Cold wallet: review the call a multisig execute dispatches - #623
Closed
n13 wants to merge 8 commits into
Closed
Conversation
`multisig.execute` now carries the proposal's inner call (chain PR #675), and the chain dispatches it only when it re-encodes to the stored bytes. The cold wallet can therefore review the executed call instead of showing a proposal id and a note saying the contents are not part of what is signed. - `CallDecoder` describes execute like approve: a nested "You are executing" call that also supplies the summary, so the headline reads SEND rather than naming the wrapper. `cancel` and `remove_expired` keep the reference note, which is still true for them. - The debug payload menu gains a `Msig execute` entry. - The standalone reference parser catches up on both calls: `approve` gained its bound call bytes on chain some time ago and was still decoding the old two-field shape, so it could not parse a current approval at all. Execute's inline nesting recurses inside the codec before `MAX_CALL_DEPTH` is reached, so decoding is bounded with `decode_with_depth_limit`.
Mirrors the firmware limit: a batch_all of 32 transfers is 1667 bytes at the worst-case encoding, 1707 inside a multisig wrapper, so 2 KiB leaves headroom. Checked on the top-level call and on each nested call's bytes; execute's inline inner call is bounded by the top-level check that contains it.
Parity with the firmware: propose, approve and execute all refuse to render an inner call larger than 2 KiB, so an oversized proposal fails the same way on every cold signer instead of producing a screen nobody can review.
…ghter limit A client limit below the chain's refuses proposals the chain accepts. Use the chain's number (10 KiB), applied to nested call bytes only — the outer extrinsic call carries a wrapper that puts a chain-valid propose past it, and stays bounded by MAX_PAYLOAD_BYTES, which rises to 12 KiB so such a proposal can arrive.
Nine signing requests, one per call the Keystone firmware parses today. Each case carries the payload, the envelope, the UR parts, one SVG per QR frame and a viewer page that animates them, so the same bytes can be unit tested and shown to a real camera. The test walks the manifest and checks every case decodes to the call it claims, including the nested call for propose, approve and execute. It also checks the manifest and the folders agree, and that the corpus targets the runtime the app bundles, so a metadata regeneration cannot leave the corpus stale unnoticed. Frame counts are the reason this is the reduced set: the payload is hex inside JSON, so QR data is roughly twice the payload, and UR splits it at 200 bytes. Even a 118 byte transfer needs 2 frames. Generated by the quantus-cli `generate_qr_fixtures` example.
Collaborator
Author
|
Superseded. This branch is stacked on #622, which is based on main before the 1.5.12+129 snapshot (78a698f) — that snapshot rewrote call_decoder.dart, added call_policy.dart, regenerated part of the bindings and deleted rust-transaction-parser/, so both branches conflict heavily. Redoing the work as a single PR on current main. Branch deleted; restorable from this page if the diff is needed as reference. |
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
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.
Stacked on #622. Review that one first. This PR is the cold wallet display, the reference parser, and a QR test corpus.
Showing the executed call
With chain#675,
multisig.executecarries the call it dispatches, and the chain only dispatches it if it matches the stored proposal. So the cold wallet can show that call instead of a proposal id under a note saying its contents are not signed.CallDecodernow describesexecutethe way it already describesapprove: a nested "You are executing" call that also supplies the summary, so the screen headlines SEND with the amount and recipient.cancelandremove_expiredkeep the old note, which is still true for them.Reference parser
rust-transaction-parserhad drifted from the chain in two ways.approvewas already broken. The chain added the bound call bytes toapprovesome time ago, and this parser still decoded the old two-field shape. It could not parse a current approval at all. It now decodes and shows the approved call.executegains the inner call. Because it is inline rather than length prefixed, SCALE decoding recurses before the parser's own depth check runs. About 200 nested executes fit in the payload cap. Decoding now usesdecode_with_depth_limit, so that fails on a counter instead of the stack. The three duplicated depth checks are now one helper.Both parsers also cap nested calls at 10 KiB, the chain's own
MaxCallSize, and raise the payload cap to 12 KiB so a chain-maximum proposal can arrive.QR test corpus
New:
cold-wallet-app/test/fixtures/qr/reduced/. Nine signing requests, one per call the Keystone firmware parses today.Each case has the payload hex, the envelope JSON, the UR parts, one SVG per QR frame, and an
index.htmlthat animates them. There is a top-levelindex.htmlto pick a case. Open it in a browser and show it to a device camera or the simulator, which scans by screenshotting the screen.A test walks the manifest and parses every case. It checks each decodes to the call it claims, including the nested call for propose, approve and execute. It also checks the manifest matches the folders on disk, and that the corpus targets the runtime the app bundles. So regenerating metadata and forgetting the corpus fails the build instead of going unnoticed.
I also ran all nine through the firmware Rust parser. All nine decode there too.
The corpus is generated, not hand written. See the quantus-cli
generate_qr_fixturesexample, added in quantus-cli#145. It encodes calls through the CLI's bundled chain metadata, so the bytes match what the chain accepts. The README next to the fixtures has the command.Why this is the reduced set
The payload is hex inside JSON, so QR data is about twice the payload size, and UR splits it at 200 bytes. Even a 118 byte transfer needs 2 frames. A chain-maximum 10 KiB call would be roughly 100 frames, which is not practical to scan. A fuller set can be added later, but big calls should be treated as a deliberate stress case, not a normal one.
set_codeis deliberately absent. It is being replaced by approving a hash for runtime upgrades.Tests
quantus_sdk 264, cold-wallet-app 60, rust-transaction-parser 24.