Merge pull request #1622 from evoskuil/master

Updates to block::populate and locktime fn names.

Author: evoskuil

include/bitcoin/system/chain/block.hpp

@@ -138,9 +138,13 @@ class BC_API block
     code connect(const context& ctx) const NOEXCEPT;
     code confirm(const context& ctx) const NOEXCEPT;
 
-    /// Populate previous outputs (and metadata.locked) internal to the block.
-    /// False if one or more populated prevouts is locked in the block context.
-    bool populate(const context& ctx) const NOEXCEPT;
+    /// Populate previous outputs internal to the block.
+    void populate() const NOEXCEPT;
+
+    /// Populate previous outputs and metadata.locked internal to the block.
+    /// Execution is shortcircuited for error with that metadata.locked set.
+    /// False if any populated prevout is immature in the block context.
+    code populate_with_metadata(const context& ctx) const NOEXCEPT;
 
 protected:
     block(const chain::header::cptr& header,

include/bitcoin/system/chain/input.hpp

@@ -44,7 +44,7 @@ class BC_API input
     typedef std::shared_ptr<const input> cptr;
 
     static bool is_relative_locktime_applied(uint32_t sequence) NOEXCEPT;
-    static bool is_locked(uint32_t sequence, size_t height,
+    static bool is_relative_locked(uint32_t sequence, size_t height,
         uint32_t median_time_past, size_t prevout_height,
         uint32_t prevout_median_time_past) NOEXCEPT;
 
@@ -117,17 +117,14 @@ class BC_API input
     size_t signature_operations(bool bip16, bool bip141) const NOEXCEPT;
 
     /// Requires metadata.height and median_time_past (otherwise returns true).
-    bool is_locked(size_t height, uint32_t median_time_past) const NOEXCEPT;
+    bool is_relative_locked(size_t height,
+        uint32_t median_time_past) const NOEXCEPT;
 
 protected:
     input(const chain::point::cptr& point, const chain::script::cptr& script,
         const chain::witness::cptr& witness, uint32_t sequence,
         bool valid) NOEXCEPT;
 
-    /// Any non-zero relative locktime value locks internally-spent input.
-    friend class transaction;
-    bool is_internal_lock() const NOEXCEPT;
-
 private:
     typedef struct { size_t nominal; size_t witnessed; } sizes;
 

include/bitcoin/system/chain/prevout.hpp

@@ -54,6 +54,7 @@ class BC_API prevout final
         uint32_t median_time_past{ max_uint32 };
 
         /// node: set to the database record of the input association.
+        /// node: it is necessary that Link::terminal derives from max_uint32.
         uint32_t link;
     };
 

include/bitcoin/system/chain/transaction.hpp

@@ -134,7 +134,7 @@ class BC_API transaction
     bool is_dusty(uint64_t minimum_output_value) const NOEXCEPT;
 
     /// Requires no metadata, true if spend in own block would be locked.
-    bool is_internal_lock(const input& in) const NOEXCEPT;
+    bool is_internally_locked(const input& in) const NOEXCEPT;
 
     /// Assumes coinbase if prevout not populated (returns only legacy sigops).
     size_t signature_operations(bool bip16, bool bip141) const NOEXCEPT;
@@ -204,7 +204,7 @@ class BC_API transaction
     /// Check (requires context).
     /// -----------------------------------------------------------------------
 
-    bool is_non_final(size_t height, uint32_t timestamp,
+    bool is_absolute_locked(size_t height, uint32_t timestamp,
         uint32_t median_time_past, bool bip113) const NOEXCEPT;
 
     /// Accept (requires prevouts).
@@ -220,7 +220,8 @@ class BC_API transaction
     /// -----------------------------------------------------------------------
 
     /// Requires input.metadata.height/median_time_past (prevout confirmation).
-    bool is_locked(size_t height, uint32_t median_time_past) const NOEXCEPT;
+    bool is_relative_locked(size_t height,
+        uint32_t median_time_past) const NOEXCEPT;
 
     /// Requires input.metadata.height (prevout confirmation).
     bool is_immature(size_t height) const NOEXCEPT;

include/bitcoin/system/error/transaction_error_t.hpp

@@ -47,14 +47,14 @@ enum transaction_error_t
 
     // accept transaction
     unexpected_witness_transaction,
-    transaction_non_final,
     premature_validation,
     unspent_duplicate,
     missing_previous_output,
     double_spend,
     coinbase_maturity,
     spend_exceeds_value,
     transaction_sigop_limit,
+    absolute_time_locked,
     relative_time_locked,
     transaction_weight_limit,
 

src/chain/block.cpp

@@ -710,10 +710,38 @@ bool block::is_unspent_coinbase_collision() const NOEXCEPT
 }
 
 // Search is unordered, forward refs (and duplicates) caught by block.check.
-bool block::populate(const chain::context& ctx) const NOEXCEPT
+void block::populate() const NOEXCEPT
 {
     if (txs_->empty())
-        return true;
+        return;
+
+    unordered_map_of_cref_point_to_output_cptr_cref points{ outputs() };
+    uint32_t index{};
+
+    // Populate outputs hash table (coinbase included).
+    for (auto tx = txs_->begin(); tx != txs_->end(); ++tx, index = 0)
+        for (const auto& out: *(*tx)->outputs_ptr())
+            points.emplace(cref_point{ (*tx)->get_hash(false), index++ }, out);
+
+    // Populate input prevouts from hash table.
+    for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
+    {
+        for (const auto& in: *(*tx)->inputs_ptr())
+        {
+            // Map chain::point to cref_point for search, should optimize away.
+            const auto point = points.find({ in->point().hash(),
+                in->point().index() });
+
+            if (point != points.end())
+                in->prevout = point->second;
+        }
+    }
+}
+
+code block::populate_with_metadata(const chain::context& ctx) const NOEXCEPT
+{
+    if (txs_->empty())
+        return error::block_success;
 
     const auto bip68 = ctx.is_enabled(chain::flags::bip68_rule);
     unordered_map_of_cref_point_to_output_cptr_cref points{ outputs() };
@@ -724,8 +752,7 @@ bool block::populate(const chain::context& ctx) const NOEXCEPT
         for (const auto& out: *(*tx)->outputs_ptr())
             points.emplace(cref_point{ (*tx)->get_hash(false), index++ }, out);
 
-    // Populate input prevouts from hash table and obtain locked state.
-    auto locked = false;
+    // Populate input prevouts from hash table and obtain maturity.
     for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
     {
         for (const auto& in: *(*tx)->inputs_ptr())
@@ -736,19 +763,23 @@ bool block::populate(const chain::context& ctx) const NOEXCEPT
 
             if (point != points.end())
             {
-                // Zero maturity coinbase spend is treated as locked.
-                const auto lock = (bip68 && (*tx)->is_internal_lock(*in));
+                // Zero maturity coinbase spend is immature.
+                const auto lock = (bip68 && (*tx)->is_internally_locked(*in));
                 const auto immature = !is_zero(coinbase_maturity) &&
                     (in->point().hash() == txs_->front()->get_hash(false));
 
                 in->prevout = point->second;
-                in->metadata.locked = immature || lock;
-                locked |= in->metadata.locked;
+                if ((in->metadata.locked = (immature || lock)))
+                {
+                    // Shortcircuit population and return above error.
+                    return immature ? error::coinbase_maturity : 
+                        error::relative_time_locked;
+                }
             }
         }
     }
 
-    return !locked;
+    return error::block_success;
 }
 
 // Delegated.

src/chain/input.cpp

@@ -399,7 +399,7 @@ bool input::is_relative_locktime_applied(uint32_t sequence) NOEXCEPT
 }
 
 // static
-bool input::is_locked(uint32_t sequence, size_t height,
+bool input::is_relative_locked(uint32_t sequence, size_t height,
     uint32_t median_time_past, size_t prevout_height,
     uint32_t prevout_median_time_past) NOEXCEPT
 {
@@ -412,29 +412,26 @@ bool input::is_locked(uint32_t sequence, size_t height,
     // BIP68: bit 22 determines if relative lock is time or block based.
     if (get_right(sequence, relative_locktime_time_locked_bit))
     {
+        // BIP68: references to median time past are as defined by bip113.
         // BIP68: change sequence to seconds by shift up by 9 bits (x 512).
         auto time = shift_left(blocks, relative_locktime_seconds_shift_left);
         auto age = floored_subtract(median_time_past, prevout_median_time_past);
         return age < time;
     }
 
+    // BIP68: when the relative lock time is block based, it is interpreted as
+    // a minimum block height constraint over the age of the input.
     const auto age = floored_subtract(height, prevout_height);
     return age < blocks;
 }
 
-bool input::is_locked(size_t height, uint32_t median_time_past) const NOEXCEPT
+bool input::is_relative_locked(size_t height,
+    uint32_t median_time_past) const NOEXCEPT
 {
     // Prevout must be found and height/median_time_past metadata populated.
     ////BC_ASSERT(!is_zero(metadata.height));
-    return is_locked(sequence_, height, median_time_past, metadata.height,
-        metadata.median_time_past);
-}
-
-// protected (tx friend)
-bool input::is_internal_lock() const NOEXCEPT
-{
-    // Internal spends have no relative height/mtp (any metadata values work).
-    return is_locked(metadata.height, metadata.median_time_past);
+    return is_relative_locked(sequence_, height, median_time_past,
+        metadata.height, metadata.median_time_past);
 }
 
 bool input::reserved_hash(hash_digest& out) const NOEXCEPT

src/chain/transaction.cpp

@@ -1047,7 +1047,7 @@ bool transaction::is_invalid_coinbase_size() const NOEXCEPT
 // Accept (contextual).
 // ----------------------------------------------------------------------------
 
-bool transaction::is_non_final(size_t height, uint32_t timestamp,
+bool transaction::is_absolute_locked(size_t height, uint32_t timestamp,
     uint32_t median_time_past, bool bip113) const NOEXCEPT
 {
     // BIP113: comparing the locktime against the median of the past 11 block
@@ -1153,7 +1153,7 @@ bool transaction::is_relative_locktime_applied(bool coinbase, uint32_t version,
         (version >= relative_locktime_min_version);
 }
 
-bool transaction::is_internal_lock(const input& in) const NOEXCEPT
+bool transaction::is_internally_locked(const input& in) const NOEXCEPT
 {
     // BIP68: not applied to the sequence of the input of a coinbase.
     BC_ASSERT(!is_coinbase());
@@ -1162,10 +1162,12 @@ bool transaction::is_internal_lock(const input& in) const NOEXCEPT
     if (version_ < relative_locktime_min_version)
         return false;
 
-    return in.is_internal_lock();
+    // Internal spends have no relative height/mtp (own metadata vs. itself).
+    return in.is_relative_locked(in.metadata.height,
+        in.metadata.median_time_past);
 }
 
-bool transaction::is_locked(size_t height,
+bool transaction::is_relative_locked(size_t height,
     uint32_t median_time_past) const NOEXCEPT
 {
     // BIP68: not applied to the sequence of the input of a coinbase.
@@ -1178,11 +1180,9 @@ bool transaction::is_locked(size_t height,
     // BIP68: references to median time past are as defined by bip113.
     const auto locked = [=](const auto& input) NOEXCEPT
     {
-        return input->is_locked(height, median_time_past);
+        return input->is_relative_locked(height, median_time_past);
     };
 
-    // BIP68: when the relative lock time is block based, it is interpreted as
-    // a minimum block height constraint over the age of the input.
     return std::any_of(inputs_->begin(), inputs_->end(), locked);
 }
 
@@ -1289,8 +1289,8 @@ code transaction::check(const context& ctx) const NOEXCEPT
 {
     const auto bip113 = ctx.is_enabled(bip113_rule);
 
-    if (is_non_final(ctx.height, ctx.timestamp, ctx.median_time_past, bip113))
-        return error::transaction_non_final;
+    if (is_absolute_locked(ctx.height, ctx.timestamp, ctx.median_time_past, bip113))
+        return error::absolute_time_locked;
 
     return error::transaction_success;
 }
@@ -1325,7 +1325,7 @@ code transaction::confirm(const context& ctx) const NOEXCEPT
 
     if (is_coinbase())
         return error::transaction_success;
-    if (bip68 && is_locked(ctx.height, ctx.median_time_past))
+    if (bip68 && is_relative_locked(ctx.height, ctx.median_time_past))
         return error::relative_time_locked;
     if (is_immature(ctx.height))
         return error::coinbase_maturity;

src/error/transaction_error_t.cpp

@@ -41,15 +41,15 @@ DEFINE_ERROR_T_MESSAGE_MAP(transaction_error)
 
     // accept transaction
     { unexpected_witness_transaction, "unexpected witness transaction" },
-    { transaction_non_final, "transaction currently non-final" },
     { premature_validation, "transaction validation under checkpoint" },
     { unspent_duplicate, "matching transaction with unspent outputs" },
     { missing_previous_output, "previous output not found" },
     { double_spend, "double spend of input" },
     { coinbase_maturity, "immature coinbase spent" },
     { spend_exceeds_value, "spend exceeds value of inputs" },
     { transaction_sigop_limit, "too many transaction embedded signature operations" },
-    { relative_time_locked, "transaction currently locked" },
+    { absolute_time_locked, "transaction absolute time locked" },
+    { relative_time_locked, "transaction relative time locked" },
     { transaction_weight_limit, "transaction weight limit exceeded" },
 
     // dconfirm transaction