fix(visualsign): render number fields as amount_v2 (VSP has no number type)#393
Open
prasanna-anchorage wants to merge 2 commits into
Open
fix(visualsign): render number fields as amount_v2 (VSP has no number type)#393prasanna-anchorage wants to merge 2 commits into
number fields as amount_v2 (VSP has no number type)#393prasanna-anchorage wants to merge 2 commits into
Conversation
…ber` type) VSP defines no `number` field type (HSM vsp.py + iOS decoder list text_v2, address_v2, amount_v2, preview_layout, list_layout, delta, accordion, diagnostic, divider) -- numeric values are `amount_v2`. Previously a `number` field decoded to `.unknown` and surfaced as unsupported. Serialize the `Number` variant as `amount_v2` at the wire layer: amount = the numeric value, abbreviation = the unit (empty for unitless numbers, which iOS renders fine -- parseAmount accepts an empty abbreviation). The in-memory `Number` variant and `create_number_field` helper are unchanged, so callers and variant-matching tests keep working; only the wire output changes. This covers every `number` producer (compute_budget, system, token_2022, jupiter_swap, ...). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
prasanna-anchorage
requested review from
Copilot,
pepe-anchor and
shahan-khatchadourian-anchorage
and removed request for
Copilot
…tion The Compute Unit Limit / Price per Compute Unit fields now serialize as amount_v2 (amount + abbreviation derived from fallback text) instead of the removed `number` type. Updates the two fixture blocks accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates VisualSign field serialization to avoid emitting the unsupported number wire type by serializing SignablePayloadField::Number as amount_v2, aligning with VSP’s supported field types and ensuring Solana “number” producers (e.g., compute budget fields, slippage) no longer decode as unknown/unsupported.
Changes:
- Serialize
SignablePayloadField::Numberasamount_v2, derivingAmountV2.abbreviationfromfallback_text. - Update CLI expected-display fixture output to reflect
amount_v2+AmountV2payload instead ofnumber+Number. - Update Jupiter swap preset test to assert
Slippageserializes asamount_v2on the wire.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/visualsign/src/lib.rs |
Changes Number-field serialization to emit amount_v2 wire shape and updates expected-field verification accordingly. |
src/parser/cli/tests/fixtures/solana-json.display.expected |
Updates expected JSON fixture output for compute-budget number fields to the new amount_v2 representation. |
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs |
Adjusts serialization assertions in tests to match the new wire Type for slippage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+276
to
+279
| let amount_v2 = SignablePayloadFieldAmountV2 { | ||
| amount: number.number.clone(), | ||
| abbreviation: Some(abbreviation), | ||
| }; |
Comment on lines
+334
to
336
| // Serialized as `amount_v2` under the hood (see `serialize_to_map`). | ||
| SignablePayloadField::Number { .. } => base_fields.push("AmountV2"), | ||
| SignablePayloadField::Amount { .. } => base_fields.push("Amount"), |
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.
VSP defines no
numberfield type — the HSMvsp.pyenum and the iOS decoder recognizetext_v2, address_v2, **amount_v2**, preview_layout, list_layout, delta, accordion, diagnostic, divider. So anumberfield decoded to.unknownand surfaced as unsupported.This serializes the
Numbervariant asamount_v2on the wire:amount= the numeric value,abbreviation= the unit (empty for unitless numbers — iOSparseAmountaccepts an empty abbreviation). The in-memoryNumbervariant and thecreate_number_fieldhelper are unchanged (callers + variant-matching tests keep working); only the wire output changes. Covers everynumberproducer (compute_budget, system, token_2022, jupiter_swap, …).Replaces #390 (which used the legacy
fix/number-fields-text-v2branch name and an earlier text_v2 approach).🤖 Generated with Claude Code