From ae34ef75371bee06b791714f9fbdb2147d1fd963 Mon Sep 17 00:00:00 2001 From: BOB450 Date: Thu, 30 Jul 2026 15:50:07 -0600 Subject: [PATCH 1/3] Fix compact filter confirmation race Fixes race that would cause a full node stall when client filters were enabled. --- .../impl/query/consensus/consensus_forks.ipp | 9 +- test/query/consensus/consensus_forks.cpp | 90 +++++++++++++++++++ 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp index a17fd431e..db52c6d9f 100644 --- a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp +++ b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp @@ -125,7 +125,7 @@ header_states CLASS::get_validated_fork(size_t& fork_point, code ec{}; // Disable filter constraint if filtering is disabled. - ////const auto filter = filter_enabled(); + const auto filter = filter_enabled(); /////////////////////////////////////////////////////////////////////////// std::shared_lock interlock{ candidate_reorganization_mutex_ }; @@ -134,9 +134,10 @@ header_states CLASS::get_validated_fork(size_t& fork_point, auto height = add1(fork_point); auto link = to_candidate(height); - // Filter body always written before validated, so the check is redundant. - while (is_block_validated(ec, link, height, top_checkpoint) - /*&& (!filter || is_filtered_body(link))*/) + // Checkpoint and milestone bypass considers association sufficient, but + // filter bodies are generated asynchronously and must precede confirmation. + while (is_block_validated(ec, link, height, top_checkpoint) && + (!filter || ec != error::bypassed || is_filtered_body(link))) { out.emplace_back(link, ec); link = to_candidate(++height); diff --git a/test/query/consensus/consensus_forks.cpp b/test/query/consensus/consensus_forks.cpp index f0854c909..4d4112657 100644 --- a/test/query/consensus/consensus_forks.cpp +++ b/test/query/consensus/consensus_forks.cpp @@ -22,4 +22,94 @@ BOOST_FIXTURE_TEST_SUITE(query_consensus_tests, test::directory_setup_fixture) +BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__bypassed_filters_pending__stops_at_first_missing) +{ + settings settings{}; + settings.path = TEST_DIRECTORY; + test::chunk_store store{ settings }; + test::query_accessor query{ store }; + BOOST_REQUIRE(!store.create(test::events_handler)); + BOOST_REQUIRE(query.initialize(test::genesis)); + BOOST_REQUIRE(query.filter_enabled()); + BOOST_REQUIRE(query.set(test::block1, + database::context{ 0, 1, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block2, + database::context{ 0, 2, 0 }, false, false)); + + const auto link1 = query.to_header(test::block1_hash); + const auto link2 = query.to_header(test::block2_hash); + BOOST_REQUIRE(query.push_candidate(link1)); + BOOST_REQUIRE(query.push_candidate(link2)); + + size_t fork_point{}; + auto fork = query.get_validated_fork(fork_point, 2); + BOOST_REQUIRE_EQUAL(fork_point, 0u); + BOOST_REQUIRE(fork.empty()); + + BOOST_REQUIRE(query.set_filter_body(link1, test::block1)); + fork = query.get_validated_fork(fork_point, 2); + BOOST_REQUIRE_EQUAL(fork.size(), 1u); + BOOST_REQUIRE(fork.front().link == link1); + BOOST_REQUIRE(fork.front().ec == error::bypassed); + + BOOST_REQUIRE(query.set_filter_body(link2, test::block2)); + fork = query.get_validated_fork(fork_point, 2); + BOOST_REQUIRE_EQUAL(fork.size(), 2u); + BOOST_REQUIRE(fork.front().link == link1); + BOOST_REQUIRE(fork.back().link == link2); + BOOST_REQUIRE(fork.front().ec == error::bypassed); + BOOST_REQUIRE(fork.back().ec == error::bypassed); +} + +BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__filters_disabled__bypass_unchanged) +{ + settings settings{}; + settings.path = TEST_DIRECTORY; + settings.filter_tx.buckets = 0; + test::chunk_store store{ settings }; + test::query_accessor query{ store }; + BOOST_REQUIRE(!store.create(test::events_handler)); + BOOST_REQUIRE(query.initialize(test::genesis)); + BOOST_REQUIRE(!query.filter_enabled()); + BOOST_REQUIRE(query.set(test::block1, + database::context{ 0, 1, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block2, + database::context{ 0, 2, 0 }, false, false)); + + const auto link1 = query.to_header(test::block1_hash); + const auto link2 = query.to_header(test::block2_hash); + BOOST_REQUIRE(query.push_candidate(link1)); + BOOST_REQUIRE(query.push_candidate(link2)); + + size_t fork_point{}; + const auto fork = query.get_validated_fork(fork_point, 2); + BOOST_REQUIRE_EQUAL(fork_point, 0u); + BOOST_REQUIRE_EQUAL(fork.size(), 2u); + BOOST_REQUIRE(fork.front().link == link1); + BOOST_REQUIRE(fork.back().link == link2); + BOOST_REQUIRE(fork.front().ec == error::bypassed); + BOOST_REQUIRE(fork.back().ec == error::bypassed); +} + +BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__milestone_filter_pending__empty) +{ + settings settings{}; + settings.path = TEST_DIRECTORY; + test::chunk_store store{ settings }; + test::query_accessor query{ store }; + BOOST_REQUIRE(!store.create(test::events_handler)); + BOOST_REQUIRE(query.initialize(test::genesis)); + BOOST_REQUIRE(query.filter_enabled()); + BOOST_REQUIRE(query.set(test::block1, + database::context{ 0, 1, 0 }, true, false)); + + const auto link = query.to_header(test::block1_hash); + BOOST_REQUIRE(query.push_candidate(link)); + + size_t fork_point{}; + const auto fork = query.get_validated_fork(fork_point); + BOOST_REQUIRE_EQUAL(fork_point, 0u); + BOOST_REQUIRE(fork.empty()); +} + BOOST_AUTO_TEST_SUITE_END() From ff8a2711b98f18e27228e30882fdcd91f1dcc2ed Mon Sep 17 00:00:00 2001 From: BOB450 Date: Thu, 30 Jul 2026 21:16:45 -0600 Subject: [PATCH 2/3] Enforce filter readiness for confirmation --- .../impl/query/consensus/consensus_forks.ipp | 5 ++-- test/query/consensus/consensus_forks.cpp | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp index db52c6d9f..6dfc4b03d 100644 --- a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp +++ b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp @@ -134,10 +134,9 @@ header_states CLASS::get_validated_fork(size_t& fork_point, auto height = add1(fork_point); auto link = to_candidate(height); - // Checkpoint and milestone bypass considers association sufficient, but - // filter bodies are generated asynchronously and must precede confirmation. + // Confirmation requires a committed filter body when filtering is enabled. while (is_block_validated(ec, link, height, top_checkpoint) && - (!filter || ec != error::bypassed || is_filtered_body(link))) + (!filter || is_filtered_body(link))) { out.emplace_back(link, ec); link = to_candidate(++height); diff --git a/test/query/consensus/consensus_forks.cpp b/test/query/consensus/consensus_forks.cpp index 4d4112657..8eed04c38 100644 --- a/test/query/consensus/consensus_forks.cpp +++ b/test/query/consensus/consensus_forks.cpp @@ -61,6 +61,34 @@ BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__bypassed_filters_pendi BOOST_REQUIRE(fork.back().ec == error::bypassed); } +BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__validated_filter_pending__waits_for_body) +{ + settings settings{}; + settings.path = TEST_DIRECTORY; + test::chunk_store store{ settings }; + test::query_accessor query{ store }; + BOOST_REQUIRE(!store.create(test::events_handler)); + BOOST_REQUIRE(query.initialize(test::genesis)); + BOOST_REQUIRE(query.filter_enabled()); + BOOST_REQUIRE(query.set(test::block1, + database::context{ 0, 1, 0 }, false, false)); + + const auto link = query.to_header(test::block1_hash); + BOOST_REQUIRE(query.push_candidate(link)); + BOOST_REQUIRE(query.set_block_valid(link)); + + size_t fork_point{}; + auto fork = query.get_validated_fork(fork_point); + BOOST_REQUIRE_EQUAL(fork_point, 0u); + BOOST_REQUIRE(fork.empty()); + + BOOST_REQUIRE(query.set_filter_body(link, test::block1)); + fork = query.get_validated_fork(fork_point); + BOOST_REQUIRE_EQUAL(fork.size(), 1u); + BOOST_REQUIRE(fork.front().link == link); + BOOST_REQUIRE(fork.front().ec == error::block_valid); +} + BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__filters_disabled__bypass_unchanged) { settings settings{}; From 6e059a3bed920d4c10b409ff17acaef26d2cac88 Mon Sep 17 00:00:00 2001 From: BOB450 Date: Thu, 30 Jul 2026 21:28:26 -0600 Subject: [PATCH 3/3] Reverted to original fix. And applied style changes. --- .../impl/query/consensus/consensus_forks.ipp | 6 ++- test/query/consensus/consensus_forks.cpp | 43 +++---------------- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp index 6dfc4b03d..77fd96a07 100644 --- a/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp +++ b/include/bitcoin/database/impl/query/consensus/consensus_forks.ipp @@ -134,9 +134,11 @@ header_states CLASS::get_validated_fork(size_t& fork_point, auto height = add1(fork_point); auto link = to_candidate(height); - // Confirmation requires a committed filter body when filtering is enabled. + // Filter body always written before validated, but validated state is not + // written in the case of bypassed blocks, it's inferred. So for bypassed + // blocks the existence of the filter must be verified. while (is_block_validated(ec, link, height, top_checkpoint) && - (!filter || is_filtered_body(link))) + (!filter || ec != error::bypassed || is_filtered_body(link))) { out.emplace_back(link, ec); link = to_candidate(++height); diff --git a/test/query/consensus/consensus_forks.cpp b/test/query/consensus/consensus_forks.cpp index 8eed04c38..7a0ce33b0 100644 --- a/test/query/consensus/consensus_forks.cpp +++ b/test/query/consensus/consensus_forks.cpp @@ -31,10 +31,8 @@ BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__bypassed_filters_pendi BOOST_REQUIRE(!store.create(test::events_handler)); BOOST_REQUIRE(query.initialize(test::genesis)); BOOST_REQUIRE(query.filter_enabled()); - BOOST_REQUIRE(query.set(test::block1, - database::context{ 0, 1, 0 }, false, false)); - BOOST_REQUIRE(query.set(test::block2, - database::context{ 0, 2, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block1, database::context{ 0, 1, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block2, database::context{ 0, 2, 0 }, false, false)); const auto link1 = query.to_header(test::block1_hash); const auto link2 = query.to_header(test::block2_hash); @@ -61,34 +59,6 @@ BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__bypassed_filters_pendi BOOST_REQUIRE(fork.back().ec == error::bypassed); } -BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__validated_filter_pending__waits_for_body) -{ - settings settings{}; - settings.path = TEST_DIRECTORY; - test::chunk_store store{ settings }; - test::query_accessor query{ store }; - BOOST_REQUIRE(!store.create(test::events_handler)); - BOOST_REQUIRE(query.initialize(test::genesis)); - BOOST_REQUIRE(query.filter_enabled()); - BOOST_REQUIRE(query.set(test::block1, - database::context{ 0, 1, 0 }, false, false)); - - const auto link = query.to_header(test::block1_hash); - BOOST_REQUIRE(query.push_candidate(link)); - BOOST_REQUIRE(query.set_block_valid(link)); - - size_t fork_point{}; - auto fork = query.get_validated_fork(fork_point); - BOOST_REQUIRE_EQUAL(fork_point, 0u); - BOOST_REQUIRE(fork.empty()); - - BOOST_REQUIRE(query.set_filter_body(link, test::block1)); - fork = query.get_validated_fork(fork_point); - BOOST_REQUIRE_EQUAL(fork.size(), 1u); - BOOST_REQUIRE(fork.front().link == link); - BOOST_REQUIRE(fork.front().ec == error::block_valid); -} - BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__filters_disabled__bypass_unchanged) { settings settings{}; @@ -99,10 +69,8 @@ BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__filters_disabled__bypa BOOST_REQUIRE(!store.create(test::events_handler)); BOOST_REQUIRE(query.initialize(test::genesis)); BOOST_REQUIRE(!query.filter_enabled()); - BOOST_REQUIRE(query.set(test::block1, - database::context{ 0, 1, 0 }, false, false)); - BOOST_REQUIRE(query.set(test::block2, - database::context{ 0, 2, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block1, database::context{ 0, 1, 0 }, false, false)); + BOOST_REQUIRE(query.set(test::block2, database::context{ 0, 2, 0 }, false, false)); const auto link1 = query.to_header(test::block1_hash); const auto link2 = query.to_header(test::block2_hash); @@ -128,8 +96,7 @@ BOOST_AUTO_TEST_CASE(query_consensus__get_validated_fork__milestone_filter_pendi BOOST_REQUIRE(!store.create(test::events_handler)); BOOST_REQUIRE(query.initialize(test::genesis)); BOOST_REQUIRE(query.filter_enabled()); - BOOST_REQUIRE(query.set(test::block1, - database::context{ 0, 1, 0 }, true, false)); + BOOST_REQUIRE(query.set(test::block1, database::context{ 0, 1, 0 }, true, false)); const auto link = query.to_header(test::block1_hash); BOOST_REQUIRE(query.push_candidate(link));