Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/bitcoin/node/chasers/chaser_validate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 10 additions & 9 deletions src/chasers/chaser_confirm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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());
Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
27 changes: 13 additions & 14 deletions src/chasers/chaser_validate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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());

Expand All @@ -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_))
Expand Down
9 changes: 5 additions & 4 deletions src/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
46 changes: 4 additions & 42 deletions src/protocols/protocol_block_in_31800.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() << ")"
Expand Down Expand Up @@ -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))
{
Expand All @@ -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));
Expand All @@ -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())
Expand Down
Loading