backport: assumeutxo M3 — crash-safe background completion (bitcoin#25740, #27862, #28050, #27746)#52
Open
PastaPastaPasta wants to merge 17 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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 |
4 tasks
PastaPastaPasta
force-pushed
the
assumeutxo/m2-evodb-roles
branch
from
552e404 to
2a37be9
Compare
PastaPastaPasta
force-pushed
the
assumeutxo/m3-background-completion
branch
from
32d5966 to
4f9d716
Compare
This is an early partial pick of bitcoin#27596; the remainder will be backported later. Upstream commit: c6af23c
Pass the validating chainstate through special transaction and quorum commitment processing instead of borrowing the active chainstate. Interpret mined-commitment records and quorum resolution relative to the caller's chain. The cached values remain reusable, but chain membership is reevaluated across reorgs and chainstates while public non-validation callers retain active-chain semantics. This prevents snapshot-seeded records from suppressing commitments or satisfying MNHF and asset-unlock quorum lookups during background validation. Add dual-chainstate coverage for a commitment seeded at a block not yet contained by the background chain, including HasQuorum and GetQuorum cache-order checks.
Emit block, tip, deterministic masternode-list, UI, and flush notifications only for the active chainstate. In particular, suppressing background ChainStateFlushed prevents a background locator from regressing wallet best-block state. Keep BlockChecked ungated because its subscribers are mining/block-submit and peer validation/relay accounting; it does not reach CMNAuth. Document all 21 B3 call-site dispositions and extend the dual-chainstate test with validation-interface and UI counters.
Check local block-data availability before building masternode-list diffs and quorum rotation info. Treat failures caused by pruning or an unvalidated snapshot base like pruned getdata: log and silently drop the plausible request without increasing the peer's misbehavior score. Malformed and implausible requests retain the pre-existing penalties.
Disable DKG participation and quorum signing until snapshot background validation completes. Enforce the refusal at CreateSigShare, the actual share-production boundary, so direct RPC, async, and queued signing paths cannot bypass it. The quorum sign RPC now returns a clear JSON-RPC error for both submit modes, and masternode status exposes the disabled participation state. Add unit coverage for the shared production-gate predicate across snapshot activation.
PastaPastaPasta
force-pushed
the
assumeutxo/m2-evodb-roles
branch
from
2a37be9 to
77a5910
Compare
…hen renaming chainstates
Discard Dash snapshot lifecycle markers only after the invalid snapshot directory rename succeeds. If the rename fails, preserving the markers keeps the existing restart recovery state recognizable while the upstream rename error is propagated to the fatal shutdown message.
Keep Dash’s mock shutdown callback while asserting the expected fatal diagnostic. The default callback reaches StartShutdown(), whose unit-test guard aborts the process before this Dash test can complete.
a733dd7 Remove unused function `reliesOnAssumedValid` (Suhas Daftuar) d4a11ab Cache block index entry corresponding to assumeutxo snapshot base blockhash (Suhas Daftuar) 3556b85 Move CheckBlockIndex() from Chainstate to ChainstateManager (Suhas Daftuar) 0ce805b Documentation improvements for assumeutxo (Ryan Ofsky) 768690b Fix initialization of setBlockIndexCandidates when working with multiple chainstates (Suhas Daftuar) d43a1f1 Tighten requirements for adding elements to setBlockIndexCandidates (Suhas Daftuar) d0d40ea Move block-storage-related logic to ChainstateManager (Suhas Daftuar) 3cfc753 test: Clear block index flags when testing snapshots (Suhas Daftuar) 272fbc3 Update CheckBlockIndex invariants for chains based on an assumeutxo snapshot (Suhas Daftuar) 10c0571 Add wrapper for adding entries to a chainstate's block index candidates (Suhas Daftuar) 471da5f Move block-arrival information / preciousblock counters to ChainstateManager (Suhas Daftuar) 1cfc887 Remove CChain dependency in node/blockstorage (Suhas Daftuar) fe86a7c Explicitly track maximum block height stored in undo files (Suhas Daftuar) Pull request description: This PR proposes a clean up of the relationship between block storage and the chainstate objects, by moving the decision of whether to store a block on disk to something that is not chainstate-specific. Philosophically, the decision of whether to store a block on disk is related to validation rules that do not require any UTXO state; for anti-DoS reasons we were using some chainstate-specific heuristics, and those have been reworked here to achieve the proposed separation. This PR also fixes a bug in how a chainstate's `setBlockIndexCandidates` was being initialized; it should always have all the HAVE_DATA block index entries that have more work than the chain tip. During startup, we were not fully populating `setBlockIndexCandidates` in some scenarios involving multiple chainstates. Further, this PR establishes a concept that whenever we have 2 chainstates, that we always know the snapshotted chain's base block and the base block's hash must be an element of our block index. Given that, we can establish a new invariant that the background validation chainstate only needs to consider blocks leading to that snapshotted block entry as potential candidates for its tip. As a followup I would imagine that when writing net_processing logic to download blocks for the background chainstate, that we would use this concept to only download blocks towards the snapshotted entry as well. ACKs for top commit: achow101: ACK a733dd7 jamesob: reACK a733dd7 ([`jamesob/ackr/27746.5.sdaftuar.rework_validation_logic`](https://github.com/jamesob/bitcoin/tree/ackr/27746.5.sdaftuar.rework_validation_logic)) Sjors: Code review ACK a733dd7. ryanofsky: Code review ACK a733dd7. Just suggested changes since the last review. There are various small things that could be followed up on, but I think this is ready for merge. Tree-SHA512: 9ec17746f22b9c27082743ee581b8adceb2bd322fceafa507b428bdcc3ffb8b4c6601fc61cc7bb1161f890c3d38503e8b49474da7b5ab1b1f38bda7aa8668675
Preserve ChainLock candidate exclusions in the new admission wrapper and keep Dash background-notification and EvoDB fixtures consistent with the tightened multi-chainstate candidate invariants.
PastaPastaPasta
force-pushed
the
assumeutxo/m3-background-completion
branch
from
4f9d716 to
f08bce5
Compare
PastaPastaPasta
force-pushed
the
assumeutxo/m2-evodb-roles
branch
from
77a5910 to
56cd37a
Compare
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
M2 (#51) wired up dual-chainstate EvoDB roles but left two integration
items open: promoting snapshot markers to the legacy key at completion, and
an independent (non-shared-cache) comparison of background vs. snapshot
state before a background chainstate is allowed to complete and replace the
snapshot. Without crash-safe completion, a power loss mid-promotion can
destroy a valid, already-validated snapshot. This is milestone 3/7. The
execution plan and per-unit ledger (M3 section) are tracked outside the repo
in a private gist, not checked in: https://gist.github.com/PastaPastaPasta/aa52b1f89fb74a0566ba3b5e15afab6a
PR 3/7 in the stacked series — base is M2 (
assumeutxo/m2-evodb-roles).What was done?
bitcoin#27862(moved here from M1 since it depends on M3'sSnapshotCompletionResult) andbitcoin#28050—FAILURE_FATALbypassesthe reindex prompt in Dash's init
else-ifstructure;FAILURE_INSUFFICIENT_DBCACHEintentionally absent untilbitcoin#27596lands in M5.
bitcoin#25740— crash-safe background-validation completion. Round 1adversarial review found four issues: crash-unsafe promote-before-swap
ordering (including missing fsync gaps), MN-list comparison that wasn't
actually independent of the shared cache, a
nulloptbase-height skip thatcould mask on-disk corruption, and unsynced activation metadata — all
fixed. Dash adaptation: crash-safe completion lifecycle (renames +
DirectoryCommitbefore a synced, idempotentPromoteSnapshotMarkers;RecoverSnapshotCleanupstate machine run at load); independent MN-listverification via a per-connect NORMAL-identity captured hash slot compared
against the activation-recorded snapshot hash at completion (not read from
shared cache). Both M2 integration TODOs (marker promotion, holistic
base-state comparison) are discharged here.
bitcoin#27746(13 upstream commits) — per-chainstate candidate admission.Round 1 review found
ResetBlockFailureFlagsbypassed per-chainstateadmission (background could admit non-base-path forks and starve the
active set) and that
InvalidateBlock's cache could reinsertChainLock-conflicting candidates — both fixed, including a
BLOCK_CONFLICT_CHAINLOCKexclusion inTryAddBlockIndexCandidateand adual-chainstate regression test.
How Has This Been Tested?
make check: exit 0, 0 failures.feature_init,feature_reindex,feature_pruning,feature_dip3_deterministicmns×2,feature_llmq_signing×2,feature_llmq_rotation,feature_llmq_chainlocks,feature_mnehf,feature_asset_locks,rpc_invalidateblock— all 12 passed.milestone gate PASS on 2026-07-11.
Breaking Changes
None. Background validation completion is now fully wired (UTXO-hash +
MN-list-hash + EvoDB-marker checks, crash-safe promotion/cleanup with
load-time recovery) but is not yet reachable —
loadtxoutsetitself isstill unexposed until M5.
Checklist: