Repository navigation
T.Softfork: consensus: 180-day rolling coinbase maturity from height 979919 until RDTS expiry - #468
BitcoinMechanic wants to merge 13 commits into
Conversation
Require 25920 blocks for mainnet rewards mined from height 973440 while their mining block's parent MTP precedes RDTS expiry. Preserve that requirement after expiry and use 100 blocks for newly mined rewards afterward. Apply the mining-time cutoff to relay and wallet maturity, report it through deployment RPCs, and use a separate validation marker so upgrades recheck inherited history. Cover expiry, depth boundaries, reorgs, wallet credit, reindexing, and legacy-marker upgrades in unit and functional tests. Document the expectation of continuing the rolling window, with future increases or decreases depending on network health.
Apply the 25920-block rule only to rewards mined from height 979919, with enforcement starting at the same height. Retain the existing 6480-block historical rule through spending height 979919 and its original release at 979920, rather than rewriting earlier consensus. Keep the legacy validation marker for that historical interval and use a new marker for the revised rolling schedule. Pre-activation rewards retain their original relay and wallet maturity instead of acquiring a retroactive 180-day policy restriction. Test the activation and historical-release boundaries, the first covered reward's full maturity, and pre-activation policy. Update the release notes to describe the revised activation and upgrade behavior.
BitcoinMechanic
changed the title
Move the rolling coinbase maturity snippet out of the archived release notes directory and use doc/release-notes-468.md, as required by the doc_release_note_snippets lint check.
The scheduled post-IBD sync wrote CoinsTip directly, without first persisting block data and the block index. A crash in a partial coins batch could therefore leave its new head absent from the block index, causing ReplayBlocks to fail with "reorganization to unknown block". This surfaced in the Windows extended feature_dbcrash CI test. Use ForceFlushStateToDisk so the existing ordered flush persists block and undo data and the block index before the coins database. Keep the scheduled readiness checks unchanged. Add a deterministic crash-recovery variant to the scheduling functional test. Force a crash during its coins write, restart without reindexing, and verify the recovered tip, UTXO count, and each coin output. The new test reproduces the CI error before the fix and passes afterward.
|
This is better than the 365 day and voiding stratum rewards discussion in the echo chamber currently. Is the intent here to void stratum rewards as well? If so, the only way I would agree to that concept is if all stratum rewards in history are voided. I believe the language in the previous release notes was, "unless you are running datum you are not mining." By that definition (if the broader community is in agreement on said definition), there have been almost no valid blocks mined since 2012. If we are going to void any stratum blocks mined since the proof of work change, to remain consistent and to maintain a valid chain, all stratum rewards in history should be void. Otherwise, if voiding rewards is outside of the scope of discussion for this PR, I am fine with 180 days. Rolling 45-90 seems better, particularly as the world's economies become more questionable, as individuals may find need of money due to external factors we have not accounted for. |
|
The intention to use a far longer maturation (180 days per this PR and/or the longer "one year" time frame (used as shorthand for "until the end of RDTS which happens on Sept 1st 2027)) remains. This PR provides a reasonable amount of time to organize retribution for malicious miners should it become necessary. Happy to include written intention to that effect or at least stress that the intention of doing that has already been expressed plenty and this PR does not intend to frustrate or undermine it. |
|
Okay. Fine with that. This is better than the 365 day CBM versions. |
|
Those would be more acceptable if they also added the rolling window approach. Having a year's worth of coinbase rewards become spendable all at once would create pointless issues for the sake of simpler code. I don't consider that a correct weighting of the pros/cons. |
|
Maybe. I think 365 days drastically underestimates the patience of the most malicious attackers. The attack on August 7, was likely far longer in the making than 365 days. |
|
If they are motivated to that level, then you need to go way further than anything increased CBM offers. Let's stay within the scope of what CBM can actually achieve. |
|
Concept ACK. 25920 blocks is a flat 180 days at the 10-minute target, ~4.9% carrying cost at 10% APR. #429's flag day holds each reward for expiry minus its own mining time, so the hold decays from 315 days for one mined today to near zero just before expiry, averaging half the window at ~157 days. It also releases every covered reward in one block, roughly 160,000 to 175,000 BTC depending on block rate. |
|
There is lots of will and left over expectation of a similar proposal that aims for "one year" instead of 180 days. As mentioned above, that "one year" is now about 315 days as that is how much time remains until RDTS expires, and moreover, the shorter time frame of 180 days should be satisfactory given the rolling window resulting in the encumbering of more "coins per day" overall than a crudely expiring "one year" approach where all encumbered coins become spendable immediately upon RDTS ending. (i.e some coins would have only been locked up for a few hours if created in blocks mined just prior to RDTS ending - whereas this proposal encumbers all newly mined coins for exactly 25,920 blocks). |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
Tested head c528017 (base 58398ba): a gcc build with lock-order checks, a sanitizer build, and a Berkeley DB build for the legacy wallet.
What it does. Rewards mined at 979919 or later each wait 25,920 blocks, about 180 days, and keep that depth after RDTS expires. Rewards mined once the parent median time past reaches expiry go back to 100.
What the title does not say. From 979920 the rewards mined from 973440 through 979918 need only the ordinary 100 blocks, exactly as in 29.4.2: 4,768 blocks and 14,911 BTC so far, about 20,260 BTC by 979918. #429 would hold those until 2027-09-01. The first commit applied the 180 days from 973440 and held every coinbase in relay for 25,920 blocks; the second commit reversed both. The description should say what the second commit reversed.
Tests. The six suites the description names pass, 44 cases and 2,608 assertions as stated; full ctest 146 of 146 with and without BDB; full functional suite 294 passed, 64 skipped, 0 failed without BDB and 343 passed, 15 skipped, 0 failed with it; the six suites and eight functional tests pass under AddressSanitizer and UBSan with nothing reported.
Mutation check. Eleven deliberate breaks of the rule. The unit test alone catches nine: six consensus, one marker, two policy. Two policy mutants survive every test in the PR: relay holding every coinbase for the long depth, which is what the first commit did and which regtest cannot observe, and the reorg filter using the flat long depth; a test for the second is in the gist.
Mixed binaries. Next to a 29.4.2 node on regtest: both agree through the old release height, split on the first depth-100 spend after it, and upgrading the 29.4.2 node's data directory rewinds 122 blocks, drops the bad block and ends on the PR node's tip. A node upgraded before the enforcement height does not rewind.
Fourth commit. A separate fix. Its crash test fails on the 29.4.2 code with "Unable to replay blocks" and passes here; the flush also empties the UTXO cache once at that point. Worth its own PR.
Notes. No testnet4 schedule, so the first live run of this rule is mainnet at 979919, about 2026-10-31. The regtest expiry option writes the RDTS expiry directly; with a blake2b activation height and no -rdtsexpiry it schedules RDTS as a side effect. Mutually exclusive with #429, which the rc1 spec still lists.
Scripts, mutants and logs: https://gist.github.com/jasonsopko/e8ea9e2d649fa44dd3d6256721d7949c
|
Concept ACK. I'd have preferred longer (i.e. 1 year CBM rolling), but if there's a split in the community, even if a minor one, I'm willing to support this so we can settle it and move on to other work. The 45-day rolling option Chris leaned toward didn't seem conservative enough to me. This is at least 4x longer, rolling, and covers every reward mined until RDTS expiry, which I think is a reasonable middle ground. Best regards, |
|
Too little time to evaluate this PR. Block height 979 919 will be in 11 days. Let's give us at least 30 days to evaluate it if the PR is not critical, please. |
|
@GregTonoski while I agree with you, that would mean the current temporary CBM SF would have already expired. |
I don't see any downsides of the possible expiry. Also, there aren't any coins from the period to void anyway. |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
Tested ACK c528017
Test results are in my review above; nothing has been pushed since. The two requests there, that the description say what the second commit reversed and that the fourth commit go in its own PR, are not blockers.
|
Concept ACK on the rolling shape. Context for where I'm coming from: I mine this chain independently — my own Knots node and my own DATUM gateway, no pool. Whatever depth ships here sits on my own paycheck, so I don't make the following argument lightly.
For the 979920 deadline itself: I support this PR shipping as-is. Raising the depth later is a strict tightening and therefore softfork-safe; getting a rolling design in before the flag day matters more than winning the depth argument this week. |
|
Concept ACK. Let's git ’er done. |
|
ACK |
Adapt kwsantiago’s FORCE_SYNC approach from PR bitcoinknots#441 to the 29.x should_write path. Preserve ordered database writes and crash recovery without evicting cached coins. Add unit and functional cache-retention checks.
Keep a covered coinbase spend and its child unmined at exact maturity, roll back one block, and require both to leave the mempool. Verify admission after restoring the tip. Replacing the reorg filter maturity with 100 makes this regression fail.
|
Concept ACK I would prefer the blocks starting from 973440 to be included, but I am willing to concede on that given the overall progress that was made. |
|
Concept ACK. Independently built and tested commit c528017 on Linux: 44 focused unit tests passed (2,608 assertions), plus 8 distinct functional scenarios covering rolling maturity, expiry/reorg behavior, upgrade revalidation, wallet reporting, and crash recovery. No blocking issues found during review. |
|
Acknowledgements of unresolved concerns/future plans: - @GregTonoski's concern that this is too hasty with blockheight 979919. We can delay things, the two trade-offs being 1) We miss the next release and 2) Maturity temporarily returns to 100 blocks. The first issue I find more significant than the second as maturity is progressively decreasing anyway per the crude design of the current 45 day test (currently only ~7 days for coins mined today). We have made the code more complex with the rolling window so thorough review is certainly warranted. Permit me some optimism in thinking the deadline can help get adequate review done in time? It will not be difficult to change the start height if we remain not ready. Further, I do not want to disrupt the current release cycle (already working on rc2 for next Knots) if it can be avoided. - Lack of follow-through on the original verbiage that committed to further encumbering coins locked for 45 days to one year (or in this case 180 days). This was a misunderstanding on my part, but something I'll argue for leaving as-is anyway. There was a (mistaken) impression that coins encumbered for 45 days during this trial period would become spendable at the end of it. While not enforced via existing code, it was laid out verbally in a prior release. So the understanding was incorrect but it is not the end of the world if it ends up not being the case that the coins do indeed become spendable again - which they do per the current code. One cost is potential downward pressure on the price which frankly I am unconcerned with and I don't think is the general motive behind these efforts anyway. Another cost is apparent indecision, lack of clarity in the community around direction. I don't believe it to be too charitable to consider that an artifact of decentralization and indeed, strict road maps that are adhered to by the letter imply a more centralized decision making process, or at least a conundrum where the path forward somehow becomes extremely obvious and is overwhelmingly accepted by everyone. That clearly wasn't going to happen here and is why we have ended up with the 180 day solution emerging as something that finally seems acceptable to enough people to make it over the finish line. In a practical sense, having 45 days of mined coins become spendable again is not a big issue in the scheme of things. The opportunistic mining we are trying to discourage will hopefully be sufficiently dis-incentivized by the introduction of the 180 day rolling CBM. The benefit of not modifying this PR to include already-mined coins since 973440 is that it would mean not having to increase the complexity of the code in a period where we have limited time. Attempting to do it anyway would likely result in the 45 day period ending before we can get this sufficiently reviewed and the current 45 CBM returning to 100 blocks/17 hours making everything spendable again, before we attempt to encumber all un-moved coinbase UTXOs with the new 180 day rolling CBM. That is frankly a complete mess and the likely outcome of trying to follow through on our original plan. The above reasons make me strongly against attempting to do this as it will likely fail. - At the end of this (when RDTS expires), make CBM fall back to 45 days, not 100 blocks: Again, this increases complexity requiring time we do not have, but more importantly, there is a strong desire not to re-litigate all this again in summer of next year (or ever - though I think that's unrealistic). Counter-intuitively, the fact that no one wants 100 block CBM will make it easier to come to an agreement about what to do next summer. The fact that some people are satisfied with 45 days means there will inevitably be a split instead and we will essentially have to have this entire conversation again. My hope is that 180 days rolling CBM simply works as intended and we can continue on with it per the current compromise that has emerged. That will be made potentially more challenging if we have a 45 day option with varying opinions on how organic any improvement has been over the trial of 180 day rolling CBM. Some will be unsatisfied and want a year, others will want to maintain 180 days, others will be satisfied with dropping it down to 45 days. Again, from what I have observed in the community the least supported option would be a drop to 100 blocks and thus that default won't serve as yet another thing to have to argue strongly for/against. It's self evidently not desirable to almost everyone, while 180 days has yet to demonstrate itself as something that will have the desired effect. It is also the shortest possible CBM to fall back to should some unknown issue present itself with lengthened CBM in general. Enshrining 45 days as a permanent rule with such a short testing period and requiring a hard fork to undo it is a little too "move fast and break things" for me. - Intentions going forward: My understanding is that the rolling 180 day CBM will be what we agree to continue should the mining landscape have continued to trend in the direction of miner-side template construction. We should see a reduction in "opportunistic", short-term mining by hashers disinterested in the network itself or its long term health. I do not know of any reliable heuristics that can be used to gauge these things, but they are discernable to some degree regardless. The path of least resistance come next summer I would like to suggest be the continuation of the 180 rolling CBM should things be in a good place. Lengthening/shortening/maintaining the time frame are all acceptable discussions to have and everyone should be aware that there are almost certainly going to be unknowns that we have yet to think of, which is why I believe this needed to be approached with a temporary soft fork in the first place. - Intention to "void" coins from malicious miners: Whilst laid out as motivation for the initial "one year" approach and somewhat accepted by virtue of the attached code being what activated on the network as the temporary 45 day CBM soft fork, there is some expectation laid out for this already. Indeed, @luke-jr has said that it's the point of extending CBM (I disagree - it is a potential tool that becomes available, and may be the most decisive in that regard, but it isn't the only one.) I have also observed objection to the notion, slippery slope arguments etc. Those are not unfounded and frankly I do not wish to increase the scope here and have this PR attempt to lay out intentions in that regard or thresholds that would establish some social-layer commitment to voiding coinbase outputs. I think it's appropriate to acknowledge that there is at least some intention that came in combination with the existing trial of extended CBM, and that extending CBM to multi-month time frames does at least allow for such retribution to realistically occur should the community ever decide that it is warranted. So without wanting to open this can of worms here, I think we should just leave it at that. 180 days is enough time to do this if it becomes suddenly a thing we want to do. I am not going to advocate for or against it here and support for this PR should not be seen as advocacy/objection to voiding any coinbase rewards in the future. |
|
Concept ACK on the rolling maturity extension as a temporary, checkable soft-fork rule that raises the cost of short-term hashing without rewriting already-accepted history. Regarding - Intention to "void" coins from malicious miners: I do not support any future mechanism that voids already-mined coinbases on the basis of an external list or human judgment. Nodes must be able to reach the same validity result from the blocks alone. Selecting specific prior outputs as unspendable reintroduces the trusted third party the protocol was designed to remove. A longer maturity window is a forward-looking rule. Retroactive invalidation of accepted coinbases is not. Support for this PR should not be read as acceptance of that path. EDIT below, because this matters: Removing the trusted third party and solving double-spending were not later interpretations of plebs about bitcoin after the protocol was introduced to the world. They are the problem statement and the proposed solution in the document that introduced the system (the white paper). They are therefore axiomatic to its stated purpose and design. Any subsequent rule that reintroduces an external party to determine the validity or spendability of already-accepted outputs stands in direct tension with that design. |
|
Concept ACK. A rolling window is clearly better than a one-time release when RDTS expires. My concern is that 180 days may not be long enough to move hashrate away from large farms and altcoin pools that mine only for quick cash and toward long-term plebs who value and hold what they mine. I'd like to see this treated as a floor, with the goal of reaching consensus before RDTS expires on extending the rolling CBM to at least one year. As a DATUM miner, I'm happy to wait longer for my rewards if it means a healthier, more decentralized hashrate distribution. |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
Tested ACK c414307
The only difference from 3395470, which I tested and approved on Oct 22, is test/functional/feature_dbcrash.py.
On Ubuntu 24.04 (x86_64), feature_dbcrash.py passes on c414307. With the new resubmit in restart_node removed, it fails when node 0 restarts after the forced crash, so the new step covers the fix.
Before rolling activation, report the historical activation, creation window, maturity and active status together. Switch to the rolling description for next-block height 979919. Add mainnet boundary, testnet4 expiry and unconfigured regtest checks, and clarify release-note reporting.
Expose an immutable network-policy maturity upper bound and return early once wallet depth exceeds it. Preserve the extra-confirmation margin. Test standard and long maturity boundaries, and require an old coinbase lookup to complete while another thread holds cs_main.
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
Tested ACK 8a4ca1b
Since my ACK on c414307, two commits change getdeploymentinfo and the wallet's coinbase maturity check. No consensus or relay code changed.
The wallet shortcut cannot change a result: its bound is the largest maturity the lookup can return, so any coin deeper than that was already counted as mature. Removing either change makes its new test fail.
On mainnet params at genesis, getdeploymentinfo now reports the 973440 rule (6480 blocks, height_end 979919), where c414307 reported the 979919 rule.
On Ubuntu 24.04 (x86_64), built with BDB, ctest and the full functional suite pass, except one p2p_blake2b_dns_immediate.py run where its single addpeeraddress did not land in addrman, the flake #428 fixes. It passed 50 of 50 reruns.
|
ACK 8a4ca1b. Both fixes look right, all tests pass, and each new test fails without its fix. |
|
Concept Re-ACK - Running DATUM mining tests with my Start9 Node. |
|
Concept ACK. |
|
Concept ACK, however, I'm more in favor for the 477 change. Overall, I think the rolling maturity 180 days is a good middle ground. It should slow things down a bit in regards the network growth which I also think is a good thing. Making it harder for the profit-only-minded people to participate in the network, having more breathing room for building new things and brainstorming how to fix the existing flaws in Bitcoin. The way I look at this, the CBM is yet another tool at our disposal, and we should try it at least once to see whether it can benefit the network decentralization. |
|
ACK |
|
Since 0237da5, getdeploymentinfo before 979919 shows the 973440 window (maturity 6480) instead of the scheduled rule. Could the scheduled 180-day rule be its own entry with active:false, so nodes can confirm what they will enforce before activation? |
|
To be clear on my position: I think we should let Long CBM expire after the 45 days, and revisit it if the situation changes. Or if there's consensus for a longer maturity outside of emergencies, address that in an ordinary softfork proposal separately. (same as Luke's position) I am planning not to deploy this PR on my node. Knots doesn't have to merge this PR. Knots software development should keep high standard. Anybody can fork it and experiment (in rush) at any quality level. |
|
I would like to mention again in here that #477 is a modification of two already existing lines in the code for this PR that makes consensus match existing policy deployed by #419 for #468, simple bugfix that should deploy. Without #477 in combination with #468 20,250BTC unlock simultaneously in consensus at the activation height of #468. This unlock is in conflict with the policy rules already in place. Policy already intends to do what #477 does but it is not enforceable without these two lines. (more lines in test, only two in the daemon.) I would like the bugfix in #468. If we are going to run this in production. |
After staying further away from the community discussion for the past 24 hours and taking more time to reflect on the situation; I am joining Greg in this position. Not because I don't think the 45 day CBM has been ineffective rather, because I believe it has. We are not allowing that change to fully sunset before assessing threats anew. This effectively disallows the ability to conduct proper evidence based fault-isolation and makes determining true success with any individual mitigating remedy impossible. While I do think an extended CBM is likely appropriate for the network at this stage in its lifecycle, jumping from 45 to 180 (etc.), is arbitrary without taking time to assess the risk of allowing the current CBM to sunset with its current expiry. This is therefore a removal of my prior Concept ACK (not a NACK). I appreciate the spirited debate within the community and appreciate those who disagree with dignity and mutual respect. |
|
Concept ACK of rCBM of 180 days. This PR is strictly worse than #478. It is however better than going back to 100 days. |
|
Concept ACK Had a long think over this. I get @GregTonoski arguments and it feels rushed. Haste is a bad advisor. OTOH, as a community, most have seen the positive impact: actually setting limits on fiat minded miners makes a difference. How much is hard to fathom. But the still fiat minded Alpha pool wants to keep to no more than 45 days (his PR) is a definite signal to me that it hurts where it matters and falling back to 100 blocks is the wrong signal. Alpha pool more or less ensures us that extending it beyond 45 days hurts fiat mining more. Now don't worry for AlphaPool: they only pay on maturity, they don't incur the cost at all... but he loves his mining community too and likes to help them out. I'm not here for the miners. In the end, they will only look at fiat, not bitcoin. 180 days seems a solid CBM to do a better job than 45 days seemingly did. |
|
Concept ACK |
|
Deployment note. From height 980019, a block spending a coinbase mined at or after 979919 at depth 100 can be accepted by 29.4.2 but rejected by 29.4.3. If a hashrate majority builds on that block, 29.4.3 nodes cannot follow the higher-work branch. Installing 29.4.3 later does not resolve this: the fixed start height requires it to reject the same historical spend. Could the release notes document detection and a tested recovery procedure? The deployment risk begins with the first eligible spend and depends on miner adoption. Miner adoption reduces that risk, but does not by itself resolve an existing split. |
Mainnet coinbase rewards mined at height 979919 or later should mature individually over 25,920 blocks (approximately 180 days) until RDTS expires on 1 September 2027. A reward mined before expiry must finish its full maturity period even when that period ends after expiry. Rewards mined at or after expiry return to 100-block maturity.
The expectation is a continuation of the 180-day rolling window, with the option to increase or decrease the window depending on the health of the network. This is the intended direction for a subsequent consensus decision; the code in this PR does not automatically extend coverage beyond the existing RDTS expiry.
This is an alternative to #467, based directly on
29.x-knotsat58398baf33e588779685ead478e6397bb28ed3d6, including #419.Consensus rule
RdtsExpiryTime(1819756800), requiresspend_height - coin_height >= 25920.The cutoff is derived from the ancestry of the branch being validated. It does not use the current wall clock, a predicted expiry height, the active chain while validating a side branch, or the candidate block's own MTP. No UTXO database format change is needed. The historical rule remains enforced on its original height schedule, including the one-block overlap at 979919.
Testnet4 keeps its existing height-based maturity schedule. Testnet3, signet, and default regtest are unchanged. Regtest can explicitly opt in using
-testcoinbasematuritylongexpiry, with the existing schedule'srelease - startsetting the test depth. When RDTS is also configured, the expiry values must agree.Relay, wallet, and reporting
Relay admission, reorg filtering, wallet credit/selection, and GUI maturity use 100 blocks for post-expiry rewards, 25,920 blocks for covered rewards mined from 979919, and the existing 6,480-block policy for older pre-activation rewards. The wallet's one-confirmation credit margin is retained. RPC category help now describes applicable maturity instead of a fixed 100 confirmations.
getdeploymentinfodescribes the historical 6,480-block schedule before rolling activation, including its active status and release height. For the rolling phase it reportscoinbase_expiry_timeinstead ofheight_end. The deployment remains active and GBT continues to advertiselong_coinbase_maturityafter expiry because old rewards remain covered.Upgrade behavior
Use a new
rolling_coinbase_maturity_180d_v2validation marker starting at 979919. Retain #419’slong_coinbase_maturitymarker for its unchanged historical rule. An existing valid historical marker avoids unnecessarily rechecking that history; nodes without it must still validate the historical rule. A marker from the earlier version of this PR cannot certify the revised schedule. Upgrades revalidate the new rule from 979919 even when the current tip is past expiry, using the existing chainstate revalidation framework. Its validation range has no fixed end height because pre-expiry rewards remain covered after expiry.A pruned node missing the block/undo data required for the rewind must reindex and redownload the missing history. Experimental rules that cached invalidity for blocks accepted by this proposal also require reindexing; the framework does not clear cached invalid blocks.
Validation
WERROR=ON,-O2 -g0, descriptor wallet and tests enabled; GUI, Berkeley DB, benchmarks, and ZeroMQ disabled.coinbase_maturity_tests,validation_chainstatemanager_tests,rdts_params_tests,wallet_tests,txvalidation_tests, andmempool_tests.git diff --check, passed. This is not a claim of a full pinned-CI lint run or GUI/BDB testing.CI crash-recovery fix
The Windows extended suite exposed a post-IBD scheduler bug: it called
CoinsTip().Sync()directly, allowing an interrupted UTXO batch to reference a block index entry that had never been persisted. Startup then failed withReplayBlocks(): reorganization to unknown block requested.Use the ordered
FlushStateToDisk(..., FORCE_SYNC)path, following @kwsantiago’s approach in #441 and adapted to this branch’sshould_writecondition, so block/undo files and the block index are written before the coins database while retaining cached UTXOs. This replaces the earlierForceFlushStateToDisk()fix, which unnecessarily emptied the cache. A new--crash-recoveryvariant offeature_sync_coins_tip_after_chain_sync.pyforces a crash during that scheduled write, then verifies normal restart, the recovered tip, and the exact coin outputs without reindexing. This regression reproduces the failure on the previous code and passes with the fix. The ordinary scheduling test remains enabled.Validation of the initial crash-recovery fix (c528017): warnings-as-errors build passed; all six selected functional cases passed, including the full 40-round
feature_dbcrash.pystress test (799 seconds), both scheduling variants, both rolling-maturity tests, and chainstate revalidation. The flush unit test passed all 1,002 assertions. Python lint with CI's pinned Ruff 0.5.5, release-note placement, whitespace, file/permission, and test-suite checks passed. These results are local Linux validation.Review follow-up
Validation of the review follow-up: warnings-as-errors build; 46 focused unit cases (3,616 assertions); all five targeted functional cases (both post-IBD scheduling variants, rolling maturity, rolling upgrade, and chainstate revalidation); pinned Python lint, release-note placement, whitespace, file/permission and test-suite checks all passed. Remote CI for the new head is pending.
NOTE: I attempted to address all community feedback in the comment below: #468 (comment)
Release-candidate alignment
Restore the exact tracked tree of
3395470f519cbe36797fe7e8dea07f7ed5c3c36e, the state used for the release candidate, using a single revert commit without rewriting published history. This removes #477’s historical 45-day extension, its subsequent policy correction, and the later DNS-test-only fix.The post-IBD ordered-write/cache-retention fixes and exact-depth mempool reorg test are already present in the restored version. The independent DNS test reliability fix (
1ef1119d1f) remains available for separate adoption; it is intentionally excluded to preserve exact release-candidate equivalence.Rollback commit:
24cbd56935. Git tree equality with3395470f51verified. Warnings-as-errors build, 46 focused unit tests (3,616 assertions), all five selected functional cases, and selected lint checks passed. Remote CI for the revert is pending.Crash-test delivery fix after RC alignment
Commit
c414307229changes onlytest/functional/feature_dbcrash.py. At c414307, production source remained identical to3395470f51; the review follow-ups below subsequently change reporting and wallet lookup performance only.Windows run 39977596916 crashed a node during a scheduled flush before the next block was delivered. Recovery restored height 300 correctly, but the unconnected test node then waited 9,600 seconds for a block it had never received. The test now passes the pending block to its recovery helper and resubmits it only if its header is unknown after restart. Known blocks must still recover without resubmission, and UTXO consistency comparisons remain enabled.
The test deterministically kills a node before initial block delivery to cover this path. The full 40-round stress test passed locally in 825 seconds, including the crash-during-recovery coverage assertion and final UTXO comparisons. Python, whitespace, file/permission and test-suite lint checks passed. Remote Windows CI for this fix is pending.
Reporting and wallet review follow-ups
0237da5824: report the historical coinbase maturity deployment before rolling activation, including consistent activation, release, maturity and active fields. The description switches to the rolling rule for next-block height 979919. Unit tests cover mainnet boundaries, testnet4 activation/release and unconfigured regtest.8a4ca1b223: skip the block lookup and cs_main acquisition for coinbase rewards whose wallet depth exceeds the maximum configured policy maturity. The upper bound includes ordinary, historical and rolling depths and comes from immutable network parameters. Preserve the extra-confirmation margin. Tests cover standard/long maturity boundaries and completion while another thread holds cs_main.Both regression tests fail when their respective fix is disabled. Final warnings-as-errors build passed; 55 targeted unit tests (727 assertions) and five functional cases passed (both rpc_blockchain transports, rolling maturity, rolling upgrade and descriptor wallet coinbase categories). The Berkeley DB wallet variant was skipped because this build lacks BDB. Selected Python, release-note, whitespace, file/permission and test-suite lint checks passed. Remote CI is pending. These follow-ups change neither consensus maturity nor wallet spendability.
AI usage: OpenAI's Astra did all this under my supervision.