diff --git a/include/bitcoin/system/chain/block.hpp b/include/bitcoin/system/chain/block.hpp index 3432ba6690..c8ced5e269 100644 --- a/include/bitcoin/system/chain/block.hpp +++ b/include/bitcoin/system/chain/block.hpp @@ -172,8 +172,6 @@ class BC_API block using hash_cref = std::reference_wrapper; using hash_hash = unique_hash_t<>; - using unordered_map_of_constant_referenced_points = - std::unordered_map; using unordered_set_of_constant_referenced_points = std::unordered_set; using unordered_set_of_constant_referenced_hashes = diff --git a/include/bitcoin/system/data/memory.hpp b/include/bitcoin/system/data/memory.hpp index 9d58027b2e..b719f4bedf 100644 --- a/include/bitcoin/system/data/memory.hpp +++ b/include/bitcoin/system/data/memory.hpp @@ -31,15 +31,6 @@ namespace system { BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT) -// shared_ptr moves are avoided in vector population by using 'new' and passing -// to shared_pointer construct a raw pointer via std::vector.emplace_back: -// std::vector.emplace_back(new block{...}). -// Otherwise emplace_back copies the shared pointer, just as would push_back: -// std::vector.emplace_back(std::make_shared(block{...})). -// Vector emplace constructs the instance using its allocator, invoking new. -// So in this case it constructs the shared_pointer which accepts the raw -// pointer as its constructor argument, passed by std::vector.emplace_back. - /// shared_ptr /// --------------------------------------------------------------------------- @@ -75,10 +66,7 @@ inline std::shared_ptr to_shared(const Type& value) NOEXCEPT template inline std::shared_ptr to_shared(Args&&... values) NOEXCEPT { - BC_PUSH_WARNING(NO_NEW_OR_DELETE) - return std::shared_ptr( - new const Type(std::forward(values)...)); - BC_POP_WARNING() + return std::make_shared(Type{ std::forward(values)... }); } /// Create shared pointer to vector of const shared pointers from moved vector. @@ -108,6 +96,13 @@ inline std::unique_ptr to_unique(const Type& value) NOEXCEPT return std::make_unique(value); } +/// Construct unique pointer to const from moved constructor parameters. +template +inline std::unique_ptr to_unique(Args&&... values) NOEXCEPT +{ + return std::make_unique(Type{ std::forward(values)... }); +} + BC_POP_WARNING() } // namespace system diff --git a/include/bitcoin/system/error/block_error_t.hpp b/include/bitcoin/system/error/block_error_t.hpp index 59dd0607f8..8f2033c347 100644 --- a/include/bitcoin/system/error/block_error_t.hpp +++ b/include/bitcoin/system/error/block_error_t.hpp @@ -55,6 +55,7 @@ enum block_error_t : uint8_t forward_reference, merkle_mismatch, block_legacy_sigop_limit, + type32_malleated, // accept block block_non_final, diff --git a/src/chain/block.cpp b/src/chain/block.cpp index 0ef7503389..6092cba185 100644 --- a/src/chain/block.cpp +++ b/src/chain/block.cpp @@ -145,11 +145,7 @@ block block::from_data(reader& source, bool witness) NOEXCEPT txs->reserve(capacity); for (size_t tx = 0; tx < capacity; ++tx) - { - BC_PUSH_WARNING(NO_NEW_OR_DELETE) - txs->emplace_back(new transaction{ source, witness }); - BC_POP_WARNING() - } + txs->push_back(to_shared(source, witness)); // This is a pointer copy (non-const to const). return txs; @@ -426,7 +422,7 @@ bool block::is_hash_limit_exceeded() const NOEXCEPT return false; // A set is used to collapse duplicates. - unordered_set_of_constant_referenced_hashes hashes; + unordered_set_of_constant_referenced_hashes hashes{}; // Just the coinbase tx hash, skip its null input hashes. hashes.emplace(txs_->front()->get_hash(false)); @@ -471,8 +467,7 @@ size_t block::malleated32_size() const NOEXCEPT { const auto malleated = txs_->size(); for (auto mally = one; mally <= to_half(malleated); mally *= two) - if (is_malleable32(malleated - mally, mally) && - is_malleated32(mally)) + if (is_malleable32(malleated - mally, mally) && is_malleated32(mally)) return mally; return zero; @@ -632,21 +627,21 @@ bool block::is_unspent_coinbase_collision() const NOEXCEPT // Search is not ordered, forward references are caught by block.check. void block::populate() const NOEXCEPT { - unordered_map_of_constant_referenced_points points{}; + std::unordered_map points{}; uint32_t index{}; // Populate outputs hash table. for (auto tx = txs_->begin(); tx != txs_->end(); ++tx, index = 0) for (const auto& out: *(*tx)->outputs_ptr()) - points.emplace(std::pair{ point{ (*tx)->get_hash(false), - index++ }, out }); + points.emplace(std::pair{ point{ (*tx)->hash(false), index++ }, + out }); // Populate input prevouts from hash table. for (auto tx = txs_->begin(); tx != txs_->end(); ++tx) { for (const auto& in: *(*tx)->inputs_ptr()) { - const auto point = points.find(std::cref(in->point())); + const auto point = points.find(in->point()); if (point != points.end()) in->prevout = point->second; } @@ -723,15 +718,22 @@ code block::confirm_transactions(const context& ctx) const NOEXCEPT // ---------------------------------------------------------------------------- // The block header is checked/accepted independently. +// context free. +// TODO: use of get_hash() in is_forward_reference makes this thread unsafe. code block::check(bool bypass) const NOEXCEPT { if (bypass) { - return is_invalid_merkle_root() ? error::merkle_mismatch : - error::block_success; + // type32_malleated is subset of is_internal_double_spend, assuming + // otherwise valid txs, as that will catch any duplicated transaction. + if (is_invalid_merkle_root()) + return error::merkle_mismatch; + if (is_malleated32()) + return error::type32_malleated; + + return error::block_success; } - // context free. // empty_block is redundant with first_not_coinbase. //if (is_empty()) // return error::empty_block; @@ -756,20 +758,24 @@ code block::check(bool bypass) const NOEXCEPT // timestamp // median_time_past +// context required. +// TODO: use of get_hash() in is_hash_limit_exceeded makes this thread unsafe. code block::check(const context& ctx, bool bypass) const NOEXCEPT { if (bypass) { const auto bip141 = ctx.is_enabled(bip141_rule); - return bip141 && is_invalid_witness_commitment() ? - error::invalid_witness_commitment : error::block_success; + + if (bip141 && is_invalid_witness_commitment()) + return error::invalid_witness_commitment; + + return error::block_success; } const auto bip34 = ctx.is_enabled(bip34_rule); const auto bip50 = ctx.is_enabled(bip50_rule); const auto bip141 = ctx.is_enabled(bip141_rule); - // context required. if (bip141 && is_overweight()) return error::block_weight_limit; if (bip34 && is_invalid_coinbase_script(ctx.height)) diff --git a/src/chain/transaction.cpp b/src/chain/transaction.cpp index b6a3816ad1..1a1d43f2db 100644 --- a/src/chain/transaction.cpp +++ b/src/chain/transaction.cpp @@ -251,11 +251,7 @@ read_puts(Source& source) NOEXCEPT puts->reserve(capacity); for (auto put = zero; put < capacity; ++put) - { - BC_PUSH_WARNING(NO_NEW_OR_DELETE) - puts->emplace_back(new Put{ source }); - BC_POP_WARNING() - } + puts->push_back(to_shared(source)); // This is a pointer copy (non-const to const). return puts; @@ -860,19 +856,16 @@ hash_digest transaction::unversioned_signature_hash( // TODO: taproot requires both single and double hash of each. void transaction::initialize_sighash_cache() const NOEXCEPT { - // This overconstructs the cache (anyone or !all), however it is simple and - // the same criteria applied by satoshi. - if (segregated_) - { - BC_PUSH_WARNING(NO_NEW_OR_DELETE) - sighash_cache_.reset(new sighash_cache - { - outputs_hash(), - points_hash(), - sequences_hash() - }); - BC_POP_WARNING() - } + // This overconstructs the cache (anyone or !all), however it is simple. + if (!segregated_) + return; + + sighash_cache_ = to_unique + ( + outputs_hash(), + points_hash(), + sequences_hash() + ); } // private diff --git a/src/error/block_error_t.cpp b/src/error/block_error_t.cpp index 456ee9e6e5..f9aced38df 100644 --- a/src/error/block_error_t.cpp +++ b/src/error/block_error_t.cpp @@ -49,6 +49,7 @@ DEFINE_ERROR_T_MESSAGE_MAP(block_error) { forward_reference, "transactions out of order" }, { merkle_mismatch, "merkle root mismatch" }, { block_legacy_sigop_limit, "too many block legacy signature operations" }, + { type32_malleated, "block is type32 malleated" }, // accept block { block_non_final, "block contains a non-final transaction" }, diff --git a/test/chain/transaction.cpp b/test/chain/transaction.cpp index f81fc91ba4..773a63dd06 100644 --- a/test/chain/transaction.cpp +++ b/test/chain/transaction.cpp @@ -1149,7 +1149,7 @@ BOOST_AUTO_TEST_CASE(transaction__is_missing_prevouts__default_inputs__true) BOOST_AUTO_TEST_CASE(transaction__is_missing_prevouts__valid_prevout__false) { const input input{ { hash_digest{}, 42 }, {}, 0 }; - input.prevout = to_shared(42, script{}); + input.prevout = to_shared(42_u64, script{}); const accessor instance { 0, diff --git a/test/error/block_error_t.cpp b/test/error/block_error_t.cpp index e6f9fffb8f..1f1cf30623 100644 --- a/test/error/block_error_t.cpp +++ b/test/error/block_error_t.cpp @@ -172,6 +172,15 @@ BOOST_AUTO_TEST_CASE(block_error_t__code__block_legacy_sigop_limit__true_exected BOOST_REQUIRE_EQUAL(ec.message(), "too many block legacy signature operations"); } +BOOST_AUTO_TEST_CASE(block_error_t__code__type32_malleated__true_exected_message) +{ + constexpr auto value = error::type32_malleated; + const auto ec = code(value); + BOOST_REQUIRE(ec); + BOOST_REQUIRE(ec == value); + BOOST_REQUIRE_EQUAL(ec.message(), "block is type32 malleated"); +} + // accept block BOOST_AUTO_TEST_CASE(block_error_t__code__block_non_final__true_exected_message)