Skip to content

fix(swap): don't attach a Solana schema the device cannot verify - #420

Open
BitHighlander wants to merge 2 commits into
developfrom
fix/solana-schema-withheld-not-rejected
Open

fix(swap): don't attach a Solana schema the device cannot verify#420
BitHighlander wants to merge 2 commits into
developfrom
fix/solana-schema-withheld-not-rejected

Conversation

@BitHighlander

Copy link
Copy Markdown
Collaborator

"[swap] SIGN FAILED: Invalid Solana instruction schema" on a relay Solana swap,
with no user interaction — the device rejected before any confirm screen.

The perverse part

Having clear-sign material made the flow WORSE than not having it:

no schema found -> needsOpaqueSolanaFallback -> consent panel -> blind sign -> works
schema found -> fallback skipped -> schema attached -> HARD REJECT

swap.ts attached the schema whenever findSolanaSchema returned one, without
checking the device could verify it. Firmware then fails the entire request
(fsm_msg_solana.h:803) and deliberately does not fall back — the comment above
it is explicit: "Present-but-invalid schema material fails the request; it never
silently degrades to blind signing." That fail-closed stance is correct; sending
material we know is unusable is not.

Verification runs signed_metadata_verify_attestation, which resolves the key via
metadata_pubkey_for -> RAM-loaded signers only, and rejects a loaded signer
unless AdvancedMode is on. Both die on power cycle. So on a stock or
just-rebooted device the outcome is not "might fail" but GUARANTEED failure —
worded as though the user's transaction were malformed.

Fix

Withhold the schema when AdvancedMode is known-off, and mirror that in the
opaque-fallback predicate so the swap takes the consent path it would have taken
had no schema existed. The user gets the opt-in they should always get instead
of a reject, consistent with #417/#419.

AdvancedMode is a NECESSARY condition for verification, so this removes the
whole guaranteed-failure class. It is not sufficient: AdvancedMode on with no
signer armed still fails, because Vault does not track which signers are loaded
(clearsignLoadSessionSigner sends the message and records an event, but keeps no
state). That residual now gets an actionable message naming the real cause
instead of "Invalid Solana instruction schema", which blames the transaction.

Closing the residual properly means tracking loaded key ids and invalidating
them on reboot/disconnect. Deliberately not done here: stale tracking state
would reintroduce exactly this bug, and the AdvancedMode check already covers
the reboot case since the policy resets too.

make test-unit 488 pass, 0 fail across 33 files, +35 btc-backend, +10 cosmos
tsc --noEmit 627, unchanged, 2 below baseline 629

"[swap] SIGN FAILED: Invalid Solana instruction schema" on a relay Solana swap,
with no user interaction — the device rejected before any confirm screen.

## The perverse part

Having clear-sign material made the flow WORSE than not having it:

  no schema found  -> needsOpaqueSolanaFallback -> consent panel -> blind sign -> works
  schema found     -> fallback skipped -> schema attached -> HARD REJECT

swap.ts attached the schema whenever findSolanaSchema returned one, without
checking the device could verify it. Firmware then fails the entire request
(fsm_msg_solana.h:803) and deliberately does not fall back — the comment above
it is explicit: "Present-but-invalid schema material fails the request; it never
silently degrades to blind signing." That fail-closed stance is correct; sending
material we know is unusable is not.

Verification runs signed_metadata_verify_attestation, which resolves the key via
metadata_pubkey_for -> RAM-loaded signers only, and rejects a loaded signer
unless AdvancedMode is on. Both die on power cycle. So on a stock or
just-rebooted device the outcome is not "might fail" but GUARANTEED failure —
worded as though the user's transaction were malformed.

## Fix

Withhold the schema when AdvancedMode is known-off, and mirror that in the
opaque-fallback predicate so the swap takes the consent path it would have taken
had no schema existed. The user gets the opt-in they should always get instead
of a reject, consistent with #417/#419.

AdvancedMode is a NECESSARY condition for verification, so this removes the
whole guaranteed-failure class. It is not sufficient: AdvancedMode on with no
signer armed still fails, because Vault does not track which signers are loaded
(clearsignLoadSessionSigner sends the message and records an event, but keeps no
state). That residual now gets an actionable message naming the real cause
instead of "Invalid Solana instruction schema", which blames the transaction.

Closing the residual properly means tracking loaded key ids and invalidating
them on reboot/disconnect. Deliberately not done here: stale tracking state
would reintroduce exactly this bug, and the AdvancedMode check already covers
the reboot case since the policy resets too.

make test-unit   488 pass, 0 fail across 33 files, +35 btc-backend, +10 cosmos
tsc --noEmit     627, unchanged, 2 below baseline 629
Follow-up: the AdvancedMode-based withholding in the previous commit was not
enough, and it inverted.

Observed: AdvancedMode off correctly withheld the schema and the device asked to
enable it ("Enable AdvancedMode to blind-sign" -> opt-in panel, as designed).
The user enabled it. The next attempt re-attached the schema, the device could
not verify it, and they hit "Invalid Solana instruction schema". Fixing the
first refusal directly caused the second.

That is the tell that predicting device capability is unwinnable here.
Verification needs a signer loaded in RAM; Vault does not track which are
loaded; and both that and AdvancedMode die on power cycle. Any predicate built
on what we can see is wrong some of the time, and the AdvancedMode one is wrong
in the most confusing possible direction.

So react instead of predict. Firmware validates schema material BEFORE drawing
any confirm screen, so the refusal costs the user nothing and shows them
nothing. On refusal, ask for blind-sign consent.

Deliberately NOT a silent re-sign without the schema. Dropping it means the
transaction gets blind-signed, and the opaque-consent panel exists to show what
it actually moves (host-side outflow simulation) before that happens. Skipping
it would trade a gate for a silent downgrade — worse than the bug.

Schema attachment is now also suppressed once allowSolanaBlindSigning is set,
or the consent retry would re-attach, be refused again, and loop.

Resulting flow, no dead ends:

  AdvancedMode off            -> schema withheld -> consent -> blind sign
  AdvancedMode on + signer    -> schema attached -> verified -> clear sign
  AdvancedMode on, no signer  -> refused -> consent -> retry w/o schema -> signs

make test-unit   488 pass, 0 fail across 33 files
tsc --noEmit     627, unchanged, 2 below baseline 629
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