feat(solana): render off-chain messages, not just transactions

[shahan-khatchadourian-anchorage]

VP2a of the NEAR Intents PR plan.

SolanaTransactionWrapper holds a transaction only, so a personal-sign message
has no representation and the parser answers one with a decode error. This adds
a Message variant: a message decodes and renders as its text.

How a message is told from a transaction

A message arrives as a JSON envelope, {"message": "..."}, and a transaction as
base64 or hex. Neither encoding alphabet contains a brace, so the two cannot
collide, and from_string tries the envelope first — a malformed envelope then
reports its own error rather than surfacing as a transaction decode failure.

The envelope carries no framing of its own because Solana adds none: the
signature covers the message bytes themselves. It exists only to tell the two
apart on a wire that carries both. An envelope with an unknown field is refused
rather than partly rendered.

Charset handling

The text is escaped, not filtered. The module-local charset_safe drops
characters it cannot render, which suits an argument preview but not text a
signer approves: dropping lets two different messages render identically. A
single marker character fails the same way less obviously, because the marker
is itself renderable — a literal ? collides with anything marked down to one.
So an unrenderable character becomes \u{...} and a literal backslash becomes
\\, which is reversible and keeps distinct messages distinct.

One consequence worth naming for review: a non-Latin script escapes character
by character, so such a message reads as a run of escapes rather than as text.
That is conservative for a signing screen, but it is a visible difference from
wallets that show the raw text, and worth settling before the render is
compared against them.

Signing and intermediate output

The signature covers the message bytes directly, so there is no message
structure to serialize — the message is its own preimage.

A message therefore produces no intermediate output. Those bytes are not a
serialized transaction message, so handing them to the transaction parser
would at best decode to nothing and at worst attach instruction metadata to a
payload that has no instructions, which a policy engine would read as real.

Tests

solana_message_tests covers the envelope decode, the rendered field, the
refusal of an unknown field, that a base64 transaction is never read as an
envelope, and two distinctness properties: a marked separator does not collide
with a literal marker, and a literal backslash cannot forge an escape.

Fuzzing

fuzz_transaction_string feeds arbitrary UTF-8 into transaction_string_to_visual_sign,
so it reaches the new {-prefixed branch. fuzz-solana.yml runs only when a PR
carries the test:fuzz label, which the path labeler does not apply — worth
adding here, since this introduces a new entry branch that accepts arbitrary
JSON.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved rendering ambiguity and incorrect intermediate metadata handling can affect signing safety.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds support for parsing and rendering Solana off-chain messages alongside transactions.

Changes:

• Adds JSON message-envelope parsing and a Message variant.
• Renders message text and preserves raw message bytes.
• Adds parsing, rendering, charset, and regression tests.

File summaries

File	Review findings
src/chain_parsers/visualsign-solana/src/core/visualsign.rs	Critical (3 votes): ? replacement is not injective. Moderate (3 votes): skip intermediate transaction metadata for messages. Moderate (1 vote): reject duplicate JSON keys. Nit (1 vote): strengthen the base64 transaction regression test.

Review details

Suppressed comments (2)

src/chain_parsers/visualsign-solana/src/core/visualsign.rs:105

• serde_json::Value overwrites repeated object keys, so an input such as {"message":"A","message":"B"} is accepted with whichever value the parser keeps. Other components may apply a different duplicate-key policy, allowing the same envelope to render/sign a different message. Deserialize into a strict envelope type (with unknown fields denied) or otherwise reject duplicate keys.

    let value: serde_json::Value = serde_json::from_str(json).map_err(|e| e.to_string())?;
    let object = value.as_object().ok_or("envelope is not a JSON object")?;
    for key in object.keys() {

src/chain_parsers/visualsign-solana/src/core/visualsign.rs:2680

• This regression test passes when from_string rejects the input, so it does not verify the behavior named by the test: a valid base64 transaction still decoding as a transaction. Use a known-good transaction fixture and assert successful parsing plus a non-Message variant; otherwise a regression that rejects all base64 transactions would still pass.

        let decoded = SolanaTransactionWrapper::from_string(tx_b64);
        assert!(
            decoded.is_err() || decoded.unwrap().inner_message().is_none(),
            "base64 input must never be read as a message envelope"

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[shahan-khatchadourian-anchorage]

Correct, and it defeats the invariant the function claimed. ? is itself ASCII-graphic so it passed through unmarked, which means innocent\nTo: attacker.sol and innocent?To: attacker.sol rendered identically.

The existing distinct_messages_stay_distinct test missed it because it only compared the newline form against the deleted form, never against a literal marker — so it asserted the property while the counterexample was one character away.

Replaced the marker with a reversible escape: an unrenderable character becomes \\u{...}, and a literal backslash becomes \\\\ so a message carrying one cannot forge an escape for a character it does not contain. Extended the distinctness test with the literal-marker case and added a_literal_backslash_cannot_forge_an_escape.

[shahan-khatchadourian-anchorage]

Agreed, and this is the one with signing-safety consequences. Fixed by returning early: the Message arm now yields no intermediate output at all rather than feeding its bytes to build_intermediate_bytes.

That also aligns the code with the position already recorded for this slice — a Solana message has no intermediate output, because the decoder decodes transactions and a message is not one. Emitting instruction metadata for a payload with no instructions is worse than emitting none, since a policy engine has no way to tell the difference.

[shahan-khatchadourian-anchorage]

Both Copilot findings are addressed in c2dc52d3 (amended, force-pushed).

Strengths worth keeping: the envelope-vs-transaction discrimination is sound — neither base64 nor hex contains a brace, so the two encodings cannot collide, and trying the envelope first means a malformed one reports its own error instead of surfacing as a transaction decode failure. Refusing unknown envelope fields rather than ignoring them is the right default for something a signer approves.

On the CodeQL alert: it names decode_accounts, create_accounts_advanced_preview_layout and decode_v0_accounts. This diff adds no logging, and those call sites number five on main and five here — untouched. The alert surfaces because the file was modified, not because this change introduced it, so I have left it alone rather than fold an unrelated fix into a one-file PR.

Verification. visualsign-solana passes in all three configurations make test exercises: default 298, --no-default-features 292, --features diagnostics 297. The full-workspace make test link OOMs in my container, so the workspace build is CI's; it passed there before this revision and should again. The test:fuzz label is on deliberately — fuzz_transaction_string feeds arbitrary UTF-8 straight into transaction_string_to_visual_sign, so it reaches the new {-prefixed branch, and the path labeler only applies chain:solana.

One open question for a human reviewer, unchanged by the fixes: a non-Latin-script message now escapes character by character, so it reads as a run of \\u{...} rather than as text. That is conservative for a signing screen, but it is a visible difference from wallets that show the raw text, and worth settling before the render is compared against them.