fix/PRO-3348/wrap-token-native-when-swapping#315
Conversation
WalkthroughThe update introduces logic for handling native and wrapped tokens in the exchange offer flow. It adds a utility module for managing wrapped token addresses and detection, and updates the offer hook to substitute native tokens with their wrapped versions when querying routes and constructing transactions, including prepending a wrapping step when needed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant useOffer Hook
participant LiFi SDK
participant WrappedTokenContract
User->>useOffer Hook: Request best offer (source, dest, chain)
useOffer Hook->>useOffer Hook: Substitute native with wrapped tokens (if needed)
useOffer Hook->>LiFi SDK: Query routes with wrapped token addresses
LiFi SDK-->>useOffer Hook: Return route
User->>useOffer Hook: Build step transactions for route
useOffer Hook->>useOffer Hook: Check if source is wrapped token
alt Source is wrapped token
useOffer Hook->>WrappedTokenContract: Prepare deposit() call to wrap native token
useOffer Hook->>useOffer Hook: Prepend wrapping step to transactions
end
useOffer Hook-->>User: Return step transactions
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 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 (4)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying x with
|
| Latest commit: |
a533099
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://67443dc3.x-e62.pages.dev |
| Branch Preview URL: | https://fix-pro-3348-wrap-token-nati.x-e62.pages.dev |
![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 (2)
src/apps/the-exchange/utils/wrappedTokens.ts (1)
22-42: LGTM! Consider adding input validation.The utility functions are well-implemented with proper case-insensitive address comparisons. The logic correctly handles:
- Checking if a token is the wrapped native token for a specific chain
- Identifying native token addresses using the predefined set
- Converting native tokens to wrapped tokens with fallback for unsupported chains
Consider adding basic input validation for edge cases:
export const isWrappedToken = (tokenAddress: string, chainId: number) => { + if (!tokenAddress || !chainId) return false; return ( tokenAddress.toLowerCase() === WRAPPED_NATIVE_TOKEN_ADDRESSES[chainId]?.toLowerCase() ); }; export const isNativeToken = (address: string) => + address && NATIVE_TOKEN_ADDRESSES.has(address.toLowerCase());src/apps/the-exchange/hooks/useOffer.tsx (1)
144-162: LGTM! Proper wrapping transaction construction.The wrapping transaction is correctly constructed:
- Uses standard
deposit()function for wrapped tokens- Sets transaction value to the wrap amount
- Targets the wrapped token contract address
Consider adding error handling for the wrapping step:
if (isWrapRequired) { + try { const wrapCalldata = encodeFunctionData({ abi: [ { name: 'deposit', type: 'function', stateMutability: 'payable', inputs: [], outputs: [], }, ], functionName: 'deposit', }); stepTransactions.push({ to: route.fromToken.address, data: wrapCalldata, value: BigInt(route.fromAmount), chainId: route.fromChainId, }); + } catch (error) { + console.error('Failed to create wrapping transaction:', error); + throw new Error('Unable to create wrapping transaction'); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/apps/the-exchange/hooks/useOffer.tsx(3 hunks)src/apps/the-exchange/utils/wrappedTokens.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
src/apps/the-exchange/hooks/useOffer.tsx (2)
24-27: LGTM! Proper import of wrapped token utilities.The imports are correctly added and will be used for native token handling in the swap flow.
44-58: LGTM! Proper normalization of native tokens for route queries.The logic correctly replaces native token addresses with their wrapped equivalents before querying routes. This approach ensures:
- Consistent token addressing for the LiFi SDK
- Better route availability since DEXs typically use wrapped tokens
- Proper handling of both native and wrapped token inputs
The implementation properly handles both source and destination tokens and uses the fallback logic for unsupported chains.
| // Not including XDAI below | ||
| export const WRAPPED_NATIVE_TOKEN_ADDRESSES: Record<number, string> = { | ||
| // Ethereum | ||
| 1: '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2', // WETH | ||
| // Polygon | ||
| 137: '0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270', // WMATIC | ||
| // Optimism | ||
| 10: '0x4200000000000000000000000000000000000006', // WETH | ||
| // Arbitrum | ||
| 42161: '0x82aF49447D8a07e3bd95BD0d56f35241523fBab1', // WETH | ||
| // Base | ||
| 8453: '0x4200000000000000000000000000000000000006', // WETH | ||
| // BNB | ||
| 56: '0xbb4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c', // WBNB | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify wrapped token contract addresses and clarify XDAI exclusion.
The wrapped native token addresses are critical for correct swap functionality. Please verify these contract addresses are accurate for each chain:
- Ethereum WETH:
0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 - Polygon WMATIC:
0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270 - Optimism WETH:
0x4200000000000000000000000000000000000006 - Arbitrum WETH:
0x82aF49447D8a07e3bd95BD0d56f35241523fBab1 - Base WETH:
0x4200000000000000000000000000000000000006 - BNB WBNB:
0xbb4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c
Also, please clarify why XDAI is excluded. If XDAI has different token wrapping mechanics or no wrapped native token, this should be documented.
🏁 Script executed:
#!/bin/bash
# Verify wrapped token contract addresses on major chains
echo "Verifying wrapped token contract addresses..."
# Check if these are standard wrapped native token contracts
echo "WETH on Ethereum (Chain ID 1):"
echo "Expected: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"
echo "WMATIC on Polygon (Chain ID 137):"
echo "Expected: 0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270"
echo "WETH on Optimism (Chain ID 10):"
echo "Expected: 0x4200000000000000000000000000000000000006"
echo "WETH on Arbitrum (Chain ID 42161):"
echo "Expected: 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1"
echo "WETH on Base (Chain ID 8453):"
echo "Expected: 0x4200000000000000000000000000000000000006"
echo "WBNB on BNB Chain (Chain ID 56):"
echo "Expected: 0xbb4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c"Length of output: 1234
🌐 Web query:
What are the correct wrapped native token contract addresses for Ethereum, Polygon, Optimism, Arbitrum, Base, and BNB Chain? Also, does XDAI (Gnosis Chain) have a wrapped native token?
💡 Result:
Here are the correct wrapped native token contract addresses for the specified chains:
-
Ethereum (Wrapped Ether - WETH):
Contract Address: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 -
Polygon (Wrapped MATIC - WMATIC):
Contract Address: 0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270 -
Optimism (Wrapped Ether - WETH):
Contract Address: 0x4200000000000000000000000000000000000006 -
Arbitrum (Wrapped Ether - WETH):
Contract Address: 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1 -
Base (Wrapped Ether - WETH):
The contract address for Base is: 0x4200000000000000000000000000000000000006 (same as Optimism) -
BNB Chain (Wrapped BNB - WBNB):
Contract Address: 0xBB4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c
Regarding XDAI (now Gnosis Chain):
Yes, Gnosis Chain does have a wrapped native token called wrapped XDAI (WXDAI).
Wrapped XDAI (WXDAI) Contract Address:
0x6A023CCd1ff6F2045C3309768eAd9E68F978f6e1
Would you like me to verify the latest contract addresses for any of these chains using web search?
Wrapped native token addresses confirmed; please document or include Gnosis Chain (XDAI).
All listed wrapped token addresses match the standard contracts:
- Ethereum (1): 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 (WETH)
- Polygon (137): 0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270 (WMATIC)
- Optimism (10): 0x4200000000000000000000000000000000000006 (WETH)
- Arbitrum (42161): 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1 (WETH)
- Base (8453): 0x4200000000000000000000000000000000000006 (WETH)
- BNB Chain (56): 0xBB4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c (WBNB)
Gnosis Chain (formerly xDai) does have a wrapped native token (WXDAI) at 0x6A023CCd1ff6F2045C3309768eAd9E68F978f6e1. Please either:
- Add it to
WRAPPED_NATIVE_TOKEN_ADDRESSES:100: '0x6A023CCd1ff6F2045C3309768eAd9E68F978f6e1', // WXDAI (Gnosis Chain)
- Or include a comment explaining why Gnosis Chain is intentionally excluded (e.g., unsupported wrapping, out of scope).
🤖 Prompt for AI Agents
In src/apps/the-exchange/utils/wrappedTokens.ts between lines 6 and 20, the
wrapped native token addresses are correct but Gnosis Chain (XDAI) is missing.
Add the Gnosis Chain wrapped native token address with chain ID 100 and address
0x6A023CCd1ff6F2045C3309768eAd9E68F978f6e1 to the WRAPPED_NATIVE_TOKEN_ADDRESSES
record, or alternatively add a comment explaining why it is excluded if
intentional.
There was a problem hiding this comment.
Not including XDAI in here
There was a problem hiding this comment.
@RanaBug I understand you're intentionally excluding XDAI. To improve code maintainability, would you mind expanding the existing comment to briefly explain why XDAI is excluded? This would help future maintainers understand the rationale behind this design decision.
✏️ Learnings added
Learnt from: RanaBug
PR: pillarwallet/x#315
File: src/apps/the-exchange/utils/wrappedTokens.ts:6-20
Timestamp: 2025-05-23T14:44:33.875Z
Learning: XDAI (Gnosis Chain) is intentionally excluded from the WRAPPED_NATIVE_TOKEN_ADDRESSES mapping in the exchange app's wrappedTokens utility.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const isWrapRequired = isWrappedToken( | ||
| route.fromToken.address, | ||
| route.fromToken.chainId | ||
| ); | ||
|
|
||
| // If wrapping is required, we will add an extra step transaction with | ||
| // a wrapped token deposit first | ||
| if (isWrapRequired) { | ||
| const wrapCalldata = encodeFunctionData({ | ||
| abi: [ | ||
| { | ||
| name: 'deposit', | ||
| type: 'function', | ||
| stateMutability: 'payable', | ||
| inputs: [], | ||
| outputs: [], | ||
| }, | ||
| ], | ||
| functionName: 'deposit', | ||
| }); | ||
|
|
||
| stepTransactions.push({ | ||
| to: route.fromToken.address, // already a wrapped token address from the SwapReceiveCard | ||
| data: wrapCalldata, | ||
| value: BigInt(route.fromAmount), | ||
| chainId: route.fromChainId, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Critical issue: Incorrect wrapping logic may cause failures for users with wrapped tokens.
The current logic has a flaw that could cause transaction failures. The code checks if route.fromToken.address is a wrapped token and adds a wrapping step, but this doesn't distinguish between:
- User specified native token → needs wrapping step
- User specified wrapped token → should NOT add wrapping step
Since getBestOffer normalizes native tokens to wrapped tokens, route.fromToken.address will always be a wrapped token address in both cases. This means users who already own wrapped tokens (e.g., WETH) would get an unnecessary wrapping step that would fail.
Suggested fix: Track the original token type to determine if wrapping is actually needed:
// In getBestOffer, track if tokens were originally native
+ const isFromTokenNative = isNativeToken(fromTokenAddress);
+ const isToTokenNative = isNativeToken(toTokenAddress);
const fromTokenAddressWithWrappedCheck = getWrappedTokenAddressIfNative(
fromTokenAddress,
fromChainId
);
// Store this information in the returned offer
selectedOffer = {
tokenAmountToReceive: processEth(bestOffer.toAmount, toTokenDecimals),
offer: bestOffer as Route,
+ isFromTokenNative,
+ isToTokenNative,
};// In getStepTransactions, use the original token type
- const isWrapRequired = isWrappedToken(
- route.fromToken.address,
- route.fromToken.chainId
- );
+ const isWrapRequired = isFromTokenNative && isWrappedToken(
+ route.fromToken.address,
+ route.fromToken.chainId
+ );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/apps/the-exchange/hooks/useOffer.tsx between lines 136 and 163, the
wrapping logic incorrectly adds a wrapping step for all wrapped tokens because
it uses route.fromToken.address, which is always a wrapped token after
normalization. To fix this, track the original token type before normalization
and only add the wrapping step if the user originally specified a native token
that requires wrapping. Adjust the logic to distinguish between native tokens
needing wrapping and already wrapped tokens that should skip this step.
There was a problem hiding this comment.
No need in this flow
There was a problem hiding this comment.
@RanaBug I understand that this isn't needed in your flow. Thanks for clarifying. Do you mind sharing a bit more about why this case doesn't need to be handled? Is it because users can only select native tokens in your UI, or is there another mechanism that prevents this edge case?
github-actions
bot
temporarily deployed
to
Preview (fix/PRO-3348/wrap-token-native-when-swapping)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit