Repository navigation
Commit 39b5bbc
committed
Merge #7869: fix: let startup block replay handle quorum commitments and asset unlocks
74dc3e5 test: replay blocks with quorum commitments and asset unlocks after a crash (pasta)
f057817 fix: replay blocks under the BLS scheme they were connected with (pasta)
4585bee fix: give replayed blocks the chain view they were connected with (pasta)
Pull request description:
## Issue being fixed or feature implemented
When a node stops in the middle of a coins-database flush (crash, power loss, OOM kill), the next start rolls the unflushed blocks forward in `ReplayBlocks()`. That runs inside `LoadChainstate()` before `LoadChainTip()`, so the chain is still empty while `RollforwardBlock()` re-processes each block's special transactions and quorum commitments. Several of those checks read the chain, and two things go wrong on a chain with quorums:
- A block containing a mined quorum commitment aborts the node: `IsMiningPhase()` asserts `nHeight <= active_chain.Height() + 1`, and the chain height is -1. The existing "skip when replaying" branches in `CQuorumBlockProcessor` do not cover that assertion, so the node crashes again on every start until `-reindex-chainstate`.
- Asset-unlock and MNHF-signal transactions resolve their quorum against the chain, and with an empty chain they find none. Replay rejects the block (`bad-assetunlock-too-old-quorum`) and startup fails with "Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate."
Separately, `RollforwardBlock()` processes blocks under whatever BLS scheme the process-wide flag holds. At that point in startup it holds the legacy default, because `VerifyLoadedChainstate()` only sets it later. So the first replayed post-V19 block runs under the legacy scheme, and an asset unlock in that block fails signature verification (`bad-assetunlock-not-verified`) even with a chain in place. `ConnectBlock()` already sets the scheme for each block with `ScopedBLSLegacyScheme`. The replay path never got that guard.
The replay range covers every block since the previous coins flush, often hours of blocks. Mainnet mines a non-null quorum commitment roughly every half hour, so in practice this recovery always fails on mainnet and needs a full `-reindex-chainstate`. That costs masternodes hours offline.
## What was done?
- `ReplayBlocks()` sets the chain tip to the parent of each block before rolling it forward. Each replayed block then sees the same chain view `ConnectBlock()` saw when the block was first connected. The tip is moved to the parent before each block, not to the block after it, so the chain ends one block short of the new coins tip. `LoadChainTip()` then still runs its full path: it sets the tip and prunes the block-index candidates that `LoadBlockIndex()` added while the chain was empty.
- `RollforwardBlock()` establishes the block's BLS scheme the same way `ConnectBlock()` does. The guard is not committed: every replayed block sets its own scheme on entry, and `VerifyLoadedChainstate()` sets the scheme for the loaded tip afterwards.
Both changes only affect local crash recovery of blocks this node already fully validated. Block validation rules, serialization and the P2P protocol are unchanged.
The CbTx best-ChainLock check is not involved: replay passes `fCheckCbTxMerkleRoots=false`, so the coinbase payload checks are skipped there.
The `chainstate.m_chain.Tip() == nullptr` "skip when replaying" branches in `CQuorumBlockProcessor` are no longer reached during replay, because replay now runs with the same chain view as `ConnectBlock()`. They are left in place to keep this change small and can be removed in a follow-up.
## How Has This Been Tested?
New step `test_replay_after_crash` at the end of `feature_asset_locks.py`, which already has the platform quorum, evo masternodes and asset unlock helpers. It crashes the regular node mid-flush twice and checks the replay each time. Each cycle restarts the node with `-dbcrashratio=1 -dbbatchsize=1`, mines without flushing, and forces a flush (`gettxoutsetinfo`) that crashes the node after its first partial batch. The node is then restarted without peers and must log `Rolling forward` for every expected block, come back at the same tip, and have the same UTXO set hash as the other node.
- Cycle 1 mines a new platform quorum commitment and then an asset unlock (index 900) signed by a platform quorum. It also checks the new quorum is listed after the restart. The commitment block is replayed before the unlock.
- Cycle 2 starts from a clean flush with the quorum already mature and mines only an asset unlock (index 901), so the unlock is the first and only block rolled forward. No earlier replayed block resets the BLS scheme, so this cycle is what independently covers the `RollforwardBlock()` scheme guard.
After the second cycle the node is restarted normally and reconnected, so the step leaves the file as it found it.
Results (macOS arm64, `--enable-debug`, full `feature_asset_locks.py` run, about 3 minutes):
- With this PR: passes, with and without `--usecli`.
- With the two `src/validation.cpp` fixes reverted (the PR base): cycle 1 fails at the restart, with the node aborting during startup on the `IsMiningPhase()` assertion (`Assertion failed: (nHeight <= active_chain.Height() + 1), function IsMiningPhase, file blockprocessor.cpp`). The commitment block is replayed before the asset unlock block, so the test reaches that crash first.
- With only the chain-tip change reverted (BLS guard kept): cycle 1 fails with the same `IsMiningPhase()` assertion.
- With only the BLS scheme guard removed (chain-tip change kept): cycle 1 still passes, because the post-V19 blocks replayed before the unlock reset the global scheme as a side effect. Cycle 2 fails: startup aborts with "Unable to replay blocks" and the log shows `RollforwardBlock(DASH): ProcessSpecialTxsInBlock for block ... failed with bad-assetunlock-not-verified`.
Also ran `feature_dbcrash.py` and the unit suites `validation_chainstatemanager_tests`, `validation_chainstate_tests`, `validation_flush_tests`, `evo_cbtx_tests`, `evo_snapshot_tests`, plus `test/lint/lint-whitespace.py`.
## Breaking Changes
None.
## 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: 2a281674e6d8e48ec66f276ba7cb65869dc54c83fa1580214fb2441e275716de75a0bb564481e3e8db476c2114e9095c7f5be9548032955add476bd7c9279a022 files changed
Lines changed: 55 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5409 | 5409 | | |
5410 | 5410 | | |
5411 | 5411 | | |
| 5412 | + | |
| 5413 | + | |
| 5414 | + | |
5412 | 5415 | | |
5413 | 5416 | | |
5414 | 5417 | | |
| |||
5507 | 5510 | | |
5508 | 5511 | | |
5509 | 5512 | | |
| 5513 | + | |
| 5514 | + | |
| 5515 | + | |
| 5516 | + | |
| 5517 | + | |
5510 | 5518 | | |
5511 | 5519 | | |
5512 | 5520 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
| 10 | + | |
9 | 11 | | |
10 | 12 | | |
11 | 13 | | |
| |||
46 | 48 | | |
47 | 49 | | |
48 | 50 | | |
| 51 | + | |
49 | 52 | | |
50 | 53 | | |
51 | 54 | | |
| |||
354 | 357 | | |
355 | 358 | | |
356 | 359 | | |
| 360 | + | |
357 | 361 | | |
358 | 362 | | |
359 | 363 | | |
| |||
1438 | 1442 | | |
1439 | 1443 | | |
1440 | 1444 | | |
| 1445 | + | |
| 1446 | + | |
| 1447 | + | |
| 1448 | + | |
| 1449 | + | |
| 1450 | + | |
| 1451 | + | |
| 1452 | + | |
| 1453 | + | |
| 1454 | + | |
| 1455 | + | |
| 1456 | + | |
| 1457 | + | |
| 1458 | + | |
| 1459 | + | |
| 1460 | + | |
| 1461 | + | |
| 1462 | + | |
| 1463 | + | |
| 1464 | + | |
| 1465 | + | |
| 1466 | + | |
| 1467 | + | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
| 1472 | + | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
| 1478 | + | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
| 1487 | + | |
1441 | 1488 | | |
1442 | 1489 | | |
1443 | 1490 | | |
0 commit comments