refactor: extend min gas limit check (develop)#3848
Conversation
📝 WalkthroughWalkthroughThe changes introduce a stricter minimum gas limit enforcement for contract calls in the EVM signer logic. The adjustment replaces a previous special-case check for a 21,000 gas limit with a more general rule, ensuring contract-related transactions have at least 100,000 gas. Corresponding tests were expanded to cover these new scenarios, and the changelog was updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GasSigner
participant CCTX
Client->>GasSigner: Request gas calculation (gasFromCCTX)
GasSigner->>CCTX: Inspect transaction type and call context
alt Coin is not native gas OR isWithdrawWithNoCall OR isRevertWithNoCall
alt Gas limit < contractCallMinGasLimit
GasSigner->>GasSigner: Set gas limit = contractCallMinGasLimit
else Gas limit >= contractCallMinGasLimit
GasSigner->>GasSigner: Use provided gas limit
end
else
GasSigner->>GasSigner: Use provided gas limit
end
GasSigner-->>Client: Return Gas struct with enforced minimum if applicable
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 @@
## develop #3848 +/- ##
===========================================
+ Coverage 64.40% 64.44% +0.03%
===========================================
Files 465 465
Lines 33716 33719 +3
===========================================
+ Hits 21714 21729 +15
+ Misses 10993 10982 -11
+ Partials 1009 1008 -1
🚀 New features to boost your workflow:
|
![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 (2)
zetaclient/chains/evm/signer/gas.go (1)
83-89: Logic extended to handle more cases appropriately.The enhanced condition properly handles different scenarios (coin type, cross-chain calls, and revert calls) to determine when to apply the minimum gas limit.
However, there appears to be a minor typo in the comment at line 87:
- // TODO: gas limit that is too low should now block outbound at all + // TODO: gas limit that is too low should not block outbound at allchangelog.md (1)
32-32: Appropriate changelog entry with typo in PR reference.The changelog entry accurately describes the purpose of the change, but contains a typo in the PR reference.
-* [3848g](https://github.com/zeta-chain/node/pull/3848) - extend min gas limit check to prevent intrinsic low gas limit +* [3848](https://github.com/zeta-chain/node/pull/3848) - extend min gas limit check to prevent intrinsic low gas limit
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)zetaclient/chains/evm/signer/gas.go(2 hunks)zetaclient/chains/evm/signer/gas_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/evm/signer/gas.gozetaclient/chains/evm/signer/gas_test.go
🧬 Code Graph Analysis (2)
zetaclient/chains/evm/signer/gas.go (3)
testutil/sample/crosschain.go (1)
InboundParams(176-191)e2e/contracts/gatewayzevmcaller/GatewayZEVMCaller.go (1)
CallOptions(33-36)pkg/contracts/sui/coin.go (1)
CoinType(4-4)
zetaclient/chains/evm/signer/gas_test.go (2)
testutil/sample/crosschain.go (3)
CrossChainTx(266-280)InboundParams(176-191)OutboundParams(209-227)zetaclient/chains/evm/signer/gas.go (1)
Gas(28-37)
🔇 Additional comments (8)
zetaclient/chains/evm/signer/gas.go (2)
17-18: Clear and descriptive constant for contract calls.The introduction of
contractCallMinGasLimitwith a value of 100,000 serves as an appropriate threshold to prevent intrinsic gas limit errors in contract interactions.
74-75: Good flag implementation for conditional logic.These boolean flags improve readability by clearly defining the conditions that determine when the minimum gas limit should be applied.
zetaclient/chains/evm/signer/gas_test.go (6)
26-75: Well-structured helper functions for comprehensive test coverage.These helper functions provide a clean way to generate different test scenarios for the various conditions in the gas limit adjustment logic.
83-94: Thorough test for ERC20 withdrawal gas limit adjustment.This test case correctly verifies that the gas limit is raised to the minimum threshold for ERC20 withdrawals when it falls below the required minimum.
95-106: Proper verification for gas withdrawals exemption.This test confirms that gas withdrawals (without cross-chain calls) correctly bypass the minimum gas limit requirement, allowing the transaction to proceed with its original limit.
107-118: Important coverage for gas withdrawals with cross-chain calls.This test validates that gas withdrawals with cross-chain calls have their gas limit properly adjusted to the minimum when necessary.
119-130: Appropriate exemption test for reverted gas withdrawals.The test confirms that simple gas withdrawal reverts (without call-on-revert) maintain their original gas limit.
131-142: Comprehensive test for reverted gas withdrawals with call-on-revert.This test ensures that gas withdrawals that are reverted with call-on-revert have their gas limit raised to the minimum threshold.
3 participants
Description
Integrate #3846
Summary by CodeRabbit