Feat/support swap oraidex osmosis #349
Conversation
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
packages/universal-swap/src/handler.ts (1)
1202-1203: Nitpick: Remove redundant initialization ofevmAddresstoundefinedVariables declared with
letare initialized toundefinedby default in JavaScript/TypeScript. You can omit the= undefinedfor brevity and clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/universal-swap/package.json (1 hunks)
- packages/universal-swap/src/handler.ts (5 hunks)
- packages/universal-swap/src/helper.ts (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/universal-swap/package.json
🧰 Additional context used
🔇 Additional comments (8)
packages/universal-swap/src/helper.ts (2)
331-341: Update generateAddress Method to Include evmInfo: ApprovedThe changes to the
generateAddressmethod correctly add the optionalevmInfoparameter, enhancing its functionality to handle EVM addresses. The types are properly specified, and the implementation is sound.
373-373: Addition of evmAddress Parameter in addOraiBridgeRoute: ApprovedThe addition of the optional
evmAddressparameter to theaddressesobject enhances flexibility in managing EVM addresses during route addition. No issues found.packages/universal-swap/src/handler.ts (6)
1218-1223: Code logic for detecting EVM chains appears correctThe implementation correctly identifies routes involving Ethereum and BSC chains using the
hasEvmflag.
1224-1227: Code logic for detecting TRON chain appears correctThe
hasTronflag correctly checks if any route involves the TRON chain.
1230-1235: Assignment ofevmAddressbased on EVM flags is appropriateThe code correctly assigns
evmAddresswhen eitherhasEvmorhasTronistrue.
1243-1245: PassingrecipientAddressandevmAddresstoaddOraiBridgeRouteThe parameters
recipientAddressandevmAddressare correctly passed to theaddOraiBridgeRoutefunction to facilitate address handling.
1255-1265: Conditional assignment ofevmInfoinaddressParamsis correctThe
addressParamsobject properly setsevmInfobased on whether theoriginalToTokenis not cosmos-based andevmAddressis defined.
1266-1268: UpdatingreceiverAddresseswithrecipientAddressThe code correctly updates
receiverAddresseswith therecipientAddressfor the current network when a recipient address is provided.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (11)
packages/universal-swap/tests/helper.spec.ts (3)
Line range hint
630-641: LGTM! Consider adding an assertion for theuniversalSwapType.The changes improve the readability of the test case by introducing the
sourceReceivervariable. The test now correctly checks the behavior ofaddOraiBridgeRoutewhen the swap route is empty.As a minor improvement, consider adding an assertion for the
universalSwapTypeproperty of the result to ensure it's set correctly.You could add the following assertion:
expect(result.universalSwapType).toEqual("other-networks-to-oraichain");
Line range hint
660-717: LGTM! Consider adding edge cases to the test suite.The updates to the
unmarshalOraiBridgeRoutetest cases improve their accuracy and realism by using more specificoraiReceivervalues. This change reflects the expected behavior of the function more closely.To further enhance the test suite, consider adding edge cases such as:
- An empty string input
- A malformed input string
- A string with extra delimiters
These additional test cases would help ensure the robustness of the
unmarshalOraiBridgeRoutefunction.
Line range hint
1-1004: Overall improvements to test cases, but some areas need attention.The changes in this file generally improve the test suite for the universal swap helper functions. The test cases are now more specific and realistic, particularly in the
unmarshalOraiBridgeRoutetests. However, there are a few areas that could use attention:
- There's a commented-out test case for
generateAddressthat should either be implemented or removed.- Some test cases could benefit from additional assertions to ensure all aspects of the function behavior are covered.
- Consider adding more edge cases to improve the robustness of the test suite.
Additionally, it might be helpful to add more descriptive comments for complex test cases to explain the purpose and expected behavior of each test.
packages/universal-swap/tests/index.spec.ts (3)
260-267: LGTM! Consider adding a comment explaining the address format.The changes to the
getKeplrAddrmethod improve test coverage by adding support for the Osmosis chain and updating the default Oraichain address. This is a good improvement for testing different chain scenarios.Consider adding a brief comment explaining the format of these test addresses (e.g., why they end with "1234" for test chains). This would improve code readability and make it easier for other developers to understand the test setup.
349-349: LGTM! Consider using a constant for test addresses.The receiver address in the IBC transfer messages has been correctly updated to use the Osmosis address ("osmo1234") instead of the previous Oraichain address. This change aligns with the modifications in the
StubCosmosWalletclass and improves the accuracy of the test cases for Osmosis-related transfers.For better maintainability and consistency, consider defining these test addresses (e.g., "osmo1234", "orai1234") as constants at the top of the file or in a separate test utilities file. This would make it easier to update all test cases if the addresses need to change in the future.
Also applies to: 438-438
993-993: LGTM! Consider grouping chain-specific test data.The receiver addresses in various IBC transfer and swap messages have been correctly updated to use the appropriate addresses for different chains (Oraichain, Osmosis, Cosmos, Noble). These changes improve the accuracy and coverage of the test cases for multi-chain scenarios.
To improve code organization and readability, consider grouping all chain-specific test data (addresses, chain IDs, etc.) into a single object or constant. This would make it easier to manage and update test data for different chains in the future. For example:
const TEST_CHAIN_DATA = { Oraichain: { address: "orai1g4h64yjt0fvzv5v2j8tyfnpe5kmnetejvfgs7g", chainId: "Oraichain" }, Osmosis: { address: "osmo1234", chainId: "osmosis-1" }, Cosmos: { address: "cosmos12zyu8w93h0q2lcnt50g3fn0w3yqnhy4flwc7p3", chainId: "cosmoshub-4" }, Noble: { address: "noble12zyu8w93h0q2lcnt50g3fn0w3yqnhy4fhddkel", chainId: "noble-1" } };Then use this object throughout the test file for consistency and easier maintenance.
Also applies to: 1054-1054
packages/universal-swap/src/helper.ts (1)
227-228: Enhance error message for invalid recipient addressThe current error message
"orai Address get source receiver invalid!"may not be clear to users. Consider rephrasing it to"Invalid Oraichain recipient address provided."for better clarity.Apply this diff to improve the error message:
- if (!isValidRecipient.isValid) throw generateError("orai Address get source receiver invalid!"); + if (!isValidRecipient.isValid) throw generateError("Invalid Oraichain recipient address provided!");packages/universal-swap/src/handler.ts (4)
Line range hint
1283-1283: Typo in method name 'caculateMinimumReceive'The method
caculateMinimumReceiveis misspelled. It should becalculateMinimumReceive.Apply this diff to fix the method name:
- async caculateMinimumReceive() { + async calculateMinimumReceive() {Remember to update all references to this method accordingly.
Line range hint
1193-1270: Consider refactoring 'processUniversalSwap' into smaller methodsThe
processUniversalSwapmethod spans numerous lines and contains complex logic, which can make it hard to read and maintain. Consider refactoring it into smaller, well-named helper methods to improve readability and maintainability.
1242-1245: Use 'evmAddress' variable consistentlyYou have already assigned
this.swapData.sender.evmtoevmAddress, so you can useevmAddressinstead when settingaddressParams.evmInfo.Apply this diff:
addressParams.evmInfo = { ...addressParams.evmInfo, - [EVM_CHAIN_IDS.ETH]: this.swapData.sender.evm, - [EVM_CHAIN_IDS.BSC]: this.swapData.sender.evm + [EVM_CHAIN_IDS.ETH]: evmAddress, + [EVM_CHAIN_IDS.BSC]: evmAddress };
1250-1253: Use 'tronAddress' variable consistentlyAfter assigning
tronAddress, you can use it instead of recomputingtronToEthAddress(this.swapData.sender.tron)when settingaddressParams.evmInfo.Apply this diff:
addressParams.evmInfo = { ...addressParams.evmInfo, - [EVM_CHAIN_IDS.TRON]: tronToEthAddress(this.swapData.sender.tron) + [EVM_CHAIN_IDS.TRON]: tronAddress };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/universal-swap/src/handler.ts (4 hunks)
- packages/universal-swap/src/helper.ts (6 hunks)
- packages/universal-swap/tests/helper.spec.ts (4 hunks)
- packages/universal-swap/tests/index.spec.ts (6 hunks)
🧰 Additional context used
🪛 Biome
packages/universal-swap/src/helper.ts
[error] 353-353: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🔇 Additional comments (3)
packages/universal-swap/tests/helper.spec.ts (1)
395-435: Consider enabling or removing the commented-out test case.This test case for
UniversalSwapHelper.generateAddressis currently disabled. If the functionality is not yet implemented or there are unresolved issues, consider adding a TODO comment explaining why it's commented out and when it should be addressed. If the test is no longer relevant, it would be better to remove it entirely to keep the test suite clean.To check if there are any other commented-out tests in the file, you can run:
✅ Verification successful
No additional commented-out tests found in
packages/universal-swap/tests/helper.spec.ts.The only commented-out test case for
UniversalSwapHelper.generateAddresshas been addressed. Ensure it is either enabled with necessary fixes or removed if obsolete to maintain a clean test suite.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other commented-out tests in the file rg '^\s*//\s*it\.' packages/universal-swap/tests/helper.spec.tsLength of output: 131
packages/universal-swap/tests/index.spec.ts (1)
524-524: LGTM! Test case updated correctly.The expected result for the "cosmoshub-4" chain ID has been updated to match the new default address set in the
StubCosmosWalletclass. This ensures consistency between the implementation and the test case.packages/universal-swap/src/handler.ts (1)
63-63: LGTM!The import statement is correct and necessary for the functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (8)
packages/universal-swap/src/handler.ts (6)
1176-1191: Improved address handling and validationThe
getToAddressUniversalSwapmethod has been enhanced to properly validate recipient addresses and handle different chain scenarios, including the special case for Tron. This is a good improvement for robustness and security.Consider extracting the chain-specific logic (e.g., Tron address handling) into a separate method for better maintainability.
1216-1247: Improved support for alpha IBC wasm swapsThe new code block enhances support for alpha IBC wasm swaps, handling various chain types including EVM and Tron. The use of array destructuring for boolean checks is efficient and readable.
Consider extracting the chain-specific logic into separate methods to improve maintainability and reduce the complexity of this method.
Line range hint
1288-1330: Enhanced minimum receive calculationThe
calculateMinimumReceivemethod has been significantly improved to handle various scenarios, including relayer fees and different token types. The changes enhance the accuracy of the calculation and provide better support for complex swap operations.Consider breaking down this method into smaller, more focused functions to improve readability and maintainability. For example, you could extract the relayer fee calculation into a separate method.
Line range hint
1364-1483: Enhanced swap message generationThe
generateMsgsSwapmethod has been significantly improved to handle various swap scenarios, including IBC wasm swaps. The additions in error handling, recipient address validation, and support for complex swap operations enhance the robustness and flexibility of the method.Consider extracting the IBC wasm specific logic into a separate method to improve readability and maintainability of this method.
Line range hint
1526-1589: Enhanced IBC Wasm transfer message generationThe
generateMsgsIbcWasmmethod has been improved to handle IBC Wasm transfers for both native and CW20 tokens. The additions enhance support for cross-network transfers and include proper error handling for invalid IBC Wasm source ports.Consider adding more detailed error messages or logging to aid in debugging potential issues with IBC Wasm transfers.
Line range hint
1-1589: Overall improvements with opportunities for further refinementThe UniversalSwapHandler class has been significantly enhanced to support a wider range of swap scenarios, including cross-chain swaps, smart routing, and IBC Wasm transfers. The additions improve error handling, address validation, and support for various token types and network configurations.
While these changes greatly improve the functionality and robustness of the class, there are opportunities for further refinement:
- Consider breaking down some of the larger methods (e.g.,
processUniversalSwap,generateMsgsSwap) into smaller, more focused functions to improve readability and maintainability.- Implement more comprehensive error handling and logging throughout the class to aid in debugging and improve the developer experience.
- Consider creating separate classes or modules for handling specific network types (e.g., EVM, Cosmos, Tron) to better organize the network-specific logic.
- Add more inline documentation or comments explaining the purpose and functionality of complex code blocks, especially in areas dealing with cross-chain operations.
These suggestions could help improve the long-term maintainability and scalability of the codebase.
packages/universal-swap/src/helper.ts (2)
228-228: Clarify the error message for invalid recipient addressThe error message
"orai Address get source receiver invalid!"is a bit unclear. Consider rephrasing it to make it more understandable to users.Apply this diff to improve the error message:
- if (!isValidRecipient.isValid) throw generateError("orai Address get source receiver invalid!"); + if (!isValidRecipient.isValid) throw generateError("Invalid Oraichain recipient address provided!");
470-470: Remove debugconsole.logstatementThe
console.logstatement appears to be used for debugging purposes. Consider removing it or replacing it with appropriate logging if necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/universal-swap/src/handler.ts (5 hunks)
- packages/universal-swap/src/helper.ts (6 hunks)
- packages/universal-swap/tests/index.spec.ts (7 hunks)
🧰 Additional context used
🪛 Biome
packages/universal-swap/src/helper.ts
[error] 354-354: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🔇 Additional comments (10)
packages/universal-swap/src/handler.ts (4)
63-63: New import addedThe addition of
EVM_CHAIN_IDSfrom@oraichain/commonsuggests that the code now handles EVM-specific chain IDs. This is a good practice for maintaining consistency and avoiding hardcoded values.
1193-1214: Enhanced cross-chain swap handlingThe
processUniversalSwapmethod has been significantly improved to handle various address types and chain scenarios. This enhancement increases the flexibility and robustness of the swap process across different networks.
Line range hint
1331-1363: Improved smart router swap message generationThe
generateMsgsSmartRouterSwapmethod has been enhanced to better handle smart router swaps, including support for affiliates and proper slippage handling. The addition of error checking for missing simulate price or user slippage improves the robustness of the method.
Line range hint
1484-1525: Improved smart route swap message buildingThe
buildSwapMsgsFromSmartRoutemethod has been enhanced to better handle smart route swap operations. The addition of minimum receive calculation using user-defined slippage and proper handling of both native and CW20 tokens improves the flexibility and accuracy of the swap process.packages/universal-swap/tests/index.spec.ts (6)
349-349: Update to receiver address in test caseThe receiver address has been changed from "orai1234" to "osmo1234".
This change is consistent with the updates made to the
StubCosmosWalletclass and appears to be intentional.
438-438: Update to receiver address in another test caseSimilar to the previous change, the receiver address has been updated from "orai1234" to "osmo1234".
This change is consistent with the updates made to the
StubCosmosWalletclass and appears to be intentional.
524-524: Update to expected address ingetUniversalSwapToAddresstestThe expected address for the "Oraichain" chain ID has been updated to "orai1g4h64yjt0fvzv5v2j8tyfnpe5kmnetejvfgs7g".
This change is consistent with the updates made to the
StubCosmosWalletclass and appears to be intentional.
993-995: Update to remote address in test caseThe remote address has been changed from "orai1234" to "orai1g4h64yjt0fvzv5v2j8tyfnpe5kmnetejvfgs7g".
This change is consistent with the updates made to the
StubCosmosWalletclass and appears to be intentional.
1054-1056: Update to remote address in another test caseSimilar to the previous change, the remote address has been updated from "orai1234" to "orai1g4h64yjt0fvzv5v2j8tyfnpe5kmnetejvfgs7g".
This change is consistent with the updates made to the
StubCosmosWalletclass and appears to be intentional.
1887-1887: Rename method fromcaculateMinimumReceivetocalculateMinimumReceiveThe method name has been corrected from
caculateMinimumReceivetocalculateMinimumReceive.This is a good correction that fixes a typo in the method name. It improves code readability and consistency.
To ensure this change is applied consistently across the codebase, you may want to run the following check:
|
Code Coverage SummaryDiff against mainResults for commit: f2e61b8 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
packages/universal-swap/src/helper.ts (2)
452-454: Improve error message clarity.The current error message "Missing router with alpha ibc wasm max length!" could be more descriptive about the actual limitation.
Apply this diff to improve the error message:
- throw generateError(`Missing router with alpha ibc wasm max length!`); + throw generateError(`EVM tokens with Alpha IBC Wasm only support single route operations.`);
456-458: Add comment explaining path filtering requirement.The path filtering logic for EVM tokens would benefit from a comment explaining why the first path is excluded.
Apply this diff to improve code documentation:
if (fromTokenIsEvm) { + // Skip the first path for EVM tokens as it represents the initial bridge operation paths = alphaRoutes.paths.filter((_, index) => index > 0); }packages/universal-swap/src/handler.ts (2)
Line range hint
1190-1267: Consider enhancing type safety for address parametersThe address parameter handling could benefit from stronger typing to prevent potential errors. Consider creating an interface for the address parameters.
Add a type definition:
interface AddressParams { oraiAddress: string; injAddress?: string; evmInfo: { [key: string]: string; }; }Then use it in the code:
- let addressParams = { + let addressParams: AddressParams = { oraiAddress, injAddress, evmInfo: {} };
Line range hint
1286-1334: Enhance readability of minimum receive calculationThe calculation logic could be more readable by extracting the fee calculations into separate helper functions.
Consider refactoring like this:
private calculateBridgeFeeAdjustment(amount: string, bridgeFee: number): number { return (bridgeFee * Number(amount)) / 100; } private calculateSlippageAdjustment(amount: string, slippage: number): number { return (slippage * Number(amount)) / 100; } async calculateMinimumReceive() { const { relayerFee, simulateAmount, originalToToken, bridgeFee = 1, userSlippage = 0 } = this.swapData; const convertSimulateAmount = simulateAmount; // Calculate relayer fee let subRelayerFee = await this.calculateRelayerFee(relayerFee, originalToToken); // Calculate adjustments const bridgeFeeAdjustment = this.calculateBridgeFeeAdjustment(convertSimulateAmount, bridgeFee); const slippageAdjustment = this.calculateSlippageAdjustment(convertSimulateAmount, userSlippage); // Calculate final amount const minimumReceive = new BigDecimal(convertSimulateAmount) .sub(bridgeFeeAdjustment) .sub(slippageAdjustment) .sub(subRelayerFee) .toString(); return Math.max(0, Math.floor(Number(minimumReceive))).toLocaleString("fullwide", { useGrouping: false }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/universal-swap/src/handler.ts (5 hunks)
- packages/universal-swap/src/helper.ts (6 hunks)
🧰 Additional context used
🪛 Biome
packages/universal-swap/src/helper.ts
[error] 354-354: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🔇 Additional comments (6)
packages/universal-swap/src/helper.ts (5)
51-53: LGTM! Good addition of address validation imports.The new imports enhance the security of the codebase by adding proper address validation functionality.
227-229: LGTM! Proper address validation implementation.The added validation ensures that only valid Oraichain addresses are processed, enhancing security.
386-387: LGTM! Good implementation of cross-chain address handling.The changes properly handle EVM and Tron addresses while maintaining compatibility with the OraiBridge protocol.
Also applies to: 417-420, 427-427
440-443: LGTM! Good documentation of supported routes.The comment clearly documents the supported cross-chain routes for Alpha IBC Wasm.
370-376: Verify evmInfo merge behavior.The current implementation merges
evmInfodirectly intocosmosAddresswhich could potentially overwrite existing keys. Consider adding a check to prevent unintended overwrites.✅ Verification successful
Let me gather more information about the structure of
evmInfoandcosmosAddressto better understand potential key conflicts.
Let me gather more information about the structure and usage of
cosmosAddressandevmInfoto better understand the merge behavior.
Let me try a different search pattern to understand the context better.
Let me try one more search pattern to understand the context of the merge operation.
Based on the code context, I can now provide a final response about the evmInfo merge behavior.
The merge of evmInfo into cosmosAddress is safe and intentional
The merge operation is part of the
generateAddresshelper method that combines different address types into a single address mapping. Here's why the merge is safe:
cosmosAddressis initially populated with Cosmos-based addresses (Oraichain and Injective)evmInfois an optional parameter containing EVM chain addresses as a simple key-value mapping- The merge is additive - it adds EVM addresses under their respective chain IDs without risk of overwriting Cosmos addresses because:
- Cosmos chain IDs use string names (e.g., "Oraichain", "injective-1")
- EVM chain IDs use hex format (e.g., "0x01", "0x38")
Therefore, there is no risk of key collisions between Cosmos and EVM addresses in the merged object.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any potential key conflicts between cosmos addresses and EVM addresses rg -A 2 "chainId.*[\"'](?:0x38|0x01|0x2b6653dc)[\"']"Length of output: 7619
Script:
#!/bin/bash # Search for the structure and usage of evmInfo and cosmosAddress ast-grep --pattern 'interface $_ { $$$ }' # Also search for type definitions and assignments rg -A 5 "type.*cosmosAddress|interface.*cosmosAddress|evmInfo.*=|cosmosAddress.*="Length of output: 355442
Script:
#!/bin/bash # Search for the structure and usage of cosmosAddress and evmInfo ast-grep --pattern 'interface $_ { evmInfo?: $_ }' # Also search for any type definitions and assignments related to these objects rg -A 5 "type.*evmInfo|interface.*evmInfo|type.*cosmosAddress|cosmosAddress.*type"Length of output: 139
Script:
#!/bin/bash # Search for the context around where cosmosAddress and evmInfo are used rg -B 5 -A 10 "cosmosAddress.*evmInfo" # Also search for any type definitions or interfaces that might contain these fields ast-grep --pattern 'type $_ = { $$$ evmInfo?: $_ $$$ }' # Look for the broader context of the file rg -l "cosmosAddress.*evmInfo" | xargs catLength of output: 149
Script:
#!/bin/bash # Search for the broader context of the file where the merge happens rg -B 10 -A 10 "cosmosAddress.*=.*\{.*\.\.\.cosmosAddress" # Also look for any type definitions or interfaces in the helper.ts file rg -A 5 "interface|type" packages/universal-swap/src/helper.ts # Get the full content of the helper.ts file around this merge rg -B 20 -A 20 "evmInfo" packages/universal-swap/src/helper.tsLength of output: 6258
packages/universal-swap/src/handler.ts (1)
1173-1188:⚠️ Potential issueInconsistent use of recipientAddress parameter
The function uses both the
recipientAddressparameter andthis.swapData.recipientAddress, which can lead to confusion. Consider using the parameter consistently throughout the method.Apply this diff:
- if (this.swapData.recipientAddress) { + if (recipientAddress) { const isValidRecipient = checkValidateAddressWithNetwork(recipientAddress, originalToToken.chainId); if (!isValidRecipient.isValid) throw generateError("Recipient address invalid!"); return originalToToken.chainId === ChainIdEnum.TRON - ? tronToEthAddress(recipientAddress) - : this.swapData.recipientAddress; + ? tronToEthAddress(recipientAddress) + : recipientAddress; } else {Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/universal-swap/tests/helper.spec.ts (3)
396-441: Enhance error handling assertions in generateAddress test.The test case has good coverage but could benefit from more specific error assertions.
Consider modifying the error handling to verify the specific error type and message:
} catch (error) { - expect(willThrow).toEqual(false); + if (willThrow) { + expect(error).toBeInstanceOf(Error); + expect(error.message).toMatch(/invalid address/i); + } else { + throw error; + } }
Line range hint
636-647: Add test cases for additional edge scenarios.While the basic functionality is covered, consider adding test cases for:
- Invalid receiver addresses
- Maximum amount limits
- Contract address validation
Would you like me to help generate additional test cases for these scenarios?
Line range hint
666-723: LGTM! Consider adding negative test cases.The test data is comprehensive and well-structured, covering various route formats and combinations.
Consider adding negative test cases for:
- Malformed route strings
- Invalid channel formats
- Missing separators
Summary by CodeRabbit
New Features
@oraichain/oraidex-universal-swappackage to 1.1.15.UniversalSwapHelper.Bug Fixes
Chores