Swap: update to handle reverts #512
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request introduces significant enhancements to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
|
Related: zeta-chain/example-contracts#219 |
fadeev
requested review from
bbbeeeee,
brewmaster012 and
a team
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/pages/developers/tutorials/swap-any.mdx (2)
71-72: Consider enhancing error messages with contextThe error types could be more informative by including relevant details about the failed operation.
- error ApprovalFailed(); - error TransferFailed(); + error ApprovalFailed(address token, address spender, uint256 amount); + error TransferFailed(address token, address to, uint256 amount);
251-273: Enhance onRevert with better error handling and eventsAdd event emission and additional error handling for failed revert scenarios.
+ event RevertHandled( + address indexed sender, + address indexed originalAsset, + address indexed targetToken, + uint256 amount, + uint256 outputAmount + ); function onRevert(RevertContext calldata context) external onlyGateway { (address sender, address zrc20) = abi.decode( context.revertMessage, (address, address) ); + require(context.amount > 0, "Zero amount in revert"); + require(zrc20 != address(0), "Invalid target token"); (uint256 out, , ) = handleGasAndSwap( context.asset, context.amount, zrc20 ); + emit RevertHandled(sender, context.asset, zrc20, context.amount, out); // ... rest of the function }src/pages/developers/tutorials/swap.mdx (2)
184-201: Consider refactoring approval checks to reduce duplication.The approval checks contain duplicated logic that could be extracted into a helper function.
Consider refactoring like this:
+ function approveToken(address token, address spender, uint256 amount) internal returns (bool) { + if (!IZRC20(token).approve(spender, amount)) { + revert ApprovalFailed(); + } + return true; + } function withdraw(...) public { if (gasZRC20 == params.target) { - if ( - !IZRC20(gasZRC20).approve( - address(gateway), - outputAmount + gasFee - ) - ) { - revert ApprovalFailed(); - } + approveToken(gasZRC20, address(gateway), outputAmount + gasFee); } else { - if (!IZRC20(gasZRC20).approve(address(gateway), gasFee)) { - revert ApprovalFailed(); - } - if ( - !IZRC20(params.target).approve(address(gateway), outputAmount) - ) { - revert ApprovalFailed(); - } + approveToken(gasZRC20, address(gateway), gasFee); + approveToken(params.target, address(gateway), outputAmount); }
207-213: Consider adding a constant for the empty address in RevertOptions.The empty address (address(0)) is used in RevertOptions. Consider defining it as a constant for better maintainability.
+ address constant EMPTY_ADDRESS = address(0); RevertOptions({ revertAddress: address(this), callOnRevert: true, - abortAddress: address(0), + abortAddress: EMPTY_ADDRESS, revertMessage: abi.encode(sender, inputToken), onRevertGasLimit: gasLimit })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/pages/developers/tutorials/swap-any.mdx(2 hunks)src/pages/developers/tutorials/swap.mdx(3 hunks)
🔇 Additional comments (4)
src/pages/developers/tutorials/swap.mdx (4)
84-84: LGTM: Well-structured state variable and error definition additions.
The new gasLimit immutable variable and ApprovalFailed error enhance the contract's gas management and error handling capabilities.
Also applies to: 88-88
95-104: LGTM: Constructor properly handles initialization and validation.
The constructor correctly validates input addresses and initializes the immutable variables, including the new gasLimit.
133-138: LGTM: Improved function organization with clear separation of concerns.
The refactoring to use handleGasAndSwap enhances code maintainability by separating the swap logic from the withdrawal process.
217-238: LGTM: Well-implemented revert handling with proper gas management.
The onRevert function correctly handles revert scenarios by:
- Properly decoding the revert context
- Utilizing handleGasAndSwap for token management
- Applying the configured gas limit
Summary by CodeRabbit
New Features
SwapToAnyTokencontract to allow swapping tokens to any target token with options for withdrawal.swapfunction for direct interactions on ZetaChain, improving user flexibility.Bug Fixes
Documentation