Fix/to adress tron #342
Fix/to adress tron #342
Conversation
WalkthroughThe pull request includes the deletion of a GitHub Actions workflow for publishing the Oraiswap v3 package, updates to the version numbers in the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
|
Code Coverage SummaryDiff against mainResults for commit: 665c312 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/universal-swap/src/handler.ts (2)
1153-1166: LGTM. Consider additional validation and error handling.The changes to the
processUniversalSwapmethod improve the handling of recipient addresses. The new validation check and the use ofChainIdEnum.TRONare good additions. However, consider the following suggestions:
- Add a null check for
recipientAddressbefore using it in the validation.- Consider using a more descriptive error message in the
generateErrorcall.- The ternary operator for setting
toAddresscould be made more readable by using an if-else statement.Consider refactoring the
toAddressassignment for better readability:- toAddress = - originalToToken.chainId === ChainIdEnum.TRON - ? tronToEthAddress(recipientAddress) - : this.swapData.recipientAddress; + if (originalToToken.chainId === ChainIdEnum.TRON) { + toAddress = tronToEthAddress(recipientAddress); + } else { + toAddress = recipientAddress; + }
234-234: LGTM. Consider additional error handling.The changes to the
getIbcMemomethod improve the handling of recipient addresses. The new validation check and the use ofChainIdEnum.TRONare good additions. However, consider the following suggestion:
- Add error handling for the case when
tronToEthAddressfails to convert the address.Consider adding a try-catch block for the
tronToEthAddressconversion:- transferAddress = toToken.chainId === ChainIdEnum.TRON ? tronToEthAddress(recipientAddress) : recipientAddress; + if (toToken.chainId === ChainIdEnum.TRON) { + try { + transferAddress = tronToEthAddress(recipientAddress); + } catch (error) { + throw generateError(`Failed to convert Tron address: ${error.message}`); + } + } else { + transferAddress = recipientAddress; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- .github/workflows/publish_oraiswap_v3.yml (0 hunks)
- packages/oraiswap-v3/package.json (2 hunks)
- packages/universal-swap/package.json (1 hunks)
- packages/universal-swap/src/handler.ts (3 hunks)
💤 Files not reviewed due to no reviewable changes (1)
- .github/workflows/publish_oraiswap_v3.yml
✅ Files skipped from review due to trivial changes (1)
- packages/universal-swap/package.json
🔇 Additional comments (4)
packages/oraiswap-v3/package.json (2)
16-16: Dependency update approved, but more context needed.The update of
@oraichain/oraidex-commonfrom ^1.1.6 to ^1.1.21 is approved. However, could you provide more information on how this update relates to the PR objective of "Fix/to adress tron"? Understanding the connection between this dependency update and the Tron-related changes would be helpful.
3-3: Please clarify the significant version jump.The package version has been updated from 0.1.8 to 0.1.20, which is a significant jump of 12 minor versions. Could you provide more context about the changes that warrant this version increase? It's unusual to see such a large version jump without corresponding changes in the package.
packages/universal-swap/src/handler.ts (2)
43-44: LGTM. Verify usage of new imports.The addition of
toDisplayandChainIdEnumimports from "@oraichain/oraidex-common" looks good. Ensure these new imports are used appropriately in the updated code.✅ Verification successful
Please run the updated shell script to verify the usage of the new imports.
Verified usage of new imports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the usage of new imports rg --type typescript 'toDisplay|ChainIdEnum' packages/universal-swap/src/handler.tsLength of output: 120
Script:
#!/bin/bash # Verify the usage of new imports with correct file type rg --type ts 'toDisplay|ChainIdEnum' packages/universal-swap/src/handler.tsLength of output: 327
Line range hint
1-1568: Overall improvements in address handling. Tests needed.The changes in this file primarily focus on improving recipient address handling and validation, which is a positive step towards more robust code. The use of
ChainIdEnumfor type-safe chain ID checks is also a good practice.However, there are a few areas that could use attention:
The TODO comments throughout the file indicate that several methods still need test cases. It's crucial to implement these tests to ensure the reliability of the code, especially given the complexity of the swap operations.
Consider implementing the suggested improvements in error handling and readability from the previous comments.
Given the complexity of this file, it might be beneficial to consider breaking it down into smaller, more manageable modules in the future.
As the
UniversalSwapHandlerclass grows in complexity, consider refactoring it into smaller, more focused classes or modules. This could improve maintainability and make it easier to test individual components.
Summary by CodeRabbit
New Features
Updates
@oraichain/oraiswap-v3from0.1.8to0.1.20.@oraichain/oraidex-commonto version^1.1.21.@oraichain/oraidex-universal-swapfrom1.1.10to1.1.11.Chores