Skip to content

feat(multisig): Keystone signing for multisig creation and proposals - #642

Merged
n13 merged 1 commit into
mainfrom
n13/multisig-keystone-signing
Sep 8, 2026
Merged

feat(multisig): Keystone signing for multisig creation and proposals#642
n13 merged 1 commit into
mainfrom
n13/multisig-keystone-signing

Conversation

@n13

@n13 n13 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #641.

Review: multisig calls vs Keystone (cold) signers

Call Software Keystone before Keystone after
Create multisig ok no path: submitExtrinsic(creator) needs a local keypair QR sign flow
Propose transfer ok no path: MultisigProposeStrategy.submit signed locally QR sign flow
Approve ok ok (MultisigActionConfirmSheet) unchanged
Execute ok ok unchanged
Cancel ok ok unchanged
Discover / disconnect read-only / local ok unchanged

Fee estimation already worked for Keystone accounts (unsigned dummy extrinsic), so only submission needed a path.

Changes

  • Create multisig: a Keystone creator skips biometric auth and opens the shared KeystoneSignScreen with the create_multisig call. The signed extrinsic goes through the same pending-creation record and poller as the local path, then returns to Accounts with the new multisig highlighted. The preflight result (draft + network fee) now feeds both paths instead of running the preflight twice.
  • Propose: MultisigProposeStrategy returns SendNeedsHardwareSignature for a Keystone member, mirroring RegularSendStrategy, and prefetches the sign payload on the review screen. prefetchSignPayload now receives the fee so the signed call uses the expiry block shown on review. proposeTransfer and the new external-signature variant share one submit-and-track path.
  • SendNeedsHardwareSignature carries terminalForHash, so the strategy decides the terminal content. Proposals keep no explorer link, same as the local path.
  • KeystoneSigningSession gains primaryLabel / secondaryLabel; the sign and verify screens caption the creation as ACTION / MULTISIG ADDRESS instead of AMOUNT / TO.
  • One Account.signsWithHardware helper replaces the two copies in the action sheet and the regular send strategy.
  • New strings: keystoneSignActionLabel, multisigCreateKeystoneAction (en + id).

Test

  • flutter test: 428 passed, including the new multisig_propose_strategy_test.
  • Manual: with a Keystone account active, Settings/Accounts → Create multisig → QR sign → scan signature → lands on Accounts with the new multisig. Multisig with a Keystone member → Propose → review → QR sign → scan → proposal appears.

Creating a multisig and proposing a transfer only signed locally, so a
Keystone (cold) member had no path: submitExtrinsic needs a local keypair.
Approve, execute and cancel already routed Keystone signers through the
QR flow; this brings the remaining two calls in line.

- Add multisig: a Keystone creator skips biometric auth and goes through
  the shared KeystoneSignScreen; the signed extrinsic is submitted via the
  same pending-creation record and poller as the local path. The single
  preflight result (draft + fee) now feeds both paths instead of running
  the preflight twice.
- Propose: MultisigProposeStrategy returns SendNeedsHardwareSignature for
  a Keystone member, mirroring RegularSendStrategy, and prefetches the sign
  payload on the review screen (the fee now reaches prefetch so the signed
  call uses the reviewed expiry block). proposeTransfer shares one
  submit-and-track path with the new external-signature variant.
- SendNeedsHardwareSignature carries terminalForHash so each strategy
  decides its terminal; proposals keep no explorer link, like the local path.
- KeystoneSigningSession gains primaryLabel/secondaryLabel so the sign and
  verify screens can caption non-transfer calls (ACTION / MULTISIG ADDRESS).
- One Account.signsWithHardware helper replaces the two copies in the
  action sheet and regular send strategy.
- Tests: propose strategy hardware outcome; pumpRef helper shared in fakes.
@n13
n13 force-pushed the n13/return-to-accounts-nav-fix branch from 105a56f to 1617545 Compare September 8, 2026 03:18
@n13
n13 force-pushed the n13/multisig-keystone-signing branch from 50385d7 to 338c27a Compare September 8, 2026 03:18
@n13 n13 added the bot-review Request automated review from review-bot label Sep 8, 2026
Base automatically changed from n13/return-to-accounts-nav-fix to main September 8, 2026 03:20

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT-6 Astra

Verdict (advisory): Approve

Reviewed head 338c27ad55926f0467e40f6661e53c12131b48e9 against base b98c1349286599ac51b2ccd3bfc2e8fa49563f12. No blocking findings.

The hardware paths reuse the existing call builders and submit-and-track behavior. The creation draft consistently supplies signing parameters and pending-account details; proposal expiry is carried from the reviewed fee breakdown into both the signing call and cache identity. Proposal success keeps its existing terminal content without adding a transfer explorer link.

Validation:

  • flutter test --no-pub --reporter expanded in mobile-app: 428 passed.
  • Four temporary review probes outside the repository passed: proposal call-parameter/cache binding and signed callback forwarding; local/external creation tracking; rollback after external submission rejection. These used mocked submission services.
  • Workspace dart format lib test --line-length=120 --output=none --set-exit-if-changed via Melos: passed.
  • git diff --check: passed.
  • Melos analysis was capped at 10 seconds. Cold wallet, miner, and SDK analysis passed; mobile analysis did not finish. GitHub currently reports successful dependency-cooldown checks, with no application CI result.

A physical Keystone round trip and live-chain submission were not exercised during this review.

@n13 n13 removed the bot-review Request automated review from review-bot label Sep 8, 2026
@n13
n13 merged commit 83f74fc into main Sep 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant