Repository navigation
feat: keep at least 4-days worth blocks after pruning #7861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Pruning | ||
| ------- | ||
|
|
||
| - Pruned nodes on mainnet, testnet and devnets now keep at least four days of | ||
| blocks (2304) below the tip instead of 288. Validation reads older blocks | ||
| from disk: the credit pool is rebuilt from its newest daily snapshot after a | ||
| restart, and rotated quorum membership reads the work blocks of earlier DKG | ||
| cycles. `pruneblockchain` clamps its height accordingly, and on testnet and | ||
| devnets pruning starts once the chain is longer than 3000 blocks | ||
| (previously 1000). Blocks that an earlier version already pruned are not | ||
| recovered; a node that fails to validate a block after upgrading needs | ||
| `-reindex`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| #include <test/util/setup_common.h> | ||
|
|
||
| #include <chainparams.h> | ||
| #include <consensus/amount.h> | ||
| #include <consensus/tx_check.h> | ||
| #include <consensus/validation.h> | ||
|
|
@@ -858,6 +859,17 @@ BOOST_FIXTURE_TEST_CASE(credit_pool_snapshot_persisted_after_transactionless_con | |
| BOOST_CHECK_EQUAL(snapshot.currentLimit, pool.currentLimit); | ||
| } | ||
|
|
||
| BOOST_FIXTURE_TEST_CASE(min_blocks_to_keep_covers_credit_pool_rebuild, BasicTestingSetup) | ||
| { | ||
| // A pool is rebuilt from a snapshot up to one period old, reading the block one window before | ||
| // each block since it. Regtest deliberately keeps Bitcoin Core's 288 and is not checked. | ||
| for (const auto& chain : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::DEVNET}) { | ||
| const auto params = CreateChainParams(*m_node.args, chain); | ||
| BOOST_CHECK_GE(params->MinBlocksToKeep(), | ||
| CCreditPoolManager::DISK_SNAPSHOT_PERIOD + params->CreditPoolPeriodBlocks()); | ||
|
Comment on lines
+868
to
+869
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If AGENTS.md reference: AGENTS.md:L137-L137 Useful? React with 👍 / 👎.
Comment on lines
+866
to
+869
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Extend pruning retention test to verify rotated LLMQ quorum depth This invariant checks only the 1152-block credit-pool requirement, so reducing retention to 1152 would still pass despite removing protection needed by rotated quorum reconstruction. GetAllQuorumMembers() reconstructs three previous quarters, and GetQuorumQuarterMembersBySnapshot() obtains each quarter's modifier through GetHashModifier(), which reads the cycle's work block from disk. Materializing older retained quorum cycles through ScanQuorums() extends that history beyond the credit-pool bound; a missing work block makes GetNonNullCoinbaseChainlock() return no signature and changes the modifier. Extend the existing per-network parameter scenario to cover the quorum-history requirement and the intended reorg margin, deriving the bound from the relevant LLMQ parameters rather than checking only the credit-pool window. source:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same as point 3 in Pasta's review, and I'm leaving it out of this PR. The test guards the bound this PR is about (credit-pool rebuild). 2304 was picked with the rotated-quorum depth in mind (the 1786 estimate plus a 288 reorg margin), but the exact depth of quorum-member reconstruction is not a single parameter expression I can derive from the code with confidence, so I'd rather not encode an unverified formula as a test. If someone works out the exact bound, a follow-up can add it next to this check.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm happy to defer the quorum-depth invariant to a follow-up that establishes the bound through the reconstruction and scanning paths, rather than encode an unverified formula in this PR's credit-pool test. Noted as intentionally deferred; I will not press it further here. |
||
| } | ||
| } | ||
|
|
||
| BOOST_AUTO_TEST_CASE(credit_pool_unlock_limit_v24) | ||
| { | ||
| // From v24 the limit is relative and net: 20% of the balance at the window start, never less | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On an upgraded pruned mainnet, testnet, or devnet datadir, the persistent
m_have_prunedflag can coexist with only the old 288-block window because this change does not restore deleted files. For explicit-checkblocksvalues between 289 and 2304, this comparison now suppresses the warning even thoughCVerifyDBstops at the missing data and startup treatsSKIPPED_MISSING_BLOCKSas success, so operators are not told that their requested verification was shortened; compare against the first actually stored block or retain the conservative old threshold for previously pruned datadirs.Useful? React with 👍 / 👎.