Simplify metadata and block populate(), style, comments,

Author: evoskuil

include/bitcoin/system/chain/block.hpp

@@ -145,12 +145,8 @@ class BC_API block
     code confirm(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;
+    /// Fails if any populated prevout is internally immature or locked.
+    code populate(const context& ctx) const NOEXCEPT;
 
 protected:
     block(stream::in::fast&& stream, bool witness) NOEXCEPT;

include/bitcoin/system/chain/prevout.hpp

@@ -25,57 +25,28 @@ namespace libbitcoin {
 namespace system {
 namespace chain {
 
-// Defaults are set so non-population issues usually imply invalidity.
 class BC_API prevout final
 {
 public:
-    ///************************************************************************
-    /// CONSENSUS: 
-    /// A height of zero is immature (unspendable) despite unspent state.
-    ///************************************************************************
-    union
-    {
-        /// Unused for coinbase.
-        /// The confirmed height of the prevout or max_uint32.
-        size_t prevout_height{ max_uint32 };
-
-        /// node: populated with database identifier for the parent tx.
-        /// Link::terminal must derive from default of max_uint32.
-        uint32_t parent_tx;
-    };
+    /// chain/node: prevout is of a coinbase tx.
+    bool coinbase{ true };
 
-    ///************************************************************************
-    /// CONSENSUS: 
-    /// A mtp of max_uint32 fails locktime maturity (until time overflow).
-    ///************************************************************************
-    union
-    {
-        /// Unused for coinbase.
-        /// The median time past at confirmed prevout block or max_uint32.
-        uint32_t median_time_past{ max_uint32 };
+    /// chain: confirmed prevout mtp or max_uint32.
+    uint32_t median_time_past{ max_uint32 };
 
-        /// node: populated with database identifier for the input/point.
-        /// Link::terminal must derive from default of max_uint32.
-        uint32_t point_link;
-    };
+    /// chain: confirmed prevout height or max_uint32.
+    size_t prevout_height{ max_uint32 };
 
-    ///************************************************************************
-    /// CONSENSUS: 
-    /// An unspent coinbase collision is immature (unspendable) and spent
-    /// collision is mature [bip30]. CB collision presumed precluded by bip34.
-    /// This is NOT guarded by system::chain confirmation checks.
-    ///************************************************************************
-    /// The confirmed height of the spender or max_uint32.
+    /// chain: confirmed spender height or max_uint32.
     uint32_t spender_height{ max_uint32 };
 
-    /// node: set via block.populate() as internal spends do not require
-    /// prevout block association for relative locktime checks. So
-    /// median_time_past is not required as locked is determined here.
-    bool locked{ true };
+    /// node: database identifier for the parent tx.
+    /// Link::terminal must align with link defaults of max_uint32.
+    uint32_t parent_tx{ max_uint32 };
 
-    /// The previous output is of a coinbase transaction.
-    /// node: populated, does not require prevout block association.
-    bool coinbase{ true };
+    /// node: database identifier for input/point.
+    /// Link::terminal must align with link defaults of max_uint32.
+    uint32_t point_link{ max_uint32 };
 };
 
 } // namespace chain

src/chain/block.cpp

@@ -751,35 +751,7 @@ bool block::is_signature_operations_limited(bool bip16,
 }
 
 // Search is unordered, forward refs (and duplicates) caught by block.check.
-void block::populate() const NOEXCEPT
-{
-    if (txs_->empty())
-        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
+code block::populate(const chain::context& ctx) const NOEXCEPT
 {
     if (txs_->empty())
         return error::block_success;
@@ -802,19 +774,15 @@ code block::populate_with_metadata(const chain::context& ctx) const NOEXCEPT
         {
             // Map chain::point to cref_point for search, should optimize away.
             const cref_point key{ in->point().hash(), in->point().index() };
-
             if (const auto it = points.find(key); it != points.end())
             {
-                const auto immature = (matures && in->point().hash() == self);
-                const auto lock = (bip68 && (*tx)->is_internally_locked(*in));
-                in->prevout = it->second;
+                if (matures && in->point().hash() == self)
+                    return error::coinbase_maturity;
+
+                if (bip68 && (*tx)->is_internally_locked(*in))
+                    return error::relative_time_locked;
 
-                // If invalid shortcircuit population and return above error.
-                if ((in->metadata.locked = (immature || lock)))
-                {
-                    return immature ? error::coinbase_maturity :
-                        error::relative_time_locked;
-                }
+                in->prevout = it->second;
             }
         }
     }