Merge pull request #658 from evoskuil/master

Remove dead code pertaining to malleation download.

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.
     /// -----------------------------------------------------------------------
 

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;
 

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;

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;
 

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;
 

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<header_t>(value));
             break;
         }
-        case chase::malleated:
-        {
-            // Re-obtain the malleated block if it is still a candidate (virtual).
-            BC_ASSERT(std::holds_alternative<header_t>(value));
-            POST(do_malleated, std::get<header_t>(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);

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;

src/chasers/chaser_check.cpp

@@ -112,24 +112,13 @@ bool chaser_check::handle_event(const code&, chase event_,
             break;
         }
         case chase::regressed:
-        {
-            BC_ASSERT(std::holds_alternative<height_t>(value));
-            POST(do_regressed, std::get<height_t>(value));
-            break;
-        }
         case chase::disorganized:
         {
             BC_ASSERT(std::holds_alternative<height_t>(value));
             POST(do_regressed, std::get<height_t>(value));
             break;
         }
         // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-        case chase::header:
-        {
-            BC_ASSERT(std::holds_alternative<header_t>(value));
-            POST(do_header, std::get<header_t>(value));
-            break;
-        }
         case chase::headers:
         {
             BC_ASSERT(std::holds_alternative<height_t>(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>(associations{ out });
-    if (set_map(map) && !purging())
-        notify(error::success, chase::download, one);
-}
-
 // get/put hashes
 // ----------------------------------------------------------------------------
 

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).
 // ----------------------------------------------------------------------------
 

src/protocols/protocol_block_in_31800.cpp

@@ -302,34 +302,24 @@ 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());
             stop(code);
             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)