highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35738: coins: parallel input prevout fetching followups

Browse Source 
8e4b7ab725 fuzz: use per-level fetch scopes in coinscache_sim (Andrew Toth)
5292386b78 doc: improve CoinsViewOverlay documentation (Andrew Toth)
d552c52b08 coins: log error reason when prevout fetch submission fails (Andrew Toth)
2ffaa6e6a7 coins: delete Sync and SetBackend on CoinsViewOverlay (Andrew Toth)
330022993f coins: filter coinbase txid from parallel input fetching (Andrew Toth)

Pull request description:

  This addresses various follow-ups requested in https://github.com/bitcoin/bitcoin/pull/35295.

  - add the coinbase txid to the filter so inputs spending the coinbase are not fetched.
  - delete Sync and SetBackend from CoinsViewOverlay
  - various logging and documentation improvements
  - improve coinscache_sim fuzzing so we continue parallel fetching while more caches are added on to the cache stack

ACKs for top commit:
  optout21:
    reACK 8e4b7ab725
  l0rinc:
    ACK 8e4b7ab725
  sedited:
    ACK 8e4b7ab725

Tree-SHA512: 38001f96be6f893e2610bb81f379ecc0c40ffd39da5bfe1f5db47db1ef2f725d80ae3f9b5e25acd64e65013176ba3ba4e3e8585cb55420b2793845c292beda23
This commit is contained in:
merge-script
2026-09-05 13:51:19 +02:00
parent 4519933391 8e4b7ab725
commit f32bfb2593
5 changed files with 49 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/coins.cpp
View File

@@ -379,12 +379,13 @@ CCoinsViewCache::ResetGuard CoinsViewOverlay::StartFetching(const CBlock& block
Assert(m_inputs.empty());
Assert(m_input_head.load(std::memory_order_relaxed) == 0);
Assert(m_input_tail == 0);
if (const auto workers_count{m_thread_pool->WorkersCount()}; workers_count > 0) {
if (const auto workers_count{m_thread_pool->WorkersCount()}; workers_count > 0 && block.vtx.size() > 1) {
// Loop through the block inputs and set their prevouts in the queue.
// Filter inputs that spend outputs created earlier in the same block. These outputs will be created
// directly in the cache from the tx that creates them, so they will not be requested from a base view.
std::unordered_set<Txid, SaltedCoinsCacheHasher> earlier_txids;
earlier_txids.reserve(block.vtx.size());
earlier_txids.emplace(block.vtx[0]->GetHash());
for (const auto& tx : block.vtx | std::views::drop(1)) {
for (const auto& input : tx->vin) {
if (!earlier_txids.contains(input.prevout.hash)) m_inputs.emplace_back(input.prevout);
@@ -402,7 +403,7 @@ CCoinsViewCache::ResetGuard CoinsViewOverlay::StartFetching(const CBlock& block
// Submit can fail if a shared owner of the thread pool outside of this class calls Stop() or
// Interrupt() on a different thread after we call WorkersCount() above. In that case parallel
// fetching will not make progress, so we clear the inputs to fall back to single threaded fetching.
LogWarning("Failed to submit prevout fetch tasks; falling back to single-threaded fetching for this block.");
LogWarning("Failed to submit prevout fetch tasks (%s); falling back to single-threaded fetching for this block.", SubmitErrorString(futures.error()));
m_inputs.clear();
StopFetching(); // Assert nothing changed if we failed to start tasks.
}

34
src/coins.h
View File

@@ -592,12 +592,16 @@ public:
};
/**
* CCoinsViewCache subclass that asynchronously fetches most block input prevouts in parallel during ConnectBlock without
* mutating the base cache.
* CCoinsViewCache subclass that asynchronously fetches most block input prevouts in parallel during ConnectBlock
* without mutating the base cache. This is achieved by fetching coins from the base view using PeekCoin() instead of
* GetCoin(), so intermediate CCoinsViewCache layers are not filled.
*
* Only used in ConnectBlock to pass as an ephemeral view that can be reset if the block is invalid.
* It provides the same interface as CCoinsViewCache.
* It adds an additional StartFetching method to provide the block.
* Used during ConnectBlock() as an ephemeral, resettable top-level view that is flushed only on success, so invalid
* blocks don't pollute the underlying cache.
*
* While this class uses threads internally to fetch coins, externally it is only safe to call its methods from a
* single "main" thread. It assumes StartFetching, StopFetching, FetchCoinFromBase, Flush and Reset will all only be
* called from the main thread.
*
* When a block is passed to StartFetching, the inputs of the block are flattened into a vector of InputToFetch
* objects. StartFetching then submits worker tasks to a ThreadPool and keeps the returned futures alive until fetching
@@ -607,17 +611,16 @@ public:
* m_inputs vector at a time. Workers race to claim inputs, so they may fetch elements in any order. If the fetched
* index is greater than or equal to the size of m_inputs, no more inputs can be fetched and false is returned.
*
* The worker claims the InputToFetch at this index, fetches the coin from the base cache and moves it into the
* The worker claims the InputToFetch at this index, fetches the coin with base->PeekCoin() and moves it into the
* InputToFetch object. The ready flag is then set with a release memory order. This allows the ready flag to be
* used as a memory fence, guaranteeing the coin being written to the object will have happened before another
* thread tests the flag with an acquire memory order.
* This assumes all base->PeekCoin() paths are safe for concurrent readers and do not mutate lower cache layers.
* This assumes all base->PeekCoin() paths are safe for concurrent readers.
*
* When a coin is requested from the cache on the main thread and is not already in cacheCoins map, FetchCoinFromBase
* checks whether the next unconsumed entry in m_inputs has the requested outpoint. On a match, m_input_tail is advanced
* and the entry's ready flag is waited on with an acquire memory order until a worker has finished fetching it. The
* coin is then moved out and returned. Since the main thread is the only consumer of validation results, it blocks
* on the specific input it needs rather than racing workers for other inputs.
* The main thread is the only consumer of the fetched coins. FetchCoinFromBase is called when a coin is requested on
* the main thread and is not already in the cache. It checks whether the next unconsumed entry in m_inputs has the
* requested outpoint. On a match, m_input_tail is advanced and the entry's ready flag is waited on with an acquire
* memory order until a worker has finished fetching it. The coin is then moved out and returned.
*
* StopFetching() is called in Flush() and in Reset() (the per-block teardown) so workers stop before the block they
* reference goes away. It stops fetching by moving m_input_head to the end of m_inputs (so workers quickly exit),
@@ -741,6 +744,9 @@ private:
std::vector<std::future<void>> m_futures{};
protected:
//! StopFetching must be called here for two reasons: InputToFetch objects hold references to the
//! block's outpoints, so they must not outlive the block being connected; and when connecting a
//! block fails, workers must not keep fetching inputs for the block that was abandoned.
void Reset() noexcept override
{
StopFetching();
@@ -769,6 +775,10 @@ public:
CCoinsViewCache::Flush(reallocate_cache);
}
//! Swapping the backend or writing through to it with Sync() is not supported while fetching.
void SetBackend(CCoinsView&) = delete;
void Sync() = delete;
//! Verify that all parallel fetched input prevouts have been consumed.
bool AllInputsConsumed() const noexcept { return m_input_tail == m_inputs.size(); }
};

2
src/test/coinsviewoverlay_tests.cpp
View File

@@ -202,6 +202,8 @@ BOOST_AUTO_TEST_CASE(access_non_input_coins)
main_cache.EmplaceCoinInternalDANGER(COutPoint{outpoint}, std::move(coin));
CoinsViewOverlay view{&main_cache, MakeStartedThreadPool()};
// The block has no non-coinbase transactions, so this fetches nothing and only creates the
// reset guard. All lookups below use the fallback path.
const auto reset_guard{view.StartFetching(block)};
// Non-input fallback hit.

4
src/test/fuzz/coins_view.cpp
View File

@@ -197,7 +197,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
coins_view_cache.Uncache(random_out_point);
},
[&] {
if (overlay) return; // // CoinsViewOverlay::SetBackend() is never called in production code
if (overlay) return; // CoinsViewOverlay::SetBackend() is never called in production code
const bool use_original_backend{fuzzed_data_provider.ConsumeBool()};
if (use_original_backend && backend_coins_view != original_backend) {
// FRESH flags valid against the empty backend may be invalid
@@ -375,7 +375,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
// If HaveCoin on the backend is true, it must also be on the cache if the coin wasn't spent.
std::optional<Coin> coin_in_backend;
bool exists_using_have_coin_in_backend;
if (dynamic_cast<CoinsViewOverlay*>(&coins_view_cache)) {
if (overlay) {
// PeekCoin does not mutate cacheCoins, so async workers can keep running.
coin_in_backend = backend_coins_view->PeekCoin(random_out_point);
exists_using_have_coin_in_backend = coin_in_backend.has_value();

34
src/test/fuzz/coinscache_sim.cpp
View File

@@ -226,8 +226,8 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
CoinsViewBottom bottom;
/** Real CCoinsViewCache objects. */
std::vector<std::unique_ptr<CCoinsViewCache>> caches;
/** Long-lived StartFetching guard (nullptr unless corresponding level is a CoinsViewOverlay). */
std::unique_ptr<OverlayFetchScope> overlay_fetch_scope;
/** Long-lived StartFetching guards, parallel to `caches` (entries are nullptr unless corresponding level is a CoinsViewOverlay). */
std::vector<std::unique_ptr<OverlayFetchScope>> fetch_scopes;
/** Simulated cache data (sim_caches[0] matches bottom, sim_caches[i+1] matches caches[i]). */
CacheLevel sim_caches[MAX_CACHES + 1];
/** Current height in the simulation. */
@@ -265,6 +265,12 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
}
};
/** Helper creating a fetch scope for the top cache (which must be a CoinsViewOverlay). */
const auto make_fetch_scope{[&] {
auto& overlay{static_cast<CoinsViewOverlay&>(*caches.back())};
return std::make_unique<OverlayFetchScope>(overlay, data.block);
}};
// Main simulation loop: read commands from the fuzzer input, and apply them
// to both the real cache stack and the simulation.
FuzzedDataProvider provider(buffer.data(), buffer.size());
@@ -275,8 +281,10 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
// Make sure there is always at least one CCoinsViewCache.
if (caches.empty()) {
caches.emplace_back(new CCoinsViewCache(&bottom, /*deterministic=*/true));
fetch_scopes.emplace_back();
sim_caches[caches.size()].Wipe();
}
assert(caches.size() == fetch_scopes.size());
// Execute command.
CallOneOf(
@@ -403,17 +411,13 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
[&]() { // Add a cache level (if not already at the max).
if (caches.size() != MAX_CACHES) {
if (overlay_fetch_scope) {
overlay_fetch_scope.reset();
sim_caches[caches.size()].Wipe();
}
// Apply to real caches.
if (provider.ConsumeBool()) {
caches.emplace_back(new CCoinsViewCache(&*caches.back(), /*deterministic=*/true));
fetch_scopes.emplace_back();
} else {
caches.emplace_back(new CoinsViewOverlay(&*caches.back(), g_thread_pool, /*deterministic=*/true));
auto& overlay{static_cast<CoinsViewOverlay&>(*caches.back())};
overlay_fetch_scope = std::make_unique<OverlayFetchScope>(overlay, data.block);
fetch_scopes.emplace_back(make_fetch_scope());
}
// Apply to simulation data.
sim_caches[caches.size()].Wipe();
@@ -423,7 +427,7 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
[&]() { // Remove a cache level.
// Apply to real caches (this reduces caches.size(), implicitly doing the same on the simulation data).
caches.back()->SanityCheck();
overlay_fetch_scope.reset();
fetch_scopes.pop_back();
caches.pop_back();
},
@@ -440,7 +444,7 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
},
[&]() { // Sync.
if (overlay_fetch_scope) return; // CoinsViewOverlay::Sync() is never called in production
if (fetch_scopes.back()) return; // CoinsViewOverlay::Sync() is never called in production
// Apply to simulation data (note that in our simulation, syncing and flushing is the same thing).
flush();
// Apply to real caches.
@@ -450,10 +454,9 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
[&]() { // Reset.
sim_caches[caches.size()].Wipe();
// Apply to real caches. Optionally start fetching again.
if (overlay_fetch_scope && provider.ConsumeBool()) {
overlay_fetch_scope.reset();
auto& overlay{static_cast<CoinsViewOverlay&>(*caches.back())};
overlay_fetch_scope = std::make_unique<OverlayFetchScope>(overlay, data.block);
if (fetch_scopes.back() && provider.ConsumeBool()) {
fetch_scopes.back().reset(); // Stop fetching before starting again.
fetch_scopes.back() = make_fetch_scope();
} else {
(void)caches.back()->CreateResetGuard();
}
@@ -514,4 +517,7 @@ FUZZ_TARGET(coinscache_sim, .init = [] { static auto setup{MakeNoLogFileContext<
assert(realcoin->nHeight == sim->second);
}
}
// Tear down the fetch scopes top down. Otherwise lower level could reset while upper level is reading from it.
while (!fetch_scopes.empty()) fetch_scopes.pop_back();
}