Conversation
WalkthroughThe changes introduce enhanced validation, error handling, and logging in the exchange flow. Key updates include stricter checks and error messages in the exchange action, improved error handling and initialization logic in the main app, more robust decoding in blockchain utilities, and clarified comments in wrapped token utilities. No public API signatures were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExchangeAction
participant OfferHook
participant BlockchainUtils
participant AppInit
User->>ExchangeAction: Click Exchange
ExchangeAction->>ExchangeAction: Validate swapToken, receiveToken, amountSwap
alt Validation fails
ExchangeAction->>User: Show error message
ExchangeAction-->>ExchangeAction: Abort
else Validation passes
ExchangeAction->>OfferHook: getBestOffer()
OfferHook-->>ExchangeAction: Return best offer or undefined
ExchangeAction->>BlockchainUtils: Decode transaction data
BlockchainUtils-->>ExchangeAction: Return decoded value or warning
ExchangeAction->>ExchangeAction: Prepare and send transactions
end
AppInit->>AppInit: On provider/wallet change
AppInit->>AppInit: Initialize config (once)
AppInit->>AppInit: Handle errors and log
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15-25 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
Preview (fix/the-exchange-bugs)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/apps/the-exchange/utils/blockchain.ts (2)
80-86: Safer decode guard — consider also verifying function nameGood hardening with array checks and bigint type guard. One small enhancement: also assert that
decoded.functionName === 'transfer'to avoid relying solely on the method selector, which is robust here but belt-and-suspenders helps with future ABI changes.
89-89: Enrich warning context for observabilityInclude basic tx/swap context (e.g.,
swapToken.symbol,decimals) in the warning to aid debugging without logging sensitive data.src/apps/the-exchange/hooks/useOffer.tsx (2)
211-213: Consistently return undefined on no routes/errors; consider unified error loggingThe switch to
undefinedis consistent and avoids placeholder objects. Consider logging via your sentry helpers for errors (parallel to the rest of the app) to keep telemetry uniform.Also applies to: 218-220
176-182: Avoid float math when applying 1% fee (precision)
The computationNumber(fromAmount) * 0.99can introduce rounding issues for tokens with many decimals. Prefer doing the 1% deduction in integer units.Example outside this hunk:
// Precise computation in wei: const fromAmountWei = parseUnits(String(fromAmount), fromTokenDecimals); const fromAmountFeeDeductedWei = (fromAmountWei * 99n) / 100n; const routesRequest: RoutesRequest = { // ... fromAmount: fromAmountFeeDeductedWei.toString(), // ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx(7 hunks)src/apps/the-exchange/hooks/useOffer.tsx(1 hunks)src/apps/the-exchange/index.tsx(3 hunks)src/apps/the-exchange/utils/blockchain.ts(1 hunks)src/apps/the-exchange/utils/wrappedTokens.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-05-23T14:44:33.911Z
Learnt from: RanaBug
PR: pillarwallet/x#315
File: src/apps/the-exchange/utils/wrappedTokens.ts:6-20
Timestamp: 2025-05-23T14:44:33.911Z
Learning: XDAI (Gnosis Chain) is intentionally excluded from the WRAPPED_NATIVE_TOKEN_ADDRESSES mapping in the exchange app's wrappedTokens utility.
Applied to files:
src/apps/the-exchange/utils/wrappedTokens.ts
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/the-exchange/utils/wrappedTokens.tssrc/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsxsrc/apps/the-exchange/hooks/useOffer.tsxsrc/apps/the-exchange/index.tsx
📚 Learning: 2025-06-17T09:20:44.533Z
Learnt from: RanaBug
PR: pillarwallet/x#334
File: src/apps/leaderboard/utils/index.tsx:91-94
Timestamp: 2025-06-17T09:20:44.533Z
Learning: In src/apps/leaderboard/utils/index.tsx, the getLastWeekMigrationData function intentionally uses currentWeek (not lastWeek) for the completedSwapWeek lookup. This is correct business logic - when retrieving last week's migration data, the function should check swap completion against the current week while using lastWeek for points and USD calculations.
Applied to files:
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsxsrc/apps/the-exchange/hooks/useOffer.tsx
📚 Learning: 2025-05-28T14:30:02.702Z
Learnt from: RanaBug
PR: pillarwallet/x#320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Applied to files:
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx
🧬 Code Graph Analysis (1)
src/apps/the-exchange/hooks/useOffer.tsx (2)
src/apps/the-exchange/utils/types.tsx (1)
SwapOffer(23-26)src/apps/the-exchange/utils/blockchain.ts (1)
processEth(18-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: unit-tests
🔇 Additional comments (8)
src/apps/the-exchange/utils/wrappedTokens.ts (1)
39-41: Comment clarity LGTM; matches intentional XDAI exclusionThe generalized fallback comment is accurate and aligns with the prior decision to intentionally exclude XDAI from WRAPPED_NATIVE_TOKEN_ADDRESSES.
src/apps/the-exchange/hooks/useOffer.tsx (2)
203-209: Return a concrete SwapOffer or undefined (good)Removal of the outer-scoped placeholder and returning a typed
SwapOfferimproves correctness and readability.
157-167: All consumers handleundefinedsafely
The only usage ofgetBestOfferis wrapped in the localgetOfferhelper in EnterAmount.tsx, which:
- Catches thrown errors and returns an empty object fallback
- Guards with
if (offer && Object.keys(offer).length)before dereferencing- Skips dispatch in the debounced caller when
offerisundefinedNo further changes are needed.
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (4)
125-137: Pre-flight token selection validation (good)Early return with telemetry keeps the flow safe and user-friendly.
138-147: Amount validation added (good)Prevents zero/negative swaps from entering the flow.
206-216: Null/empty step transactions guard (good)The additional null check avoids crashes when LiFi yields no steps.
257-270: Safe optional chaining for calldata and logging (good)Using
?.toString() ?? ''avoids undefined issues when building batch entries.src/apps/the-exchange/index.tsx (1)
69-74: One-time config init — verify behavior on provider changesThe ref gate prevents re-init, which is good for stability. If the provider instance can change at runtime (wallet reconnect, account change with a new client), consider resetting the ref when the provider identity changes.
If relevant, track and compare the provider object:
const prevProvider = useRef<any>(null); useEffect(() => { if (!provider) return; if (prevProvider.current && prevProvider.current !== provider) { configInitialized.current = false; } prevProvider.current = provider; // ... then run your guarded init }, [provider, walletAddress]);Also applies to: 152-153
Deploying x with
|
| Latest commit: |
0bcb9e9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9bda0d59.x-e62.pages.dev |
| Branch Preview URL: | https://fix-the-exchange-bugs.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (fix/the-exchange-bugs)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/apps/the-exchange/hooks/useOffer.tsx (2)
341-347:totalNativeRequireddouble-counts swap amount when wrappingThe intention is “swap amount + fee”, but the current formula adds the full amount twice (
fromAmountBigIntand againfromAmountBigInt - feeAmount), so the balance check requires almost 2× the real need.-totalNativeRequired = fromAmountBigInt; -if (isWrapRequired) { - totalNativeRequired += fromAmountBigInt - feeAmount; -} +totalNativeRequired = fromAmountBigInt; // swap amount +if (isWrapRequired) { + totalNativeRequired = fromAmountBigInt; // still just swap amount +} +totalNativeRequired += feeAmount; // plus 1 % fee
519-524: Numeric comparison is lexicographic – convert to BigInt first
formatUnitsreturns strings; comparing them with>=compares string bytes, not numbers ('20' > '100'⇒true).
Parse them back toBigInt(or useBig.js) before comparison.-const isEnoughAllowance = isAllowance - ? formatUnits(isAllowance, step.action.fromToken.decimals) >= - formatUnits(BigInt(step.action.fromAmount), step.action.fromToken.decimals) +const isEnoughAllowance = isAllowance + ? isAllowance >= BigInt(step.action.fromAmount) : undefined;
🧹 Nitpick comments (1)
src/apps/the-exchange/index.tsx (1)
252-253: Trim unnecessary dependency to prevent redundant effect runs
walletAddressis not used inside this effect.
Removing it avoids a (harmless) re-render path and keeps the dependency list minimal.-}, [provider, walletAddress]); +}, [provider]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx(7 hunks)src/apps/the-exchange/hooks/useOffer.tsx(18 hunks)src/apps/the-exchange/index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/the-exchange/hooks/useOffer.tsxsrc/apps/the-exchange/index.tsx
📚 Learning: 2025-06-17T09:20:44.533Z
Learnt from: RanaBug
PR: pillarwallet/x#334
File: src/apps/leaderboard/utils/index.tsx:91-94
Timestamp: 2025-06-17T09:20:44.533Z
Learning: In src/apps/leaderboard/utils/index.tsx, the getLastWeekMigrationData function intentionally uses currentWeek (not lastWeek) for the completedSwapWeek lookup. This is correct business logic - when retrieving last week's migration data, the function should check swap completion against the current week while using lastWeek for points and USD calculations.
Applied to files:
src/apps/the-exchange/hooks/useOffer.tsx
📚 Learning: 2025-05-23T14:44:33.911Z
Learnt from: RanaBug
PR: pillarwallet/x#315
File: src/apps/the-exchange/utils/wrappedTokens.ts:6-20
Timestamp: 2025-05-23T14:44:33.911Z
Learning: XDAI (Gnosis Chain) is intentionally excluded from the WRAPPED_NATIVE_TOKEN_ADDRESSES mapping in the exchange app's wrappedTokens utility.
Applied to files:
src/apps/the-exchange/hooks/useOffer.tsx
📚 Learning: 2025-05-28T14:30:02.702Z
Learnt from: RanaBug
PR: pillarwallet/x#320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Applied to files:
src/apps/the-exchange/hooks/useOffer.tsxsrc/apps/the-exchange/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
2 participants
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
Bug Fixes
Refactor