fix: require chainlock signatures only after V20 activation#405
fix: require chainlock signatures only after V20 activation#405xdustinface merged 1 commit intov0.42-devfrom
Conversation
Pre-V20 blocks have no chainlock signatures in `MnListDiff` responses, but we currently incorrectly trigger failures when they are missing below v20 activation height. V20 activation heights (from Dash Core src/chainparams.cpp): - Mainnet: 1,987,776 - Testnet: 905,100 Added `v20_activation_height()` to `NetworkExt` and check in `apply_diff` and `from_diff` to only require signatures for post-V20 blocks. See DIP-0029.
📝 WalkthroughWalkthroughThis change adds network-aware V20 activation height tracking to the masternode list module. A new Changes
Sequence DiagramsequenceDiagram
participant Engine as MasternodeListEngine
participant Diff as apply_diff
participant Network as NetworkExt
participant Sigs as Signature Validation
Engine->>Diff: apply_diff(..., network)
Diff->>Network: v20_activation_height()
Network-->>Diff: activation_height
alt diff_end_height >= v20_activation_height
Diff->>Sigs: Check signatures present
alt Signatures available
Sigs->>Sigs: Safe retrieve via get().copied().flatten()
Sigs-->>Diff: VerifyingChainLockSignaturesType
Diff-->>Engine: Result(MasternodeList, signature)
else Missing signatures
Sigs-->>Diff: IncompleteSignatureSet error
Diff-->>Engine: Err(IncompleteSignatureSet)
end
else Pre-V20 (diff_end_height < activation_height)
Diff->>Sigs: Skip strict signature check
Sigs->>Sigs: Use available signatures if present
Sigs-->>Diff: Optional signature result
Diff-->>Engine: Result(MasternodeList, optional_sig)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
ZocoLini
left a comment
There was a problem hiding this comment.
Not sure about having a method with hardcoded heights where v20 starts, there is no other way to know it?? the v doesn't come in the data provided by the network??
Not since it's activated i think. We could probably at some point introduce a chainparams module which mirrors |
Pre-V20 blocks have no chainlock signatures in
MnListDiffresponses, but we currently incorrectly trigger failures when they are missing below v20 activation height.V20 activation heights (from Dash Core src/chainparams.cpp):
Added
v20_activation_height()toNetworkExtand check inapply_diffandfrom_diffto only require signatures for post-V20 blocks.See DIP-0029.
Summary by CodeRabbit