[Claudius-Maginificent]

TL;DR: The app now signs each action with a key the network will accept. It skips expired keys and keys bound to another contract, prefers keys without limits in the same order as Platform's own wallet library, and unlocks the wallet that actually holds the chosen key. This also changes key choice on protocol 13 networks, as a behaviour fix. DashPay contact QR codes now use the standard DashPay format through Platform's wallet library, so other DashPay wallets can read them.

User story

As a user, I want the app to sign with a key that can actually sign the action, so that my actions are not refused because of an expired, used-up or out-of-scope key (IDN-024).

Scenario

An identity holds an expired HIGH key, a HIGH key bound to DashPay, and a CRITICAL key without limits. The user deletes a document of another contract.

• Before: the app took the first key that matched the security levels, which could be the expired or the DashPay-bound key, and the network refused the action.
• Now: the app skips both and signs with the CRITICAL key (a document type accepts a stronger key than it requires), and the unlock prompt asks for the wallet that holds that key.

If no key on the device can sign the action, the form now says so instead of stopping silently.

Detailed discussion

Behaviour change on protocol 13 networks

This changes key choice on every network, including mainnet and testnet on protocol 13. It is a behaviour fix, not a protocol 14 feature:

• Wrong-scope keys: a key bound to another contract or document type was refused by the network. It is now skipped automatically and not offered in the manual chooser.
• Stronger keys: a document type now accepts a key stronger than the level it requires, which matches Platform's rule. Before, the app looked only at the exact level, so an identity holding only a stronger key could not sign.
• DashPay key types: DashPay documents are signed with an ECDSA key (ECDSA_SECP256K1 or ECDSA_HASH160), as upstream platform-wallet's DashPay writes require.
• Private half on this device: automatic choice only takes keys whose private half this device can obtain. A key filed under a wallet that is not on this device, or in the old encrypted form, is passed over for the next usable key. Lookups that only point the user at a key, such as Check Owner Key and Manage Transfer Key, still find keys whose private half is not here.
• Unlock prompt and owner: the wallet to unlock, and the owner of an identity stored without a wallet link, follow the automatically chosen key, not the identity's DPNS signing key. On the username screen, a key chosen under Advanced brings its own wallet to unlock; on other screens the unlock wallet does not yet follow a key chosen under Advanced, as on v1.0-dev.

On protocol 13 no key carries limits or an expiry, so the limit and expiry rules have no effect there.

What the rules are

• Automatic choice keeps only keys that are:

  • live (not disabled, not expired at the current time);
  • held on this device;
  • in scope: unbound, or bound to the action's contract or document type;
  • at a security level the action accepts. A document type that requires MASTER accepts only MASTER keys, as Platform does.

  Among those it follows upstream platform-wallet's usable_authentication_key order: every key without limits, at any accepted level, comes before any key with a budget or an expiry, then key id order. So a delegated key's budget is not spent while an unlimited key is held. Every automatic choice, for DashPay contact requests, usernames, document, token, contract and credit actions, uses this order. DashPay contact details (contactInfo) still pick their key the old way (the first matching key of any type); fix(dashpay): save contact details through platform-wallet (DIP-15) #1102 moves them to platform-wallet's writer, which chooses the key itself. The key is picked again when the contract or document type changes.

• Key types: every automatic choice and every key chooser takes only key types Platform verifies (ECDSA_SECP256K1, ECDSA_HASH160, BLS12_381), never EDDSA_25519_HASH160 or BIP13_SCRIPT_HASH. When the identity holds a key that cannot sign here (disabled, expired, bound to another token or app, or not on this device), the token transfer, claim, credit transfer and withdrawal forms say so instead of opening with a disabled button.

• Username registration: the key levels now satisfy both the preorder and the domain document.

• The manual chooser leaves out keys bound to another contract or document type. It keeps expired and limited keys, with a warning, because the user may know better. A DPNS key the user chose explicitly may have expired; Platform decides by network time.

• When the only key that can sign a token action is bound to a contract group, the form opens its key chooser with a note, so the user can choose that key if the group includes the token's contract.

• Keys as held now: keys are judged as the identity holds them now.

  • A key whose budget or expiry was raised on the network is still the same key, so its stored private half stays matched and a stale stored copy is not reported expired.
  • A key that gains or loses a limit is a different key.

• Scope per screen: every caller passes the action's scope.

  • Document and token actions pass their contract or document type.
  • Identity, transfer, withdrawal and contract-registration transitions pass none (non-batch).
  • DashPay documents use the levels each document type accepts.

DashPay contact QR codes

Before, the app derived the QR code's auto-accept key from an identity encryption key used as a wallet seed, and wrote its own dash:?di=<identity> format, so no other DashPay wallet could use its codes or verify its requests. Contact QR codes now go through upstream platform-wallet:

• Showing a code: DashPayView::build_auto_accept_qr builds it. The key is derived from the identity's wallet seed, which is read through the secret seam for one session only. The app's crypto provider exports only an auto-accept key path on the current network and refuses every other request.
• Scanning a code: upstream's parse_dashpay_contact_uri and decode_auto_accept_key_blob read it. An expired code is refused before anything is sent. The contact is found from the code's username, and upstream's sign_auto_accept_proof signs the request's proof over the sender, that contact and the request's account reference.
• Following upstream: a code shows the identity's username and works for one hour. The validity and account index fields are gone. Codes in the old di= format are refused.
• Removed: the app's own QR format, key derivation and proof signing, and its owner-side proof check, which nothing called.

Refusals

The refusals Platform gives for a signing key get dedicated messages that say what to do:

• expired;
• spending limit used up;
• action costs more than the remaining limit (both amounts are shown);
• bound to another contract;
• bound to a contract but used for a non-batch action.

Upstream reference

Where DET and Platform's crates implement the same rule, upstream is the reference.

• Used now: the order of automatic choice and the DashPay key types follow upstream platform-wallet, and contact QR codes use its codecs and derivation.
• Still in DET: the selector, the bounds check, the accepted-levels range and the DPNS explicit-key check. Their upstream versions are not public at the pinned Platform revision (usable_authentication_key, first_available, allowed_signing_security_levels and resolve_authentication_signing_key are crate-private, and dpp's public bounds check needs a built transition). Each place carries a TODO(upstream) marker naming what upstream must expose. A follow-up PR, blocked by feat(platform-wallet)!: expose protocol-14-aware signing-key selection platform#5588, replaces them.

Scope

• contactInfo: this PR does not touch DashPay contact details. fix(dashpay): save contact details through platform-wallet (DIP-15) #1102 moves them to platform-wallet, whose writer chooses the signing key itself.
• Follow-up, on top of fix(dashpay): save contact details through platform-wallet (DIP-15) #1102 and after feat(platform-wallet)!: expose protocol-14-aware signing-key selection platform#5588 and #5546: accepting a request from a contact QR code automatically on the owner's side (upstream's drain_auto_accepts_verified), and sending and accepting contact requests through platform-wallet. Both need a full DashPay crypto provider.
• Only key choice. Adding keys with limits or contract bounds, raising limits and the key list markers come in follow-up PRs.
• The refusal mappings for those flows come with them.

Known limitations (by design)

• The DashPay profile writer (unchanged from v1.0-dev) lets platform-wallet pick its signing key through QualifiedIdentity::can_sign_with, which accepts any filed key placement. If a profile-capable key this device cannot sign with has a lower id than a usable one, saving the profile fails before broadcast (no fee). Follow-up tracked in refactor(dashpay)!: sync DashPay through platform-wallet and show it from shared snapshots #1134: a profile signer that also requires holds_obtainable_key.
• BLS keys and usernames: username registration signs with ECDSA keys only (ECDSA_SECP256K1, ECDSA_HASH160), for parity with upstream platform-wallet's DPNS writes. Platform would accept a BLS key, so an identity whose only registration-capable key is a BLS key cannot register a username from DET.
• Explicitly chosen expired or contract-group keys: the backend accepts a username registration key the user picked even if it is expired or bound to a contract group, and leaves the final check to Platform. The Usernames readiness gate still applies the automatic rule, though. An identity whose only registration-capable keys are expired or group-bound is therefore told it has no suitable key, and never reaches the Advanced key chooser.

Testing

• Library tests for the touched modules pass (dashpay, qualified_identity, identity_key_usability, wallet_backend::auto_accept, document_action_screen, dpns: 794 passed). DashPay UI tests (kittest) pass (11).
• cargo clippy --all-features --all-targets -- -D warnings and cargo fmt --all -- --check pass.
• New tests cover:
  • automatic choice that skips disabled, expired, out-of-scope and unheld keys and prefers unlimited ones;
  • a stronger key signing a document type, and an unlimited key at any accepted level beating a limited one, in both document selectors;
  • the extended key judged by its live copy;
  • raised limits keeping the stored key matched;
  • DashPay key levels and key types;
  • contact QR codes against upstream: a proof DET signs verifies with upstream's verifier only for its sender, recipient and account reference; the exported key equals upstream's derivation from the wallet seed; expired and old-format codes are refused; the provider refuses every other request;
  • the unlock wallet following a contract-bound key, and the key chosen on the username screen;
  • the unlinked identity's owner;
  • the "no usable key" notice;
  • informational key lookups;
  • the refusal mappings.
• No live network or GUI flows were run.

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

🤖 Generated with Claude Code

https://claude.ai/code/session_012ujJpB9DBhvtbjhYB3aG9Q

PR Hygiene · 1e47ac2

• Bots — coderabbitai skipped after the window · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it
• Self-review — not asked of a bot author
• Build failed
• Approvals
  • files with no dedicated owner (docs/user-stories.md, src/backend_task/dashpay.rs, src/backend_task/dashpay/auto_accept_handler.rs and 54 more) — lklimek
  • lklimek requested changes — waiting for them to re-review or dismiss

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

• New Features
  • DashPay contact QR codes use the standard format, display the contact’s username, and expire after one hour. Scanning a valid code sends a contact request that can be accepted automatically; expired or unreadable codes are rejected before sending.
  • Transfer screens include a shortcut to manage keys when no transfer key is available.
• Improvements
  • Signing-key selection accounts for key status, expiry, permissions, security level, and the action being performed. Automatic selection prefers unrestricted eligible keys; advanced selection shows relevant caveats and warnings.
  • Wallet selection follows the key used for an action. Username registration and token actions use keys eligible for their requirements, with clearer guidance when a key cannot sign.

[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: 895ff8a
• Latest commit, not reviewed: 1e47ac2

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.

Walkthrough

The PR adds shared signing-key eligibility and selection rules, then applies them to identity, document, DPNS, DashPay, and token actions. It also replaces the DashPay QR proof flow with DIP-15 contact QR generation, parsing, and contact-request submission.

Changes: Signing rules and DashPay contact flows

Layer / File(s)	Summary
Define signing-key eligibility and selection src/model/identity_key_usability.rs, src/model/qualified_identity/*, src/model/dpns_usernames.rs, src/model/qualified_identity/encrypted_key_storage.rs, src/backend_task/error.rs	Signing scopes, caveats, and requirements drive key selection by purpose, security level, key type, bounds, expiry, and local signability. SDK signing refusals map to typed task errors. Key identity comparison and DPNS selection use updated rules.
Apply scoped signing and resolve action wallets src/context/*, src/ui/helpers.rs, src/ui/contracts_documents/*, src/ui/dashpay/*, src/ui/identity/*, src/ui/tokens/*, src/backend_task/identity/register_dpns_name.rs, tests/backend-e2e/measure_dpns_registration.rs, tests/kittest/identity_usernames.rs	Document, identity, DPNS, DashPay, and token actions use scoped key selection. Screens resolve wallets from signing keys. Regression tests cover key selection, wallet resolution, and ownership fallback.
Implement DIP-15 contact QR flow src/model/dashpay.rs, src/wallet_backend/auto_accept.rs, src/backend_task/dashpay/*, src/backend_task/error.rs, src/ui/dashpay/qr_*, docs/user-stories.md	The wallet backend builds contact QR URIs. The scanner parses ContactQr values and sends them through the contact-request task, which signs the proof. The generator removes account-index and validity controls. Documentation describes the QR format and behavior.

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant QRScannerScreen
  participant ContactQr
  participant DashPayTask
  participant ContactRequests
  QRScannerScreen->>ContactQr: Parse URI and check expiry
  ContactQr-->>QRScannerScreen: Parsed QR or parse error
  QRScannerScreen->>DashPayTask: Send contact request with QR
  DashPayTask->>ContactRequests: Submit request using QR username
  ContactRequests->>ContactQr: Sign proof for sender, recipient, and account reference

Loading

Suggested reviewers: lklimek, thepastaclaw

Merge Risk: Low · previous review 895ff

Users with multiple wallets may see the wrong unlock prompt, and some simple-mode token actions may fail late instead of rejecting an unusable key earlier. These are bounded risks that can be accepted with owner awareness or fixed before merging.

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: selecting signing keys accepted by Platform on protocol 14.
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

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/dash-evo-tool
• Branch: fix/protocol14-signing-key-selection
• File: .coderabbit.yaml
• Commit: 895ff8a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4d11f396-e6de-4c92-aa99-56bba82b2ac6

Commits

Reviewing files that changed from the base of the PR and between 573c26c and 895ff8a.

Files selected for processing: 5

• src/model/identity_key_usability.rs
• src/model/qualified_identity/mod.rs
• src/ui/helpers.rs
• src/ui/tokens/claim_tokens_screen.rs
• src/ui/tokens/transfer_tokens_screen.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.

[thepastaclaw]

⛔ Final review complete — 1 blocking finding(s) (commit 1e47ac2) · triage: critical · Phase 2 only (no Phase 1 for this repository)

[Claudius-Maginificent]

Re latest review-body findings:

Avoid a wallet error while waiting for a manual token key (FIXED): Fixed in 39d41fd. The token action, transfer, purchase, pricing and config constructors now look up the unlock wallet only when a key was picked (key.map_or(Ok(None), …), e.g. src/ui/tokens/token_action_screen.rs:221). Real lookup errors still raise their banner. Test: opening_the_form_without_an_automatic_key_raises_no_wallet_error.

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

[github-actions]

Waiting for bot review — coderabbitai not yet · 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 2 only (no Phase 1 for this repository)

One verified automatic-selection mismatch remains: profile writes can select an unavailable key even though the profile screen selects an obtainable alternative. The 36 prior findings reconcile to 30 fixes, five tracked deferrals, and one withdrawal. Verification was static-only against head 39d41fd; current CI shows Clippy passing and Test Suite pending.

🔴 1 blocking

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This large, intricate change directly alters signing-key eligibility and selection in src/model/identity_key_usability.rs and src/model/qualified_identity/mod.rs, plus private-key availability and wallet-unlock handling across transaction flows.
• Phase 1 reviewers: not run (disabled for this repository)
• 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 — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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 `src/ui/dashpay/profile_screen.rs`:
- [BLOCKING] src/ui/dashpay/profile_screen.rs:141: Use the same availability policy in the upstream profile signer
  The new wallet lookup uses `dashpay_signing_key`, which filters candidates through `holds_obtainable_key`, but the actual profile write uses a different availability policy. `WalletBackend::dashpay_write_profile` passes the plain `QualifiedIdentity` to both upstream profile writers; platform-wallet's `profile_signing_key` selects through `Signer::can_sign_with`, whose implementation accepts any matching stored placement. With a lower-ID eligible HIGH ECDSA key stored only as `PrivateKeyData::Encrypted` and a later obtainable HIGH ECDSA key, the screen selects the latter, while the writer selects the former and fails during private-key resolution instead of using the available key. The existing `ensure_profile_signing_key` preflight also uses the weaker predicate and does not prevent this. Supply an external signer whose availability predicate uses `holds_obtainable_key` while delegating signing to the existing secret-access path, leaving selection in platform-wallet, and add a two-key regression covering profile creation and update. This is a missed automatic-selection sibling, not the manual-wallet synchronization tracked in #1137; deferring it requires a linked follow-up covering this specific writer path.

Dismissed by maintainer decision: the remaining blocker (DashPay profile writer's signer availability policy) predates this PR on v1.0-dev and is moved to #1134's scope — see #1117 (comment)

[github-actions]

@coderabbitai review

No review for 39d41fd9 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

[github-actions]

@thepastaclaw review

No review for 39d41fd9 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

[github-actions]

Ready for review — files with no dedicated owner: lklimek · re-review or resolve: lklimek.
Full checklist in the description.

[lklimek]

@thepastaclaw review

[github-actions]

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 /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 2 only (no Phase 1 for this repository)

Verified head 0044d4d against the source and pinned Platform implementation: 30 prior findings are fixed, six have tracked deferrals, and one remains withdrawn. One in-scope blocker remains in token signing-key reconciliation after identity refresh; the DPNS contract-group finding depends on membership that the pinned implementation cannot establish for DPNS. This was a static review with no builds or tests run locally; current-head Clippy passes and Test Suite remains pending.

🔴 1 blocking

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

🔴 Blocking: Reconcile automatic key selection when a token identity refreshes
src/ui/tokens/token_action_screen.rs:460-467

The stricter constructor leaves common.selected_key and common.selected_wallet unset when no key is currently usable, but refresh() updates only the identity. When that identity gains a usable candidate, the readiness gate recomputes Usable and opens the ordinary form without selecting the key that submission requires. For example, open the form with an expired CRITICAL key, extend its expiry on-chain, and fetch the identity again: RefreshIdentity persists the updated identity and publishes an Identities cache update, which calls this refresh method, but confirmation still fails with "No signing key selected". Before this change, the constructor retained that public key, so the expiry-extension case did not lose the selection. Adding a CRITICAL key through this form's Add key shortcut and returning through PopScreenAndRefresh also leaves the newly usable form without a selected key. Transfer, claim, purchase, and pricing forms have the same identity-only refresh pattern. Reconcile automatic selection and its unlock wallet when identity data changes, while preserving a deliberate manual choice, and add a regression covering unavailable candidate → usable candidate → refresh → submit. This is separate from the deferred synchronization after a manual chooser change in #1137.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This large, cross-cutting change directly alters signature key eligibility and selection in src/model/identity_key_usability.rs and private-key availability and signing behavior in src/model/qualified_identity/mod.rs, meeting both the intricacy and critical key-handling surface requirements.
• Phase 1 reviewers: not run (disabled for this repository)
• 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 — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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 `src/ui/tokens/token_action_screen.rs`:
- [BLOCKING] src/ui/tokens/token_action_screen.rs:460-467: Reconcile automatic key selection when a token identity refreshes
  The stricter constructor leaves `common.selected_key` and `common.selected_wallet` unset when no key is currently usable, but `refresh()` updates only the identity. When that identity gains a usable candidate, the readiness gate recomputes `Usable` and opens the ordinary form without selecting the key that submission requires. For example, open the form with an expired CRITICAL key, extend its expiry on-chain, and fetch the identity again: `RefreshIdentity` persists the updated identity and publishes an Identities cache update, which calls this refresh method, but confirmation still fails with "No signing key selected". Before this change, the constructor retained that public key, so the expiry-extension case did not lose the selection. Adding a CRITICAL key through this form's Add key shortcut and returning through `PopScreenAndRefresh` also leaves the newly usable form without a selected key. Transfer, claim, purchase, and pricing forms have the same identity-only refresh pattern. Reconcile automatic selection and its unlock wallet when identity data changes, while preserving a deliberate manual choice, and add a regression covering unavailable candidate → usable candidate → refresh → submit. This is separate from the deferred synchronization after a manual chooser change in #1137.

[Claudius-Maginificent]

Fixed in 1e47ac2. The token action, transfer, claim, purchase and pricing forms now call pick_key_if_none (src/ui/tokens/mod.rs:92) at the end of refresh() (e.g. src/ui/tokens/token_action_screen.rs:469). While no key is chosen, it picks the automatic key and its unlock wallet from the refreshed identity, so a key that became usable since the form opened is signed with. A key already chosen, automatically or by hand, is kept; syncing the unlock wallet after a manual choice stays with #1137. Regression tests: a_refresh_picks_the_claim_key_that_became_usable (also checks that a manual choice survives) and a_refresh_picks_the_token_action_key_that_became_usable (expired key → live wallet-derived key → refresh() → key and wallet set).

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

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. 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 2 only (no Phase 1 for this repository)

Static verification of head 1e47ac2 confirms the token-refresh correction and reconciles all 38 prior findings: 31 fixed, six intentionally deferred with tracking, and one withdrawn. One blocking cross-purpose key-ordering defect remains, alongside two non-blocking suggestions concerning document-create scope and QR integration coverage. Current CI reports Test Suite, Clippy, and Platform pin guard passing; PR Hygiene remains pending, and no local builds or tests were run.

🔴 1 blocking | 🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-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: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The large, cross-cutting diff directly changes signature authorization and private-key availability and selection in src/model/identity_key_usability.rs and src/model/qualified_identity/mod.rs, alongside encrypted key handling in src/model/qualified_identity/encrypted_key_storage.rs.
• Phase 1 reviewers: not run (disabled for this repository)
• 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 — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (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 `src/model/qualified_identity/mod.rs`:
- [BLOCKING] src/model/qualified_identity/mod.rs:1337-1344: Prefer unlimited keys across both accepted token-signing purposes
  The outer find_map completes AUTHENTICATION selection before considering TRANSFER, so unlimited-first ordering applies only within each purpose. An obtainable, budgeted CRITICAL authentication key therefore wins over an obtainable, unlimited CRITICAL transfer key. This unnecessarily spends the delegated budget; when that budget is exhausted, transfer and claim submissions fail despite an available unrestricted signing key. The pinned DPP BatchTransitionV1 accepts either purpose for a single token transfer or claim, and IDN-024 explicitly prefers an unlimited held key over a limited one. Rank eligible candidates across both purposes by limits first, retaining authentication preference within the same limits class. Add a regression with a limited authentication key and an unlimited transfer key.

In `src/ui/contracts_documents/document_action_screen.rs`:
- [SUGGESTION] src/ui/contracts_documents/document_action_screen.rs:2132-2139: Include create-consumption bounds in the upstream-selector follow-up
  This scope covers only the named document type, including for creates. The pinned drive-abci advanced-structure validator also checks every consumable_document_type_names() entry: a key bound to the created type cannot authorize consumption of another type. An earlier unlimited key bound only to the created type can consequently be selected ahead of an unrestricted key, and Platform rejects the create through its paid out-of-bounds path. The owning create-aware validation is being exposed in dashpay/platform#5588 and consolidated in dashpay/platform#5660. Include this caller explicitly in dashpay/dash-evo-tool#1133: its current patch still supplies DocumentType rather than DocumentCreate. Keep the rule upstream instead of adding another DET-side copy, and cover a create that consumes another document type.

In `src/ui/dashpay/qr_code_generator.rs`:
- [SUGGESTION] src/ui/dashpay/qr_code_generator.rs:474-480: Exercise QR generation through the production secret-session path
  The two-wallet regression verifies the ownership lookup and unlock-wallet helper but never generates a QR code. The separate interoperability tests construct AutoAcceptKeyExport from a supplied seed and assemble the URI directly, so neither test exercises GenerateAutoAcceptQrCode through dashpay_build_auto_accept_qr, SecretScope::HdSeed, and the upstream builder. Incorrect wallet or secret-scope wiring in that production chain could therefore leave all these tests green. Add an offline async regression with a wired context, two runtime-generated wallets, and a cached username. Dispatch the production task, parse its returned URI, sign a proof, and verify that it succeeds against the linked owner's wallet and fails against the unrelated wallet.

[thepastaclaw]

🔴 Blocking: Prefer unlimited keys across both accepted token-signing purposes

The outer find_map completes AUTHENTICATION selection before considering TRANSFER, so unlimited-first ordering applies only within each purpose. An obtainable, budgeted CRITICAL authentication key therefore wins over an obtainable, unlimited CRITICAL transfer key. This unnecessarily spends the delegated budget; when that budget is exhausted, transfer and claim submissions fail despite an available unrestricted signing key. The pinned DPP BatchTransitionV1 accepts either purpose for a single token transfer or claim, and IDN-024 explicitly prefers an unlimited held key over a limited one. Rank eligible candidates across both purposes by limits first, retaining authentication preference within the same limits class. Add a regression with a limited authentication key and an unlimited transfer key.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

[thepastaclaw]

🟡 Suggestion: Include create-consumption bounds in the upstream-selector follow-up

This scope covers only the named document type, including for creates. The pinned drive-abci advanced-structure validator also checks every consumable_document_type_names() entry: a key bound to the created type cannot authorize consumption of another type. An earlier unlimited key bound only to the created type can consequently be selected ahead of an unrestricted key, and Platform rejects the create through its paid out-of-bounds path. The owning create-aware validation is being exposed in dashpay/platform#5588 and consolidated in dashpay/platform#5660. Include this caller explicitly in #1133: its current patch still supplies DocumentType rather than DocumentCreate. Keep the rule upstream instead of adding another DET-side copy, and cover a create that consumes another document type.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

[thepastaclaw]

🟡 Suggestion: Exercise QR generation through the production secret-session path

The two-wallet regression verifies the ownership lookup and unlock-wallet helper but never generates a QR code. The separate interoperability tests construct AutoAcceptKeyExport from a supplied seed and assemble the URI directly, so neither test exercises GenerateAutoAcceptQrCode through dashpay_build_auto_accept_qr, SecretScope::HdSeed, and the upstream builder. Incorrect wallet or secret-scope wiring in that production chain could therefore leave all these tests green. Add an offline async regression with a wired context, two runtime-generated wallets, and a cached username. Dispatch the production task, parse its returned URI, sign a proof, and verify that it succeeds against the linked owner's wallet and fails against the unrelated wallet.

_{source: gpt-6.1-sol (phase2-reviewer: rust-quality)}