feat(platform)!: revise shielded identity-create denominations in protocol version 13 - #4206
Conversation
…tocol version 13 Protocol version 13 revises the fixed exit-denomination set for IdentityCreateFromShieldedPool (Type 20) transitions: 0.03 and 0.25 DASH are added and 0.3 DASH is retired, giving 0.03 / 0.1 / 0.25 / 0.5 / 1 DASH. The v8 set (0.1 / 0.3 / 0.5 / 1) is unchanged for protocol version 12 chain replay. The builder and wallet note selection read the set from the platform version, so they follow automatically; the hardcoded pickers in the Swift and Kotlin example apps and the docstrings that spell the set out are updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds protocol-version refresh APIs and observable app state, updates v13 shielded identity denomination rules, and makes Kotlin and Swift identity-creation denomination selection and submission validation depend on the active protocol version. ChangesProtocol-gated shielded identity creation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AppState
participant SystemQueries
participant NativeQueries
participant Network
participant CreateIdentityView
AppState->>SystemQueries: refresh protocol version
SystemQueries->>NativeQueries: request protocol version
NativeQueries->>Network: fetch current version
Network-->>NativeQueries: version
NativeQueries-->>SystemQueries: version result
SystemQueries-->>AppState: publish current version
AppState-->>CreateIdentityView: update protocol-version state
CreateIdentityView->>CreateIdentityView: select active denomination set
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #4206 +/- ##
=============================================
+ Coverage 65.42% 87.51% +22.09%
=============================================
Files 26 2666 +2640
Lines 2707 336904 +334197
=============================================
+ Hits 1771 294837 +293066
- Misses 936 42067 +41131
🚀 New features to boost your workflow:
|
…ocol version The example-app denomination pickers hardcoded a single set, but the exit-denomination set changes at protocol version 13 (0.03 and 0.25 DASH added, 0.3 DASH retired). Offer the pre-v13 set until the network reports protocol version 13, then the revised set: - iOS: AppState now stores the version learned by the existing refreshProtocolVersion ratchet (reset on network switch, stale-SDK guarded), and CreateIdentityView picks the set from it. The submit gate also requires the chosen denomination to still be in the live set in case a refresh retires it mid-session. - Kotlin: bridge dash_sdk_refresh_protocol_version through JNI (it was never exposed), surface it as SystemQueries.refreshProtocolVersion, refresh on every SDK (re)build in the app's AppState, and gate the CreateIdentityScreen picker the same way, re-snapping a picked denomination if the set changes under it. An unknown version (refresh pending or failed) falls back to the pre-v13 set, matching every currently deployed network. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/v0/state_transition_validation.rs (1)
210-218: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPin these tests to explicit protocol versions.
Both tests use
PlatformVersion::latest(), so they validate whichever version is currently newest rather than specifically proving the PV13 set. Use explicit PV13 fixtures and add a PV12 acceptance check for0.3 DASHto preserve the replay contract.Also applies to: 229-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/v0/state_transition_validation.rs` around lines 210 - 218, Update the denomination validation tests should_accept_each_member_denomination and the related test around the second denomination case to use an explicit PV13 fixture instead of PlatformVersion::latest(). Add a separate PV12 acceptance assertion for the 0.3 DASH denomination, preserving the existing PV13 coverage and replay behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`:
- Around line 1330-1335: Update the public API documentation at
PlatformWalletManager.kt:1330-1335, FundingNative.kt:119-123, and
funding.rs:655-657 to state that callers must select denominations supported by
the active platform version, documenting PV12 as 0.1/0.3/0.5/1.0 DASH and PV13
as 0.03/0.1/0.25/0.5/1.0 DASH. Keep the comments consistent across all three
sites.
---
Nitpick comments:
In
`@packages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/v0/state_transition_validation.rs`:
- Around line 210-218: Update the denomination validation tests
should_accept_each_member_denomination and the related test around the second
denomination case to use an explicit PV13 fixture instead of
PlatformVersion::latest(). Add a separate PV12 acceptance assertion for the 0.3
DASH denomination, preserving the existing PV13 coverage and replay behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5987775e-8fc9-4d7d-bd22-c1d685d905dd
📒 Files selected for processing (18)
packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/state/AppState.ktpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/identity/CreateIdentityScreen.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/QueriesNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/queries/PlatformQueries.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/rs-dpp/src/shielded/sighash.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/v0/mod.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/identity_create_from_shielded_pool_transition/v0/state_transition_validation.rspackages/rs-drive/src/state_transition_action/action_convert_to_operations/shielded/identity_create_from_shielded_pool_transition.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rspackages/rs-platform-version/src/version/v13.rspackages/rs-platform-wallet/src/wallet/shielded/note_selection.rspackages/rs-unified-sdk-jni/src/funding.rspackages/rs-unified-sdk-jni/src/queries.rspackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/TEST_PLAN.md
Address review feedback: the membership tests validated whatever PlatformVersion::latest() resolves to, so they'd silently drift when a later version revises the set. Pin the v13 acceptance/rejection tests to PV13, and add PV12-pinned tests proving the replay contract: the original set (including the 0.3 DASH retired at v13) stays accepted under PV12, and the v13 additions (0.03, 0.25) are rejected there. Also make the denomination docstrings in the Kotlin SDK and JNI layer version-aware (both sets, keyed to the active protocol version) instead of listing only the v13 set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…alance-transition-amounts-da0109
Issue being fixed or feature implemented
IdentityCreateFromShieldedPool (Type 20) transitions must spend one of a fixed set of exit denominations from the shielded pool. For protocol version 13 the set is revised: 0.03 and 0.25 DASH are added, and 0.3 DASH is retired.
What was done?
Consensus (PV13):
shielded_identity_create_denominationsinDRIVE_ABCI_VALIDATION_VERSIONS_V9(used only by PV13) to[0.03, 0.1, 0.25, 0.5, 1]DASH in credits. The v8 set (0.1 / 0.3 / 0.5 / 1) is unchanged for protocol version 12 chain replay.v13.rsandv9.rsto record the revision.should_accept_each_member_denominationnow iterates the new set, and a newshould_reject_retired_denominationtest asserts 0.3 DASH is rejected at latest. Sighash-distinctness and drive conversion tests that used 0.3 DASH as an arbitrary value now use 0.25 DASH so they don't reference a retired denomination.Example-app pickers (protocol-version-gated):
The iOS and Android example apps hardcode the denomination set (there is no FFI getter), so they now gate it on the network's reported protocol version instead of assuming one set: the pre-v13 set (with 0.3) is offered until the network reports PV13, then the revised set (with 0.03/0.25). An unknown version (refresh pending/failed) falls back to the pre-v13 set, matching every currently deployed network.
AppStatestores the version learned by the existingrefreshProtocolVersionproven ratchet (reset on network switch, stale-SDK guarded);CreateIdentityViewpicks the set from it, and the submit gate requires the chosen denomination to still be in the live set.dash_sdk_refresh_protocol_versionis newly bridged through JNI and surfaced asSystemQueries.refreshProtocolVersion(); the app'sAppStaterefreshes on every SDK (re)build, andCreateIdentityScreengates the picker the same way, re-snapping a picked denomination if the set changes under it.rs-dppshielded builder) and wallet note selection (rs-platform-wallet) read the set from the platform version and follow automatically; docstrings inrs-unified-sdk-jni, the Kotlin SDK, and the SwiftTEST_PLAN.mdwere updated.How Has This Been Tested?
cargo test -p dpp identity_create_from_shielded/denomination/sighash— all pass, including the new retired-denomination rejection test.cargo test -p drive identity_create_from_shielded— conversion/conservation tests pass.cargo test -p drive-abci identity_create_from_shielded_pool— execution-path tests pass (they use 0.1 DASH, a member of both sets).cargo test -p platform-wallet select— note-selection membership gate tests pass.cargo check -p drive-abci,cargo check -p rs-unified-sdk-jni, andcargo fmt --allclean../gradlew :sdk:compileDebugKotlin :app:compileDebugKotlinclean.Breaking Changes
Consensus change gated on protocol version 13 (not yet activated): the accepted exit-denomination set for Type-20 transitions changes. A 0.3 DASH transition valid under PV12 is rejected once v13 activates; 0.03 and 0.25 DASH become valid. PV12 validation is untouched, and the example-app pickers follow the network's reported version, so 0.3 remains offered until activation.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests