Repository navigation
feat(bridge-status-controller): pass swap asset ids to Tron, Solana, and Bitcoin snap requests - #10775
Merged
Conversation
Battambang
force-pushed
the
feat/tron-flow-classification-analytics
branch
2 times, most recently
from
October 9, 2026 14:49
cd64d52 to
1d9e239
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add missing no-ID coverage, correct the bridge snapshot fixture, and update the stale validation comment.
2 open findings
What changed in this PR
Updates Tron Snap transaction requests with asset IDs so swaps and bridges are classified correctly.
Changes:
- Adds conditional source and destination asset IDs for Tron.
- Updates tests and snapshots.
- Updates the changelog and lint suppressions.
| File | Description |
|---|---|
packages/bridge-status-controller/src/utils/transaction.test.ts |
Tests Tron asset-ID propagation. |
packages/bridge-status-controller/src/utils/snaps.ts |
Adds asset IDs to Tron request options. |
packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap |
Updates request snapshots. |
packages/bridge-status-controller/CHANGELOG.md |
Documents the behavior change. |
oxlint-suppressions.json |
Updates lint suppression count. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…ests Include the sourceAssetId and destAssetId in the Tron signAndSendTransaction options so the Tron snap can classify the transaction as a same-chain swap or a cross-chain bridge, mirroring the Stellar snap.
Battambang
force-pushed
the
feat/tron-flow-classification-analytics
branch
from
October 9, 2026 15:48
1d9e239 to
44b7ded
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…quests Include the sourceAssetId and destAssetId in the Solana signAndSendTransaction options so the Solana snap can classify the transaction as a same-chain swap or a cross-chain bridge, mirroring the Stellar snap. The Solana and Stellar branches share the same asset-id-only options shape, so they are combined into a single condition.
…equests Include the sourceAssetId and destAssetId in the Bitcoin signAndSendTransaction options so the Bitcoin snap can classify the transaction as a same-chain swap or a cross-chain bridge, mirroring the Stellar snap. The Bitcoin snap ignores unknown options, so no strict rollout ordering is required.
…ichain-flow-classification-analytics
…ltichain-flow-classification-analytics # Conflicts: # oxlint-suppressions.json # packages/bridge-status-controller/CHANGELOG.md # packages/bridge-status-controller/src/utils/snaps.ts
…ultichain-flow-classification-analytics # Conflicts: # oxlint-suppressions.json # packages/bridge-status-controller/CHANGELOG.md # packages/bridge-status-controller/src/utils/snaps.ts
…t ids Replace the mixed isStellarTrade/isBitcoinTrade guards with isStellarChainId/isBitcoinChainId so all three non-Tron chains use the same chain-id check, since getClientRequest already receives srcChainId. The Tron branch is unchanged as it reads fields from the trade object.
Battambang
changed the title
Add the pull request reference to the combined Tron, Solana, and Bitcoin changelog entry.
Battambang
commented
Oct 10, 2026
Comment on lines
+96
to
+99
| if ( | ||
| isStellarChainId(srcChainId) || | ||
| isSolanaChainId(srcChainId) || | ||
| isBitcoinChainId(srcChainId) |
Contributor
Author
There was a problem hiding this comment.
Why srcChainId instead of trade
Two reasons: consistency, and the trade shape does not always tell you the chain.
1. The trade shape is a lossy proxy for the chain
The strategy layer establishes that trade shape maps 1:1 to chain, but note the Stellar case in strategy/index.ts:
case ChainId.STELLAR:
return txs.every((tx) => typeof tx === 'string' || isStellarTrade(tx));A Stellar trade is either a string or an { xdr / xdrBase64 } object. So isStellarTrade(trade) is false for a string-shaped Stellar trade, and that string is indistinguishable from a Solana trade by shape alone. The chain id is the only reliable signal for Stellar-vs-Solana strings, which is exactly why the original Solana check used isSolanaChainId(srcChainId) (there is no isSolanaTrade).
2. Consistency: one condition, one kind of check
Before, the condition mixed two kinds of check:
isStellarTrade(trade) || // trade-shape guard
isSolanaChainId(srcChainId) || // chain guard
isBitcoinTrade(trade) // trade-shape guardgetClientRequest already has srcChainId in scope (it derives scope from it on the first line), so using chain guards for all three makes the condition uniform:
isStellarChainId(srcChainId) ||
isSolanaChainId(srcChainId) ||
isBitcoinChainId(srcChainId)3. It is also slightly more accurate
isStellarChainId accepts both pubnet and testnet (XlmScope.Pubnet / XlmScope.Testnet), whereas isStellarTrade only reflects the payload shape.
Behavior equivalence
| Chain | Before | After | Same? |
|---|---|---|---|
| Stellar (object xdr) | isStellarTrade -> true |
isStellarChainId -> true |
Yes |
| Stellar (string xdr) | isStellarTrade -> false |
isStellarChainId -> true |
After is more correct |
| Solana | isSolanaChainId -> true |
isSolanaChainId -> true |
Yes |
| Bitcoin | isBitcoinTrade -> true |
isBitcoinChainId -> true |
Yes |
| Tron | not in this condition | not in this condition | Yes (separate branch) |
| EVM | all false | all false | Yes |
The only behavioral difference is the string-shaped Stellar case, where the new version is correct and the old one would have silently omitted the asset ids.
…he swap fixture The Solana swap fixture set destChainId to Solana but gave the destination asset an eip155 CAIP id. Now that the swap asset ids are forwarded to the snap, that mismatch would classify the same-chain swap as a bridgeSend, contradicting the fixture's own swap_type of single_chain. Use a Solana asset id for the destination asset and regenerate the affected snapshots.
…he swap fixture The Solana swap fixture set destChainId to Solana but gave the destination asset an eip155 CAIP id. Now that the swap asset ids are forwarded to the snap, that mismatch would classify the same-chain swap as a bridgeSend, contradicting the fixture's own swap_type of single_chain. Use a Solana asset id for the destination asset and regenerate the affected snapshots.
… requests handleNonEvmTx always attached the quote's source and destination asset ids to the signAndSendTransaction options, including for the token approval that precedes a swap or bridge. The Tron snap gives those asset ids precedence over the contract type, so an approve was classified as a swap or bridgeSend instead of tokenApprove. Add an isApproval flag and only attach the asset ids to the main trade. The approval request now falls back to the contract-type classification.
# Conflicts: # packages/bridge-status-controller/src/utils/snaps.ts
Battambang
marked this pull request as draft
October 13, 2026 07:26
Julink-eth
reviewed
Oct 13, 2026
Contributor
Author
|
Snap release PR: MetaMask/internal-snaps#443 |
Julink-eth
marked this pull request as ready for review
October 13, 2026 09:16
Julink-eth
marked this pull request as draft
October 13, 2026 09:16
Julink-eth
previously approved these changes
Oct 13, 2026
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
infiniteflower
approved these changes
Oct 13, 2026
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.
Explanation
Current state
getClientRequestbuilds thesignAndSendTransactionoptions per non-EVM chain. Before this PR it forwarded the swap/bridgesourceAssetId/destAssetIdonly for Stellar. Tron, Solana, and Bitcoin therefore could not see the swap asset ids, so their snaps could not tell a same-chain swap from a cross-chain bridge and reportedunknown(Tron) or a balance-derivedsend(Solana, Bitcoin) for those transactions.Solution
Forward
sourceAssetIdanddestAssetIdfor Tron, Solana, and Bitcoin, mirroring the existing Stellar behavior.visibleflag and contracttypein the same options object.isStellarChainId,isSolanaChainId,isBitcoinChainId) rather than trade-shape guards, sincegetClientRequestalready receivessrcChainIdand a Stellar trade can also be a plain base64 string.Each snap then compares the CAIP-2 chains of the two asset ids to classify the transaction as a same-chain
swapor a cross-chainbridgeSend.Non-obvious points
handleNonEvmTxnow takes anisApprovalflag and only attaches the asset ids to the main trade. Without this, the approval request would inherit the swap asset ids, and the Tron snap (which gives the asset ids precedence over the contract type) would classify anapproveasswap/bridgeSendinstead oftokenApprove.isTronTradebecause it needs to readvisibleand the contract type off the trade object.destChainIdto Solana but gave the destination asset aneip155CAIP id. Now that the asset ids are forwarded, that mismatch would classify the same-chain swap as abridgeSend, so the fixture was corrected to use a Solana asset id.References
Checklist
Note
Medium Risk
Changes non-EVM transaction submission payloads that snaps use for tx classification; incorrect approval vs main-trade handling could mislabel user-facing flows, though behavior is additive when asset ids are omitted.
Overview
Non-EVM snap
signAndSendTransactionrequests now includesourceAssetIdanddestAssetIdinoptionsfor Solana, Bitcoin, and Tron (in addition to Stellar), so snaps can distinguish same-chain swaps from cross-chain bridges. Tron still mergesvisibleand contracttypeinto the same options object.handleNonEvmTxgains anisApprovalflag (wired from the non-EVM strategy for approval txs) so swap/bridge asset ids are not sent on token approvals—avoiding misclassification when Tron would otherwise treat an approve as swap/bridge.Tests and snapshots were updated for correct CAIP asset ids (e.g. Solana native/USDC, Tron TRC20) and to assert asset ids on main trades but not approvals.
Reviewed by Cursor Bugbot for commit ec899ef. Bugbot is set up for automated code reviews on this repo. Configure here.