Merge pull request #726 from evoskuil/master

Don't fire events for bypassed validate/confirm, shrink initial point table.

Author: evoskuil

include/bitcoin/node/chasers/chaser_validate.hpp

@@ -60,9 +60,9 @@ class BCN_API chaser_validate
     virtual code populate(bool bypass, const system::chain::block& block,
         const system::chain::context& ctx) NOEXCEPT;
     virtual void tracked_complete_block(const code& ec,
-        const database::header_link& link, size_t height) NOEXCEPT;
+        const database::header_link& link, size_t height, bool bypassed) NOEXCEPT;
     virtual void complete_block(const code& ec,
-        const database::header_link& link, size_t height) NOEXCEPT;
+        const database::header_link& link, size_t height, bool bypassed) NOEXCEPT;
 
     // Override base class strand because it sits on the network thread pool.
     network::asio::strand& strand() NOEXCEPT override;

src/chasers/chaser_confirm.cpp

@@ -177,6 +177,7 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
     {
         const auto link = query.to_candidate(height);
         auto ec = query.get_block_state(link);
+        auto bypassed = true;
 
         if (ec == database::error::unassociated)
         {
@@ -224,13 +225,6 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
                     return;
                 }
 
-                // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-                // Failure here was previously result of bug in hashmap, which
-                // caused both iteration across full prevout table and missing
-                // prevout tx records intermittently in confirmation query.
-                // This would prevent subsequent confirmation progress. This
-                // has been resolved.
-                // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                 notify(ec, chase::unconfirmable, link);
                 fire(events::block_unconfirmable, height);
                 LOGR("Unconfirmable block [" << height << "] " << ec.message());
@@ -249,6 +243,9 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
                 fault(error::confirm6);
                 return;
             }
+
+            // Confirmable query executed successfully.
+            bypassed = false;
         }
         else if (ec == database::error::block_confirmable)
         {
@@ -284,8 +281,12 @@ void chaser_confirm::do_bump(height_t) NOEXCEPT
         set_position(height);
 
         notify(error::success, chase::confirmable, height);
-        fire(events::block_confirmed, height);
-        ////LOGV("Block confirmed: " << height);
+
+        if (!bypassed)
+        {
+            fire(events::block_confirmed, height);
+            ////LOGV("Block confirmed: " << height);
+        }
     }
 }
 

src/chasers/chaser_validate.cpp

@@ -168,7 +168,7 @@ void chaser_validate::do_bump(height_t) NOEXCEPT
         }
         else if (ec == database::error::block_unconfirmable)
         {
-            complete_block(ec, link, height);
+            complete_block(ec, link, height, true);
             return;
         }
 
@@ -179,7 +179,7 @@ void chaser_validate::do_bump(height_t) NOEXCEPT
 
         if (bypass && !filter_)
         {
-            complete_block(database::error::success, link, height);
+            complete_block(database::error::success, link, height, true);
         }
         else
         {
@@ -227,7 +227,7 @@ void chaser_validate::validate_block(const header_link& link,
     }
 
     // Return to strand to handle result.
-    POST(tracked_complete_block, ec, link, ctx.height);
+    POST(tracked_complete_block, ec, link, ctx.height, bypass);
 }
 
 code chaser_validate::populate(bool bypass, const chain::block& block,
@@ -279,16 +279,16 @@ code chaser_validate::validate(bool bypass, const chain::block& block,
 
 // The size of the job is not relevant to the backlog cost.
 void chaser_validate::tracked_complete_block(const code& ec,
-    const header_link& link, size_t height) NOEXCEPT
+    const header_link& link, size_t height, bool bypassed) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
     --backlog_;
-    complete_block(ec, link, height);
+    complete_block(ec, link, height, bypassed);
 }
 
 void chaser_validate::complete_block(const code& ec, const header_link& link,
-    size_t height) NOEXCEPT
+    size_t height, bool bypassed) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
@@ -305,21 +305,20 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
         fire(events::block_unconfirmable, height);
         LOGR("Invalid block [" << height << "] " << ec.message());
 
-        // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
         // Stop the network in case of an unexpected invalidity (debugging).
         // This is considered a bug, not an invalid block arrival (for now).
-        // This usually manifests as accept failure invalid_witness (!checked),
-        // in which case the witness data is simply not present, but bip141 is
-        // active and the output indicates a witness transaction.
-        fault(ec);
-        // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+        ////fault(ec);
 
         return;
     }
 
     notify(ec, chase::valid, possible_wide_cast<height_t>(height));
-    fire(events::block_validated, height);
-    ////LOGV("Block validated: " << height);
+
+    if (!bypassed)
+    {
+        fire(events::block_validated, height);
+        ////LOGV("Block validated: " << height);
+    }
 
     // Prevent stall by posting internal event, avoid hitting external handlers.
     if (is_zero(backlog_))

src/parser.cpp

@@ -82,8 +82,9 @@ parser::parser(system::chain::selection context) NOEXCEPT
     configured.database.output_size = 25'300'000'000;
     configured.database.output_rate = 5;
 
-    configured.database.point_buckets = 1'750'905'073;
-    configured.database.point_size = 24'000'000'000;
+    // Full size too big for mini, so reduced to compressed size.
+    configured.database.point_buckets = 546'188'501;
+    configured.database.point_size = 8'389'074'978;
     configured.database.point_rate = 5;
 
     configured.database.puts_size = 6'300'000'000;
@@ -697,12 +698,12 @@ options_metadata parser::load_settings() THROWS
     (
         "database.point_buckets",
         value<uint32_t>(&configured.database.point_buckets),
-        "The number of buckets in the point table head, defaults to '1750905073'."
+        "The number of buckets in the point table head, defaults to '546188501'."
     )
     (
         "database.point_size",
         value<uint64_t>(&configured.database.point_size),
-        "The minimum allocation of the point table body, defaults to '24000000000'."
+        "The minimum allocation of the point table body, defaults to '8389074978'."
     )
     (
         "database.point_rate",

src/protocols/protocol_block_in_31800.cpp

@@ -304,21 +304,6 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
     const auto checked = is_under_checkpoint(height) ||
         query.is_milestone(link);
 
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-    // Failed on block [363353] with block_internal_double_spend (!checked).
-    // The block object remains in scope during the call, but the pointer owns
-    // the memory. There is a local pointer object copied above, but it may be
-    // deleted once it is no longer referenced. This makes the block reference
-    // below weak. The message pointer object is also held by the calling
-    // closure, however it can be deleted by the compiler due to subsequent
-    // non-use. The copied block pointer holds the block object, but it may be
-    // also deleted if not referenced. Passing a block pointer into check,
-    // allowing block->check() to be called would resolve that issue, but not
-    // the issue of calling set_code (see below). But it's not clear how ~block
-    // could have occured here with query.set_code(*block) pending below.
-    // It hasn't failed when using a prevout table, which seems unrelated.
-    // But if it's a race to overwrite freed block memory, it's very possible.
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
     // 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
@@ -330,11 +315,6 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
             code == system::error::invalid_witness_commitment ||
             code == system::error::block_malleated)
         {
-            // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-            // These references to block didn't keep it in scope.
-            // But because block is const they could precede the check. Could
-            // be a compiler optimization as these are called by check(true).
-            // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
             LOGR("Uncommitted block [" << encode_hash(hash) << ":" << height
                 << "] from [" << authority() << "] " << code.message()
                 << " txs(" << block->transactions() << ")"
@@ -363,18 +343,6 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
     // Commit block.txs.
     // ........................................................................
 
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-    // For early segwit blocks this often fails to set the witness.
-    // There is no failure code, the witness is just empty and stored empty.
-    // That causes a validation failure with invalid_witness (!checked).
-    // It may have also failed in a way that invalidates confirmation queries.
-    // Passing a block pointer here would only hold the block in scope until
-    // it iterates transactions, with no subsequent reference. Then transaction
-    // pointers could hold themselves in scope, however block owns their memory
-    // and ~block() frees that memory even if the transaction pointer is alive.
-    // It hasn't failed when using a prevout table, which seems unrelated.
-    // But if it's a race to overwrite freed block memory, it's very possible.
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
     // This invokes set_strong when checked. 
     if (const auto code = query.set_code(*block, link, checked))
     {
@@ -385,17 +353,11 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
         return false;
     }
 
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-    // TODO: verify that ~block() free during contained object access is the
-    // problem by switching to the default memory allocator w/non-prevout run
-    // and the code below disabled.
-    // TODO: pass shared_ptr in to check(block) and set_code(block) from here.
-    // This may guarantee the pointer (vs. block&) lifetime until return.
+    // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
     // This is an attempt to keep the shared pointer in scope.
     // Given that block is const this could also be reordered prior to check().
     // But this is not called in this scope, only by message deserialization.
-    // Passing the block pointer through the store
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+    // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
     if (!block->is_valid())
     {
         stop(fault(error::protocol2));
@@ -411,10 +373,10 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
     notify(ec, chase::checked, height);
     fire(events::block_archived, height);
 
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+    // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
     // block->serialized_size may keep block in scope during set_code above.
     // However the compiler may reorder this calculation since block is const.
-    // ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+    // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
     count(block->serialized_size(true));
     map_->erase(it);
     if (is_idle())