Merge pull request libbitcoin#1432 from evoskuil/master

Factor and fix malleability checks.

include/bitcoin/system/chain/block.hpp

@@ -91,6 +91,8 @@ class BC_API block
     uint64_t claim() const NOEXCEPT;
     hash_digest hash() const NOEXCEPT;
     bool is_malleable() const NOEXCEPT;
+    bool is_malleable_duplicate() const NOEXCEPT;
+    bool is_malleable_coincident() const NOEXCEPT;
     bool is_segregated() const NOEXCEPT;
     size_t serialized_size(bool witness) const NOEXCEPT;
     size_t signature_operations(bool bip16, bool bip141) const NOEXCEPT;

src/chain/block.cpp

@@ -19,10 +19,10 @@
 #include <bitcoin/system/chain/block.hpp>
 
 #include <algorithm>
-#include <cfenv>
 #include <iterator>
 #include <memory>
 #include <numeric>
+#include <set>
 #include <type_traits>
 #include <unordered_map>
 #include <unordered_set>
@@ -274,23 +274,6 @@ size_t block::serialized_size(bool witness) const NOEXCEPT
 // Connect.
 // ----------------------------------------------------------------------------
 
-// Subset of is_internal_double_spend if sha256 collisions cannot happen. This
-// is because each tx must have an input and for there to be no double spend in
-// the block the inputs must be unique (and only one coinbase). As the
-// is_internal_double_spend check invalidates any block with duplicated txs,
-// there can be no tx hash duplication within the merkle tree. And a block that
-// fails block.check is not archived, and its header remains potentially valid.
-////bool block::is_distinct_transaction_set() const
-////{
-////    // A set is used to collapse duplicates.
-////    std::set<hash_digest> hashes;
-////
-////    for (const auto& tx: *txs_)
-////        hashes.insert(tx->hash(false));
-////
-////    return hashes.size() == txs_->size();
-////}
-
 bool block::is_empty() const NOEXCEPT
 {
     return txs_->empty();
@@ -456,26 +439,38 @@ bool block::is_hash_limit_exceeded() const NOEXCEPT
 }
 
 // This is not part of validation. Should be called after *invalidation* to
-// determine if the invalidity is universal (otherwise do not cache invalid).
+// determine if the invalidity is universal (otherwise do not cache invalid),
+// and as an abbreviated validation when under checkpoint/milestone.
 // lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html
 bool block::is_malleable() const NOEXCEPT
 {
-    // A two tx block cannot be malleable as coinbase is singular, otherwise
-    // if the last two non-witness tx hashes match then the id is malleable.
-    const auto count = txs_->size();
-    if (count > two && is_even(count) &&
-        txs_->at(sub1(count))->hash(false) == txs_->at(count)->hash(false))
-    {
-        return true;
-    }
+    return is_malleable_coincident() || is_malleable_duplicate();
+}
 
-    // Hash of two same concatenated leaves is same as doubling one odd leaf.
+// Repeated tx hashes is a subset of is_internal_double_spend.
+// This form of malleability also implies current block instance is invalid.
+bool block::is_malleable_duplicate() const NOEXCEPT
+{
+    // A set is used to collapse duplicates.
+    std::set<hash_digest> hashes;
+
+    BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
+    for (const auto& tx: *txs_)
+        hashes.insert(tx->hash(false));
+    BC_POP_WARNING()
+
+    return hashes.size() == txs_->size();
+}
+
+// If all non-witness tx serializations are 64 bytes the id is malleable.
+// This form of malleability does not imply current block instance is invalid.
+bool block::is_malleable_coincident() const NOEXCEPT
+{
     const auto two_leaf_size = [](const transaction::cptr& tx) NOEXCEPT
     {
         return tx->serialized_size(false) == two * hash_size;
     };
 
-    // If all non-witness tx serializations are 64 bytes the id is malleable.
     return std::all_of(txs_->begin(), txs_->end(), two_leaf_size);
 }