Carry the proposal's inner call on multisig execute - #622
Closed
n13 wants to merge 4 commits into
Closed
Conversation
Regenerates the polkadart bindings against the runtime from chain PR #675 and threads the proposal's stored call through `multisig.execute`, so the executor signs the call that dispatches instead of an opaque proposal id — in both the in-app and the hardware (Keystone) flow. Unlike `approve`, execute takes a `Box<RuntimeCall>`: inline, no length prefix. `CallDecoder.decodeRuntimeCall` decodes the stored bytes back into the call, so a proposal the bundled metadata cannot decode fails before signing rather than producing a call the chain rejects. The regenerated metadata also trims the runtime surface, which the clients have to follow: - pallet-recovery is gone. `RecoveryService` and the recovery describer go with it; nothing in the apps called them. - Balances `force_*` and `upgrade_accounts`, Treasury `set_treasury_portion` and every Utility call but `batch_all` are gone, along with their describers. - `ReversibleTransfers::GuardianIndex` is gone. `getInterceptedAccounts` now walks the `HighSecurityAccounts` map and keeps the entries naming the guardian, preserving the entrusted-accounts feature. - The cold wallet's `force_transfer` debug payload is dropped; the display case it covered (funds leaving an account that is not the signer) is now exercised through a multisig proposal. Tests: execute round-trips the stored bytes, encodes inline (asserted against approve's length-prefixed encoding), and rejects undecodable or trailing bytes; `requireCallBytes` guards both resubmitting actions.
This was referenced Aug 29, 2026
A call a signer cannot review is one they cannot meaningfully approve, so bound what propose, approve and execute will build. Sized by the largest call we ever expect to put through a multisig: a `batch_all` of 32 transfers (double the 16 a batch is expected to carry). At the worst-case encoding of every field that is 1667 bytes, or 1707 inside a multisig wrapper, so 2 KiB leaves headroom. The chain's own `MaxCallSize` is 10 KiB; this is deliberately tighter. The limit sits on the multisig builders, not on `CallDecoder.decodeRuntimeCall`: `system.set_code` and `preimage.note_preimage` legitimately carry a runtime blob, and the chain's 10 KiB proposal bound already keeps them out of a multisig.
…er limit A client limit below the chain's refuses proposals the chain accepts, leaving a multisig no cold signer can act on. Use the chain's number: 10 KiB, from `pallet_multisig::Config::MaxCallSize`.
…data The polkadart bindings were regenerated against spec 147 / tx 6, but `bundledSpecVersion` and `bundledTransactionVersion` still said 136 / 3. That made `ParsedPayload.specMatchesBundled` false for every real payload, so the cold wallet would have warned that the call may have been decoded against shifted indices when nothing was wrong. Also raises the Dart parser's payload cap to 12 KiB, matching the firmware and the reference parser, so a payload one accepts is not rejected by another.
Collaborator
Author
|
Superseded. Based on main before the 1.5.12+129 snapshot (78a698f), which regenerated part of the bindings and rewrote call_decoder.dart / app_constants.dart / multisig_service.dart, leaving this branch conflicting. The execute-with-inner-call change is still needed — main is spec 146 and multisig.execute still takes only (multisigAddress, proposalId) — so it will be redone as one PR on current main together with the cold wallet side (#623). Branch deleted; restorable from this page if the diff is needed as reference. |
adamtpang
pushed a commit
to adamtpang/quantus-apps
that referenced
this pull request
Sep 4, 2026
…uantus-Network#575) * fix: display and sign multisig proposal call from chain storage (M3) The multisig approve/execute/cancel confirm sheets rendered recipient and amount built from GraphQL indexer JSON, while the signed extrinsic only committed to a proposal id. A compromised indexer could label a malicious drain proposal as a benign transfer to every co-signer. The confirm sheet now loads the authoritative inner call bytes from on-chain Multisig.Proposals storage before showing the sheet content: - transfer calls (balances transfer_*, reversible schedule_transfer*) display the decoded recipient and amount from those bytes - any other call is shown as raw hex bytes - if the on-chain proposal cannot be loaded, the sheet fails closed and the confirm button stays disabled The approve extrinsic uses the new pallet API (chain PR Quantus-Network#622) which takes the inner call bytes and rejects approvals that are not byte-equal to the stored payload (CallMismatch), binding the signature to the reviewed call. The same bytes feed the Keystone QR signing flow, both for the displayed details and the unsigned payload. Generated planck bindings for the multisig pallet were regenerated from a local node running the current chain runtime (dart run polkadart_cli:generate against ws://127.0.0.1:9944); only the four files with semantic changes were kept. ProposalData dropped callWeight in the new runtime, so the regenerated storage codec is required to decode Proposals at all. Addresses finding M3 of the 2026-07-22 mobile wallet security audit. * style: apply dart format * chore: remove unnecessary import * fix: address review feedback on on-chain proposal call display - Unify transfer-call decoding in MultisigProposal.decodeTransferCall (was duplicated between MultisigService and MultisigProposal._decodeCallRaw) and cover it with unit tests: all 4 transfer variants, non-transfer call, non-Id destination, malformed bytes, trailing bytes. - Reject calls with trailing bytes after decode (fall back to raw hex). - Enrich open proposals with on-chain call data at fetch time so the proposal list and detail sheet show chain truth instead of indexer JSON; a stored call that is not a recognized transfer clears the indexer details. - Render raw call hex in a constrained scrollable block in the confirm sheet and middle-truncate it on the Keystone signing screen. - Disconnect the RPC provider after multisig storage queries. Verified the un-regenerated decode-path files against a fresh polkadart codegen from wss://a1-planck.quantus.cat: pallet_balances/call.dart, multi_address.dart and all pallet_multisig files are byte-identical; pallet_reversible_transfers/call.dart differs only in doc comments; runtime_call.dart pallet indices match (Balances=2, ReversibleTransfers=11, Multisig=19) - the committed file only carries extra variants (Scheduler=8, Referenda=10, ConvictionVoting=12) absent from the live runtime, which cannot affect transfer decoding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the hot wallet for the runtime in chain#675.
Goes with keystone3-firmware #19. The cold wallet side is a follow-up PR stacked on this one.
What changed on chain
multisig.executenow takes the call it dispatches:The chain only dispatches it if it re-encodes to the stored proposal bytes. So the signer sees the real call, hardware wallets included.
One catch.
approvetakesBoundedVec<u8>, which is length prefixed.executetakesBox<RuntimeCall>, which is inline with no prefix. SobuildExecuteCalldecodes the stored bytes back into a call rather than passing them through. That is what the newCallDecoder.decodeRuntimeCalldoes. If the bundled metadata cannot decode the proposal, it fails before signing instead of building a call the chain would reject.Both flows are covered.
estimateExecuteFeeandsubmitExecuteExtrinsictake the bytes and fetch them when missing, exactly like approve. The Keystone flow gets them from the confirm sheet's existingloadCallBytes, which execute already called to show the proposal.requireCallBytesreplaces the inline throw approve had, and now guards both actions.Call size limit
Propose, approve and execute refuse a call over 10 KiB, which is the chain's own
MaxCallSize. Using the chain's number matters. Anything smaller would refuse proposals the chain accepts, and then no cold signer could act on them.The limit sits on the multisig builders, not on
CallDecoder.decodeRuntimeCall.system.set_codeandpreimage.note_preimagelegitimately carry a runtime blob, and the chain's 10 KiB proposal bound already keeps those out of a multisig.Runtime surface changes
Regenerating the bindings is not just the execute field. The new runtime removes a lot, and the clients have to follow:
pallet-recoveryRecoveryServiceand the recovery describer, nothing used themforce_transfer,force_unreserve,force_set_balance,force_adjust_total_issuance,upgrade_accountsset_treasury_portionbatch_all_dispatchAswith themReversibleTransfers::GuardianIndexGuardianIndex. The guardian to accounts reverse index is gone, and
getInterceptedAccountswas its only caller. It backs the entrusted accounts screen. Rather than drop the feature, it now pagesstate_getKeysPagedover theHighSecurityAccountsmap and keeps the entries naming this guardian. The map isBlake2_128Concat, so each key ends with the unhashedAccountId32.Cold wallet debug payload.
force_transferwas the example of "funds leave an account that is not the signer". A multisig proposal has the same property, so that case uses one now.A bug this found
The bindings are now spec 147 and tx version 6, but
bundledSpecVersionandbundledTransactionVersionstill said 136 and 3. That madeParsedPayload.specMatchesBundledfalse for every real payload, so the cold wallet would have warned that a call may have been decoded against shifted indices when nothing was wrong. Fixed.The Dart parser's payload cap also goes from 8 KiB to 12 KiB, matching the firmware and the reference parser, so one parser does not reject what another accepts.
Tests
buildExecuteCallround-trips the stored bytes exactly. Its inline encoding is checked by comparing its length against approve's length-prefixed encoding. Bytes that do not decode, trailing bytes and oversized calls are all rejected.requireCallBytesreturns the bytes and throws a namedStateErrorwithout them.All with
--exclude-tags=native, as CI runs them: quantus_sdk 261, mobile-app 295, cold-wallet-app 40.