Conversation
WalkthroughThis update integrates the LiFi SDK into the exchange application, refactoring swap offer retrieval and transaction preparation to use LiFi's APIs. It removes legacy Etherspot swap logic, updates token and offer types, and ensures price data is propagated throughout token selection and swap flows. Supporting test and type changes are included, along with dependency additions. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant EnterAmount as EnterAmount Component
participant useOffer as useOffer Hook
participant LiFiSDK as LiFi SDK
participant ExchangeAction as ExchangeAction Component
UI->>EnterAmount: User enters swap details
EnterAmount->>useOffer: getBestOffer(params)
useOffer->>LiFiSDK: getRoutes(params)
LiFiSDK-->>useOffer: Returns available routes
useOffer-->>EnterAmount: Returns best offer
UI->>ExchangeAction: User confirms swap
ExchangeAction->>useOffer: getStepTransactions(route, account)
useOffer->>LiFiSDK: getStepTransaction for each step
useOffer-->>ExchangeAction: Returns step transactions
ExchangeAction->>Etherspot: Add transactions to batch and send
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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: |
9531023
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f9c28ea8.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3203-lifi-sdk-imple.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3203/lifi-sdk-implementation)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (1)
83-87: Good URL cleaning implementationCleaning the URL parameters after they've been processed is a good practice to prevent unintended reapplication if users refresh the page.
Consider adding a brief comment explaining the purpose of this URL cleaning for future maintainers:
// clean the URL after the token has been set + // This prevents reapplication of parameters if user refreshes the page const url = new URL(window.location.href); url.searchParams.delete('asset'); url.searchParams.delete('blockchain'); window.history.replaceState({}, '', url.toString());src/apps/the-exchange/index.tsx (1)
34-53: Secure configuration of LiFi SDK.The LiFi SDK is configured correctly with proper integrator name, EVM provider, and API key. The implementation of
switchChainis particularly well-done, creating a new wallet client with the correct chain configuration.Consider adding error handling for the case when a chainId is not found in the supported chains list.
switchChain: async (chainId) => // Switch chain by creating a new wallet client createWalletClient({ account: (provider as WalletClient).account, chain: supportedChains.find( (chain) => chain.id === chainId - ) as Chain, + ) as Chain || (() => { + console.error(`Chain with ID ${chainId} not supported`); + throw new Error(`Chain with ID ${chainId} not supported`); + })(), transport: http(), // eslint-disable-next-line @typescript-eslint/no-explicit-any }) as any,src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (1)
146-168: Clever workaround for removing individual transactions from batch.The implementation creates a custom solution to remove a single transaction from a batch by removing all chain transactions and re-adding all except the one to remove.
While this works, it could be optimized to avoid unnecessary state updates:
const removeOneTransaction = (transactionId: string, chainId: number) => { - // All transactions for this chain - const chainTransactions = globalTransactionsBatch.filter( - (tx) => tx.chainId === chainId - ); - // Remove all transactions from UI state - chainTransactions.forEach((tx) => { - removeFromBatch(tx.id as string); - }); - - // Wait for state update - setTimeout(() => { - // Adding back all transactions except the one we wanted to remove - chainTransactions.forEach((tx) => { - if (tx.id !== transactionId) { - addToBatch(tx); - } - }); - }, 0); + // Get all transactions for this chain except the one to remove + const chainTransactionsToKeep = globalTransactionsBatch.filter( + (tx) => tx.chainId === chainId && tx.id !== transactionId + ); + + // Remove the specific transaction directly + removeFromBatch(transactionId); + + // If the transaction removal changes batch structure in ways that require rebuild, + // only then perform the full rebuild + if (chainTransactionsToKeep.length > 0 && needsRebuild(transactionId, chainId)) { + // Remove all remaining transactions for this chain + chainTransactionsToKeep.forEach((tx) => { + removeFromBatch(tx.id as string); + }); + + // Re-add all transactions to keep + setTimeout(() => { + chainTransactionsToKeep.forEach((tx) => { + addToBatch(tx); + }); + }, 0); + } };Note: The
needsRebuildfunction would need to be implemented based on your specific requirements for when a full batch rebuild is necessary.src/apps/the-exchange/hooks/useOffer.tsx (2)
79-109: Enhance error logging in isAllowanceSetThe error message in the catch block is generic. Consider adding more context to help with debugging.
- console.error('Failed to check token allowance:', error); + console.error(`Failed to check token allowance for token ${tokenAddress} on chain ${chainId}:`, error);
128-141: Simplify the allowance check logicThe complex condition for checking if the token has enough allowance could be refactored for better readability.
- const isEnoughAllowance = isAllowance - ? formatUnits(isAllowance, step.action.fromToken.decimals) >= - formatUnits( - BigInt(step.action.fromAmount), - step.action.fromToken.decimals - ) - : undefined; - - // Here we are checking if this is not a native/gas token and if the allowance - // is not set, then we manually add an approve transaction - if ( - !isZeroAddress(step.action.fromToken.address) && - !isEnoughAllowance - ) { + // If token is not native and needs approval (no allowance or insufficient allowance) + const needsApproval = !isZeroAddress(step.action.fromToken.address) && ( + !isAllowance || + formatUnits(isAllowance, step.action.fromToken.decimals) < + formatUnits(BigInt(step.action.fromAmount), step.action.fromToken.decimals) + ); + + if (needsApproval) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
package.json(1 hunks)src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx(1 hunks)src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx(1 hunks)src/apps/the-exchange/components/EnterAmount/test/EnterAmount.test.tsx(1 hunks)src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx(4 hunks)src/apps/the-exchange/components/ExchangeAction/test/ExchangeAction.test.tsx(3 hunks)src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx(1 hunks)src/apps/the-exchange/hooks/useOffer.tsx(2 hunks)src/apps/the-exchange/hooks/usePillarSwapAssets.tsx(0 hunks)src/apps/the-exchange/index.tsx(2 hunks)src/apps/the-exchange/utils/types.tsx(2 hunks)src/apps/token-atlas/components/TokensSlider/TokensSlider.tsx(1 hunks)src/apps/token-atlas/components/TokensSlider/test/TokenSlider.test.tsx(1 hunks)src/apps/token-atlas/types/types.tsx(1 hunks)src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx(3 hunks)src/services/tokensData.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/apps/the-exchange/hooks/usePillarSwapAssets.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (1)
src/services/tokensData.ts (1)
chainNameToChainIdTokensData(237-258)
src/apps/the-exchange/index.tsx (2)
src/apps/the-exchange/hooks/useReducerHooks.tsx (1)
useAppSelector(6-6)src/utils/blockchain.ts (1)
supportedChains(139-148)
src/apps/the-exchange/components/ExchangeAction/test/ExchangeAction.test.tsx (1)
src/apps/the-exchange/utils/types.tsx (1)
SwapOffer(23-26)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (27)
src/apps/token-atlas/types/types.tsx (1)
22-22: Good addition of price property to SelectedTokenTypeThe addition of the optional price property to the SelectedTokenType interface is appropriate and aligns with the Lifi SDK integration. Using the optional modifier ensures backward compatibility.
src/apps/token-atlas/components/TokensSlider/TokensSlider.tsx (1)
126-126: LGTM - Price property handling looks goodThe implementation correctly handles the token price, with a proper fallback to 0 when the price data is unavailable.
src/apps/token-atlas/components/TokensSlider/test/TokenSlider.test.tsx (1)
137-137: Good test update for price propertyThe test has been properly updated to include the price property with the default value, maintaining test consistency with the implementation changes.
src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (1)
79-79: LGTM - Consistent price propagationThe price property from the selected token is properly included in the payload, maintaining consistency with the other components in the price data flow.
src/services/tokensData.ts (1)
281-343: Good implementation of token price propagation.The changes correctly filter API responses to only include 'token' or 'asset' types before processing, and add the price property from the original item to the token objects. This properly aligns with the LiFi SDK integration requirements, ensuring that price data is available throughout the token selection and swap flows.
The implementation maintains all the existing token filtering logic while adding the necessary price data in a clean, non-disruptive way.
package.json (1)
22-25: New dependencies added for LiFi integrationThe addition of
@etherspot/modular-sdkand@lifi/sdksupports the implementation of the LiFi SDK in the exchange app as described in the PR objectives.src/apps/the-exchange/components/EnterAmount/test/EnterAmount.test.tsx (1)
73-78: Well-structured LiFi SDK mock for testsThe mock correctly simulates the LiFi SDK's core methods that would be used in the application, allowing tests to run without making actual API calls.
src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (1)
137-142: Consistent LiFi SDK mocking across test filesThis mock matches the implementation in other test files, ensuring consistent test behavior across components that utilize the LiFi SDK.
src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx (2)
102-132: Verify all needed parameters are passed to getBestOfferThe implementation correctly creates the params object with all necessary details for the LiFi SDK transaction, including token addresses, chain IDs, and decimals from both source and destination tokens.
65-65:Details
❌ Incorrect review comment
Updated useOffer hook signature
The parameter has been removed from the useOffer hook call, reflecting the refactored implementation that likely now sources chain information directly from Redux state.
🏁 Script executed:
#!/bin/bash # Verifying the updated useOffer hook signature # Let's check how the useOffer hook is defined now # Look for the useOffer hook definition rg -A 10 "export default function useOffer" src/apps/the-exchange/hooks/Length of output: 74
I couldn’t locate the
useOfferdefinition with the initial pattern. Let’s search more broadly across the repo for how it’s declared:This will help us verify the new hook signature.
🏁 Script executed:
#!/bin/bash # Broad search for useOffer declaration across the repo rg -n "function useOffer" -C 3 src rg -n "const useOffer" -C 3 src rg -n "export .*useOffer" -C 3 srcLength of output: 852
Let’s inspect the
useOfferimplementation to confirm it now sources chain info internally:
🏁 Script executed:
#!/bin/bash # Check for Redux selectors or internal chain resolution in useOffer rg -n "useSelector" -C 3 src/apps/the-exchange/hooks/useOffer.tsx rg -n "useAppSelector" -C 3 src/apps/the-exchange/hooks/useOffer.tsx # See where getNetworkViem is called rg -n "getNetworkViem" -C 3 src/apps/the-exchange/hooks/useOffer.tsx # Search for any direct “chain” references rg -n "chain" -C 4 src/apps/the-exchange/hooks/useOffer.tsxLength of output: 3137
Updated useOffer hook signature
TheuseOfferhook no longer accepts any arguments. It now returns helper methods (e.g.getBestOffer,isAllowanceSet) that each expect achainId(and other params) when you call them.• Ensure all calls to
getBestOffer({ … })andisAllowanceSet({ … })pass the correctchainIdin their argument objects.
• No Redux selectors are used insideuseOffer; chain IDs are provided per-call rather than pulled from state.Likely an incorrect or invalid review comment.
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (4)
2-2: Good refactoring to use the LiFi SDK integration.The addition of
useWalletAddressfrom the Etherspot transaction kit and updating the hook usage to accessgetStepTransactionsfromuseOffersets up the component properly for the new LiFi SDK integration.Also applies to: 17-17, 53-54
93-96: LGTM: Transaction preparation using LiFi SDK.The implementation correctly retrieves step transactions for the current offer, passing the wallet address in the right format with TypeScript type safety.
98-117: Handle empty or undefined step transactions before the loop.The code assumes
stepTransactionswill be an array when entering the loop at line 100, but there's no explicit check that it's non-empty beyond theif (stepTransactions)check.Add an additional check before the loop to ensure
stepTransactionsis non-empty:if (stepTransactions) { + if (stepTransactions.length === 0) { + setErrorMessage('No transactions to process. Please try again.'); + setIsAddingToBatch(false); + return; + } // eslint-disable-next-line no-plusplus for (let i = 0; i < stepTransactions.length; ++i) {
101-102: Verify properties on step transactions.The implementation correctly extracts
value,data, andtofrom step transactions. Consider using optional chaining for safer access to potentially undefined properties.Also applies to: 108-109, 115-116
src/apps/the-exchange/index.tsx (1)
2-4: Good integration of Etherspot and LiFi.The imports of necessary modules from Etherspot transaction kit and LiFi SDK are appropriate for the SDK integration.
Also applies to: 26-26
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (2)
42-46: LGTM: Enhanced batch management hooks.The addition of
addToBatchalongside other functions fromuseGlobalTransactionsBatchprovides a complete set of batch management capabilities.
225-229: Good update to use the new removal function.The replacement of the direct
removeFromBatchcall with the newremoveOneTransactionfunction correctly implements the more robust transaction removal approach.src/apps/the-exchange/utils/types.tsx (4)
2-3: LGTM: Updated imports for the LiFi SDK integration.The change from
@lifi/typesto@lifi/sdkfor theRoutetype and addition ofHexfromviemare appropriate for the SDK integration.
25-25: Good type narrowing for offer property.Restricting the
offerproperty to only theRoutetype from LiFi SDK improves type safety by removing the previous union type withExchangeOffer.
33-42: Well-defined transaction type structure.The
StepTransactiontype clearly defines the structure needed for blockchain transactions, with appropriate optional properties.
44-44: Good enumeration of step types.The
StepTypeunion type provides a clear enumeration of all possible step categories, which will help with type checking throughout the application.src/apps/the-exchange/components/ExchangeAction/test/ExchangeAction.test.tsx (3)
53-86: Greatly improved mock data structure!The detailed
mockBestOfferobject now accurately represents the LiFi SDK'sRoutestructure, providing a more realistic testing scenario. The inclusion of comprehensive fields like token details, price information, and the insurance object will make tests more robust.🧰 Tools
🪛 Biome (1.9.4)
[error] 52-86: Do not export from a test file.
(lint/suspicious/noExportsInTest)
99-104: Good addition of required mocksThese mock implementations for
useEtherspotUtilsanduseWalletAddressprovide the necessary stubs for testing the component with the LiFi integration.
114-119: LiFi SDK mock implementation looks goodThe mock for the LiFi SDK correctly stubs out the asynchronous methods that would be used in the component, ensuring tests can run in isolation from external dependencies.
src/apps/the-exchange/hooks/useOffer.tsx (3)
1-8: LGTM! Great SDK integrationThe import of LiFi SDK components looks good - you're importing only the specific functions and types needed for this implementation.
10-16: Good use of viem libraryThe switch to using viem for blockchain interactions is a good choice, as it provides a more modern and type-safe API compared to alternatives.
118-120:Details
❌ Incorrect review comment
Address ESLint disabled rules
Multiple ESLint rules are disabled in the code, which may indicate potential issues with the code structure. The
no-restricted-syntaxrule for the for-of loop andno-await-in-loopsuggest that there might be a better way to structure this code.Consider refactoring to use Promise.all() for parallel execution of steps that don't depend on each other:
// Example approach (conceptual) const stepPromises = await Promise.all( route.steps.map(async (step) => { const transactions = []; const isAllowance = await isAllowanceSet({...}); // Add approval transaction if needed if (needsApproval(step, isAllowance)) { transactions.push(createApprovalTransaction(step)); } // Add swap transaction const updatedStep = await getStepTransaction(prepareStep(step, fromAccount)); if (updatedStep.transactionRequest) { transactions.push(createSwapTransaction(updatedStep)); } return transactions; }) ); return stepPromises.flat();Is sequential processing of steps necessary for LiFi swaps? Let's verify:
🌐 Web query:
Does LiFi SDK require steps to be processed sequentially or can they be processed in parallel?💡 Result:
Based on the LI.FI SDK documentation, steps within a route must be executed sequentially rather than in parallel. When executing a route, each step depends on the outcome of the previous one, making sequential execution necessary for the process to work correctly[6].
The documentation specifically states: "Each step should be executed sequentially, as each step depends on the outcome of the previous one"[6]. This sequential requirement exists because later steps in a cross-chain transfer or swap operation typically rely on the successful completion of earlier steps, ensuring the integrity of the entire transaction flow.
It's important to note that while the steps themselves must be executed in sequence, LI.FI as a technology does support certain parallel capabilities in other contexts. For example, when using the
getStepTransactionfunction to prepare transaction data for execution, developers need to handle each step one after another in the correct order[6].In contrast, quote objects are returned with transaction data already included, so they don't require the additional
getStepTransactioncall and can be executed immediately, though still in the proper sequence[6].This sequential approach ensures reliability in cross-chain operations, especially when bridging between different blockchain networks where transaction confirmation on one chain may be a prerequisite for operations on another chain.
Citations:
- 1: https://docs.li.fi/integrate-li.fi-sdk/li.fi-sdk-overview
- 2: https://li.fi/knowledge-hub/why-li-fi-is-excited-about-eclipse-and-the-modular-future/
- 3: https://github.com/lifinance/sdk
- 4: https://www.digitalxplore.org/up_proc/pdf/367-152698903535-39.pdf
- 5: https://www.buildbear.io/docs/tutorials/lifi-plugin-tutorial
- 6: https://docs.li.fi/integrate-li.fi-sdk/execute-routes-quotes
- 7: https://pmc.ncbi.nlm.nih.gov/articles/PMC10181365/
- 8: https://www.controldesign.com/connections/remote-connectivity/article/11319362/12-steps-to-li-fi-implementation
- 9: https://www.tec.gov.in/pdf/Studypaper/lifi%20study%20paper%20-%20approved.pdf
- 10: https://www.oledcomm.net/blog/how-does-lifi-work/
Sequential execution is required – suppressing ESLint rules is intentional
The LiFi SDK documentation specifies that each step in a route must be executed one after another, as each step depends on the outcome of the previous one. Therefore, using
awaitinside a loop and disablingno-restricted-syntax/no-await-in-loophere is appropriate. To make this clear to future readers, you can consolidate and annotate the disables:// LiFi SDK requires sequential execution of steps // eslint-disable-next-line no-restricted-syntax, no-await-in-loop for (const step of route.steps) { // … }Likely an incorrect or invalid review comment.
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3203/lifi-sdk-implementation)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/apps/the-exchange/hooks/useOffer.tsx (4)
73-79: Improve error handling in getBestOfferThe current implementation returns an empty object cast as
SwapOfferwhen an error occurs. This could lead to runtime errors if the consuming code expects allSwapOfferproperties to be present.Consider returning a more explicit error indication or defined default values:
- return {} as SwapOffer; + return { + tokenAmountToReceive: 0, + offer: { + id: '', + fromChainId: fromChainId, + toChainId: toChainId, + fromAmount: '0', + toAmount: '0', + fromToken: { address: fromTokenAddress, chainId: fromChainId, decimals: fromTokenDecimals, symbol: '', name: '' }, + toToken: { address: toTokenAddress, chainId: toChainId, decimals: toTokenDecimals, symbol: '', name: '' }, + steps: [] + } + };
148-180: Use imported erc20Abi instead of hardcoding the ABIYou're importing
erc20Abibut not using it for the approval transaction. Using the imported ABI would be more maintainable.- const calldata = encodeFunctionData({ - abi: [ - { - inputs: [ - { - internalType: 'address', - name: 'spender', - type: 'address', - }, - { - internalType: 'uint256', - name: 'value', - type: 'uint256', - }, - ], - name: 'approve', - outputs: [ - { - internalType: 'bool', - name: '', - type: 'bool', - }, - ], - stateMutability: 'nonpayable', - type: 'function', - }, - ], - functionName: 'approve', - args: [ - step.estimate.approvalAddress as `0x${string}`, - BigInt(step.estimate.fromAmount), - ], + const calldata = encodeFunctionData({ + abi: erc20Abi, + functionName: 'approve', + args: [ + step.estimate.approvalAddress as `0x${string}`, + BigInt(step.estimate.fromAmount), + ],
213-224: Add guards for optional transaction parametersWhen extracting values from
updatedStep.transactionRequest, there's no check if properties likegasLimitorgasPriceare defined. This could lead to runtime errors if they're undefined.- const { to, data, value, gasLimit, gasPrice, chainId, type } = - updatedStep.transactionRequest; - - stepTransactions.push({ - to, - data: data as `0x${string}`, - value: BigInt(`${value}`), - gasLimit: BigInt(`${gasLimit}`), - gasPrice: BigInt(`${gasPrice}`), - chainId, - type, - }); + const { to, data, value, gasLimit, gasPrice, chainId, type } = + updatedStep.transactionRequest; + + stepTransactions.push({ + to, + data: data as `0x${string}`, + value: value ? BigInt(`${value}`) : BigInt(0), + ...(gasLimit && { gasLimit: BigInt(`${gasLimit}`) }), + ...(gasPrice && { gasPrice: BigInt(`${gasPrice}`) }), + chainId, + ...(type && { type }), + });
226-228: Improve error handling in getStepTransactionsThe catch block only logs errors but doesn't provide any indication to the caller that something went wrong. Consider adding more context and returning some information about the failure.
} catch (error) { console.error('Failed to get step transactions:', error); + // Consider adding some error state to the return value + // or throwing a more specific error with context }
🧹 Nitpick comments (2)
src/apps/the-exchange/hooks/useOffer.tsx (2)
39-52: Consider making bridge filter options configurableThe bridge deny list is currently hardcoded. Making these options configurable would provide more flexibility and potentially better swap results in different scenarios.
const routesRequest: RoutesRequest = { fromChainId, toChainId, fromTokenAddress, toTokenAddress, fromAmount: `${parseUnits(`${fromAmount}`, fromTokenDecimals)}`, options: { bridges: { - deny: ['across', 'multichain', 'hyphen', 'hop'], + deny: bridgesDenyList || ['across', 'multichain', 'hyphen', 'hop'], }, }, };
124-139: Consider optimizing sequential async operations in the loopThe current implementation has sequential async calls within a loop, which can be slow for routes with many steps. Consider using Promise.all to parallelize when possible.
- // eslint-disable-next-line no-restricted-syntax - for (const step of route.steps) { - // eslint-disable-next-line no-await-in-loop - const isAllowance = await isAllowanceSet({ - owner: fromAccount, - spender: step.estimate.approvalAddress, - tokenAddress: step.action.fromToken.address, - chainId: step.action.fromChainId, - }); + // Prepare allowance check requests for all steps + const allowancePromises = route.steps.map(step => + isAllowanceSet({ + owner: fromAccount, + spender: step.estimate.approvalAddress, + tokenAddress: step.action.fromToken.address, + chainId: step.action.fromChainId, + }) + ); + + // Execute all allowance checks in parallel + const allowances = await Promise.all(allowancePromises); + + // Process each step with its corresponding allowance result + for (let i = 0; i < route.steps.length; i++) { + const step = route.steps[i]; + const isAllowance = allowances[i];Note: This optimization may need to be adapted based on your specific requirements. The subsequent transaction processing would still need to be sequential since each transaction might depend on the previous one.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx(4 hunks)src/apps/the-exchange/hooks/useOffer.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just a note but it's not a show stopper, LGTM
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores
@etherspot/modular-sdkand@lifi/sdk.