[Claudius-Maginificent]

TL;DR

Since protocol version 14 an identity key can carry a budget, an expiry or contract bounds, and Platform refuses a key that doesn't fit the transition. Only two of the wallet's own writes (DashPay profile and contactInfo) knew that. Clients such as Dash Evo Tool kept their own copies of the rules. This PR:

• makes the wallet's signing-key selection public and generalizes it,
• adds the dpp helpers it needs to decide before a transition is built,
• moves every platform-wallet key picker onto it,
• stops two pickers from choosing disabled keys,
• picks keys the way DIP-11 intends (weakest sufficient level; DIP-11 key types before ECDSA_HASH160) and accepts ECDSA_HASH160 keys, which Dash Evo Tool identities hold,
• judges DPNS and contact-request keys against the same contract snapshot the transition is built from.

Issue being fixed or feature implemented

• Clients duplicated the key policy. usable_authentication_key was pub(crate) and only covered AUTHENTICATION keys for one document type. Dash Evo Tool re-implemented it, together with dpp's contract-bounds rule and the "levels accepted for a requirement" rule, and its copy has already drifted from the wallet's ordering.
• Most wallet writes were not protocol-14 aware. These operations took the first key at an accepted level, ignoring bounds, expiry and limits: DPNS registration and marketplace, contact-request send, generic document create, token operations, contract create/update, identity update and credit transfer/withdrawal. An identity holding a key bound elsewhere, or an application key with a budget, got a transition that Platform refused.
• Disabled keys were chosen. credit_signing_key and the identity-update MASTER picker passed allow_disabled = true. Platform refuses a disabled signing key unconditionally: dpp's verify_public_key_is_enabled refuses it before signing, and Drive's identity-signed validation refuses it after.
• Routine writes used the strongest key. Selection walked keys in key-id order, so with the usual layout (1 CRITICAL, 2 HIGH) a HIGH write such as a contact request signed with the CRITICAL key. DIP-11 has clients gate stronger keys behind stronger authentication, so that meant a strong-auth prompt on routine writes.
• ECDSA_HASH160 identities could not sign. Identities created by older Dash Evo Tool versions hold ECDSA_HASH160 authentication keys. Platform accepts them on every identity-signed transition, but automatic selection read only ECDSA_SECP256K1 keys.
• Key and transition could disagree on the contract. The wallet chose a DPNS or contact-request key from one contract snapshot (the bundled contract at the SDK's possibly stale protocol version, or a separate fetch) while the SDK built the transition from another. After a protocol or system-contract upgrade the wallet could pass a key Platform refuses, or refuse a valid one.
• Contract create was stricter than consensus. The wallet asked for a CRITICAL key; consensus accepts HIGH or CRITICAL for a create.

What was done?

dpp (no consensus behaviour change)

• SecurityLevel::accepted_for_requirement(requirement): the levels a key may carry to meet a document type's requirement. Tests pin it to combined_security_level_requirement for document and token batches.
• ContractBoundsScope (Contract / DocumentType(&str) / Token(position)) and ContractBounds::check_scope(contract_id, scope): the bounds verdict before a transition is built. check_batched_transition delegates to it. The existing per-transition tests stay green and pin the verdicts as unchanged.
• IdentityPublicKey::can_sign_non_batch(): a contract-bound AUTHENTICATION key signs only batches. A test pins it to the pre-sign check for every purpose and every bounds kind. The wallet's SigningScope::bounds_check uses it, so the rule is written once.

rs-sdk (additive)

• Sdk::prepare_dpns_name_registration / Sdk::submit_dpns_name_registration: prepare takes the network's current, proved DPNS contract, builds the preorder and domain documents, and returns a DpnsNameRegistration stating the signing-level requirement and contract id of that snapshot. Submit signs and submits exactly those documents. register_dpns_name* are now prepare + submit.
• Sdk::prepare_contact_request / Sdk::submit_contact_request: prepare refreshes the protocol version, takes the network's current, proved DashPay contract and returns a PreparedContactRequest holding the contactRequest document type of that snapshot; submit builds and submits as exactly that type. create_contact_request and send_contact_request keep their signatures and use one snapshot each.
• Sdk::fetch_current_system_data_contract (in platform::system_data_contract) is the shared "proved at the parsed protocol version" fetch behind both.

platform-wallet: new public API, re-exported from the platform_wallet crate root

• SigningScope<'a>: NonBatch / DocumentType { contract_id, document_type_name } / Contract { contract_id } / Token { contract_id, position }. SigningScope::bounds_check(&key) returns dpp's three-way BatchedTransitionBoundsCheck, using the same ContractBoundsScope consensus uses. For NonBatch it asks IdentityPublicKey::can_sign_non_batch.
• usable_signing_key(identity, signer, purpose, security_levels, key_types, scope) -> Result<Option<&IdentityPublicKey>, dash_sdk::Error>:
  • Automatic selection order (DIP-11): unlimited keys before limited ones; within each group, DIP-11 key types (ECDSA_SECP256K1, BLS12_381) before ECDSA_HASH160, then the weakest sufficient security level, then the lowest key id. Security levels are minimums: a stronger key signs only when the signer cannot reach a weaker one. Consensus accepts either, so no on-chain behaviour changes.
  • It skips disabled keys, expired keys, out-of-bounds keys and group-bound keys. If the system clock reads before the Unix epoch, an expiry cannot be checked, so it also skips any key that has one.
  • Ok(None) means no key is eligible. Err means an eligible key exists but the signer cannot use it.
• check_signing_key(key, signer, purposes, security_levels, key_types, scope) -> Result<(), dash_sdk::Error> checks a key the caller chose explicitly, using the same rules. purposes lists every purpose the transition accepts; a withdrawal, for example, takes TRANSFER or OWNER. Each refusal is the consensus SignatureError Platform itself returns, checked in Drive's order: InvalidIdentityPublicKeyTypeError, InvalidSignaturePublicKeyPurposeError, InvalidSignaturePublicKeySecurityLevelError, PublicKeyIsDisabledError; then PublicKeyExpiredError, ContractBoundedKeyOutOfBoundsError or ContractBoundedKeyNonBatchError. A group-bound key is accepted and left to Platform. A key the signer cannot use gets the existing signer-unavailable error. With the clock before the Unix epoch, a key with an expiry is refused with dash_sdk::Error::Config.
• PlatformWalletError::NoUsableSigningKey { identity_id, purpose, security_levels, key_types, transition }: no key of the identity can sign transition. It is distinct from the signer-unavailable error. In the FFI it keeps the code InvalidIdentityData had (ErrorUnknown), so the Swift and Kotlin result enums are unchanged.
• select_own_encryption_key is now public.

platform-wallet: own selectors moved onto it

• Key types. WALLET_SIGNING_KEY_TYPES is now [ECDSA_SECP256K1, ECDSA_HASH160] (was [ECDSA_SECP256K1]). This is a public API change: the constant is re-exported from the crate root and its type changes from [KeyType; 1] to [KeyType; 2]. The wallet's own document, token, contract and identity-update selectors pick from it, so ECDSA_HASH160 keys (Dash Evo Tool identities) are accepted by automatic selection wherever consensus accepts them, and a DIP-11 key type is still preferred when the identity holds both. Credit transfer/withdrawal selection and every explicit-key check use PLATFORM_SIGNING_KEY_TYPES (ECDSA_SECP256K1, BLS12_381, ECDSA_HASH160), the set Drive verifies.
• DPNS registration uses a whole-DPNS-contract scope, because it writes preorder and then domain. One key signs both documents, so its level must meet the stronger of their two requirements. register_name_with_signer and register_name_with_external_signer check or select the key from the DpnsNameRegistration returned by prepare_dpns_name_registration and submit that same value. Public signatures are unchanged, and the extra DPNS contract round-trip is gone.
• DPNS marketplace (set price, delist, transfer, purchase) resolves the contract once and passes it through the state lookup, key selection and transition build, using the document type's scope. Generic document create uses the same scope.
• Contact-request send selects the key from the PreparedContactRequest (the contactRequest type of the network's current DashPay contract) and hands the same value to submit_contact_request. Public signatures are unchanged.
• Tokens use SigningScope::Token: the key's bounds must cover that token.
• Contract create accepts a HIGH or CRITICAL key (weakest sufficient first, so HIGH when the identity has one); contract update requires CRITICAL. Identity update and credit transfer/withdrawal use NonBatch.
• When no key qualifies, every operation that picks its own key returns NoUsableSigningKey instead of an InvalidIdentityData string: contract create/update, identity update, token operations, document create, DPNS registration and marketplace, contact-request send.
• Not touched: the profile and contactInfo writers keep their own key-type lists and InvalidIdentityData errors (see Follow-ups). They do not use WALLET_SIGNING_KEY_TYPES.
• Explicit keys are checked before any request and refused exactly where Platform refuses them. Each check accepts every key type Drive verifies. The rules per operation:
  • Document replace, delete, transfer, set-price and purchase: an AUTHENTICATION key at a level the document type accepts, with bounds covering the document type.
  • register_name_with_signer: an AUTHENTICATION key at the level the DPNS preorder and domain documents require, whose bounds cover the whole DPNS contract.
  • Credit transfer: a CRITICAL TRANSFER key. Credit withdrawal: a CRITICAL TRANSFER key, or a CRITICAL OWNER key only when no to_address is given (Platform refuses an OWNER key for a withdrawal to a chosen address, WithdrawalOutputScriptNotAllowedWhenSigningWithOwnerKeyError). Automatic selection follows the same rules. The key must be enabled and unexpired.
  • Every token_*_with_signer: a CRITICAL AUTHENTICATION key whose bounds cover the token. token_transfer_with_signer and token_claim_with_signer also take a CRITICAL TRANSFER key, which consensus allows for a batch of exactly one token transfer or claim.
• Credit transfer/withdrawal and identity update skip disabled keys. This is the first commit, which can be cherry-picked on its own.
• The AvailableSigningKey trait is removed, since nothing calls it any more. usable_authentication_key becomes a thin wrapper, so the profile and contactInfo call sites are untouched.

How Has This Been Tested?

• Added since the last review round (RED first, then green):
  • Automatic selection order: a HIGH write signs with a HIGH key rather than a CRITICAL one; a DIP-11 key type goes before ECDSA_HASH160 of the same group; a ECDSA_HASH160-only identity still signs. Credit keys: CRITICAL TRANSFER keys {3 HASH160, 4 SECP256K1} pick 4 (pinned in credit_signing_key tests).
  • Contract create signs with a HIGH key; contract update still requires CRITICAL.
  • DPNS registration, the DPNS marketplace and contact requests check/select the key from the same contract snapshot the transition is built from (rs-sdk prepare_* tests share one fixture that raises a document type's signatureSecurityLevelRequirement).
• RED first. These tests failed against the old selectors and pass now:
  • should_skip_disabled_transfer_keys_before_owner_fallback
  • should_skip_a_disabled_master_key
  • should_choose_a_document_key_whose_bounds_cover_the_document_type
  • should_choose_an_unlimited_document_key_before_a_limited_one
  • should_pick_only_a_credit_key_of_a_type_platform_verifies
  • should_sign_credit_operations_only_with_a_critical_key
  • should_require_the_level_the_contact_request_document_type_requires
  • should_refuse_an_explicit_document_key_platform_would_refuse: extended so it also accepts ECDSA_HASH160 and BLS12_381 keys and refuses EDDSA_25519_HASH160.
  • should_register_with_a_key_that_covers_both_dpns_document_types
  • should_sign_a_contact_request_with_a_key_bound_to_contact_requests_or_unbound
  • should_sign_a_token_operation_with_a_key_that_covers_the_whole_contract
  • should_sign_a_contract_transition_only_with_a_key_free_of_contract_bounds
  • should_report_no_usable_contract_key_as_a_typed_error
  • should_report_no_usable_master_key_as_a_typed_error
  • should_report_no_usable_token_key_as_a_typed_error
  • should_refuse_an_explicit_registration_key_before_any_network_call
  • should_refuse_an_explicit_credit_key_platform_would_refuse
  • should_refuse_an_explicit_token_key_before_any_network_call
  • should_accept_an_explicit_transfer_key_for_a_lone_token_transfer_or_claim
  • should_accept_an_explicit_token_key_of_any_type_platform_verifies
  • should_report_a_token_scope_as_the_token_consensus_checks
  • should_not_trust_an_expiry_without_a_valid_clock
  • should_require_the_strongest_level_of_both_registration_documents
  • should_not_sign_a_withdrawal_to_an_address_with_an_owner_key
• New unit tests cover usable_signing_key, check_signing_key and bounds_check, plus dpp agreement tests that tie the new helpers to the existing consensus logic.
  • should_deny_a_non_batch_signature_exactly_when_dpp_says_the_key_cannot_sign_one: covers every purpose dpp decodes and every bounds kind.
  • should_keep_no_usable_signing_key_on_its_former_code_with_the_context_in_the_message (platform-wallet-ffi).
• cargo test -p platform-wallet -p dash-sdk passes on 155f8f1667; cargo clippy -p platform-wallet -p dash-sdk --all-features --tests -- -D warnings is clean.
• cargo test -p platform-wallet-ffi --lib error:: passed.
• cargo test -p dpp --lib --features state-transition-signing,bls-signatures passed, filtered to the touched modules.
• cargo clippy -p dpp -p platform-wallet -p platform-wallet-ffi --all-targets --all-features -- -D warnings is clean.

Breaking Changes

• WALLET_SIGNING_KEY_TYPES changes type from [KeyType; 1] to [KeyType; 2] (ECDSA_SECP256K1, ECDSA_HASH160). It is re-exported from the platform_wallet crate root. Code that destructures or indexes it as a one-element array no longer compiles; passing it as &WALLET_SIGNING_KEY_TYPES to usable_signing_key still works. No in-repo consumer outside the crate uses it.
• PlatformWalletError gains the variant NoUsableSigningKey. That breaks an exhaustive match on PlatformWalletError that has no wildcard arm. The FFI result codes, Swift and Kotlin are unchanged.
• Everything else new is additive, including the rs-sdk prepare_*/submit_* methods. platform-wallet function signatures are unchanged.

Behaviour changes that clients will see:

• Wallet operations pick different keys than before when an identity holds contract-bound, limited, expired or disabled keys. In each case the old choice was one Platform refused, or a budgeted key spent while an unlimited one was available.
• Automatic selection is weakest-sufficient-first. A HIGH write signs with a HIGH key even when a CRITICAL key has a lower id. This also applies to credit keys.
• ECDSA_HASH160 keys now sign (automatic selection accepts them), after any DIP-11-type key of the same group.
• Contract create may now be signed with a HIGH key; contract update still requires CRITICAL.
• DPNS registration and contact-request send judge the key against the network's current contract, not the bundled one, so the outcome can differ from before after a protocol or system-contract upgrade.
• When no key qualifies, an operation that picks its own key returns NoUsableSigningKey instead of InvalidIdentityData(String). FFI hosts still see ErrorUnknown, but the message text changes.
• Explicit keys for document operations, DPNS registration, credit transfer/withdrawal and token operations are refused up front when Platform would refuse them, with Platform's typed error. Before, the refusal came from Platform after a network round-trip, or as an InvalidIdentityData string.
  • A disabled or expired explicit TRANSFER or OWNER key is now refused before signing.
  • An explicit credit key whose type Drive cannot verify, such as BIP13_SCRIPT_HASH, is refused up front.
• A withdrawal to a chosen address no longer falls back to an OWNER key, and refuses an explicit one. Platform refused those withdrawals anyway.
• Credit transfer and withdrawal choose and accept only CRITICAL keys. Platform refused any other level there.
• Explicit-key refusals for a wrong purpose, key type or security level, or a disabled key, are consensus SignatureErrors rather than ProtocolError variants such as WrongPublicKeyPurposeError.
• If the system clock reads before the Unix epoch, keys with an expiry are neither chosen nor accepted explicitly.

In-place changes to shipped generations

None. Drive and drive-abci are unchanged. The dpp edits are on the client signing side. verify_identity_key_bounds (signing path, state-transition-signing) is refactored to use can_sign_non_batch without changing behaviour, which an exhaustive purpose × bounds test pins. The new helpers check_scope, accepted_for_requirement and can_sign_non_batch mirror Drive's rules, but Drive does not call them.

• BatchTransitionV1::purpose_requirement (rs-dpp/.../batch_transition/v1/identity_signed.rs). Protocol versions 9 to 15 select batch structure version 1 (batch_state_transition default 1 from STATE_TRANSITION_SERIALIZATION_VERSIONS_V2). The method now delegates to the new pure BatchTransitionV1::signing_key_purposes. Consensus cannot change, because the refactor is output-identical:
  • TokenTransitionActionTypeGetter::action_type() returns Transfer exactly for TokenTransition::Transfer and Claim exactly for TokenTransition::Claim. Those are the only transitions the old as_transition_token_transfer() / as_transition_token_claim() checks matched.
  • The lone-transition condition is unchanged, and every other batch still returns [AUTHENTICATION].
  • The method reads no PlatformVersion.
  • Pinned by v1_signing_key_purposes_pin_every_lone_token_action_and_any_other_batch, v1_purpose_requirement_answers_through_signing_key_purposes and the existing v1_purpose_requirement_* tests.

Follow-ups (not in this PR)

• Consensus call sites. Left unchanged on purpose, to keep consensus paths untouched. Each could move onto the new dpp helpers:
  • combined_security_level_requirement (dpp) and Drive state_transition_action/batch/mod.rs could use SecurityLevel::accepted_for_requirement.
  • StateTransition::verify_identity_key_bounds (dpp) and drive-abci validate_state_transition_identity_signed/v1 could use IdentityPublicKey::can_sign_non_batch for the non-batch rule.
• dpp auto-pick of disabled keys. dpp's own auto-pick in IdentityCreditTransferTransition / IdentityCreditWithdrawalTransition / IdentityCreditTransferToAddressesTransition / ShieldFromIdentityTransition try_from_identity passes allow_disabled = true to get_first_public_key_matching, the same disabled-key problem as the wallet bug fixed here.
• Conflict with fix(platform-wallet): support HASH160 DashPay contact signing keys #5546. fix(platform-wallet): support HASH160 DashPay contact signing keys #5546 widens the DashPay contactInfo key types to ECDSA_HASH160. It conflicts textually with this PR in contact_requests.rs (the contact-request selector moves onto document_signing_key and the prepared request here). Whichever PR merges second resolves the conflict. profile.rs and contact_info.rs are untouched here, and fix(platform-wallet): support HASH160 DashPay contact signing keys #5546 owns contactInfo.
• profile and contactInfo. Both writers keep their own key-type lists and still return InvalidIdentityData when no key qualifies. After both PRs merge, they can move onto WALLET_SIGNING_KEY_TYPES and NoUsableSigningKey.
• Swift selection. selectSigningKey in swift-sdk StateTransitionExtensions.swift still picks document keys with preferCritical: true and asks for CRITICAL on contract create, contrary to the policy here. It belongs in rs-platform-wallet key_selection.
• Dedicated FFI code. A dedicated NoUsableSigningKey FFI result code would need the Swift and Kotlin result enums mirrored in a coordinated change.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
• If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

🤖 Generated with Claude Code

PR Hygiene · cdb38c5

• Bots — thepastaclaw requested changes — dismiss the review or push a fix
• Self-review — not asked of a bot author
• Build failed
• Approvals
  • dpp (packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs, packages/rs-dpp/src/identity/identity_public_key/mod.rs, packages/rs-dpp/src/identity/identity_public_key/security_level.rs and 3 more) — QuantumExplorer or shumkov
  • rs-platform-wallet-ffi (packages/rs-platform-wallet-ffi/src/error.rs) — HashEngineering or ZocoLini or llbartekll or romchornyi
  • rs-platform-wallet (packages/rs-platform-wallet/src/error.rs, packages/rs-platform-wallet/src/lib.rs, packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs and 24 more) — HashEngineering or ZocoLini or llbartekll or romchornyi
  • rust-sdk (packages/rs-sdk/src/platform/dashpay/contact_request.rs, packages/rs-sdk/src/platform/dashpay/mod.rs, packages/rs-sdk/src/platform/dpns_usernames/mod.rs and 1 more) — lklimek or shumkov

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• Improvements
  • Signing keys are checked against each operation’s security requirements, supported key types, availability, and applicable contract, document, or token scope.
  • Key selection supports additional eligible key types and distinguishes between missing keys and keys that cannot be used, with clearer error details.
  • Contact requests and DPNS registrations use the relevant contract’s signing requirements.
• Bug Fixes
  • Credit withdrawals now select signing keys based on whether a destination address is provided.
  • Signing-key checks account for the specific token or document involved and reject ineligible keys before proceeding.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 129e5fc
• Latest commit, not reviewed: cdb38c5

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: feat/platform-wallet-public-signing-key-selection
• File: .coderabbit.yaml
• Commit: 129e5fc
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 04c879b7-a8ff-4a7b-97fa-14d3a9955884

Commits

Reviewing files that changed from the base of the PR and between 155f8f1 and 129e5fc.

Files selected for processing: 16

• packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/tests.rs
• packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/v1/identity_signed.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/burn.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/claim.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/destroy_frozen_funds.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/freeze.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/pause.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/purchase.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/resume.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/set_price.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/transfer.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/unfreeze.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/update_config.rs
• packages/rs-sdk/src/platform/dpns_usernames/mod.rs

Files skipped from review as they are similar to previous changes: 2

• packages/rs-platform-wallet/src/wallet/identity/network/tokens/destroy_frozen_funds.rs
• packages/rs-platform-wallet/src/wallet/identity/network/tokens/freeze.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The wallet now selects and validates signing keys against operation scope, security requirements, key type, status, expiry, and signer availability. Document, contract, identity, credit, and token flows use shared checks. The SDK prepares contact requests and DPNS registrations against fetched contract snapshots.

Changes: Signing key validation

Layer / File(s)	Summary
Scope and security rules packages/rs-dpp/src/identity/identity_public_key/..., packages/rs-dpp/src/state_transition/mod.rs, packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/...	Contract bounds now support whole-contract, document-type, and token scopes. New helpers define non-batch eligibility, accepted security levels, and batch signing purposes.
Shared selection and typed errors packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs, packages/rs-platform-wallet/src/wallet/identity/network/signing_key.rs, packages/rs-platform-wallet/src/error.rs, packages/rs-platform-wallet/src/lib.rs, packages/rs-platform-wallet/src/wallet/identity/network/mod.rs, packages/rs-platform-wallet-ffi/src/error.rs	Shared helpers filter and validate keys by purpose, level, type, status, expiry, scope, and signer availability. The wallet adds NoUsableSigningKey and exports the helpers. The FFI conversion maps that error to ErrorUnknown and retains its rendered context.
Prepared registrations and contract snapshots packages/rs-sdk/src/platform/dashpay/contact_request.rs, packages/rs-sdk/src/platform/dashpay/mod.rs, packages/rs-sdk/src/platform/dpns_usernames/mod.rs, packages/rs-sdk/src/platform/system_data_contract.rs, packages/rs-sdk/src/platform.rs, packages/rs-platform-wallet/src/wallet/identity/network/sdk_writer.rs	The SDK exposes prepared contact-request and DPNS-registration data with contract IDs and signing requirements. Submission uses the prepared data. System-contract fetching checks protocol-version consistency.
Document, contract, identity, and credit operations packages/rs-platform-wallet/src/wallet/identity/network/document.rs, packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs, packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs, packages/rs-platform-wallet/src/wallet/identity/network/dpns_marketplace.rs, packages/rs-platform-wallet/src/wallet/identity/network/contract.rs, packages/rs-platform-wallet/src/wallet/identity/network/update.rs, packages/rs-platform-wallet/src/wallet/identity/network/withdrawal.rs, packages/rs-platform-wallet/src/wallet/masternode_withdrawal.rs	Document and DPNS operations check keys against document or contract scope. Contract, identity, and credit operations use shared selection or explicit-key validation. Withdrawal selection depends on whether the destination is a chosen address.
Token operations packages/rs-platform-wallet/src/wallet/identity/network/tokens/*	Token key selection and explicit-key validation include contract, token-position, and action context. Token operations check supplied keys before transition construction.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant WalletOperation
  participant KeySelection
  participant ContractBounds
  participant Signer
  WalletOperation->>KeySelection: request or validate key for operation scope
  KeySelection->>ContractBounds: check contract, document, or token bounds
  KeySelection->>Signer: check key availability
  KeySelection-->>WalletOperation: return eligible key or validation error

Loading

Possibly related PRs

• dashpay/platform#5546: Both changes address key types accepted for contact-request signing; this pull request routes selection through shared scope-aware checks and includes ECDSA_HASH160 in wallet signing types.

Merge Risk: Minimal · previous review 129e5

No confirmed issue blocks merging after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: exposing protocol-14-aware signing-key selection in platform-wallet.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

⛔ Final review complete — 1 blocking finding(s) (commit cdb38c5) · triage: critical

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[Claudius-Maginificent]

Review from a consumer's side. Dash Evo Tool would replace its own copy of the protocol-14 key rules with this API, so this checks the PR against two needs: the wallet's own pickers have to be fixed, and a client has to be able to choose and check a key before it builds a transition.

What a client uses

• usable_signing_key + SigningScope: picks a key before anything is built. The client needs that key to know which wallet to unlock, and to show "no usable key" before the user submits.
• check_signing_key: checks a key the user picked by hand, and returns Platform's own typed refusal.
• SigningScope::bounds_check: lets the client warn about a contract-bound or group-bound key in a key picker.
• SecurityLevel::accepted_for_requirement: the accepted-levels rule, so clients stop restating it.

The protocol-14 fixes to the wallet's own pickers (bounds, expiry, unlimited-first order, skipping disabled keys) are needed in their own right. The move from allowed_signing_security_levels to accepted_for_requirement, and check_batched_transition now calling check_scope, also remove two copies of the same rule. Both are good.

What has no consumer

• IdentityPublicKey::can_sign_non_batch: only a test calls it. SigningScope::bounds_check and verify_identity_key_bounds each implement the rule themselves.
• select_own_encryption_key becoming public: no caller outside the crate, and unrelated to signing-key selection.
• The second re-export path platform_wallet::wallet::identity::{check_signing_key, usable_signing_key, SigningScope, select_own_encryption_key}: the crate-root re-export already covers it.

Dropping these would make the PR smaller without losing anything a client or the wallet uses. Details inline.

Follow-ups, not for this PR

• The automatic operations still report "no key" as InvalidIdentityData(String), so a caller cannot tell it apart from other failures without matching on text.
• The explicit-key paths (register_name_with_signer, the token *_with_signer variants, an explicit key passed for a credit transfer or withdrawal) still pass the key on without the protocol-14 check that the explicit document operations now get.

Neither blocks this PR; both are inline as notes.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The protocol-14 key-selection refactor is broadly correct and the changed wallet paths now reject disabled, expired, limited, and out-of-scope keys during automatic selection. Several non-blocking consistency and ownership issues remain, including incomplete validation for explicitly supplied credit keys and loss of token-position information for group-bound token scopes. Rust wallet and workspace checks are skipped in the supplied CI snapshot, while no relevant code check is reported as failed.

🟡 6 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Validate explicit credit keys with the same eligibility policy
packages/rs-platform-wallet/src/wallet/identity/network/signing_key.rs:78-87

The automatic credit path now uses usable_signing_key, so it filters disabled, expired, contract-bound, and otherwise ineligible keys. An explicitly supplied credit key still goes directly through require_available, which checks only whether the signer can access it. Consequently, callers of transfer_credits_with_signer and the explicit withdrawal path can select a disabled, expired, wrong-purpose, wrong-level, wrong-type, or contract-bound key and only discover the refusal later during transition signing or Platform validation. Route explicit credit keys through the same purpose, level, type, disabled, expiry, and NonBatch bounds checks used by automatic selection.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)}

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, cross-cutting change to signing-key selection and validation that directly affects cryptographic key handling, signatures, identity updates, and funds movement across platform-wallet and DPP.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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-platform-wallet/src/wallet/identity/network/signing_key.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/signing_key.rs:78-87: Validate explicit credit keys with the same eligibility policy
  The automatic credit path now uses `usable_signing_key`, so it filters disabled, expired, contract-bound, and otherwise ineligible keys. An explicitly supplied credit key still goes directly through `require_available`, which checks only whether the signer can access it. Consequently, callers of `transfer_credits_with_signer` and the explicit withdrawal path can select a disabled, expired, wrong-purpose, wrong-level, wrong-type, or contract-bound key and only discover the refusal later during transition signing or Platform validation. Route explicit credit keys through the same purpose, level, type, disabled, expiry, and `NonBatch` bounds checks used by automatic selection.

In `packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs:29-43: Preserve the token position in token signing scopes
  Token operations pass `SigningScope::Contract`, so a `ContractGroup`-bound key is evaluated with `ContractBoundsScope::Contract` and the returned membership requirement contains `ContractGroupMember::Contract`. Consensus evaluates each token transition with its specific `TokenContractPosition` and returns `ContractGroupMember::Token(position)`. Automatic selection currently skips group-bound keys, so this is conservative for existing wallet flows, but the public `bounds_check` API exposes an inaccurate preflight result for token callers. Add a token-specific signing scope carrying the token position and use it for token operations, while retaining `Contract` for operations that truly cover the whole contract.

In `packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:48-64: Delegate non-batch bounds checks to the DPP helper
  The `NonBatch` branch implements the contract-bound rule locally by returning `Denied` for every contract-bound AUTHENTICATION key. The same PR adds `IdentityPublicKey::can_sign_non_batch()` in DPP as the protocol-owned predicate used to express this consensus rule. Keeping a second implementation in the wallet currently produces the same result, but future protocol changes can make wallet preselection and DPP signing disagree. Use the DPP helper for this branch and keep the wallet layer responsible for signer availability and ordering.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:91-98: Do not treat a pre-epoch clock as Unix epoch
  The new selector uses `now_ms()` as an eligibility check for key expiry, but `now_ms()` returns `0` when `SystemTime` is before the Unix epoch. On a device with an invalid or unset clock, a key with any positive expiry is therefore treated as unexpired and may be selected, despite the selector promising that returned keys are currently eligible. The resulting transition is then rejected when Platform evaluates it against block time. Make clock failure explicit or fail closed for expiry-bearing keys instead of silently using epoch zero.

In `packages/rs-dpp/src/identity/identity_public_key/security_level.rs`:
- [SUGGESTION] packages/rs-dpp/src/identity/identity_public_key/security_level.rs:109-120: Make accepted security levels the single DPP-owned rule
  `accepted_for_requirement` documents itself as the consensus rule, but `combined_security_level_requirement` still independently constructs the CRITICAL/HIGH/MEDIUM range. The agreement tests show that both implementations currently match, but they do not prevent a future protocol change from updating one and not the other. Keep the batch method responsible for aggregating the strongest requirement, then delegate construction of the accepted level list to this helper.

In `packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/dpns.rs:48-59: Derive the DPNS registration requirement from both document types
  DPNS registration signs both `preorder` and `domain`, but the shared selector is called with a hardcoded `SecurityLevel::HIGH`. The marketplace selector already obtains its requirement from the fetched contract. If either DPNS document type's requirement changes, registration can select a key that does not satisfy the combined batch requirement and fail later, while marketplace operations continue to follow the contract. Resolve both document types from the fetched contract and pass the strongest requirement to the shared selector.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• DPP automatic identity transition pickers still allow disabled keys — The DPP constructors for identity credit transfer, withdrawal, transfer-to-addresses, and shield-from-identity still pass allow_disabled = true to get_first_public_key_matching, so they can choose a disabled key before sign_external rejects it. These are pre-existing DPP call sites that the PR explicitly leaves unchanged while fixing platform-wallet selection; they should be tracked as a separate DPP hardening change.
  • Follow-up: Create or link a separate rs-dpp issue covering enabled-only automatic key selection and fallback tests for all four constructors.

[github-actions]

Ready for review — dpp: QuantumExplorer or shumkov · rs-platform-wallet: HashEngineering or ZocoLini or llbartekll or romchornyi.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The reviewed head fixes the previously identified signing-key selection, token-purpose, contract-snapshot, bounds-ordering, and DPNS redaction issues. Five non-blocking issues remain: local-clock expiry decisions, duplicated document-create and security-level protocol rules, the missing Unreleased changelog entry, and acceptance of a stale system contract after a failed protocol refresh. No blocking correctness issue was found; several CI and policy checks were still pending in the supplied snapshot.

🟡 5 suggestion(s)

4 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate change to identity signing-key selection, contract-bound authorization, signature construction, and funds-related wallet operations across DPP and platform-wallet, directly affecting cryptographic key handling and signing correctness in functions such as key selection and signing-key generation.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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 final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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-sdk/src/platform/system_data_contract.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/system_data_contract.rs:66-83: Do not accept a stale system contract after a failed protocol refresh
  `fetch_current_system_data_contract()` calls `refresh_protocol_version()`, but that method deliberately swallows refresh failures and returns the SDK's previously learned version. If the SDK starts at an older version, the refresh fails, and a node still returns a proof at that old version, `contract_proved_at_parsed_version()` can see matching metadata and accept the stale contract even though the network has upgraded. Require a successful current-version observation before accepting the contract, or propagate the inability to establish the current protocol version instead of treating the cached version as authoritative.

In `packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:146-149: Use an authoritative or conservative time source for expiry eligibility
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4393955798)
  Automatic and explicit key checks compare expiry with the device's local wall clock, while Platform evaluates expiry against consensus block time. If the device clock lags the network, the wallet can select or approve a key that is already expired when the transition is processed, causing rejection after signing and submission. Use trusted SDK response metadata or a last verified protocol timestamp where available; otherwise apply a conservative skew margin or fail closed for expiry-bearing keys.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:117-125: Centralize document-create consumed-type bounds semantics in DPP
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4393955780)
  `SigningScope::DocumentCreate` independently walks `consumable_document_type_names()` and applies `check_scope`, while Drive's batch advanced-structure validation performs the same aggregation. The implementations currently agree, but future changes to consume semantics or contract bounds can make wallet preflight select a key that consensus rejects, or reject one consensus accepts. Add a DPP-owned document-create scope verdict and have both the wallet and Drive consume it, leaving stateful contract-group membership resolution in Drive. This remains untracked, so it cannot yet be reasonably deferred.

In `packages/rs-dpp/src/identity/identity_public_key/security_level.rs`:
- [SUGGESTION] packages/rs-dpp/src/identity/identity_public_key/security_level.rs:109-120: Make accepted security levels the single DPP-owned rule
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4388917936)
  `accepted_for_requirement()` independently constructs the accepted CRITICAL/HIGH/MEDIUM range, while `combined_security_level_requirement()` retains a second construction of the same rule. The agreement tests protect the current output but do not prevent a future protocol change from updating one implementation without the other. Keep batch processing responsible for finding the strongest requirement, then delegate accepted-level construction to `SecurityLevel::accepted_for_requirement()`.

In `packages/rs-platform-wallet/src/error.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/error.rs:89-104: Record the breaking wallet error API in the Unreleased changelog
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4390783260)
  `PlatformWalletError::NoUsableSigningKey` is a new public enum variant and breaks exhaustive Rust matches. The PR description documents the change, but the active `CHANGELOG.md` Unreleased section does not. The repository history includes breaking feature commits such as `60cc5f4508` that update the Unreleased section in the PR, so add an entry describing the variant and noting that FFI result codes remain unchanged.

[Claudius-Maginificent]

Correction to my earlier reply on the CHANGELOG.md suggestion: "entries are added at release time, not in PRs" was inaccurate, since some v5.1-dev PRs do add Unreleased entries by hand. The decision (no hand-written entry) stands. The real reason: the changelog is generated at release from conventional commits. This PR's title carries ! and a BREAKING CHANGE footer, so the generator records the NoUsableSigningKey break. A hand-written Unreleased entry would show up twice.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The PR fixes most of the protocol-14 signing-key selection issues, but several correctness and ownership concerns remain. The Rust wallet still relies on potentially stale cached system contracts, uses local wall-clock time for consensus expiry decisions, and duplicates document-create and security-level rules; the Swift SDK also continues to bypass the new Rust-owned selection policy. Changed-package CI checks were still queued at the captured time.

🔴 1 blocking | 🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Route Swift document operations through the Rust signing-key owner
packages/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift:29-55

The first-party Swift path still ranks signing candidates in Swift through KeyManager.rankSigningCandidates and hardcodes CRITICAL for both contract creation and update. Its document create/replace/delete/transfer/price/purchase operations therefore bypass the Rust policy added by this PR, including contract bounds, expiry, disabled-key handling, weakest-sufficient levels, and supported HASH160 keys. This leaves a second signing-key policy in a mobile SDK and can cause Swift to select a key that Platform rejects or require unnecessarily strong authentication. Marshal the operation and contract/document context through the Rust platform-wallet selector rather than maintaining policy in Swift.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

4 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); 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: critical by gpt-6.1-sol (effort low) — This is a large cross-cutting change that modifies consensus-relevant signing-key selection, contract-bound authorization, cryptographic signing eligibility, and funds-moving wallet operations, notably in key_selection.rs, signing_key.rs, and the DPP identity-key validation code.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift:29-55: Route Swift document operations through the Rust signing-key owner
  The first-party Swift path still ranks signing candidates in Swift through `KeyManager.rankSigningCandidates` and hardcodes CRITICAL for both contract creation and update. Its document create/replace/delete/transfer/price/purchase operations therefore bypass the Rust policy added by this PR, including contract bounds, expiry, disabled-key handling, weakest-sufficient levels, and supported HASH160 keys. This leaves a second signing-key policy in a mobile SDK and can cause Swift to select a key that Platform rejects or require unnecessarily strong authentication. Marshal the operation and contract/document context through the Rust platform-wallet selector rather than maintaining policy in Swift.

In `packages/rs-sdk/src/platform/system_data_contract.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/system_data_contract.rs:17-22: Do not accept a stale system contract after a failed protocol refresh
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4405158169)
  Although the former refresh helper was removed, the replacement still returns any contract supplied by the context provider before making a proved network fetch. The trusted provider cache does not verify the requested protocol version when it finds a known contract, so an in-place-updated DashPay contract can remain stale after a protocol or system-contract upgrade. Contact-request preparation then builds and selects a key against that stale schema, and Platform rejects the transition only after signing. Require current-version provenance for cached contracts or invalidate/refetch them when freshness cannot be established.

In `packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:146-149: Use an authoritative or conservative time source for expiry eligibility
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4393955798)
  Automatic selection compares key expiry with the device's local wall clock, while Platform evaluates expiry against consensus block time. The new pre-epoch fail-closed behavior only handles an unreadable clock; a clock that lags the network can still select a key that Platform already considers expired. The wallet can therefore sign and submit a transition that is rejected after the signing operation, potentially consuming fees or nonces. Use trusted block-time metadata when available or apply a conservative margin to expiry-bearing keys.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:117-130: Centralize document-create consumed-type bounds semantics in DPP
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4393955780)
  `SigningScope::DocumentCreate` now manually walks `consumable_document_type_names()` and applies `check_scope`, while Drive's batch advanced-structure validation independently performs the same consumed-type aggregation. This PR introduced the wallet-side copy as part of its new public preflight policy. The current implementations agree, but a future change to consume semantics or bounds handling can make wallet selection accept a key that consensus rejects. Add a DPP-owned document-create scope verdict and have both wallet preflight and Drive consume it, leaving only state-dependent contract-group membership resolution in Drive.

In `packages/rs-dpp/src/identity/identity_public_key/security_level.rs`:
- [SUGGESTION] packages/rs-dpp/src/identity/identity_public_key/security_level.rs:109-120: Make accepted security levels the single DPP-owned rule
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4388917936)
  `SecurityLevel::accepted_for_requirement` presents the accepted-level set as the protocol rule, but the batch requirement implementation still constructs the CRITICAL/HIGH/MEDIUM range independently, and Drive has another equivalent construction. The agreement tests pin today's values but do not prevent a future protocol change from updating only one implementation. Keep batch code responsible for finding the strongest requirement, then delegate accepted-level construction to the DPP helper and update the consensus callers together.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Add a dedicated FFI result code for NoUsableSigningKey — The new Rust error is intentionally mapped to the existing generic FFI error code, so Swift and Kotlin ABI enums remain unchanged. Hosts cannot distinguish NoUsableSigningKey without inspecting the message, but assigning a new stable code requires a coordinated platform-wallet-ffi, Swift, and Kotlin API change outside this PR.
  • Follow-up: Track a coordinated FFI and mobile API update that assigns and propagates a dedicated NoUsableSigningKey result code.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The Rust-side signing policy is substantially centralized and the current snapshot/bounds changes are mostly correct. One blocking layer-ownership issue remains: Swift still independently selects keys and bypasses the new Rust policy. Additional non-blocking issues remain around stale cached system contracts, local-clock expiry checks, omitted BLS keys from automatic selection, and duplicated security-level policy; Rust workspace tests and several package checks were still pending at the captured CI state.

🔴 1 blocking | 🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Route Swift document operations through the Rust signing-key owner
packages/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift:29-55

Swift selectSigningKey still owns signing policy: it hardcodes CRITICAL for contract operations and prefers CRITICAL for document operations, while only delegating signer availability to Rust. The helper does not apply the Rust selector's protocol-14 bounds, expiry, disabled-key, key-type, or weakest-sufficient-level rules. All Swift document operations continue to call it, so supported Swift clients can select a different key or reject a valid HIGH, BLS, HASH160, or bounded-key path than rs-platform-wallet. Move selection through the Rust wallet/FFI API and leave Swift responsible only for marshalling and signer callbacks.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, security-auditor)}

3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This is a large cross-cutting change that directly alters signing-key selection, authentication requirements, contract-bound validation, and funds-related wallet operations across DPP and platform-wallet code, including the critical key-handling surface in key_selection.rs and signing_key.rs.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), 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; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift`:
- [BLOCKING] packages/swift-sdk/Sources/SwiftDashSDK/FFI/StateTransitionExtensions.swift:29-55: Route Swift document operations through the Rust signing-key owner
  Swift `selectSigningKey` still owns signing policy: it hardcodes CRITICAL for contract operations and prefers CRITICAL for document operations, while only delegating signer availability to Rust. The helper does not apply the Rust selector's protocol-14 bounds, expiry, disabled-key, key-type, or weakest-sufficient-level rules. All Swift document operations continue to call it, so supported Swift clients can select a different key or reject a valid HIGH, BLS, HASH160, or bounded-key path than `rs-platform-wallet`. Move selection through the Rust wallet/FFI API and leave Swift responsible only for marshalling and signer callbacks.

In `packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:40-50: Include BLS keys in automatic document and token selection
  `WALLET_SIGNING_KEY_TYPES` contains only `ECDSA_SECP256K1` and `ECDSA_HASH160`, while `PLATFORM_SIGNING_KEY_TYPES` also includes `BLS12_381`. The generic selector documents and implements DIP-11 ordering with BLS before HASH160, but document, contract, identity-update, and token automatic selectors pass the narrower array. Consequently, an identity with only a valid BLS AUTHENTICATION key can fail automatic selection with `NoUsableSigningKey`, even though Platform verifies BLS identity signatures and the explicit-key paths accept it. Include BLS in the automatic set or document and enforce a deliberate signer-level reason why BLS is credit-only.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/key_selection.rs:146-149: Use an authoritative or conservative time source for expiry eligibility
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4393955798)
  Automatic selection compares key expiry with the device's local `SystemTime`, while Platform evaluates expiry against consensus block time. A device clock that lags the network can select a key already expired according to Platform, causing the subsequent transition to fail after signing preparation. The typed Platform refusal and no-fee behavior limit the impact, but the selector still reports an unusable key as eligible and creates an avoidable availability failure. Use trusted protocol time metadata when available or apply a conservative expiry margin.

In `packages/rs-sdk/src/platform/system_data_contract.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/system_data_contract.rs:10-23: Do not accept a stale system contract after a failed protocol refresh
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4405158169)
  The earlier `fetch_current_system_data_contract` refresh implementation is gone, but the replacement `fetch_system_data_contract` still returns any contract held by the context provider for `self.version()` without establishing a fresh network observation or proving that the cached contract is current. `prepare_contact_request` relies on this helper. During a system-contract or protocol upgrade, contact-request preparation can therefore use an obsolete DashPay schema and build a transition that Platform rejects. Require a fresh proof-backed version observation for the current-contract path, or expose that freshness is unavailable instead of treating the cached contract as authoritative.

In `packages/rs-dpp/src/identity/identity_public_key/security_level.rs`:
- [SUGGESTION] packages/rs-dpp/src/identity/identity_public_key/security_level.rs:109-120: Make accepted security levels the single DPP-owned rule
  (existing thread: https://github.com/dashpay/platform/pull/5588#discussion_r4388917936)
  `SecurityLevel::accepted_for_requirement` is presented as the consensus rule, but `combined_security_level_requirement` still independently constructs the CRITICAL/HIGH/MEDIUM range in the batch-transition implementation. The current agreement tests prove only that the two implementations match today; they do not prevent a future protocol change from updating one without updating the other. Keep batch logic responsible for aggregating the strongest requirement, then delegate accepted-level construction to this DPP helper when the consensus-side follow-up is made.

[thepastaclaw]

🟡 Suggestion: Include BLS keys in automatic document and token selection

WALLET_SIGNING_KEY_TYPES contains only ECDSA_SECP256K1 and ECDSA_HASH160, while PLATFORM_SIGNING_KEY_TYPES also includes BLS12_381. The generic selector documents and implements DIP-11 ordering with BLS before HASH160, but document, contract, identity-update, and token automatic selectors pass the narrower array. Consequently, an identity with only a valid BLS AUTHENTICATION key can fail automatic selection with NoUsableSigningKey, even though Platform verifies BLS identity signatures and the explicit-key paths accept it. Include BLS in the automatic set or document and enforce a deliberate signer-level reason why BLS is credit-only.

Suggested change

	/// The key types the wallet picks its own signing keys from, except for a credit transfer or
	/// withdrawal key, which may be any of [`PLATFORM_SIGNING_KEY_TYPES`]. Pass these as
	/// `key_types` to [`usable_signing_key`] to choose the key the wallet itself would; a key the
	/// caller chose is checked against [`PLATFORM_SIGNING_KEY_TYPES`] instead.
	///
	/// `ECDSA_HASH160` is here so identities whose keys hold only the hash of the public key, the
	/// default layout older Dash Evo Tool versions created, keep signing wherever Platform
	/// accepts the key; [`usable_signing_key`] still prefers an `ECDSA_SECP256K1` key, the type
	/// DIP-11 defines.
	pub const WALLET_SIGNING_KEY_TYPES: [KeyType; 2] =
	[KeyType::ECDSA_SECP256K1, KeyType::ECDSA_HASH160];
	pub const WALLET_SIGNING_KEY_TYPES: [KeyType; 3] = [
	KeyType::ECDSA_SECP256K1,
	KeyType::BLS12_381,
	KeyType::ECDSA_HASH160,
	];

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)}