fix(platform-version): gate shielded-pool block methods to protocol v12#3782
Conversation
Protocol v11 shared DRIVE_ABCI_METHOD_VERSIONS_V7 with v12. The Medusa shielded-pool PR (#3177, b33f8ec) added four shielded block-processing methods to V7 as Some(0): store_nullifiers_to_recent_block_storage, cleanup_recent_block_storage_nullifiers, record_shielded_pool_anchor, prune_shielded_pool_anchors. Because V7 is referenced by both v11.rs and v12.rs, this silently activated them on protocol v11 too. These run every block and read the shielded credit pool subtree [ShieldedBalances 0x34, MAIN_SHIELDED_CREDIT_POOL_KEY 'M' 0x4d], which only exists from v12 onward (created by transition_to_version_12 or a v12 genesis). On a v11 state the subtree is absent, so the read aborts consensus with: 'storage: grovedb: path parent layer not found: could not get key 4d for parent [52]'. Observed on testnet hp-masternode-7 (drive 4.0.0-beta.1): crash-looped in ProcessProposal on every v11 block starting at height 354892 (the first block it processed after the binary was deployed). The rest of testnet runs drive 3.0.1, whose V7 had no shielded fields, and was unaffected. Fix: keep V7 as the v11 method set with the four shielded methods None (restores 3.0.1 v11 behavior); add DRIVE_ABCI_METHOD_VERSIONS_V8 (= V7 + shielded Some(0)) and point v12 at it. v11.rs is unchanged. Regression test (platform-version shielded_pool_gating_tests) would have caught this in CI: before fix v11 record_shielded_pool_anchor == Some(0) -> FAILS (left=Some(0), right=None); after fix v11 == None and v12 == Some(0) -> PASSES. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR gates four shielded-pool block-processing methods to protocol v12+ by disabling them in v7 (used by protocol v11), defining a new v8 method set with them enabled, wiring v12 to use v8, and validating the gating via tests that confirm the methods are inactive on v11 and active on v12. ChangesShielded Pool Method Gating
🎯 3 (Moderate) | ⏱️ ~22 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
✅ Review complete (commit 36f7f9e) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Targeted, correct consensus-safety fix. V7 (the method set used by PLATFORM_V11) is restored to its 3.0.1 behavior with the four shielded-pool block-processing methods set to None, and the new V8 struct (used only by PLATFORM_V12) carries the Some(0) activations. V7 vs V8 differ in exactly those four fields, no other protocol version references V8, and the added tests pin the invariant at both v11 and v12.
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "277047132170cc569f5ba4932bec5c857537fe2723758b1b161c4d4a0ef87f05"
)Xcode manual integration:
|
Issue being fixed or feature implemented
drive 4.0.0-beta.1cannot process any protocol-v11 block. A node syncing testnet on this build crash-loops inProcessProposalwith:0x34=RootTree::ShieldedBalances(52);0x4d=MAIN_SHIELDED_CREDIT_POOL_KEY("M"). The shielded credit pool subtree[52, "M"]is created only from protocol v12 onward (transition_to_version_12, or a v12 genesis via init structure v3). On a protocol-v11 state it does not exist.Root cause: protocol v11 and v12 both reference the shared
DRIVE_ABCI_METHOD_VERSIONS_V7. The Medusa shielded-pool PR (#3177,b33f8ec75b) added four shielded block-processing methods to V7 asSome(0):store_nullifiers_to_recent_block_storagecleanup_recent_block_storage_nullifiersrecord_shielded_pool_anchorprune_shielded_pool_anchorsBecause V7 is shared, this silently turned them on for v11 too. They run on every block (independent of transactions) and unconditionally read
[52, "M"], aborting consensus.This was not introduced by #2920 (which switched v11 from V6→V7 for checkpoints): at release 3.0.1, V7 had no shielded fields, so v11 was fine. The rest of testnet runs
dashpay/drive:3.0.1and is unaffected — only the lone4.0.0-beta.1node broke.Observed on testnet
hp-masternode-7at height 354892 — the first block the beta binary processed after it was deployed (the previous binary had committed cleanly up to 354891).What was done?
DRIVE_ABCI_METHOD_VERSIONS_V7(the protocol-v11 method set) to its 3.0.1 behavior: the four shielded methods are back toNone.DRIVE_ABCI_METHOD_VERSIONS_V8(= V7 plus the four shielded methodsSome(0)) and pointedv12.rsat it.v11.rsis unchanged (still references V7).Net effect: shielded-pool block processing is gated to protocol v12+, where the
[52, "M"]subtree is guaranteed to exist. The legitimately-v11 parts of V7 (checkpoints, address recent-block-storage from #2866) are untouched.How Has This Been Tested?
New unit test
platform-version::shielded_pool_gating_testspins the invariant with a red→green transition against this exact bug:PlatformVersion::get(11).drive_abci.methods.block_end.record_shielded_pool_anchor == Some(0)→ test FAILS (left: Some(0),right: None)Nonefor all four methods; v12 →Some(0)→ PASSESGround truth confirmed against the live network: every other testnet masternode runs
dashpay/drive:3.0.1, whoseV7has no shielded fields — this change restores exact v11 parity with them.Breaking Changes
None. This is a consensus-relevant version-map correction, but no chain state was ever produced under the buggy v11 rules (a v11 node with the bug crashes before it can commit), and the fix restores parity with the released 3.0.1 network. The shielded pool remains a v12 feature; the v12 upgrade migration (
transition_to_version_12) is idempotent (grove_insert_if_not_existsper node) and creates[52, "M"]and its children when v12 activates.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores