fix: chainflip swap explorer link and solana compute budget#12178
Open
gomesalexandre wants to merge 7 commits intodevelopfrom
Open
fix: chainflip swap explorer link and solana compute budget#12178gomesalexandre wants to merge 7 commits intodevelopfrom
gomesalexandre wants to merge 7 commits intodevelopfrom
Conversation
The status-by-id endpoint returns the real Chainflip network swap ID in status.swapId, but we were using the broker deposit channel ID from swap creation for the scan.chainflip.io link. The channel ID points to a completely different (or nonexistent) swap. Now we extract the real swapId from the status response and only use it for the explorer link once available. Before deposit detection (when swapId isn't in the response yet), we fall back to the regular chain explorer link instead of a wrong Chainflip scan link. closes #11899 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Chainflip deposit addresses are program-owned accounts that require more compute than a regular SOL transfer. The simulated compute units were too low, causing "Computational budget exceeded" on broadcast. Apply a 50k floor with 1.6x margin to match Jupiter's recommendation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughDestructures and propagates Chainflip swap native IDs (swapId → chainflipSwapId) through swapper, types, trade-execution, and action-center flows; Solana compute units/fee calculations apply a ceil(max(simulated, 50_000) * 1.6) boost and computeUnitLimit/fee returns are stringified. Changes
Sequence DiagramsequenceDiagram
participant API as Chainflip API
participant Swapper as ChainflipSwapper\n(endpoints.ts)
participant Types as Types / QuoteStore
participant Exec as tradeExecution.ts
participant Subscriber as useSwapActionSubscriber
participant UI as Action Center / UI
API->>Swapper: respond to checkTradeStatus() with { status: { swapEgress, swapId, ... } }
Swapper->>Swapper: compute boosted computeUnits = ceil(max(simulatedUnits, 50000) * 1.6)\nstringify computeUnitLimit\ncompute boosted fee via ratio
Swapper->>Exec: return TradeStatus including chainflipSwapId (swapId ?? undefined)
Exec->>Subscriber: emit status event with chainflipSwapId
Subscriber->>Subscriber: resolve/merge chainflipSwapId into swap.metadata
Subscriber->>UI: build tx link using chainflipSwapId?.toString()
UI->>API: user views scan.chainflip.io/swaps/<chainflipSwapId>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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)
📝 Coding Plan
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/tradeExecution.ts (1)
91-119:⚠️ Potential issue | 🟠 Major
chainflipSwapIdis fetched but never emitted in status events.
fetchTradeStatus()now returnschainflipSwapId, but_execWalletAgnostic()drops it when constructingStatusArgs. This preventsuseTradeExecutionfrom receiving and persisting the resolved Chainflip swap id during polling.💡 Minimal fix
const { status, message, buyTxHash, relayerTxHash, relayerExplorerTxLink, actualBuyAmountCryptoBaseUnit, + chainflipSwapId, } = await queryClient.fetchQuery({ queryKey: tradeStatusQueryKey(swap.id, updatedSwap.sellTxHash), queryFn: () => fetchTradeStatus({ @@ const payload: StatusArgs = { stepIndex, status, message, buyTxHash, relayerTxHash, actualBuyAmountCryptoBaseUnit, + chainflipSwapId, }Also applies to: 268-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/tradeExecution.ts` around lines 91 - 119, fetchTradeStatus()/swapper.checkTradeStatus now returns chainflipSwapId but _execWalletAgnostic constructs StatusArgs without it, so the status events never carry the Chainflip swap id; update _execWalletAgnostic (and the other similar block around the 268-315 range) to include chainflipSwapId in the StatusArgs you emit/return so the value flows through StatusArgs -> status events -> useTradeExecution for persistence; look for references to StatusArgs, _execWalletAgnostic, fetchTradeStatus/checkTradeStatus, and ensure chainflipSwapId is added to the returned object and any emitted event payloads.
🤖 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/swappers/ChainflipSwapper/endpoints.ts`:
- Around line 238-239: The code is converting Chainflip's swapId (a u64 provided
as a string) to Number, which risks precision loss; update the type for
chainflipSwapId in packages/swapper/src/types.ts to accept string and stop
coercing swapId to Number: keep the destructured status: { swapEgress, swapId }
as a string, remove the Number(...) wrappers in the return statements (the three
places that convert swapId to Number in endpoints.ts), and ensure any callers
that build the status URL use the string value unchanged (refer to the swapId
symbol and the functions that return it).
---
Outside diff comments:
In `@src/lib/tradeExecution.ts`:
- Around line 91-119: fetchTradeStatus()/swapper.checkTradeStatus now returns
chainflipSwapId but _execWalletAgnostic constructs StatusArgs without it, so the
status events never carry the Chainflip swap id; update _execWalletAgnostic (and
the other similar block around the 268-315 range) to include chainflipSwapId in
the StatusArgs you emit/return so the value flows through StatusArgs -> status
events -> useTradeExecution for persistence; look for references to StatusArgs,
_execWalletAgnostic, fetchTradeStatus/checkTradeStatus, and ensure
chainflipSwapId is added to the returned object and any emitted event payloads.
🪄 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: ba12dcff-c33d-4458-a43b-515863920290
📒 Files selected for processing (6)
packages/swapper/src/swappers/ChainflipSwapper/endpoints.tspackages/swapper/src/swappers/ChainflipSwapper/types.tspackages/swapper/src/types.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/lib/tradeExecution.ts
swapId is a u64 on-chain, can exceed JS Number.MAX_SAFE_INTEGER. Remove Number() coercion from status response, widen type to number | string to accept both SDK (number) and status API (string). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
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/swappers/ChainflipSwapper/endpoints.ts`:
- Around line 172-184: The fee-quoting path must mirror the boosted compute
budget used in getUnsignedSolanaTransaction(); in getSolanaTransactionFees()
compute the same computeUnits value (e.g., computeUnits =
Math.ceil(Math.max(Number(fast.chainSpecific.computeUnits), 50_000) * 1.6)) or
otherwise apply the same floor and 1.6 multiplier, and use that adjusted
computeUnitLimit when deriving the SOL fee (instead of returning the original
fast.txFee from the unadjusted simulation) so the returned fee matches the
increased compute-unit limit used in
buildSendApiTransaction/getUnsignedSolanaTransaction.
In
`@src/components/MultiHopTrade/components/TradeConfirm/hooks/useChainflipStreamingProgress.tsx`:
- Around line 26-29: The guard that treats swapId as falsy should be changed to
a nullish check so numeric 0 remains valid: in getChainflipStreamingSwap (and
the useQuery gate inside useChainflipStreamingProgress) replace the truthy
checks like `if (!swapId)` / `if (!queryEnabled)` with explicit null/undefined
checks using `swapId == null` / `swapId != null` (or the inverse) so both
numeric 0 and string IDs are accepted while still excluding null/undefined.
🪄 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: 512f3b9c-e48c-42ca-9c15-d8e6942546fd
📒 Files selected for processing (4)
packages/public-api/src/lib/quoteStore.tspackages/swapper/src/swappers/ChainflipSwapper/endpoints.tspackages/swapper/src/types.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useChainflipStreamingProgress.tsx
src/components/MultiHopTrade/components/TradeConfirm/hooks/useChainflipStreamingProgress.tsx
Outdated
Show resolved
Hide resolved
…Id checks - getSolanaTransactionFees now mirrors the same 1.6x compute unit boost as getUnsignedSolanaTransaction to avoid underquoting fees - Use nullish checks (== null / != null) for chainflipSwapId since 0 is a valid u64 swap ID but falsy in JS Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
Errors:
Please ensure the type definitions are in sync with the implementation. |
Contributor
Author
bad bot ser pls pull @NeOMakinG |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts (1)
223-225:⚠️ Potential issue | 🟠 MajorUse a nullish guard for
chainflipSwapId(Line 224), not a truthy guard.
if (!chainflipSwapId)rejects valid0swap IDs. This can incorrectly throw for a valid u64 ID. Use== nullinstead.🔧 Proposed fix
- if (!chainflipSwapId) throw Error(`chainflipSwapId is required`) + if (chainflipSwapId == null) throw Error(`chainflipSwapId is required`)Based on learnings: In
packages/swapper/src/swappers/ChainflipSwapper,chainflipSwapIdintentionally usesnumber | string, and0is valid while string swap IDs must flow without coercion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts` around lines 223 - 225, The guard for chainflipSwapId currently uses a truthy check which will reject valid numeric IDs like 0; update the validation to use a nullish check (chainflipSwapId == null) so only null or undefined cause the Error, and keep the variable type as number | string so string IDs pass without coercion—locate the check around the chainflipSwapId declaration in endpoints.ts and replace the truthy guard with a nullish guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts`:
- Around line 223-225: The guard for chainflipSwapId currently uses a truthy
check which will reject valid numeric IDs like 0; update the validation to use a
nullish check (chainflipSwapId == null) so only null or undefined cause the
Error, and keep the variable type as number | string so string IDs pass without
coercion—locate the check around the chainflipSwapId declaration in endpoints.ts
and replace the truthy guard with a nullish guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31bfc8c5-45b1-4d99-bb30-0ed863404248
📒 Files selected for processing (2)
packages/swapper/src/swappers/ChainflipSwapper/endpoints.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useChainflipStreamingProgress.tsx
0 is a valid u64 swap ID but falsy in JS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Re: outside-diff comment on line 224 ( |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts (1)
32-33:⚠️ Potential issue | 🟠 MajorUse nullish checks for all
chainflipSwapIdguards.Lines 32, 72, 108, 132, 158, 201 still use falsy checks (
!chainflipSpecific?.chainflipSwapId) and will incorrectly reject validchainflipSwapId = 0(a valid u64 value). Line 224 is already correct withchainflipSwapId == null.Replace all falsy checks with nullish checks:
Proposed fix
- if (!chainflipSpecific?.chainflipSwapId) throw Error('Missing swap id') + if (chainflipSpecific?.chainflipSwapId == null) throw Error('Missing swap id')Apply to lines 32, 72, 108, 132, 158, 201.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts` around lines 32 - 33, Replace all falsy guards that use "!chainflipSpecific?.chainflipSwapId" with nullish checks that detect only null/undefined (e.g. "chainflipSpecific?.chainflipSwapId == null") so a valid 0 value is not rejected; update every occurrence of the pattern involving the chainflipSpecific variable and its chainflipSwapId property (the checks at the top-level guards in endpoints.ts) to use "== null" (or "=== undefined" if you prefer explicitness) before throwing the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts`:
- Around line 32-33: Replace all falsy guards that use
"!chainflipSpecific?.chainflipSwapId" with nullish checks that detect only
null/undefined (e.g. "chainflipSpecific?.chainflipSwapId == null") so a valid 0
value is not rejected; update every occurrence of the pattern involving the
chainflipSpecific variable and its chainflipSwapId property (the checks at the
top-level guards in endpoints.ts) to use "== null" (or "=== undefined" if you
prefer explicitness) before throwing the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ca43731-a430-482e-8a58-873489977749
📒 Files selected for processing (1)
packages/swapper/src/swappers/ChainflipSwapper/endpoints.ts
NeOMakinG
approved these changes
Mar 17, 2026
Collaborator
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
QA Code Review ✅
PR: fix: chainflip swap explorer link and solana compute budget
Code Analysis
1. Chainflip Swap ID Fix
- Changed
chainflipSwapIdtype fromnumbertonumber | stringfor flexibility - Now extracts real
swapIdfrom Chainflip API status response - Propagates swap ID through status checks → trade execution → swap metadata
- Fixes broken explorer links by using actual swap ID
2. Solana Compute Budget Fix
- Increased compute units by 1.6x multiplier with 50,000 minimum floor
- Prevents "Computational budget exceeded" errors on Chainflip deposit addresses
- Fee estimation updated to match actual compute budget
3. Testing
✅ App builds and loads successfully
✅ Code changes are correct and comprehensive
Recommendation: Approve - Both fixes are well-implemented and address real user issues.
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
2 participants
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
Two fixes for Chainflip swaps:
1. Wrong explorer link in action center (#11899)
"View Transaction" linked to
scan.chainflip.io/swaps/<broker_channel_id>(e.g./swaps/47079) instead of the real Chainflip network swap ID (e.g./swaps/1403671). The broker deposit channel ID was stored at swap creation time, but the real swap ID only becomes available from the status API after deposit detection.Fix: extract the real
swapIdfrom thestatus-by-idresponse and use it for the explorer link. Before deposit detection (whenswapIdisn't in the response yet), fall back to the regular chain explorer link instead of a wrong Chainflip scan link.2. SOL -> USDT(SOL) via Chainflip fails with "Computational budget exceeded"
Chainflip deposit addresses are program-owned accounts that need more compute than a regular SOL transfer. The simulated compute units were too low, causing broadcast failure.
Fix: apply a 50k floor with 1.6x margin (matching Jupiter's recommendation) for Chainflip Solana transactions.
Issue (if applicable)
closes #11899
Risk
Low - read-only status field extraction for explorer links, and compute budget increase for Solana Chainflip txs (higher budget = slightly higher fee but tx actually lands).
Chainflip swaps: explorer links in action center, and Solana sell-side transaction broadcast.
Testing
Engineering
Swap link fix:
scan.chainflip.io/swaps/<real_swap_id>Compute budget fix:
Operations
Screenshots (if applicable)
Swap link fix
https://jam.dev/c/3ab56ba6-2ee7-44ab-bfdd-626d27afe313
SOL compute budget fix
Summary by CodeRabbit
New Features
Bug Fixes