From 6974f2be9dfcb9198810eba7927153efbb7acd4f Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 31 Jan 2025 17:27:27 -0500 Subject: [PATCH 1/3] Don't fire events for bypassed validate or confirm. --- .../bitcoin/node/chasers/chaser_validate.hpp | 4 ++-- src/chasers/chaser_confirm.cpp | 19 +++++++++--------- src/chasers/chaser_validate.cpp | 20 +++++++++++-------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/include/bitcoin/node/chasers/chaser_validate.hpp b/include/bitcoin/node/chasers/chaser_validate.hpp index d6dc0f066..83b7b7040 100644 --- a/include/bitcoin/node/chasers/chaser_validate.hpp +++ b/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; diff --git a/src/chasers/chaser_confirm.cpp b/src/chasers/chaser_confirm.cpp index 624afee3d..b8ad9282c 100644 --- a/src/chasers/chaser_confirm.cpp +++ b/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); + } } } diff --git a/src/chasers/chaser_validate.cpp b/src/chasers/chaser_validate.cpp index d2c64b8d8..f56cfc5e8 100644 --- a/src/chasers/chaser_validate.cpp +++ b/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()); @@ -318,8 +318,12 @@ void chaser_validate::complete_block(const code& ec, const header_link& link, } notify(ec, chase::valid, possible_wide_cast(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_)) From 9272c875a84285cf2f0d20f8bd329d896eb26656 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 31 Jan 2025 17:28:02 -0500 Subject: [PATCH 2/3] Comments on recent fixes, disable debugging halt. --- src/chasers/chaser_validate.cpp | 7 +--- src/protocols/protocol_block_in_31800.cpp | 46 ++--------------------- 2 files changed, 5 insertions(+), 48 deletions(-) diff --git a/src/chasers/chaser_validate.cpp b/src/chasers/chaser_validate.cpp index f56cfc5e8..c9c762981 100644 --- a/src/chasers/chaser_validate.cpp +++ b/src/chasers/chaser_validate.cpp @@ -305,14 +305,9 @@ 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; } diff --git a/src/protocols/protocol_block_in_31800.cpp b/src/protocols/protocol_block_in_31800.cpp index 428273b1a..7efe207ad 100644 --- a/src/protocols/protocol_block_in_31800.cpp +++ b/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()) From 53ca381005623d818252d441e6da19d8acfb2801 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 31 Jan 2025 18:04:11 -0500 Subject: [PATCH 3/3] Initial point table too big for mini, reduce to compressed size. --- src/parser.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/parser.cpp b/src/parser.cpp index 3e33f11e0..94e89c686 100644 --- a/src/parser.cpp +++ b/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(&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(&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",