fix: refund unused tokens on revert #41
Conversation
📝 WalkthroughWalkthroughThe pull request introduces similar changes to two contracts: Changes
Sequence DiagramsequenceDiagram
participant Sender
participant Contract
participant AssetToken
Sender->>Contract: Initiate Cross-Chain Transaction
Contract->>Contract: Transaction Fails
alt Asset Recovery Conditions Met
Contract->>AssetToken: Transfer Asset Back to Sender
alt Transfer Fails
Contract->>Contract: Revert with TransferFailed Error
else Transfer Succeeds
AssetToken->>Sender: Asset Returned
end
end
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
Documentation and Community
|
![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
🧹 Nitpick comments (1)
contracts/nft/contracts/zetachain/UniversalNFTCore.sol (1)
267-272: LGTM! Consistent implementation with UniversalTokenCore.The implementation correctly handles gas token refunds during cross-chain NFT transfer failures, maintaining consistency with the token contract's behavior.
Consider extracting this common revert handling logic into a shared library to maintain consistency and reduce code duplication between NFT and Token contracts.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
contracts/nft/contracts/zetachain/UniversalNFTCore.sol(1 hunks)contracts/token/contracts/zetachain/UniversalTokenCore.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test (contracts/token)
- GitHub Check: test (contracts/nft)
- GitHub Check: slither (contracts/token, token.sarif)
- GitHub Check: slither (contracts/nft, nft.sarif)
🔇 Additional comments (1)
contracts/token/contracts/zetachain/UniversalTokenCore.sol (1)
252-257: LGTM! Important fix for token recovery.The added code properly handles the refund of unused tokens during cross-chain revert scenarios, preventing tokens from being locked in the contract.
Let's verify the token recovery implementation in other similar contracts:
✅ Verification successful
Token recovery implementation verified across contracts ✅
The same token recovery pattern is correctly implemented in both UniversalTokenCore and UniversalNFTCore contracts, ensuring consistent and secure handling of token refunds during cross-chain reverts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for similar token recovery patterns in other contracts # Look for contracts that might need similar fixes # Search for onRevert functions that might need token recovery ast-grep --pattern 'function onRevert(RevertContext calldata $_) { $$$ }' # Search for potential token transfers in revert scenarios rg -U "function onRevert.*?\{.*?transfer.*?\}" -A 5Length of output: 131
Script:
#!/bin/bash # Search for onRevert functions with a simpler pattern echo "=== Searching for onRevert functions ===" ast-grep --pattern 'function onRevert' echo -e "\n=== Searching for ZRC20 transfer calls ===" rg "IZRC20.*transfer" -B 2 -A 2 echo -e "\n=== Searching for RevertContext usage ===" rg "RevertContext" -B 2 -A 2Length of output: 4034
When a transfer fails, refund unused ZRC-20 tokens to the original sender on ZetaChain.
Summary by CodeRabbit