node: move index ownership to NodeContext #35474

w0xlt commented at 6:01 PM on June 6, 2026: contributor

This removes the remaining process-global index ownership and stores node-owned indexes in NodeContext instead.

Indexes moved:

g_txindex
g_txospenderindex
g_coin_stats_index
g_filter_indexes

NodeContext now owns these index instances, while node.indexes remains a non-owning list used for common index startup, shutdown, and getindexinfo handling.

Behavior should be unchanged, apart from the key order in getindexinfo output. Indexes are still created from the same startup options, started in the same init step, stopped before destruction, and destroyed while validation_signals is still alive so they can unregister safely.

The block filter index registry is moved to node/indexes.{h,cpp} so index/ code does not depend on NodeContext. PeerManager receives a small lookup callback instead of reaching into global state.

Review notes:

The first four commits are small and mostly mechanical: move txindex, txospenderindex, and coinstatsindex ownership to NodeContext, then make getindexinfo use node.indexes. The first commit also adds custom deleters in node/indexes.cpp, so node/context.cpp does not include index headers, which would create circular dependencies.
The fifth commit adds a block filter index lookup callback to PeerManager::Options, still backed by the global registry.
The last commit is the main one to review carefully. It moves block filter index ownership to NodeContext, moves the block filter registry to node/indexes.{h,cpp}, and points the PeerManager lookup at NodeContext instead of global state.

DrahtBot commented at 6:01 PM on June 6, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35474.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	sedited, fjahr

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36447 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36447.svg"> (test: Misc fixes for pending validation events races by maflcko)
#36244 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36244.svg"> (validation, net: Process blocks asynchronously and reduce cs_main contention by w0xlt)
#36167 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36167.svg"> ([RFC] Enable -Wunused by fanquake)
#35642 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35642.svg"> (headersync: do parameter search at runtime by sipa)
#34132 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34132.svg"> (coins, dbwrapper: remove error catcher, make point-read failures fatal by l0rinc)
#31260 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/31260.svg"> (scripted-diff: Type-safe settings retrieval by ryanofsky)
#30437 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/30437.svg"> (ipc: add bitcoin-mine test program by ryanofsky)
#24230 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/24230.svg"> (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
#19461 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19461.svg"> (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19460.svg"> (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
#10102 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/10102.svg"> (Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

node::InitBlockFilterIndex(m_node, [&]{ return interfaces::MakeChain(m_node); }, BlockFilterType::BASIC, 1_MiB, true, false) in src/test/blockfilter_index_tests.cpp

2026-09-30 19:27:32

sedited commented at 7:48 PM on June 6, 2026: contributor

Concept ACK

sedited requested review from fjahr on Jun 9, 2026

Sjors commented at 8:22 AM on June 17, 2026: member

How does this interact with #24230?

w0xlt commented at 5:01 PM on June 17, 2026: contributor

#24230 still keeps process-global index ownership.

So this PR is mostly orthogonal to it: #24230 changes how indexes sync with the chain and removes index dependencies on node internals, while this PR moves index ownership into NodeContext.

There is no conceptual conflict. In terms of code there will be rebase conflicts after one or the other lands, since both touch the index/init/RPC/REST paths.

Removing process-global index ownership seems aligned with the multiprocess direction anyway.

fjahr commented at 2:41 PM on June 21, 2026: contributor

Concept ACK

willcl-ark added the label Refactoring on Jul 9, 2026

willcl-ark added the label UTXO Db and Indexes on Jul 9, 2026

DrahtBot added the label Needs rebase on Jul 14, 2026

sedited commented at 9:38 AM on August 17, 2026: contributor

Ping for rebase @w0xlt

w0xlt force-pushed on Aug 18, 2026

w0xlt commented at 10:00 PM on August 18, 2026: contributor

Rebased. Thanks @sedited

DrahtBot removed the label Needs rebase on Aug 18, 2026

DrahtBot added the label Needs rebase on Aug 21, 2026

w0xlt force-pushed on Sep 15, 2026

w0xlt commented at 11:03 PM on September 15, 2026: contributor

Reabsed

DrahtBot removed the label Needs rebase on Sep 16, 2026

node: move txindex ownership to NodeContext

Replace the g_txindex global with NodeContext::txindex and pass the
index to GetTransaction() explicitly instead of reading the global.

Index types are destroyed through deleters defined in node/indexes.cpp
because including index headers in node/context.cpp would create
circular dependencies (index/base.cpp includes node/context.h). These
can become plain unique_ptrs once index code no longer depends on
NodeContext.

bdaf67c2c8

node: move txospenderindex ownership to NodeContext 3df63408a0

node: move coinstatsindex ownership to NodeContext 6833bda571

rpc: get index info from NodeContext indexes

getindexinfo now iterates NodeContext::indexes instead of querying each
index type separately. The set of reported indexes is unchanged, but
when several indexes are enabled the keys in the result follow the
order in which the indexes are registered at startup.

3a1d8b3087

net_processing: get block filter index via PeerManager::Options

Add a get_block_filter_index callback to PeerManager::Options and use it
instead of calling the global GetBlockFilterIndex() directly. Callers
still pass the global lookup, so behavior is unchanged.

This lets the next commit move block filter index ownership without
net_processing depending on NodeContext.

39069ac64e

node: move block filter index ownership to NodeContext fc948ca8e4

in src/node/context.h:53 in 3c05b4eb83

  49 | @@ -49,6 +50,20 @@ namespace node {
  50 |  class KernelNotifications;
  51 |  class Warnings;
  52 |  
  53 | +// Keep NodeContext from depending on index modules while still owning indexes.

fjahr commented at 11:09 PM on September 26, 2026:

Are the custom deleters really needed? NodeContext already handles this for its other unique_ptr members via the destructor here: https://github.com/bitcoin/bitcoin/blob/ed7dd7cf4e1561a97edf72eba29a67b14e28c717/src/node/context.h#L95 Since node/context.cpp and index/*.cpp are both in bitcoin_node, I think you could just add the index includes there and use plain std::unique_ptr<TxIndex> etc.

w0xlt commented at 8:28 PM on September 30, 2026:

Thanks! Plain unique_ptrs would compile, but index/base.cpp includes node/context.h, so including the index headers in node/context.cpp introduces new circular dependencies:

index/base -> node/context -> index/blockfilterindex -> index/base
index/base -> node/context -> index/coinstatsindex -> index/base
index/base -> node/context -> index/txindex -> index/base
index/base -> node/context -> index/txospenderindex -> index/base

The deleters are defined in node/indexes.cpp to avoid that.

They can be dropped in a follow-up once index/base.cpp no longer needs node/context.h. #24230 already removes most of that dependency, leaving only the AbortNode() call in FatalErrorf(). Replacing that with a Chain method would allow plain unique_ptrs here and also remove the existing index/base -> node/context -> net_processing -> ... lint exception.

If you'd rather add these to EXPECTED_CIRCULAR_DEPENDENCIES for now, I'm happy to switch to plain unique_ptrs.

I've updated the code comment and commit message to make the reason clearer.

w0xlt force-pushed on Sep 30, 2026

w0xlt commented at 8:33 PM on September 30, 2026: contributor

PR description updated.

DrahtBot added the label Needs rebase on Oct 7, 2026

DrahtBot commented at 2:20 PM on October 7, 2026: contributor

🐙 This pull request conflicts with the target branch and needs rebase.