feat(swap-widget): thorchain support - #12318
Merged
Merged
Conversation
… tx data - Dispatch on transactionData.type discriminated union instead of stale chain-flag fallbacks - Drop legacy relay/butter/solana metadata fields and nested quote.quote.steps escape hatch from ApiQuoteStep/QuoteResponse - Drop dead top-level transactionData; add affiliateAddress to match public-api response - Extract shared getErrorMessage helper that handles viem shortMessage and wallet RPC error objects - Standardize user-facing error voice with debug context in console.error - Tighten machine selector to avoid re-rendering on every input/asset/amount change Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughSwap approval and execution hooks were refactored to use snapshot-based context reading, extract transaction execution into chain-specific helpers, add Thorchain transaction metadata propagation, and consolidate error handling across EVM, UTXO, and Solana flows. ChangesSwap Execution and Approval Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/swap-widget/src/hooks/useSwapApproval.ts (1)
47-71: ⚡ Quick win
sellAmountBaseUnitguard fires after chain switch.The validation at line 68 that aborts with
APPROVAL_ERRORis evaluated only after the chain-switch at lines 50–53 has already executed. A missing or zerosellAmountBaseUnittherefore triggers a wallet chain-switch popup with no subsequent transaction.♻️ Proposed fix — move the guard up, before the chain switch
+ if (!sellAmountBaseUnit || sellAmountBaseUnit === '0') { + actorRef.send({ type: 'APPROVAL_ERROR', error: 'No sell amount specified' }) + return + } + const requiredChainId = getEvmNetworkId(sellAsset.chainId) const client = walletClient as WalletClient const currentChainId = await client.getChainId() if (currentChainId !== requiredChainId) { await switchOrAddChain(client, requiredChainId) } // ... chain config setup ... - if (!sellAmountBaseUnit || sellAmountBaseUnit === '0') { - actorRef.send({ type: 'APPROVAL_ERROR', error: 'No sell amount specified' }) - return - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swap-widget/src/hooks/useSwapApproval.ts` around lines 47 - 71, The guard for sellAmountBaseUnit should run before any wallet chain switching to avoid triggering a wallet popup when there's no amount; move the sellAmountBaseUnit check (the block that sends actorRef.send({ type: 'APPROVAL_ERROR', ... })) above the code that derives requiredChainId/getEvmNetworkId(sellAsset.chainId) and before creating/using walletClient and calling client.getChainId()/switchOrAddChain; ensure functions referenced (sellAmountBaseUnit, actorRef.send, getEvmNetworkId, walletClient, client.getChainId, switchOrAddChain) remain unchanged otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swap-widget/src/hooks/useSwapApproval.ts`:
- Around line 31-38: The destructuring from actorRef.getSnapshot().context pulls
sellAsset without checking it, then useSwapApproval accesses sellAsset.assetId
which can throw; fix by guarding sellAsset before using it (e.g., after
obtaining { quote, sellAsset, sellAmountBaseUnit } from
actorRef.getSnapshot().context, check if sellAsset is truthy and if not send
actorRef.send({ type: 'APPROVAL_ERROR', error: 'Missing sellAsset in context' })
and return), and only proceed to compute sellAssetAddress when sellAsset is
present; update references in useSwapApproval to use this guard so no property
access occurs on undefined.
---
Nitpick comments:
In `@packages/swap-widget/src/hooks/useSwapApproval.ts`:
- Around line 47-71: The guard for sellAmountBaseUnit should run before any
wallet chain switching to avoid triggering a wallet popup when there's no
amount; move the sellAmountBaseUnit check (the block that sends actorRef.send({
type: 'APPROVAL_ERROR', ... })) above the code that derives
requiredChainId/getEvmNetworkId(sellAsset.chainId) and before creating/using
walletClient and calling client.getChainId()/switchOrAddChain; ensure functions
referenced (sellAmountBaseUnit, actorRef.send, getEvmNetworkId, walletClient,
client.getChainId, switchOrAddChain) remain unchanged otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9083441a-3167-42fe-892c-1385c5f2f25b
📒 Files selected for processing (4)
packages/swap-widget/src/hooks/useSwapApproval.tspackages/swap-widget/src/hooks/useSwapExecution.tspackages/swap-widget/src/types/index.tspackages/swap-widget/src/utils/errors.ts
Populate `step.thorchainTransactionMetadata` (to/data/value/memo) for EVM, UTXO, cosmossdk, Solana, and TRON in `getL1RateOrQuote`, and add a cosmossdk `getThorTxData` helper. Lets downstream consumers build the deposit tx without re-fetching inbound address data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Read deposit address/memo/value from the step's `thorchainTransactionMetadata` instead of doing a runtime `resolveDepositContext` inbound-address fetch. Drops the `ThorLikeQuote`/`DepositExtractionContext` plumbing and the DEPOSIT_ADDRESS_UNAVAILABLE failure mode now that the swapper populates the metadata at quote time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Uncomment THORChain in the SwapperName enum and its icon/color in the swapper constants so it shows up in the route selector. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kaladinlight
changed the title
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts (1)
450-477: 💤 Low valueCosmos branch: no
Promise.allSettledwrapping after hoisting vault fetch.After the refactor,
cosmossdk.getThorTxDataandcosmosChainAdapter.getFeeDataare awaited at the top of the case but not wrapped in try/catch (unlike EVM/UTXO/Solana/Tron which usePromise.allSettledper route). A throw here propagates out ofgetL1RateOrQuoterather than producing aTradeQuoteError.UnsupportedTradePairErr result. Since these failures are shared across all routes the practical impact is small (the cosmos sell flow is also deferred per the PR), but consider catching and returningErr(makeSwapErrorRight(...))for symmetry with the other branches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts` around lines 450 - 477, The CosmosSdk branch in getL1RateOrQuote currently awaits cosmosChainAdapter.getFeeData and cosmossdk.getThorTxData at the top and will throw out of getL1RateOrQuote on failure; wrap those awaits in a try/catch (or use Promise.allSettled like the other branches) so failures produce an Err result instead of throwing, and in the catch return Err(makeSwapErrorRight({ cause: error, code: TradeQuoteError.UnsupportedTradePair, details: 'cosmos tx/fee fetch failed' })) so perRouteValues mapping still matches other chains and errors are surfaced as a TradeQuoteError via makeSwapErrorRight.packages/swapper/src/thorchain-utils/cosmossdk/getThorTxData.ts (2)
22-24: 💤 Low valueNative RUNE/CACAO returns
{ vault: '' }, which silently suppresses cosmostransactionDatadownstream.The empty-string vault flows into
getL1RateOrQuote.ts(line 463-467) asthorchainTransactionMetadata.to = '', andextractCosmosTransactionDatainpackages/public-api/src/routes/quote/extractTransactionData.tschecksif (step.thorchainTransactionMetadata?.to), which is falsy for''. Net effect: native cosmos sells returntransactionData: undefined. This is acceptable given the PR explicitly defers Cosmos sells pendinguseCosmosSigning, but consider either (a) returning a discriminated shape (e.g.,{ kind: 'msgDeposit' } | { kind: 'msgSend', vault: string }) so callers can branch explicitly, or (b) leaving a TODO referencing the deferred MsgDeposit support so this doesn't get lost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/thorchain-utils/cosmossdk/getThorTxData.ts` around lines 22 - 24, getThorTxData currently returns { vault: '' } for native assets which causes downstream checks (e.g., getL1RateOrQuote.ts and extractCosmosTransactionData) to treat the value as falsy and drop transactionData; change the return to an explicit discriminated shape (for example return { kind: 'msgDeposit' } for native deposits and { kind: 'msgSend', vault: string } for normal sends) and update the getThorTxData return type/interface accordingly so callers (getL1RateOrQuote and extractCosmosTransactionData) can branch on kind; alternatively, if you prefer minimal change, add a clear TODO in getThorTxData referencing deferred MsgDeposit support and ensure the returned shape is unambiguous (not an empty string) so it isn't treated as falsy.
28-30: 💤 Low valueThrows
unwrapErr()instead of returningResult.Per coding guidelines, swapper code should use
Result<T, E>from@sniptt/monadsrather than throwing. The pattern matches sibling helpers (evm.getThorTxDataetc.), so changing it in isolation isn't worth it, but worth noting for a future cleanup pass — especially because this helper is now awaited directly ingetL1RateOrQuote.ts(line 454) withoutPromise.allSettledwrapping, so a throw propagates out of the whole quote function. As per coding guidelines, "UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/thorchain-utils/cosmossdk/getThorTxData.ts` around lines 28 - 30, The current getThorTxData implementation throws on inbound lookup failure (it calls res.unwrapErr()), violating the Result<T,E> pattern; change it to return an Err(...) instead of throwing: after calling getInboundAddressDataForChain(daemonUrl, sellAsset.assetId, true, swapperName) check res.isErr() and return Err(res.unwrapErr()); otherwise continue and wrap successful payloads with Ok(...). Update getThorTxData's Promise return type to Promise<Result<..., ...>> if needed and ensure you import and use Ok/Err from `@sniptt/monads` so callers (e.g., getL1RateOrQuote) receive a Result instead of exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/swapper/src/thorchain-utils/cosmossdk/getThorTxData.ts`:
- Around line 22-24: getThorTxData currently returns { vault: '' } for native
assets which causes downstream checks (e.g., getL1RateOrQuote.ts and
extractCosmosTransactionData) to treat the value as falsy and drop
transactionData; change the return to an explicit discriminated shape (for
example return { kind: 'msgDeposit' } for native deposits and { kind: 'msgSend',
vault: string } for normal sends) and update the getThorTxData return
type/interface accordingly so callers (getL1RateOrQuote and
extractCosmosTransactionData) can branch on kind; alternatively, if you prefer
minimal change, add a clear TODO in getThorTxData referencing deferred
MsgDeposit support and ensure the returned shape is unambiguous (not an empty
string) so it isn't treated as falsy.
- Around line 28-30: The current getThorTxData implementation throws on inbound
lookup failure (it calls res.unwrapErr()), violating the Result<T,E> pattern;
change it to return an Err(...) instead of throwing: after calling
getInboundAddressDataForChain(daemonUrl, sellAsset.assetId, true, swapperName)
check res.isErr() and return Err(res.unwrapErr()); otherwise continue and wrap
successful payloads with Ok(...). Update getThorTxData's Promise return type to
Promise<Result<..., ...>> if needed and ensure you import and use Ok/Err from
`@sniptt/monads` so callers (e.g., getL1RateOrQuote) receive a Result instead of
exceptions.
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts`:
- Around line 450-477: The CosmosSdk branch in getL1RateOrQuote currently awaits
cosmosChainAdapter.getFeeData and cosmossdk.getThorTxData at the top and will
throw out of getL1RateOrQuote on failure; wrap those awaits in a try/catch (or
use Promise.allSettled like the other branches) so failures produce an Err
result instead of throwing, and in the catch return Err(makeSwapErrorRight({
cause: error, code: TradeQuoteError.UnsupportedTradePair, details: 'cosmos
tx/fee fetch failed' })) so perRouteValues mapping still matches other chains
and errors are surfaced as a TradeQuoteError via makeSwapErrorRight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d987e18-8216-4945-a1fb-8f1a68db475b
📒 Files selected for processing (10)
packages/public-api/src/routes/quote/extractTransactionData.tspackages/public-api/src/routes/quote/getQuote.tspackages/public-api/src/routes/quote/types.tspackages/public-api/src/routes/quote/utils.tspackages/swap-widget/src/constants/swappers.tspackages/swap-widget/src/types/index.tspackages/swapper/src/thorchain-utils/cosmossdk/getThorTxData.tspackages/swapper/src/thorchain-utils/cosmossdk/index.tspackages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/types.ts
💤 Files with no reviewable changes (1)
- packages/public-api/src/routes/quote/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/swap-widget/src/types/index.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kaladinlight
changed the title
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.
Description
Enables THORChain as a routable swapper in the swap-widget end-to-end. Three layers:
1.
swapper— populatestep.thorchainTransactionMetadataat quote timegetL1RateOrQuotenow stamps athorchainTransactionMetadata: { to, data?, value?, memo? }on each step, for every chain namespace it handles (EVM, UTXO, cosmossdk, Solana, TRON). Newcosmossdk/getThorTxDatahelper mirrors the existingevm/utxo/solana/tronhelpers — handles the special case of native-asset deposits on the swapper's own chain (RUNE on THORChain, CACAO on MAYAChain) which useMsgDepositand have notoaddress.This makes the swapper the single source of truth for the deposit tx params. The field is additive and optional, so existing consumers (main web app, etc.) are unaffected.
2.
public-api— read deposit tx data from the stepextractTransactionDatanow readsto/memo/valuefromstep.thorchainTransactionMetadatafor both UTXO (utxo_deposit) and cosmos (cosmos) tx types, and from EVM steps where the metadata carries router-call calldata.Drops the runtime
resolveDepositContextinbound-address fetch and theThorLikeQuote/DepositExtractionContextplumbing — the swapper now provides this at quote time. Also drops theDEPOSIT_ADDRESS_UNAVAILABLE(503) failure mode that fired when THORNode was slow on the second fetch.3.
swap-widget— wire up THORChain + clean upuseSwapExecutionSwapperName.Thorchainand its icon/color so it shows up in the route selector and quotes modal.transactionData.type('evm' | 'utxo_deposit' | 'utxo_psbt' | 'solana' | 'cosmos') via a flat switch inuseSwapExecution. Per-branch helpers (executeEvm,executeUtxoDeposit,executeSolana) own their own wallet-state preconditions. Deferred types (utxo_psbt,cosmos) throw a clear "not yet supported" error rather than silently falling through.packages/swap-widget/src/types/index.ts:relayTransactionMetadata,butterSwapTransactionMetadata,solanaTransactionMetadatafromApiQuoteStep(consolidated intotransactionData).quote.quote.stepsescape hatch fromQuoteResponse.QuoteResponse.transactionDatafield — public-api only emits it insidesteps[i].affiliateAddress?: stringto match what public-api actually returns.getErrorMessagehelper inpackages/swap-widget/src/utils/errors.tsthat prefers viem'sshortMessage, handles plain RPC error objects ({ code: 4001, message: 'User rejected' }), and falls back gracefully. Replaces fragileJSON.stringify(error)paths.console.error.s.context(which re-rendered on every input/asset/amount change). Pull machine context fromactorRef.getSnapshot()at fire time.End-user widget coverage in this PR: EVM sells (router contract call) and UTXO sells (
utxo_depositwith memo as OP_RETURN). Cosmos sells (RUNE / ATOM via THORChain) are plumbed through the swapper + public-api, but the widget still throws "not yet supported" — landing cosmos signing requires auseCosmosSigninghook + AppKit cosmos adapter and is out of scope here.Issue (if applicable)
closes #
Risk
Cross-package:
swapper,public-api,swap-widget.TradeQuoteStep. No on-chain behavior change. Main web app picks up the new field but doesn't read it; existing THORChain integration there is unaffected./v1/swap/quote(not just the widget). Sameto/memo/valuevalues — just pulled from the swapper at quote time rather than re-fetched. Removes theDEPOSIT_ADDRESS_UNAVAILABLE503 path.Testing
Engineering
End-to-end against a local public-api at
http://localhost:3005:txData.type === 'evm', router contract call broadcasts,txHashreturned.txData.type === 'utxo_deposit',bitcoin.sendTransferbroadcasts to vault with memo in OP_RETURN.cosmostransactionData); widget execution should show "This swap is not yet supported — please try a different route" rather than silent failure.shortMessage(e.g. "User rejected the request") rather than a JSON-stringified error blob.POST /v1/swap/quotewith a THORChain-routable pair → response carriessteps[0].transactionDatawith the deposit address/memo/value populated, noDEPOSIT_ADDRESS_UNAVAILABLE503.Operations
The widget itself is not yet shipped to end users.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Refactoring