Repository navigation
chore: merge v5.0-dev into v5.1-dev - #5296
Conversation
…lls (#5235) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… (PV14) (#5240) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ral (#5241) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…l as insufficient credits
Platform refuses an identity credit withdrawal or credit transfer to
addresses with IdentityInsufficientBalanceError when the balance cannot
cover the amount plus the transition's minimum fee. Both wallet paths
stringified it into InvalidIdentityData ("Failed to withdraw credits:
Protocol error: Insufficient identity <id> balance <n> required <n>"),
so hosts could only show the raw protocol text.
Promote it to the typed InsufficientIdentityCredits (FFI code 39, with
Platform's required/available figures) the way the DPNS marketplace
already does, through a shared promote_identity_insufficient_balance
helper that the document-trade promotion now reuses.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…_identity_insufficient_balance_or Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ared replace (PV14) (#5253) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Ivan Shumkov <ivanshumkov@gmail.com>
…on map parser (#5072) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tled deletion (PV14) (#5260) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…l as insufficient credits (#5206)
… at registration (PV14) (#5284) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…dev-into-v5.1-dev
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
🌳 GroveDB structure This pull request changes the described GroveDB structure. Open it in the structure viewer: new nodes glow, removed ones stay as ghosts, and the tour walks through each change. Added (10 nodes)
Changed (1 node)
Compared |
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-10-05T17:37:32.528Z |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
|
⛔ Final review complete — 1 blocking finding(s) (commit faf2ad1) · triage: critical |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix. Wait for the missing reviews, or a writer can post |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Confirmed one blocking double-spend vulnerability and three suggestions in the incoming token shielded-pool changes: two missing WASM API capabilities and a calibrated fee parameter outside the versioned schedule. Verification was static; no builds or tests were run locally. At the supplied CI snapshot, completed build and test checks were passing, while Rust workspace tests, the main Test Suite, browser shard 1, and PR Hygiene remained pending.
🔴 1 blocking | 🟡 3 suggestion(s)
4 finding(s) not shown inline (GitHub refused the PR diff as too large)
🔴 Blocking: Reject nullifiers reused across pool operations in the same batch
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_shielded_pool_common/mod.rs:143-149
The seen set is local to each bundle, and the subsequent storage reads only check pre-batch state. Batch state validation validates every sub-transition before their combined operations are applied. Neither basic structure validation nor the additional CheckTx nullifier probe rejects reuse across sub-transitions. Consequently, an identity with sufficient credit fees can submit two independently valid TokenShieldedTransfer bundles with distinct valid contract nonces that spend the same note against the same recorded anchor and produce different replacement notes.
Storage application does not reject this batch under the shipped configuration. token_balance_writes excludes TokenShieldedTransfer, and batching_consistency_verification defaults to false. The repeated nullifier inserts collapse to one keyed operation, while the pinned GroveDB commitment-tree preprocessing appends both bundles' outputs. The ordinary item writes in this transfer batch do not activate GroveDB's separate backward-reference conflict checks. Neither the pool total nor the token supply changes, so balance conservation does not detect the duplicated spendable value. The replacements can subsequently be unshielded in separate transitions, consuming tokens backing other holders' notes.
Track nullifiers across accepted batch pool operations using a set keyed by (token_id, nullifier), including shielded document payments, and reject a conflicting sub-transition before lowering its actions. Add a regression test with two separately valid transfer bundles spending the same note within one batch under the shipped consistency-check configuration.
source: gpt-6.1-sol (phase2-reviewer: security-auditor)
🟡 Suggestion: Marshal the pool threshold's change-control rules into V1
packages/wasm-dpp2/src/tokens/configuration/token_configuration.rs:201-209
The constructor accepts minimumPoolNotesForOutgoing but provides no option or setter for minimumPoolNotesForOutgoingChangeRules. Enabling the pool upgrades through TokenConfigurationV1::from_v0, whose dedicated threshold rules authorize NoOne for both changes and administration. An extra JavaScript property is ignored by the options deserializer, and tokens_configuration_from_js_value copies only the Rust inner configuration into the contract, so attaching that property to the wrapper does not preserve it either.
Tokens created through this constructor therefore have an immutable threshold, despite the new API documentation describing subsequent issuer updates. Native authorization checks use these dedicated rules rather than the token's other change rules. Expose an optional ChangeControlRules option and marshal it into the V1 configuration, preserving the restrictive default when omitted. Add a regression test that checks the rules retained in the constructed data contract.
source: gpt-6.1-sol (phase2-reviewer: ffi-engineer)
🟡 Suggestion: Expose JavaScript factories for the three new threshold change items
packages/wasm-dpp2/src/tokens/configuration_change_item/token_configuration_change_item.rs:136-144
The new match arms expose the threshold variants through getters, but JavaScript cannot construct them through the typed API. The configuration_change_item/items modules provide factories for existing variants such as MaxSupplyItem, but none for MinimumPoolNotesForOutgoing, its control group, or its admin group. TokenConfigurationChangeItem also has no constructor or object/JSON parser.
TokenConfigUpdateTransition::constructor extracts updateTokenConfigurationItem through try_from_options, which requires an actual WASM wrapper rather than a plain object. Thus, even a token created elsewhere with rules authorizing threshold updates cannot receive these updates through the JavaScript factories. Add the three exported factories using the existing value, control-group, and admin-group patterns, and test that each resulting item survives construction of a TokenConfigUpdateTransition.
source: gpt-6.1-sol (phase2-reviewer: ffi-engineer)
🟡 Suggestion: Store the calibrated token-unshield allowance in the fee schedule
packages/rs-dpp/src/shielded/mod.rs:214
This 230-byte figure is calibrated pricing with discretionary headroom, not an encoded-field width: the comment derives it from measured storage credits and rounds it upward. compute_token_unshield_with_shielded_fee_fee uses the shared constant to determine the exact credit amount required by minimum-fee validation, but the allowance itself is independent of PlatformVersion::fee_version.
The current activation is replay-safe, but changing this calibration later would also change the accepted fee for historical PV14/PV15 transitions unless a separate behavior generation were introduced. The repository's fee conventions place pricing parameters in named schedules. Add the allowance to the owning fee parameters, configure it in the unreleased FEE_VERSION3 schedule used by PV14/PV15, and read it through platform_version in the shared helper used by builders and validation. Preserve the frozen historical fee-serialization structures.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, intricate diff adds token shielded-pool funds movement and consensus validation, including cryptographic signing changes in packages/rs-dpp/src/shielded/sighash.rs and new token shield/unshield transition implementations. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_shielded_pool_common/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_shielded_pool_common/mod.rs:143-149: Reject nullifiers reused across pool operations in the same batch
The `seen` set is local to each bundle, and the subsequent storage reads only check pre-batch state. Batch state validation validates every sub-transition before their combined operations are applied. Neither basic structure validation nor the additional CheckTx nullifier probe rejects reuse across sub-transitions. Consequently, an identity with sufficient credit fees can submit two independently valid `TokenShieldedTransfer` bundles with distinct valid contract nonces that spend the same note against the same recorded anchor and produce different replacement notes.
Storage application does not reject this batch under the shipped configuration. `token_balance_writes` excludes `TokenShieldedTransfer`, and `batching_consistency_verification` defaults to false. The repeated nullifier inserts collapse to one keyed operation, while the pinned GroveDB commitment-tree preprocessing appends both bundles' outputs. The ordinary item writes in this transfer batch do not activate GroveDB's separate backward-reference conflict checks. Neither the pool total nor the token supply changes, so balance conservation does not detect the duplicated spendable value. The replacements can subsequently be unshielded in separate transitions, consuming tokens backing other holders' notes.
Track nullifiers across accepted batch pool operations using a set keyed by `(token_id, nullifier)`, including shielded document payments, and reject a conflicting sub-transition before lowering its actions. Add a regression test with two separately valid transfer bundles spending the same note within one batch under the shipped consistency-check configuration.
In `packages/wasm-dpp2/src/tokens/configuration/token_configuration.rs`:
- [SUGGESTION] packages/wasm-dpp2/src/tokens/configuration/token_configuration.rs:201-209: Marshal the pool threshold's change-control rules into V1
The constructor accepts `minimumPoolNotesForOutgoing` but provides no option or setter for `minimumPoolNotesForOutgoingChangeRules`. Enabling the pool upgrades through `TokenConfigurationV1::from_v0`, whose dedicated threshold rules authorize `NoOne` for both changes and administration. An extra JavaScript property is ignored by the options deserializer, and `tokens_configuration_from_js_value` copies only the Rust inner configuration into the contract, so attaching that property to the wrapper does not preserve it either.
Tokens created through this constructor therefore have an immutable threshold, despite the new API documentation describing subsequent issuer updates. Native authorization checks use these dedicated rules rather than the token's other change rules. Expose an optional `ChangeControlRules` option and marshal it into the V1 configuration, preserving the restrictive default when omitted. Add a regression test that checks the rules retained in the constructed data contract.
In `packages/wasm-dpp2/src/tokens/configuration_change_item/token_configuration_change_item.rs`:
- [SUGGESTION] packages/wasm-dpp2/src/tokens/configuration_change_item/token_configuration_change_item.rs:136-144: Expose JavaScript factories for the three new threshold change items
The new match arms expose the threshold variants through getters, but JavaScript cannot construct them through the typed API. The `configuration_change_item/items` modules provide factories for existing variants such as `MaxSupplyItem`, but none for `MinimumPoolNotesForOutgoing`, its control group, or its admin group. `TokenConfigurationChangeItem` also has no constructor or object/JSON parser.
`TokenConfigUpdateTransition::constructor` extracts `updateTokenConfigurationItem` through `try_from_options`, which requires an actual WASM wrapper rather than a plain object. Thus, even a token created elsewhere with rules authorizing threshold updates cannot receive these updates through the JavaScript factories. Add the three exported factories using the existing value, control-group, and admin-group patterns, and test that each resulting item survives construction of a `TokenConfigUpdateTransition`.
In `packages/rs-dpp/src/shielded/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/shielded/mod.rs:214: Store the calibrated token-unshield allowance in the fee schedule
This 230-byte figure is calibrated pricing with discretionary headroom, not an encoded-field width: the comment derives it from measured storage credits and rounds it upward. `compute_token_unshield_with_shielded_fee_fee` uses the shared constant to determine the exact credit amount required by minimum-fee validation, but the allowance itself is independent of `PlatformVersion::fee_version`.
The current activation is replay-safe, but changing this calibration later would also change the accepted fee for historical PV14/PV15 transitions unless a separate behavior generation were introduced. The repository's fee conventions place pricing parameters in named schedules. Add the allowance to the owning fee parameters, configure it in the unreleased `FEE_VERSION3` schedule used by PV14/PV15, and read it through `platform_version` in the shared helper used by builders and validation. Preserve the frozen historical fee-serialization structures.
|
@thepastaclaw thanks. Replies to the final review on faf2ad1: 🔴 Reject nullifiers reused across pool operations in the same batch: not reachable. The scenario needs two pool operations (two
Two separate state transitions in one block are applied in sequence on the block transaction, so the second one's The cross-sibling gap is real if the cap is ever raised. It belongs with the other "needs cross-sibling tracking before lifting the cap" items already documented on 🟡 The three suggestions (WASM threshold change-control rules, WASM factories for the threshold change items, the token-unshield allowance in the fee schedule) are also on #4760's code. They're out of scope for a merge-only PR. Any follow-up should land on The only change this PR makes on top of the merge is faf2ad1, which regenerates 🤖 Posted autonomously by Claude on behalf of pasta. |
Basic explanation
What this does: Brings
v5.1-devup to date withv5.0-devby merging the 11 PRs that landed onv5.0-devsince the last forward merge (218cb89).Value: Among those 11 PRs, #5257 fixes the Swift SDK CI job on
v5.1-dev. Xcode 27 and the iOS 27 simulator landed on the self-hosted Mac runner on 2026-10-02. Since then,run_tests.shpicks a simulator by name only (iPhone 16 Pro, which exists only on iOS 18.6).xcodebuildthen looks for that name on the newest OS (iOS 27), finds no match, and fails withUnable to find a device matching the provided destination specifier. #5257 selects the simulator by UDID instead. Example failure: https://github.com/dashpay/platform/actions/runs/37319971317 (PR #5287).Risks: Low to moderate. The merge applied without conflicts, but it carries PV14 consensus changes into
v5.1-dev.PLATFORM_V15uses exactly the same sub-version tables asPLATFORM_V14(I compared every non-comment field assignment inv14.rsandv15.rs), so v15 picks up every change. The GroveDB structure snapshot combines v5.0-dev's shielded-pool trees with v5.1-dev's latest-protocol-version pinning (#5154). Thers-drivestructure tests in CI are the check for that.Issue being fixed or feature implemented
v5.1-devwas 11 PRs behindv5.0-dev, including the simulator-selection fix every Swift SDK job onv5.1-devcurrently needs.What was done?
Merged
origin/v5.0-dev(8061667) intoorigin/v5.1-dev(231dcbb) with no conflicts. Incoming PRs:Please merge with a merge commit (not squash) so the
v5.0-devhistory stays shared withv5.1-dev.How Has This Been Tested?
git mergeapplied with no conflicts.PLATFORM_V14andPLATFORM_V15after the merge: they are identical.Breaking Changes
None of its own. It carries the already-reviewed PV14 breaking changes listed above (#5284, #5260, #4760, #5253, #5240) into
v5.1-dev.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
faf2ad1/skip-botsproceeds without the ones not yet reported/self-reviewedgithub(.github/workflows/release.yml) — ktechmidas or shumkovbook/src/SUMMARY.md,book/src/addresses/platform-addresses.md,book/src/contract-keywords.mdand 113 more) — QuantumExplorer or shumkovdashmate(packages/dashmate/test/unit/config/configFile/tenderdashImageMigration.spec.js) — ktechmidas or shumkovjs-wasm-sdk(packages/js-evo-sdk/src/contracts/facade.ts,packages/wasm-sdk/src/state_transitions/contract.rs) — shumkovdpp(packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json,packages/rs-dpp/src/balances/total_tokens_balance/mod.rs,packages/rs-dpp/src/data_contract/associated_token/token_configuration/accessors/mod.rsand 197 more) — QuantumExplorer or shumkovrs-drive-abci(packages/rs-drive-abci/Cargo.toml,packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs,packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rsand 128 more) — QuantumExplorer or shumkovrs-drive(packages/rs-drive/grovedb-structure.json,packages/rs-drive/src/drive/contract/insert/insert_contract/v2/mod.rs,packages/rs-drive/src/drive/contract/moderation/team_action_tests.rsand 146 more) — QuantumExplorer or shumkovrs-platform-wallet-ffi(packages/rs-platform-wallet-ffi/src/error.rs) — HashEngineering or ZocoLini or llbartekll or romchornyirs-platform-wallet(packages/rs-platform-wallet/src/error.rs,packages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rs,packages/rs-platform-wallet/src/wallet/identity/network/withdrawal.rsand 3 more) — HashEngineering or ZocoLini or llbartekll or romchornyirust-sdk(packages/rs-sdk/src/platform/documents/transitions/create.rs,packages/rs-sdk/src/platform/documents/transitions/delete.rs,packages/rs-sdk/src/platform/documents/transitions/purchase.rsand 6 more) — lklimek or shumkovswift-sdk(packages/swift-sdk/run_tests.sh,packages/swift-sdk/scripts/fixtures/simulators-ios-18-6-and-27.json,packages/swift-sdk/scripts/select_simulator.pyand 2 more) — llbartekll or romchornyiWhen every box is checked the
PR Hygienecheck passes and this can merge.