feat(solana): POST /solana/decode-transaction — decode without signing - #449
Merged
Merged
Conversation
The browser extension asks the user to approve a dApp transaction BEFORE it calls /solana/sign-transaction, so the decode inside the signing gate runs too late to be reviewed: the extension's approval card had nothing to render and showed "N/A" over a real transfer. This serves the same buildSolanaDecodedInfo the signing gate runs, so the two screens cannot drift. No wallet, no device, no overlay — not a signing route. A parse failure returns an explicit solanaDecodeError and requiresBlindSigningConsent: true, never a partial decode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The extension asks for approval before the vault sees the transaction, so this endpoint is the first — and for an unknown program the only — place a human-readable warning can come from. Returning just the decoded rows makes the caller invent its own wording, and two vocabularies for one transaction means the user reads whichever is softer. assessSigningRisk already produces the sentences the vault's own approval overlay shows, and it lives in src/shared, so the bun side returns the assessment rather than the caller re-deriving it. A decode failure returns one too: silence there reads as "nothing to worry about". Deliberately NOT returned: deviceClearSigns. Whether the DEVICE can clear-sign depends on the certified lookup in the signing gate — a network round-trip that is firmware-version dependent — so a decode must not promise it. The fixture is a real mainnet Cee-lo bet captured from the extension (SoltoshiDICE, tag 0x36, 1,000 SDICE, 0.01 SOL). Its wager moves by CPI from inside the game program, so no transfer instruction appears in the bytes: exactly the case that must read as "KeepKey cannot read this" rather than as an empty, reassuring summary.
…n vouches for A dapp bet is the case the approval screen was worst at. The wager moves by CPI from inside the game program, so there is no transfer instruction in the bytes: the overlay could say "app code KeepKey cannot read" and nothing else — not the amount, not the deposit, not what would be left. That is true but useless, and a user who sees only that learns to click through it. Three things, in the order a user reads them. WHAT IT PUTS UP. A certified envelope carries the reviewed schema the delegate signed, so the values in the instruction can be read back at the offsets that schema fixes — the same offsets the device reads. describeCertifiedSolanaTransaction does that, but only after confirming the envelope's payload IS the local catalog entry's serialization and the firmware's own rule applies it, so the vault renders what was signed rather than what a service said. Token amounts get a ticker only from the delegate's attestation of that exact mint; otherwise raw base units and the full mint, because a ticker nobody signed for is how a fake token passes for a real one. The SOL the call names is totalled with the network fee read from the ComputeBudget bytes — "what it asks for, not a limit on what the program can move". WHAT IS LEFT. simulateSolanaHoldings answers the question a decoder cannot, for any program: it watches native SOL plus the fee payer's token accounts this transaction NAMES (Solana requires every account a CPI touches to be listed, so an account absent from the list cannot change) and reports the post-state from one simulateTransaction call. Post-state only — the delta design is still wrong for the reason solana-outflow.ts documents. Wired into both /solana/decode-transaction and the REST signing preview, returned as its own simulatedOutflow field, labelled "checked on this computer", with a budget so a dead RPC cannot hold the approval window shut. It is an estimate, so it is fenced: it runs AFTER every gate is decided, it is added to the risk bar at 'low' where the level is a maximum, and a failed simulation says "could not simulate ... that is not the same as nothing moves". A test asserts no gate in rest-api.ts is ever assigned from it. THE VOUCH. The overlay now says what the signature covers, in as many words: the program's address, the name of the instruction, the layout of the values — and that it is "not a check of this site, of where the money goes, or of whether the program treats you fairly". The dapp name sits next to it as "Asked for by", never as something KeepKey endorses. A test forbids the words safe, verified, trusted and guarantee in that block, because the certified path is the one that skips the blind-signing consent. Driven by the live pair already in the fixtures: the SoltoshiDICE Blackjack join and the production Worker's own response to it (round 86, 1,000 SDICE, session key, 1 h), plus the captured Cee-lo bet for the unreadable case.
…olve The figure is presented as "up to X SOL", so understating it is the one way it can be wrong. It is read from the ComputeBudget instructions, and an instruction whose program id comes from an unresolved lookup table cannot be read at all — a SetComputeUnitPrice hiding there would be missed and the sentence would promise a ceiling the transaction can exceed. Nothing is shown instead. The bar already says "Part of this transaction is hidden and could not be looked up" in that case, which is the honest summary.
Airplane mode is a promise about outbound traffic, and this check is outbound traffic that did not exist before: a simulate plus two account listings, one of which sends the user's own address to whichever RPC is configured. Adding that silently to a setting whose whole point is "ask nobody anything" is not a trade the user made. With the setting on it returns "offline mode is on, so this computer made no network call" — an unavailable reason like any other, so the sentence reads as "could not check" and nothing downstream mistakes it for "nothing moves". Scope: only this check. The instruction decode and the certified lookup on the same route already went out over the network in airplane mode; that predates this change and is left alone rather than folded into it.
… total "SOL this call puts up, network fee included: 0.010005 SOL" read as the whole ask and was not. It left out account rent, which this call does charge: the Cee-lo dapp's own code computes what the user needs as deposit + solAccountRentLamports + 5000 and warns in as many words about "a refundable 0.01 SOL deposit plus network and account fees". On the captured bet that is an understatement of roughly 2x, and the trailing "not a limit on what the program can move" did not cure it — rent IS what the call asks for. Two figures with two provenances now, and no sum of them: SOL named in this call: 0.01 SOL (Deposit), plus up to 0.000005 SOL of network fee. Account rent is extra and is not in these bytes, so this is not the total SOL leaving your wallet, and it is not a limit on what the program can move. The lamports come out of the instruction's own bytes under the signed schema and are labelled with the label that schema gives them; the ceiling comes from this transaction's ComputeBudget instructions. Rent is in neither, so the copy says so rather than quietly dropping it into a total. This also settles a contradiction between the two branches: the catalog entry for the Cee-lo bet already says "the SOL figure on the device is the deposit, not the total SOL leaving the wallet", which is the opposite of what this sentence claimed. The exact string is pinned in the test — it is the number a user decides on.
…in to agree
The comment over this branch claimed "Identity only from the delegate's own
token attestation for this exact mint. A ticker from anywhere else is how a
fake token gets a trusted name" — a guarantee the host does not enforce.
Nothing here verifies the attestation's signature: the registry only
shape-validates tokenInfo (mint decodes to 32 bytes, symbol matches a charset,
decimals 0..9, signerKeyId 0x80) and no code path recovers or checks the
signer. So a ClearSign response carrying the wrong symbol for the right mint
rendered in the overlay as a trusted ticker.
The device does verify that signature, so this was display integrity rather
than signing safety — but the catalog entry already pins the identity locally,
and it was sitting unused one scope up. A ticker now renders only when the
attestation agrees with that pin on mint, symbol and decimals; a disagreement
on any of them, an entry that pins no token, or no attestation at all leaves
the amount in raw base units with the full mint, exactly as before. Note that
the attestation carries no token program, so the pin's tokenProgram has no
counterpart to compare against here; the comment now says what is actually
compared instead of claiming more.
This is the rule the clearsign Worker already applies on its side ("Solana
token identity contradicts the reviewed catalog pin; not certified"); the host
was the side that took the delegate's word for it.
Tested in both directions: the real production envelope still renders
"1,000 SDICE", and a symbol swap, a decimals shift and an attestation for a
different mint each fall back to "1,000,000,000 base units of token 4nCm…pump".
… dropped
A watched token account whose simulated post-state came back null or
unreadable was skipped with nothing said: splAmount returned null and the loop
simply did not push it. The length guard passed — simulateTransaction still
returns one entry per requested address — so the result looked complete, and
the sentence a user reads became "if this goes through, your account Gu83…4Xux
would hold 0.012345678 SOL." The wager had vanished, and silence there reads
as "that token is not moving".
Not a synthetic case. simulateTransaction returns null for an address that
does not exist at the post-state, which is exactly what a program that closes
the player's token account produces, and also for entries the RPC did not
load. The mint-resolution path one function up already takes the opposite
decision for the same reason ("reporting SOL while one watched mint silently
dropped out would read as 'that token is not moving'"); the simulation path
did not.
It now carries a note, and the copy renders it:
Token balances are incomplete: the simulation returned no readable state
for TokA…t111, so what that account holds afterwards was not established.
The SOL figure is still answered — it was established — but the token side is
stated as not established rather than left out.
Two smaller accuracy fixes in the same paths:
- "simulation returned no account states" fired whenever the count merely
differed from the number of watched accounts, including when the simulation
did return states. It now names both counts.
- holdingsAfter dropped `note` whenever `unavailable` was set, so a token
lookup failure followed by a simulation failure reported only the second and
lost the fact that the token side was never attempted. Both are kept, and
formatSimulatedHoldings renders the note on the unavailable branch too.
Regression test uses the real Cee-lo fixture with a simulation reply of
[{lamports: 12345678}, null].
…ade-up zero Three residuals from the adversarial review of the simulation floor: - A mismatched token attestation still produced a ticker, one line over. The pin cross-check lived in the describe path while the simulation was handed the same unverified attestation 13 lines later. Both paths now go through certifiedTokenIdentities: a symbol renders only where the delegate's attestation and the reviewed catalog entry's own pin agree, and anything else is raw base units plus the full mint. - An unreadable fee-payer balance became "would hold 0 SOL". Absent is now absent: no figure, and a note saying it was not established. The token side already worked this way; index 0 was the exception the comment claimed it was not. - The rent caveat sat inside the fee branch, so an unresolved lookup table withheld the fee figure and left the deposit standing alone as if it were the total — the exact shape the caveat exists to prevent.
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.
Pairs with keepkey/keepkey-client#152.
Why
The extension collects the user's approval for a dApp transaction before it calls
/solana/sign-transaction. Every bit of review this vault produces — the decode, the risk bar, the blind-signing policy — runs inside that signing gate, which is too late for the first screen the user actually sees. So the extension showedTO: N/A / AMOUNT: N/Aover a real mainnet transfer.What
POST /solana/decode-transaction, bearer-authed, taking{ raw_tx }(base64) and returning{ solanaDecoded, requiresBlindSigningConsent }or{ solanaDecodeError, requiresBlindSigningConsent: true }.It calls the same
buildSolanaDecodedInfo+requiresSolanaBlindSigningConsentthe signing gate calls, so there is one decoder and one set of words, not two that drift. Deliberately not a signing route: norequireWallet, no device, no approval overlay — a pure function of the bytes plus one ALT read for v0.On a parse failure it mirrors the signing gate: an explicit error string, never a partial decode dressed up as a summary. The caller is expected to render that as a refusal to review.
Verified live
Against this build running as the dev vault:
setComputeUnitLimit(units=120000),setComputeUnitPrice(microLamports=1000), SPL TokentransferChecked(amount=2000, decimals=6), Memo v2 — withrequiresBlindSigningConsent: false{"raw_tx":"AAAA"}returnsSolanaTxParseError: Malformed Solana transaction: unreasonable signature count (0)withrequiresBlindSigningConsent: true{}gives a 400 validation error namingraw_tx