Skip to content

Conversation

@zZoMROT
Copy link
Contributor

@zZoMROT zZoMROT commented Jan 25, 2025

Static Code Analysis (readability, compactness):

The removal of the fixed gas limitation. The logic remains straightforward and functional while improving compatibility with L2 solutions such as zkSync, where gas requirements can be unpredictable.

Dynamic Code Analysis (external APIs, interaction flows):

The previous fixed gas limit of 5000 units posed challenges in zkSync environments, where certain operations require more gas than initially anticipated. Although the contracts compiled successfully, runtime failures occurred in production due to insufficient gas allocation. By removing the arbitrary gas cap, the contract can now better handle interactions with zkSync and other L2 solutions, ensuring more reliable and adaptable execution.

Efficiency (gas costs, computational complexity, memory requirements):

Allowing the call to use the full available gas improves the reliability of transactions by reducing the likelihood of failures caused by gas constraints. However, this change may lead to higher gas consumption if the recipient contract is inefficient. It is important to monitor the gas usage in production and optimize interactions where possible to avoid unnecessary costs.

Opinion, trade-offs and other thoughts (optional):

Opinion, trade-offs and other thoughts (optional):
The removal of the gas limit increases compatibility with zkSync and future-proofs the contract against evolving gas dynamics. However, it introduces a potential risk of reentrancy attacks if the called contract contains malicious or complex logic. Proper precautions, such as reentrancy guards and adherence to the checks-effects-interactions pattern, should be considered to mitigate these risks. The change aligns with the practical deployment experience and improves overall robustness.

@codecov
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0268e85) to head (ec56086).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #176   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          342       342           
  Branches        65        65           
=========================================
  Hits           342       342           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@galekseev galekseev merged commit 5bdc603 into master Jan 27, 2025
8 checks passed
@galekseev galekseev deleted the feature/no-gas-limit branch January 27, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants