From b58be1d65b7a30ada1b7d82bea409fa7b2cf2550 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 28 Jun 2024 21:53:53 -0400 Subject: [PATCH 1/2] Remove dead code pertaining to malleation download. --- include/bitcoin/node/chase.hpp | 11 ------- include/bitcoin/node/chasers/chaser_block.hpp | 3 -- include/bitcoin/node/chasers/chaser_check.hpp | 1 - .../bitcoin/node/chasers/chaser_header.hpp | 3 -- .../bitcoin/node/chasers/chaser_organize.hpp | 3 -- .../node/impl/chasers/chaser_organize.ipp | 12 ++------ src/chasers/chaser_block.cpp | 5 ---- src/chasers/chaser_check.cpp | 30 ------------------- src/chasers/chaser_header.cpp | 11 ------- 9 files changed, 2 insertions(+), 77 deletions(-) diff --git a/include/bitcoin/node/chase.hpp b/include/bitcoin/node/chase.hpp index 561e8a0c7..3f5cbcb4a 100644 --- a/include/bitcoin/node/chase.hpp +++ b/include/bitcoin/node/chase.hpp @@ -68,10 +68,6 @@ enum class chase /// Candidate Chain. /// ----------------------------------------------------------------------- - /// A candidate header has been disassociated due to malleation (header_t). - /// Issued by 'header' and handled by 'check' (redownload). - header, - /// A new candidate branch exists from given branch point (height_t). /// Issued by 'block' and handled by 'confirm'. blocks, @@ -88,17 +84,10 @@ enum class chase /// Issued by 'organize' and handled by 'check'. regressed, - /// Late-stage Invalidity. - /// ----------------------------------------------------------------------- - /// unchecked, unvalid or unconfirmable was handled (height_t). /// Issued by 'organize' and handled by 'validate' (disorgs candidates). disorganized, - /// unarchived block was determined to be malleated (header_t). - /// Issued by 'block_in_31800', handled by 'organize' (redownload). - malleated, - /// Validation. /// ----------------------------------------------------------------------- diff --git a/include/bitcoin/node/chasers/chaser_block.hpp b/include/bitcoin/node/chasers/chaser_block.hpp index a445df14e..04bdcd092 100644 --- a/include/bitcoin/node/chasers/chaser_block.hpp +++ b/include/bitcoin/node/chasers/chaser_block.hpp @@ -56,9 +56,6 @@ class BCN_API chaser_block code validate(const system::chain::block& block, const chain_state& state) const NOEXCEPT override; - /// Notify check chaser to redownload the block (nop). - void do_malleated(header_t link) NOEXCEPT override; - /// Determine if state is top of a storable branch (always true). bool is_storable(const chain_state& state) const NOEXCEPT override; diff --git a/include/bitcoin/node/chasers/chaser_check.hpp b/include/bitcoin/node/chasers/chaser_check.hpp index 7d32b2a72..ae0166e12 100644 --- a/include/bitcoin/node/chasers/chaser_check.hpp +++ b/include/bitcoin/node/chasers/chaser_check.hpp @@ -61,7 +61,6 @@ class BCN_API chaser_check event_value value) NOEXCEPT; virtual void do_bump(height_t height) NOEXCEPT; - virtual void do_header(header_t height) NOEXCEPT; virtual void do_checked(height_t height) NOEXCEPT; virtual void do_headers(height_t branch_point) NOEXCEPT; virtual void do_regressed(height_t branch_point) NOEXCEPT; diff --git a/include/bitcoin/node/chasers/chaser_header.hpp b/include/bitcoin/node/chasers/chaser_header.hpp index c36fe4b5c..b48ba5159 100644 --- a/include/bitcoin/node/chasers/chaser_header.hpp +++ b/include/bitcoin/node/chasers/chaser_header.hpp @@ -59,9 +59,6 @@ class BCN_API chaser_header code validate(const system::chain::header& header, const chain_state& state) const NOEXCEPT override; - /// Notify check chaser to redownload the block. - void do_malleated(header_t link) NOEXCEPT override; - /// Determine if state is top of a storable branch. bool is_storable(const chain_state& state) const NOEXCEPT override; diff --git a/include/bitcoin/node/chasers/chaser_organize.hpp b/include/bitcoin/node/chasers/chaser_organize.hpp index 2cb7dfdd6..b89da30e3 100644 --- a/include/bitcoin/node/chasers/chaser_organize.hpp +++ b/include/bitcoin/node/chasers/chaser_organize.hpp @@ -78,9 +78,6 @@ class chaser_organize virtual code validate(const Block& block, const chain_state& state) const NOEXCEPT = 0; - /// Notify check chaser to redownload the block. - virtual void do_malleated(header_t link) NOEXCEPT = 0; - /// Determine if state is top of a storable branch. virtual bool is_storable(const chain_state& state) const NOEXCEPT = 0; diff --git a/include/bitcoin/node/impl/chasers/chaser_organize.ipp b/include/bitcoin/node/impl/chasers/chaser_organize.ipp index 00a7d7dc5..6a25240ed 100644 --- a/include/bitcoin/node/impl/chasers/chaser_organize.ipp +++ b/include/bitcoin/node/impl/chasers/chaser_organize.ipp @@ -100,13 +100,6 @@ bool CLASS::handle_event(const code&, chase event_, event_value value) NOEXCEPT POST(do_disorganize, std::get(value)); break; } - case chase::malleated: - { - // Re-obtain the malleated block if it is still a candidate (virtual). - BC_ASSERT(std::holds_alternative(value)); - POST(do_malleated, std::get(value)); - break; - } case chase::stop: { return false; @@ -179,9 +172,8 @@ void CLASS::do_organize(typename Block::cptr block_ptr, return; }; - // blocks-first may return malleation error, in which case another peer may - // return the good block of the same hash. headers-first cannot detect - // malleation here, so the block_in protocol sends chase::malleated. + // Headers are late validated, with malleations ignored upon download. + // Blocks are fully validated (not confirmed), so malleation is non-issue. if ((ec = validate(*block_ptr, *state))) { handler(ec, height); diff --git a/src/chasers/chaser_block.cpp b/src/chasers/chaser_block.cpp index 37ecbc27b..f587bb5a0 100644 --- a/src/chasers/chaser_block.cpp +++ b/src/chasers/chaser_block.cpp @@ -130,11 +130,6 @@ code chaser_block::validate(const block& block, return block.connect(ctx); } -void chaser_block::do_malleated(header_t) NOEXCEPT -{ - // blocks are guarded against malleation before being stored. -} - bool chaser_block::is_storable(const chain_state&) const NOEXCEPT { return true; diff --git a/src/chasers/chaser_check.cpp b/src/chasers/chaser_check.cpp index 25401fa41..dabc233d1 100644 --- a/src/chasers/chaser_check.cpp +++ b/src/chasers/chaser_check.cpp @@ -112,11 +112,6 @@ bool chaser_check::handle_event(const code&, chase event_, break; } case chase::regressed: - { - BC_ASSERT(std::holds_alternative(value)); - POST(do_regressed, std::get(value)); - break; - } case chase::disorganized: { BC_ASSERT(std::holds_alternative(value)); @@ -124,12 +119,6 @@ bool chaser_check::handle_event(const code&, chase event_, break; } // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - case chase::header: - { - BC_ASSERT(std::holds_alternative(value)); - POST(do_header, std::get(value)); - break; - } case chase::headers: { BC_ASSERT(std::holds_alternative(value)); @@ -244,25 +233,6 @@ void chaser_check::do_headers(height_t) NOEXCEPT notify(error::success, chase::download, added); } -// A malleable block was found to be malleated, so re-download. -void chaser_check::do_header(header_t link) NOEXCEPT -{ - BC_ASSERT(stranded()); - - association out{}; - if (!archive().get_unassociated(out, link)) - { - // False return may be the result of existing association (still error). - fault(error::get_unassociated); - return; - } - - // Add even if purging, as this header applies to the subsequent rage. - const auto map = std::make_shared(associations{ out }); - if (set_map(map) && !purging()) - notify(error::success, chase::download, one); -} - // get/put hashes // ---------------------------------------------------------------------------- diff --git a/src/chasers/chaser_header.cpp b/src/chasers/chaser_header.cpp index bf30c9ff3..3d6c54ff4 100644 --- a/src/chasers/chaser_header.cpp +++ b/src/chasers/chaser_header.cpp @@ -115,17 +115,6 @@ code chaser_header::validate(const header& header, return system::error::block_success; } -// A malleable block was found to be invalid due to malleation. -void chaser_header::do_malleated(header_t link) NOEXCEPT -{ - BC_ASSERT(stranded()); - - // Announce a single header that requires (re)download. - // Since it is in the candidate chain, it must presently be missing. - if (archive().is_candidate_header(link)) - notify(error::success, chase::header, link); -} - // Storable methods (private). // ---------------------------------------------------------------------------- From 8626467137d2d5beec208ffac7cd6d0d4535dcc7 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 28 Jun 2024 21:54:14 -0400 Subject: [PATCH 2/2] Refactor handle_receive_block. --- src/protocols/protocol_block_in_31800.cpp | 26 +++++++---------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 0c3892811..5ea11ed4b 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/src/protocols/protocol_block_in_31800.cpp @@ -302,15 +302,16 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, const auto checked = is_under_checkpoint(height) || query.is_milestone(link); - // Transaction commitments and malleation are checked under bypass. - // Invalidity is only stored in the case where a strong header has been - // stored, later to be found out as invalid. The invalidity prevents repeat - // processing of the same invalid chain but is not logically necessary. It - // is not stored in the case where the block is malleated32/64. + // Tx commitments and malleation are checked under bypass. Invalidity is + // only stored when a strong header has been stored, later to be found out + // as invalid and not malleable. Stored invalidity prevents repeat + // processing of the same invalid chain but is not logically necessary. if (const auto code = check(*block_ptr, it->context, checked)) { + // These imply that we don't have actual block represented by the hash. if (code == system::error::invalid_transaction_commitment || - code == system::error::invalid_witness_commitment) + code == system::error::invalid_witness_commitment || + code == system::error::block_malleated) { LOGR("Uncommitted block [" << encode_hash(hash) << ":" << height << "] from [" << authority() << "] " << code.message()); @@ -318,18 +319,7 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec, return false; } - if (code == system::error::block_malleated) - { - LOGR("Malleated block [" << encode_hash(hash) << ":" << height - << "] from [" << authority() << "] " << code.message()); - - // event sent to re-obtain the correct block. - notify(error::success, chase::malleated, link); - stop(code); - return false; - } - - // Mark unconfirmable non-malleated block. + // Actual block represented by the hash is unconfirmable. if (!query.set_block_unconfirmable(link)) { LOGF("Failure setting block unconfirmable [" << encode_hash(hash)