backport: bitcoin#25494 index interface decoupling#57
Draft
PastaPastaPasta wants to merge 9 commits into
Draft
Conversation
…otifications Add interfaces::BlockInfo so block notifications can pass block metadata through the high-level chain interface. Dash adaptation: preserve the existing mempool notification signatures, wallet batch/conflict handling, CoinJoin cache invalidation, and ChainLock/InstantSend notifications. Upstream commit: a0b5b4a
…to indexes Passing abstract Chain interface will let indexes run in separate processes. This commit does not change behavior in any way.
…ne function This commit does not change behavior in any way.
Replace overriden index Init() methods that use the best block CBlockIndex* pointer with pure CustomInit() callbacks that are passed the block hash and height. This gets rid of more CBlockIndex* pointer uses so indexes can work outside the bitcoin-node process. It also simplifies the initialization call sequence so index implementations are not responsible for initializing the base class. There is a slight change in behavior here since now the best block pointer is loaded and checked before the custom index init functions are called instead of while they are called.
Replace WriteBlock method with CustomAppend and pass BlockInfo struct instead of CBlockIndex* pointer This commit does not change behavior in any way.
Replace Rewind method with CustomRewind and pass block hashes and heights instead of CBlockIndex* pointers This commit does not change behavior in any way.
Replace CommitInternal method with CustomCommit and use interfaces::Chain instead of CChainState to generate block locator. This commit does not change behavior in any way, except in the (m_best_block_index == nullptr) case, which was added recently in bitcoin#24117 as part of an ongoing attempt to prevent index corruption if bitcoind is interrupted during startup. New behavior in that case should be slightly better than the old behavior (skipping the entire custom+base commit now vs only skipping the base commit previously) and this might avoid more cases of corruption.
Migrate Dash-specific address, timestamp, and spent indexes to the Chain-owned BaseIndex constructor and the BlockInfo/BlockKey append and rewind hooks introduced by bitcoin#25494.
Migrate Dash-specific index implementations and shared test setup to the Chain-owned BaseIndex APIs, preserve required locking, and update the expected circular dependency set.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Dash's later assumeutxo backports rely on
src/kernel/chain.{h,cpp}, but those files and the high-level index notification interfaces were originally introduced by bitcoin#25494. Backporting the complete upstream PR preserves the real file history and lets later backports contain only their own attributable hunks.What was done?
interfaces::BlockInfo,kernel::MakeBlockInfo, andkernel/chain.{h,cpp}.interfaces::Chain,BlockInfo, andBlockKeyAPIs.Upstream PR: bitcoin#25494
How Has This Been Tested?
make -j7./src/test/test_dash --run_test=blockfilter_index_tests,coinstatsindex_tests,txindex_teststest/functional/test_runner.py feature_addressindex.py feature_spentindex.py feature_timestampindex.py feature_index_prune.pytest/lint/lint-circular-dependencies.pygit diff --check origin/develop...HEADAll completed successfully on macOS arm64.
Breaking Changes
No externally visible breaking changes. Internal index constructors and extension hooks now use the high-level chain interfaces introduced upstream.
Checklist: