Merge pull request #1408 from evoskuil/master

Change block.populate() to allow forward refs and own coinbase ref.

include/bitcoin/system/chain/block.hpp

@@ -105,8 +105,7 @@ class BC_API block
     code connect(const context& ctx) const NOEXCEPT;
     code confirm(const context& ctx) const NOEXCEPT;
 
-    /// Populate previous output metadata internal to the block.
-    /// Does not populate forward references (consensus limited).
+    /// Populate previous outputs (only, no metadata) internal to the block.
     void populate() const NOEXCEPT;
 
 protected:

include/bitcoin/system/chain/prevout.hpp

@@ -52,7 +52,7 @@ class BC_API prevout final
     /// For a non-coinbase input this indicates spent at height.
     bool spent{ true };
 
-    // The output is of a coinbase transaction.
+    /// The output is of a coinbase transaction.
     bool coinbase{ false };
 };
 

src/chain/block.cpp

@@ -564,48 +564,32 @@ bool block::is_unspent_coinbase_collision() const NOEXCEPT
     if (txs_->empty() || txs_->front()->inputs_ptr()->empty())
         return false;
 
-    // May only commit  duplicate coinbase that is already confirmed spent.
+    // May only commit duplicate coinbase that is already confirmed spent.
+    // Metadata population defaults coinbase to spent (not a collision).
     return !txs_->front()->inputs_ptr()->front()->metadata.spent;
 }
 
+// Search is not ordered, forward references are caught by block.check.
 void block::populate() const NOEXCEPT
 {
-    // Coinbase, outputs only, inputs only.
-    if (txs_->size() < 3u)
-        return;
-
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
-    std::unordered_map<point, output::cptr> map{};
-
-    // Skip coinbase tx.
-    auto tx = std::next(txs_->begin());
+    std::unordered_map<point, output::cptr> points{};
     uint32_t index{};
 
-    // Outputs only (first tx).
-    for (const auto& out: *(*tx)->outputs_ptr())
-        map.emplace(point{ (*tx)->hash(false), index++ }, out);
+    // Populate outputs hash table.
+    for (auto tx = txs_->begin(); tx != txs_->end(); ++tx, index = 0)
+        for (const auto& out: *(*tx)->outputs_ptr())
+            points.emplace(point{ (*tx)->hash(false), index++ }, out);
 
-    // Search is ordered, no forward references or coinbase spend (consensus).
-    for (++tx; tx != std::prev(txs_->end()); ++tx)
+    // Populate input prevouts from hash table.
+    for (auto tx = txs_->begin(); tx != txs_->end(); ++tx)
     {
         for (const auto& in: *(*tx)->inputs_ptr())
         {
-            const auto element = map.find(in->point());
-            if (element != map.end())
-                in->prevout = element->second;
+            const auto point = points.find(in->point());
+            if (point != points.end())
+                in->prevout = point->second;
         }
-
-        index = 0;
-        for (const auto& out: *(*tx)->outputs_ptr())
-            map.emplace(point{ (*tx)->hash(false), index++ }, out);
-    }
-
-    // Inputs only (last tx).
-    for (const auto& in: *(*tx)->inputs_ptr())
-    {
-        const auto element = map.find(in->point());
-        if (element != map.end())
-            in->prevout = element->second;
     }
 
     BC_POP_WARNING()
@@ -668,10 +652,11 @@ code block::connect_transactions(const context& ctx) const NOEXCEPT
 code block::confirm_transactions(const context& ctx) const NOEXCEPT
 {
     code ec;
-
-    for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
-        if ((ec = (*tx)->confirm(ctx)))
-            return ec;
+
+    if (!is_empty())
+        for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
+            if ((ec = (*tx)->confirm(ctx)))
+                return ec;
 
     return error::block_success;
 }

src/chain/transaction.cpp

@@ -1284,7 +1284,7 @@ code transaction::accept(const context&) const NOEXCEPT
 
 // Do NOT invoke on coinbase.
 // Node performs these checks through database query.
-// This assume that prevout and metadata caching are completed on all inputs.
+// This assumes that prevout and metadata caching are completed on all inputs.
 code transaction::confirm(const context& ctx) const NOEXCEPT
 {
     BC_ASSERT(!is_coinbase());
@@ -1312,8 +1312,8 @@ code transaction::connect(const context& ctx) const NOEXCEPT
 {
     BC_ASSERT(!is_coinbase());
 
-    ////if (is_coinbase())
-    ////    return error::transaction_success;
+    if (is_coinbase())
+        return error::transaction_success;
 
     code ec;
     using namespace machine;