Closed m-Peter closed 1 month ago
The pull request modifies the blockGasLimit
constant in the api/encode_transaction.go
file, increasing its value from 15,000,000
to 120,000,000
to support larger transactions. Additionally, enhancements were made to the test cases in tests/web3js/eth_non_interactive_test.js
to validate block retrieval and fee history, including assertions related to the new gas limit.
File | Change Summary |
---|---|
api/encode_transaction.go | Updated blockGasLimit from 15_000_000 to 120_000_000 to accommodate larger transactions. |
tests/web3js/eth_non_interactive_test.js | Enhanced test cases for block retrieval and fee history, including assertions for gasLimit . |
Objective | Addressed | Explanation |
---|---|---|
Gas Limit should be increased to accommodate higher gas usage (558) | ✅ |
tests/web3js/eth_non_interactive_test.js
indicating potential connections in transaction handling tests.api/api.go
, which may relate to transaction processing and limits, similar to adjustments made in this PR.Improvement
🐰 In the meadow where transactions bloom,
A gas limit raised, dispelling the gloom.
With tests enhanced, we hop with delight,
Validating blocks, our future is bright!
So here's to the changes, let’s cheer and play,
For a smoother tomorrow, hip-hip-hooray! 🎉
api/encode_transaction.go (1)
`12-12`: **Verify the rationale behind setting the block gas limit to `120M`.** The linked issue #558 suggests increasing the `gasLimit` to the maximum cap of `30M`, given that the `gasUsed` exceeded `15M` of the limit. However, the updated constant value of `120M` is significantly higher than the suggested limit. Please consider the following: - Provide a clear justification for setting the block gas limit to `120M` and ensure it aligns with the system's requirements and constraints. - Consider using a more conservative limit, such as the `30M` mentioned in the linked issue, unless there is a compelling reason for the higher value. - Conduct thorough testing and validation to assess the impact of the increased gas limit on the system's performance, security, and resource utilization.tests/web3js/eth_non_interactive_test.js (2)
`31-31`: **LGTM!** The assertion correctly verifies that the `block.gasLimit` matches the updated value of `120000000n`, which aligns with the PR objective of increasing the block gas limit to 120M. --- `383-383`: **Verify the `gasUsedRatio` value.** Please ensure that the updated `gasUsedRatio` value of `[0, 0.006205458333333334]` is correct and aligns with the expected behavior considering the increased block gas limit. Provide additional context or justification for the change if necessary.
Closes: https://github.com/onflow/flow-evm-gateway/issues/558
Description
Update the constant's value to
120M
, as that is the theoretical block gas limit.For contributor use:
master
branchFiles changed
in the Github PR explorerSummary by CodeRabbit
New Features
blockGasLimit
to support larger transactions.Bug Fixes