feat: thorchain solana swapper integration#12036
Conversation
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
getThorTxData, getUnsignedSolanaTransaction, getSolanaTransactionFees. Follows tron/ pattern: vault lookup + SPL Memo Program instruction for fee estimation, hdwallet SolanaTxInstruction for buildSendApiTransaction. SPL token support forward-compat via tokenId passthrough. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Replaces the hard-reject stub with a full Memo Program fee estimation case. Uses TransactionInstruction (web3.js) for getFeeData, returns ThorUtxoOrCosmosTradeRateOrQuote routes. Also exports solana from thorchain-utils/index.ts. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds getUnsignedSolanaTransaction, getSolanaTransactionFees to endpoints.ts and executeSolanaTransaction to ThorchainSwapper.ts. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Detects SPL Memo Program instruction, base58-decodes data to UTF-8, delegates to shared thormaya.Parser. Adds bs58 dependency. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Solana support across the ThorChain swapper: case-sensitive asset ID lookups, new Solana utilities (vault lookup, unsigned tx builder, fee estimator), Solana-aware rate/quote flow, swapper endpoints and execution method, parser support for THORChain memos, type additions, and asset map updates. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Swapper as ThorchainSwapper
participant Utils as Thorchain Utils (solana)
participant Adapter as SolanaAdapter
participant Node as THORNode
Client->>Swapper: getUnsignedSolanaTransaction(tradeQuote)
Swapper->>Utils: getUnsignedSolanaTransaction(args, swapperName)
Utils->>Utils: validate quote & extract memo
Utils->>Node: getThorTxData(sellAsset, config)
Node-->>Utils: { vault }
Utils->>Utils: build memo instruction + transfer instruction
Utils->>Adapter: getFeeData({ instructions, memo, tokenId? })
Adapter-->>Utils: { fast: { txFee, computeUnitLimit } }
Utils->>Swapper: return SolanaSignTx (unsigned)
Client->>Swapper: executeSolanaTransaction(signedTx)
Swapper->>Adapter: signAndBroadcastTransaction(signedTx)
Adapter-->>Swapper: broadcast result
Swapper-->>Client: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts (1)
10-15: Extract shared Solana memo/instruction builders to avoid drift.
MEMO_PROGRAM_IDand memo/transfer instruction construction are duplicated across multiple thorchain Solana utilities. Consolidating these into one helper would reduce divergence between quote, fee, and execution paths.Also applies to: 39-43, 45-57, 60-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts` around lines 10 - 15, The constants MEMO_PROGRAM_ID and COMPUTE_BUDGET_INSTRUCTION_OVERHEAD_CU and the logic that builds Solana memo and transfer instructions (used in getUnsignedSolanaTransaction and other thorchain Solana utilities) are duplicated; extract them into a single shared helper module (e.g., solanaInstructionHelpers) that exports MEMO_PROGRAM_ID, COMPUTE_BUDGET_INSTRUCTION_OVERHEAD_CU, and functions like buildSolanaMemoInstruction(memo: string) and buildSolanaTransferInstruction(params) and update getUnsignedSolanaTransaction and the other utilities to import and use these shared symbols instead of redefining the constants/instruction construction locally so all quote, fee, and execution paths use the same implementations.packages/unchained-client/src/solana/parser/thorchain.ts (1)
8-10: PrefertypeoverinterfaceforParserArgs.Line 8 can be converted to a type alias to align with the project’s TS conventions.
Proposed fix
-export interface ParserArgs { +export type ParserArgs = { midgardUrl: string -} +}As per coding guidelines: "Prefer
typeoverinterfacefor type definitions in ShapeShift project".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unchained-client/src/solana/parser/thorchain.ts` around lines 8 - 10, Change the exported ParserArgs declaration from an interface to a type alias to follow project TS conventions: replace the "export interface ParserArgs { midgardUrl: string }" declaration with a "export type ParserArgs = { midgardUrl: string }" type alias, leaving the exported name ParserArgs and its property midgardUrl unchanged so all usages of ParserArgs continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts`:
- Around line 456-467: The fee estimation in getL1RateOrQuote uses
adapter.getFeeData with only a memoInstruction and an empty to:'' which
underestimates Solana fees; update the flow to call getThorTxData (the same
helper used in getSolanaTransactionFees.ts) to obtain the vault/transfer details
and construct the transfer instruction, then pass both the memoInstruction and
the transfer instruction(s) in chainSpecific.instructions to adapter.getFeeData
(instead of only memoInstruction) so fee estimation mirrors execution path;
locate the adapter.getFeeData call, the memoInstruction creation, and the
getThorTxData helper to implement this change.
In `@packages/swapper/src/thorchain-utils/solana/getSolanaTransactionFees.ts`:
- Around line 35-39: The lamports conversion in getSolanaTransactionFees.ts
currently uses Number(sellAmountIncludingProtocolFeesCryptoBaseUnit) when
building transferInstruction (SystemProgram.transfer), which loses precision for
values > Number.MAX_SAFE_INTEGER; modify the fee-estimation logic in
getSolanaTransactionFees (and mirror in getUnsignedSolanaTransaction if present)
to either convert using BigInt(sellAmountIncludingProtocolFeesCryptoBaseUnit) or
first guard with a safe-integer check (e.g., throw or clamp if >
Number.MAX_SAFE_INTEGER) before using Number, ensuring
transferInstruction.fromPubkey/toPubkey lamports are computed without silent
precision loss and keeping the original string value passed later to
buildSendApiTransaction unchanged.
In `@packages/unchained-client/src/solana/parser/thorchain.ts`:
- Around line 23-26: Wrap the memo decode and parse block in a try-catch so
invalid base58 or parsing errors don’t throw and break transaction processing:
guard the Buffer.from(base58.decode(memoInstruction.data)).toString('utf8') and
the call to this.parser.parse(memo, tx.txid) with a try { ... } catch (err) {
console.error(...) } and return undefined/null on error to mirror existing error
handling patterns; also change the exported ParserArgs declaration from an
interface to a type (rename `interface ParserArgs` to `type ParserArgs = { ...
}`) to conform to the TypeScript style guideline.
In `@scripts/generateTradableAssetMap/utils.ts`:
- Around line 79-80: The code lowercases assetReference unconditionally which
breaks case-sensitive Solana SPL mint addresses; update the logic so that when
the asset namespace is ASSET_NAMESPACE.splToken (or the chain is
KnownChainIds.SolanaMainnet) you do NOT call assetReference.toLowerCase(), but
keep lowercasing for other namespaces; locate the branch that returns
ASSET_NAMESPACE.splToken and the code that calls assetReference.toLowerCase()
and make the lowercase step conditional on namespace !==
ASSET_NAMESPACE.splToken (or chain !== KnownChainIds.SolanaMainnet).
---
Nitpick comments:
In `@packages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts`:
- Around line 10-15: The constants MEMO_PROGRAM_ID and
COMPUTE_BUDGET_INSTRUCTION_OVERHEAD_CU and the logic that builds Solana memo and
transfer instructions (used in getUnsignedSolanaTransaction and other thorchain
Solana utilities) are duplicated; extract them into a single shared helper
module (e.g., solanaInstructionHelpers) that exports MEMO_PROGRAM_ID,
COMPUTE_BUDGET_INSTRUCTION_OVERHEAD_CU, and functions like
buildSolanaMemoInstruction(memo: string) and
buildSolanaTransferInstruction(params) and update getUnsignedSolanaTransaction
and the other utilities to import and use these shared symbols instead of
redefining the constants/instruction construction locally so all quote, fee, and
execution paths use the same implementations.
In `@packages/unchained-client/src/solana/parser/thorchain.ts`:
- Around line 8-10: Change the exported ParserArgs declaration from an interface
to a type alias to follow project TS conventions: replace the "export interface
ParserArgs { midgardUrl: string }" declaration with a "export type ParserArgs =
{ midgardUrl: string }" type alias, leaving the exported name ParserArgs and its
property midgardUrl unchanged so all usages of ParserArgs continue to work.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (23)
e2e/screenshots/thor_solana/all-swaps-pending.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-rune-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-rune-pending.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-rune-quote.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-usdc-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-usdc-pending.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/eth-to-usdc-quote.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/rune-to-sol.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-rune-v2-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-rune-v2-quote.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-rune-v2-success.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-failed-compute-budget.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-fixed-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-fixed-quote.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-quote-e2e.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-success.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-v3-confirm.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-v3-quote.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune-v3-result.pngis excluded by!**/*.pnge2e/screenshots/thor_solana/sol-to-rune.pngis excluded by!**/*.pngpackages/swapper/src/swappers/ThorchainSwapper/generated/generatedTradableAssetMap.jsonis excluded by!**/generated/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
packages/swapper/src/swappers/MayachainSwapper/utils/poolAssetHelpers/poolAssetHelpers.tspackages/swapper/src/swappers/ThorchainSwapper/ThorchainSwapper.tspackages/swapper/src/swappers/ThorchainSwapper/endpoints.tspackages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.test.tspackages/swapper/src/swappers/ThorchainSwapper/utils/poolAssetHelpers/poolAssetHelpers.tspackages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/thorchain-utils/index.tspackages/swapper/src/thorchain-utils/solana/getSolanaTransactionFees.tspackages/swapper/src/thorchain-utils/solana/getThorTxData.tspackages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.tspackages/swapper/src/thorchain-utils/solana/index.tspackages/swapper/src/types.tspackages/types/src/base.tspackages/unchained-client/package.jsonpackages/unchained-client/src/solana/parser/index.tspackages/unchained-client/src/solana/parser/thorchain.tspackages/unchained-client/src/solana/parser/types.tsscripts/generateTradableAssetMap/utils.ts
- include transfer instruction in quote-time solana fee estimation (mirrors execution path) - use bigint for lamport conversion to avoid precision loss above 9007 SOL - wrap base58 decode in try-catch in unchained-client thorchain parser - interface -> type for ParserArgs - skip toLowerCase for spl token mint addresses in asset map generation Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts`:
- Around line 470-477: The fee estimation call to adapter.getFeeData is missing
the Solana-specific tokenId in the chainSpecific payload, causing
under-estimated SPL-token fees; update the code that constructs chainSpecific
for the getFeeData call (the block passing to adapter.getFeeData in
getL1RateOrQuote) to include tokenId (use the tokenId available in the
quote/asset context) alongside from, instructions (memoInstruction,
transferInstruction) and ensure tokenId is forwarded so quote-time fees match
execution-time behavior.
In `@packages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts`:
- Around line 51-58: The fee simulation call to adapter.getFeeData (the const {
fast } = await adapter.getFeeData(...)) omits chainSpecific.tokenId causing
mismatched estimates vs execution; update that getFeeData invocation to include
chainSpecific.tokenId (pass the same tokenId used later during transaction
construction) so the simulated fees reflect SPL-token routes exactly, ensuring
the tokenId value is forwarded into the chainSpecific object used for fee
estimation.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
e2e/screenshots/thor_solana/sol-to-rune-success-final.pngis excluded by!**/*.png
📒 Files selected for processing (5)
packages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/thorchain-utils/solana/getSolanaTransactionFees.tspackages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.tspackages/unchained-client/src/solana/parser/thorchain.tsscripts/generateTradableAssetMap/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/unchained-client/src/solana/parser/thorchain.ts
- scripts/generateTradableAssetMap/utils.ts
packages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts
Show resolved
Hide resolved
… accuracy Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts (1)
452-460: Hoist THOR tx data lookup to avoid duplicated per-route network calls.
solana.getThorTxData()is invariant for this request and currently invoked once per route. Reusing a single promise keeps behavior the same while reducing duplicate I/O and failure fan-out.♻️ Proposed refactor
case CHAIN_NAMESPACE.Solana: { const adapter = assertGetSolanaChainAdapter(sellAsset.chainId) const sendAddress = (input as CommonTradeQuoteInput).sendAddress + const thorTxDataPromise = sendAddress + ? solana.getThorTxData({ sellAsset, config, swapperName }) + : undefined const maybeRoutes = await Promise.allSettled( perRouteValues.map(async (route): Promise<T> => { const memo = getMemo(route) const protocolFees = getProtocolFees(route.quote) const feeData = await (async (): Promise<QuoteFeeData> => { - if (!sendAddress) return { networkFeeCryptoBaseUnit: undefined, protocolFees } - const { vault } = await solana.getThorTxData({ sellAsset, config, swapperName }) + if (!thorTxDataPromise) return { networkFeeCryptoBaseUnit: undefined, protocolFees } + const { vault } = await thorTxDataPromise const memoInstruction = new TransactionInstruction({ keys: [], programId: new PublicKey(SOLANA_MEMO_PROGRAM_ID), data: Buffer.from(memo, 'utf8'), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts` around lines 452 - 460, The code calls solana.getThorTxData() inside the perRouteValues.map handler, causing redundant identical network calls and failure fan-out; hoist a single promise by calling solana.getThorTxData({ sellAsset, config, swapperName }) once before building perRouteValues (or before Promise.allSettled) and reuse that result (or its promise) inside the per-route async function where current code references { vault } and memoInstruction, updating references in the per-route block (used by maybeRoutes, getMemo, getProtocolFees, feeData) to use the hoisted value instead of calling solana.getThorTxData repeatedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts`:
- Around line 452-460: The code calls solana.getThorTxData() inside the
perRouteValues.map handler, causing redundant identical network calls and
failure fan-out; hoist a single promise by calling solana.getThorTxData({
sellAsset, config, swapperName }) once before building perRouteValues (or before
Promise.allSettled) and reuse that result (or its promise) inside the per-route
async function where current code references { vault } and memoInstruction,
updating references in the per-route block (used by maybeRoutes, getMemo,
getProtocolFees, feeData) to use the hoisted value instead of calling
solana.getThorTxData repeatedly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/thorchain-utils/solana/getUnsignedSolanaTransaction.ts
NeOMakinG
left a comment
There was a problem hiding this comment.
Code Review — PR #12036
Verdict: ✅ APPROVE
Summary: Clean THORChain Solana swapper integration following the established Tron pattern. Adds SOL as a valid sell/buy asset through THORChain, with proper case-sensitive base58 handling, SPL token forward-compatibility, and a Solana transaction parser for unchained-client.
Architecture
Follows the exact same structure as the Tron integration:
thorchain-utils/solana/module withgetThorTxData,getUnsignedSolanaTransaction,getSolanaTransactionFeesThorchainSwapper.executeSolanaTransaction+ endpoint wiring- Solana case in
getL1RateOrQuote.tsreplaces the'Solana is not supported'stub - Asset map generation updated for SOL chain + SPL token namespace
What's Good
-
Case-sensitive base58 fix — Removing
.toLowerCase()fromassetIdToThorPoolAssetIdMapandassetIdToMayaPoolAssetIdMaplookups is critical. Solana addresses are base58-encoded and case-sensitive..toLowerCase()would corrupt them. Test added to verify. -
SPL token forward-compat —
getTokenStandardFromChainIdreturnssplTokenfor Solana, andassetReferenceskips.toLowerCase()for SPL tokens. When THORChain adds SPL pools, no new code needed. -
Compute budget overhead —
COMPUTE_BUDGET_INSTRUCTION_OVERHEAD_CU = 300accounts for the 2 ComputeBudgetProgram instructions (150 CU each) that aren't included in fee estimation. Good detail that prevents "compute budget exceeded" errors. -
Memo Program instruction — Uses
MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr(Solana's Memo Program) to attach THORChain memos. This is the correct pattern — same as how THORChain's own Solana integration works. -
Transaction parser —
thorchain.tsdecodes base58-encoded memo data from the Memo Program instruction, delegates to the shared thorchain parser. Error handling withconsole.error+ gracefulreturn undefined. -
Tests — Pool asset helpers tested for
SOL.SOL ↔ solAssetIdroundtrip, including case-sensitivity verification.
Minor Observations (Non-blocking)
-
Memo/transfer instruction duplication across 3 files (
getL1RateOrQuote,getSolanaTransactionFees,getUnsignedSolanaTransaction). This follows the same pattern as Tron — each file builds instructions independently. Could be extracted to a shared helper, but consistency with existing patterns is fine. -
allowanceContract: '0x0'in the Solana case — Solana doesn't have ERC-20 allowances. This is a typed-system placeholder consistent with UTXO/Cosmos paths. -
GetSolanaTradeQuoteInputBaseandGetSolanaTradeQuoteInputare identical — follows Tron scaffolding pattern for when wallet-specific fields are added later.
Risk Assessment
Low risk — Additive only, no existing swap flows touched. SOL.SOL pool is live on THORChain mainnet. The .toLowerCase() removal is safe because EVM asset IDs in CAIP format already use lowercase hex addresses. CI passes including static analysis.
Browser Test
Swapper-level change — quote/execution logic lives in packages, not UI components. E2E screenshots in the PR demonstrate working SOL ↔ RUNE swaps. No additional browser test needed.
LGTM 👍
Description
Wires up SOL as a valid sell/buy asset in the THORChain swapper. SOL.SOL pool is confirmed live on THORChain mainnet (
status: Available,trading_halted: false). Follows the Tron pattern: native SOL sent to vault with an SPL Memo Program instruction carrying the THORChain memo.What this enables:
adapter.getFeeDatawith the memo instruction for accurate compute unit pricingtokenId)What's in this PR:
SolanaChainIdtype in@shapeshiftoss/types@shapeshiftoss/swapperyarn generate:tradable-asset-mapnow outputsSOL.SOLthorchain-utils/solana/module:getThorTxData,getUnsignedSolanaTransaction,getSolanaTransactionFeesgetL1RateOrQuote.tsSolana case — replaces the'Solana is not supported'stubThorchainSwapperendpoints + execution wiringunchained-clientSolana THORChain parseTx parser (Memo Program detection, base58 decode, thormaya delegation)Issue (if applicable)
closes #12034
Risk
Low — new chain support, additive only, no existing swap flows touched. Solana adapter already exists and is battle-tested via Jupiter/Relay.
THORChain native SOL swaps only. Inbound vault:
GWYXY7c6SVMkuhmDq2LT1Hj6qGkDFCqmdZWJdCeHUgEN.Testing
Engineering
Manual test pairs to verify:
Operations
SOL.SOL pool is live, no flag needed. Operations can test in staging with a small SOL → BTC swap.
Screenshots
SOL → RUNE quote
SOL → RUNE confirm
SOL → RUNE result (1 Pending, swap being processed)
RUNE → SOL quote (reverse direction)
ETH → RUNE regression test
ETH → USDC sanity check
All swaps pending
Summary by CodeRabbit
New Features
Bug Fixes