Repository navigation
backport: bitcoin#25494 index interface decoupling - #57
Closed
PastaPastaPasta wants to merge 9 commits into
Closed
PastaPastaPasta wants to merge 9 commits into
PastaPastaPasta wants to merge 9 commits into
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 |
PastaPastaPasta
added a commit
that referenced
this pull request
Oct 29, 2026
… partial #59 0b33fee Merge bitcoin-core/bitcoin-maintainer-tools#92: github-merge: Always include repo_from in the merge commit message (pasta) fe057ee partial bitcoin-core/bitcoin-maintainer-tools#59: github-merge: Format pull_reference in one place (pasta) Pull request description: ## Issue being fixed or feature implemented `contrib/devtools/github-merge.py` writes merge subjects as `Merge #N: ...` no matter which repo the PR came from. The v23.1.9 merge-back (dashpay#7961) brings in merges made in dashpay/dash-internal, such as `Merge #2: fix(consensus): reject negative transaction nVersion after DIP3`. Next to public merges like `Merge dashpay#7486: ...` in the same history, there is no way to tell that `#2` is a dash-internal PR rather than dashpay#2. Bitcoin Core fixed this after `github-merge.py` moved to bitcoin-core/bitcoin-maintainer-tools (bitcoin#16625, which Dash never took). That is why their history reads `Merge bitcoin/bitcoin#N` / `Merge bitcoin-core/gui#N`. Dash's copy of the script never picked up the change. ## What was done? Backports two bitcoin-core/bitcoin-maintainer-tools commits to Dash's `github-merge.py`: - partial bitcoin-core/bitcoin-maintainer-tools#59 (commit 6e05dc8 only): format the PR reference in one place (`pull_reference`) and use it in the merge subject, the merge-details header and the error messages. The rest of #59 (`push_mirrors`) is unrelated and is omitted. Dash's `print_merge_details()` has no `message` argument, so only its first parameter changes. - bitcoin-core/bitcoin-maintainer-tools#92 (commit 3811ce1): always include the repo in the reference. Upstream uses `repo_from`, which comes from bitcoin-core/bitcoin-maintainer-tools#57 (`--repo-from` for monotree fetches). Dash doesn't have #57, so this uses `repo` (`githubmerge.repository`), which is what `repo_from` defaults to upstream. No Dash-specific adaptation is needed for dash-internal. When run in a checkout with `githubmerge.repository = dashpay/dash-internal`, merges come out as `Merge dashpay/dash-internal#N`. Before / after: ``` Merge #2: fix(consensus): reject negative transaction nVersion after DIP3 Merge dashpay/dash-internal#2: fix(consensus): reject negative transaction nVersion after DIP3 ``` ## How Has This Been Tested? Ran the patched script in a throwaway clone against open PRs and answered `x` at the sign prompt, so nothing was committed or pushed. The `git commit` call was wrapped to print the generated subject: - `githubmerge.repository = dashpay/dash`, PR 7962 → `Merge dashpay#7962: ci: download Docker release binaries with the workflow token` - `githubmerge.repository = dashpay/dash-internal`, PR 13 onto `v23.1.x` → `Merge dashpay/dash-internal#13: ci: let Docker release builds download binaries from a private repo` The merge-details header printed `dashpay#7962 ... into develop` and `dashpay/dash-internal#13 ... into v23.1.x`. ## Breaking Changes None. Only the merge commit subject format changes, to `Merge <owner>/<repo>#N`, which matches the `merge-script` merges already on develop. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: d014624d1b0d47dcf0040d2e76b4bfaadb3f878840acb11ec1af0c476cc97e97463be016c0937aa2229a7b45e4cc35b718681115c643c55a198afa3cbaf09cb0
Owner
Author
|
Superseded by merged dashpay#7462 (bitcoin#25494 index interface decoupling), followed by dashpay#8042. 🤖 Posted autonomously by Codex on behalf of pasta. |
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
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: