Merge pull request #1641 from evoskuil/master

Refactor is_invalid_witness_commitment, use cref to avoid copies.

Author: evoskuil

include/bitcoin/system/chain/block.hpp

@@ -200,6 +200,8 @@ class BC_API block
 
     // context free
     hash_digest generate_merkle_root(bool witness) const NOEXCEPT;
+    bool get_witness_commitment(hash_cref& commitment) const NOEXCEPT;
+    bool get_witness_reservation(hash_cref& reservation) const NOEXCEPT;
 
     // contextual
     uint64_t reward(size_t height, uint64_t subsidy_interval,

include/bitcoin/system/chain/input.hpp

@@ -110,7 +110,7 @@ class BC_API input
 
     bool is_final() const NOEXCEPT;
     bool is_roller() const NOEXCEPT;
-    bool reserved_hash(hash_digest& out) const NOEXCEPT;
+    bool reserved_hash(hash_cref& out) const NOEXCEPT;
 
     /// Assumes coinbase if prevout not populated (returns only legacy sigops).
     size_t signature_operations(bool bip16, bool bip141) const NOEXCEPT;

include/bitcoin/system/chain/output.hpp

@@ -85,7 +85,7 @@ class BC_API output
     /// Methods.
     /// -----------------------------------------------------------------------
 
-    bool committed_hash(hash_digest& out) const NOEXCEPT;
+    bool committed_hash(hash_cref& out) const NOEXCEPT;
     size_t signature_operations(bool bip141) const NOEXCEPT;
     bool is_dust(uint64_t minimum_output_value) const NOEXCEPT;
 

include/bitcoin/system/hash/functions.hpp

@@ -184,6 +184,12 @@ inline bool operator!=(const bc::system::hash_cref& left,
 {
     return !(left == right);
 }
+
+inline bool operator!=(const bc::system::hash_cref& left,
+    const bc::system::hash_digest& right) NOEXCEPT
+{
+    return !(left.get() == right);
+}
 } // namespace std
 
 /// Extend std and boost namespaces with djb2_hash.

src/chain/block.cpp

@@ -592,6 +592,30 @@ size_t block::segregated() const NOEXCEPT
     return std::count_if(txs_->begin(), txs_->end(), count_segregated);
 }
 
+// Last output of commitment pattern holds the committed value (bip141).
+bool block::get_witness_commitment(hash_cref& commitment) const NOEXCEPT
+{
+    if (txs_->empty())
+        return false;
+
+    const auto& outputs = *txs_->front()->outputs_ptr();
+    for (const auto& output: std::views::reverse(outputs))
+        if (output->committed_hash(commitment))
+            return true;
+
+    return false;
+}
+
+// Coinbase input witness must be 32 byte witness reserved value (bip141).
+bool block::get_witness_reservation(hash_cref& reservation) const NOEXCEPT
+{
+    if (txs_->empty())
+        return false;
+
+    const auto& inputs = *txs_->front()->inputs_ptr();
+    return !inputs.empty() && inputs.front()->reserved_hash(reservation);
+}
+
 // The witness merkle root is obtained from wtxids, subject to malleation just
 // as the txs commitment. However, since tx duplicates are precluded by the
 // malleable32 (or complete) block check, there is no opportunity for this.
@@ -601,26 +625,19 @@ bool block::is_invalid_witness_commitment() const NOEXCEPT
     if (txs_->empty())
         return false;
 
-    const auto& coinbase = txs_->front();
-    if (coinbase->inputs_ptr()->empty())
-        return false;
-
+    // Witness data (segregated) disallowed if no commitment (bip141).
     // If no block tx has witness data the commitment is optional (bip141).
-    if (!is_segregated())
-        return false;
+    hash_cref commit{ null_hash };
+    if (!get_witness_commitment(commit))
+        return is_segregated();
 
-    // If there is a valid commitment, return false (valid).
-    // Coinbase input witness must be 32 byte witness reserved value (bip141).
-    // Last output of commitment pattern holds the committed value (bip141).
-    hash_digest reserved{}, committed{};
-    if (coinbase->inputs_ptr()->front()->reserved_hash(reserved))
-        for (const auto& output: std::views::reverse(*coinbase->outputs_ptr()))
-            if (output->committed_hash(committed))
-                if (committed == sha256::double_hash(
-                    generate_merkle_root(true), reserved))
-                    return false;
+    // If there is a witness reservation there must be a commitment (bip141).
+    hash_cref reserve{ null_hash };
+    if (!get_witness_reservation(reserve))
+        return true;
 
-    return true;
+    // If there is a valid commitment, return false (valid).
+    return commit != sha256::double_hash(generate_merkle_root(true), reserve);
 }
 
 //*****************************************************************************
@@ -851,6 +868,7 @@ code block::identify() const NOEXCEPT
     return error::block_success;
 }
 
+// bip141 should be disabled when the node is not accepting witness data.
 code block::identify(const context& ctx) const NOEXCEPT
 {
     const auto bip141 = ctx.is_enabled(bip141_rule);
@@ -894,6 +912,7 @@ code block::check() const NOEXCEPT
 // median_time_past
 
 // TODO: use of get_hash() in is_hash_limit_exceeded makes this thread unsafe.
+// bip141 should be disabled when the node is not accepting witness data.
 code block::check(const context& ctx) const NOEXCEPT
 {
     const auto bip141 = ctx.is_enabled(bip141_rule);

src/chain/input.cpp

@@ -430,12 +430,14 @@ bool input::is_relative_locked(size_t height,
         metadata.height, metadata.median_time_past);
 }
 
-bool input::reserved_hash(hash_digest& out) const NOEXCEPT
+bool input::reserved_hash(hash_cref& out) const NOEXCEPT
 {
-    if (!witness::is_reserved_pattern(get_witness().stack()))
+    const auto& stack = get_witness().stack();
+    if (!witness::is_reserved_pattern(stack))
         return false;
 
-    std::copy_n(get_witness().stack().front()->begin(), hash_size, out.begin());
+    // Guarded by is_reserved_pattern.
+    out = unsafe_array_cast<uint8_t, hash_size>(stack.front()->data());
     return true;
 }
 

src/chain/output.cpp

@@ -31,6 +31,7 @@ namespace system {
 namespace chain {
 
 BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
+BC_PUSH_WARNING(NO_ARRAY_INDEXING)
 
 // This is a consensus critical value that must be set on reset.
 const uint64_t output::not_found = sighash_null_value;
@@ -182,20 +183,17 @@ const chain::script::cptr& output::script_ptr() const NOEXCEPT
 // Methods.
 // ----------------------------------------------------------------------------
 
-bool output::committed_hash(hash_digest& out) const NOEXCEPT
+bool output::committed_hash(hash_cref& out) const NOEXCEPT
 {
     const auto& ops = script_->ops();
     if (!script::is_commitment_pattern(ops))
         return false;
 
-    // The four byte offset for the witness commitment hash (bip141).
+    // Offset four bytes for witness commitment head (bip141).
+    const auto start = std::next(ops[1].data().data(), sizeof(witness_head));
 
-    // More efficient [] dereference is guarded above.
-    BC_PUSH_WARNING(NO_ARRAY_INDEXING)
-    const auto start = std::next(ops[1].data().begin(), sizeof(witness_head));
-    BC_POP_WARNING()
-
-    std::copy_n(start, hash_size, out.begin());
+    // Guarded by is_commitment_pattern.
+    out = unsafe_array_cast<uint8_t, hash_size>(start);
     return true;
 }
 
@@ -221,6 +219,7 @@ bool output::is_dust(uint64_t minimum_value) const NOEXCEPT
     return value_ < minimum_value && !script_->is_unspendable();
 }
 
+BC_POP_WARNING()
 BC_POP_WARNING()
 
 // JSON value convertors.