fix: disable callOnRevert in GatewayEVM call #407
Conversation
skosito
requested review from
andresaiello,
brewmaster012,
lumtis,
CharlieMc0,
fadeev and
fbac
as code owners
|
Warning Rate limit exceeded@skosito has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request include the addition of a new error declaration 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
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
==========================================
+ Coverage 84.23% 84.27% +0.04%
==========================================
Files 8 8
Lines 387 388 +1
Branches 122 123 +1
==========================================
+ Hits 326 327 +1
Misses 61 61 ☔ View full report in Codecov by Sentry. |
![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)
v2/contracts/Errors.sol (1)
8-8: LGTM! Consider adding a brief comment for the new error.The addition of the
CallOnRevertNotSupported()error aligns well with the PR objective to disable the callOnRevert feature. It follows the existing naming convention and structure within the interface.Consider adding a brief comment above the new error to explain its purpose, similar to how the interface itself is documented. This would improve code readability and maintainability. For example:
/// @notice Error thrown when callOnRevert is not supported error CallOnRevertNotSupported();v2/contracts/evm/GatewayEVM.sol (1)
344-344: Consider adding unit tests for the new error conditionAdding unit tests to verify that the function correctly reverts when
revertOptions.callOnRevertis true would enhance test coverage and ensure the new logic functions as expected.Would you like assistance in creating these unit tests or opening a GitHub issue to track this task?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- v2/contracts/Errors.sol (1 hunks)
- v2/contracts/evm/GatewayEVM.sol (1 hunks)
- v2/test/GatewayEVM.t.sol (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
v2/contracts/evm/GatewayEVM.sol (1)
344-344: Properly handling unsupportedcallOnRevertoptionThe added check correctly ensures that the function reverts with
CallOnRevertNotSupported()whenrevertOptions.callOnRevertis true, effectively disabling the unsupported feature as intended.v2/test/GatewayEVM.t.sol (3)
401-401: Addition ofINotSupportedMethodsinterface is appropriateThe inclusion of
INotSupportedMethodsinterface provides access to necessary error declarations used in the tests.
457-457: SettingcallOnReverttofalseinrevertOptionsProperly initializing
callOnReverttofalseensures that tests behave as expected when this feature is disabled.
681-687: Test correctly verifies rejection whencallOnRevertistrueThe test accurately ensures that calling with
callOnRevertset totruereverts with the expected error, enforcing thatcallOnRevertis not supported.
currently not supported on protocol side, so disabling here as well
Summary by CodeRabbit
New Features
callfunction in theGatewayEVMcontract for calling omnichain smart contracts without asset transfer.CallOnRevertNotSupported()to enhance error handling for unsupported method calls.Bug Fixes
deposit,depositAndCall,setCustody, andsetConnectorfunctions to prevent initialization errors.Tests