Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

increase mainnet & Sepolia gas limit to 36M #8249

Merged

Conversation

daniellehrner
Copy link
Contributor

PR description

Sets the mainnet gas limit to 36M. Sets the gas limit for Sepolia, Holesky and Ephemery to DEFAULT_TARGET_GAS_LIMIT_TESTNET. The idea is to have a separate constant for tesnets in order to increase the gas limt there first before we do it on mainnet.

This PR also removes GasLimitCalculator from BesuControllerBuilder, because BesuController does not use it. It seems it was removed at some point, but was still present as dead code in BesuControllerBuilder.

Fixed Issue(s)

fixes #8245

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

… from BesuControllerBuilder

Signed-off-by: Daniel Lehrner <[email protected]>
@Gabriel-Trintinalia
Copy link
Contributor

The system call gas limit is 30M regardless of the block gas limit

@daniellehrner daniellehrner changed the title increase mainnet % Sepolia gas limit to 36M increase mainnet & Sepolia gas limit to 36M Feb 6, 2025
@@ -18,12 +18,12 @@
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"prevRandao": "0x0000000000000000000000000000000000000000000000000000000000000000",
"blockNumber": "0x1",
"gasLimit": "0x1c9c380",
"gasLimit": "0x1ca35ef",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the gasLimit has changed because the new limit is 36M: We start with a block of 30M and increase gas limit for the next block by 30M / 1024.

Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one place we might want to default to testnet default gas

@@ -79,7 +79,7 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
*/
private static final double TRY_FILL_BLOCK = 1.0;

private static final long DEFAULT_TARGET_GAS_LIMIT = 30000000L;
private static final long DEFAULT_TARGET_GAS_LIMIT = 36_000_000L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not also want to use the testnet check and different defaults here and here?

* href="https://eips.ethereum.org/EIPS/eip-2935#block-processing">EIP-2935</a> This value is
* independent of the gas limit of the block
*/
private static final long SYSTEM_CALL_GAS_LIMIT = 30_000_000L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if anything triggers an issue in pectra, I bet it would be some client using the block limit for system call gas limit

@fab-10
Copy link
Contributor

fab-10 commented Feb 11, 2025

Please add a CHANGELOG entry

Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changelog entry

@macfarla
Copy link
Contributor

@daniellehrner is this one ready to merge?

@daniellehrner
Copy link
Contributor Author

@daniellehrner is this one ready to merge?

Yes, I think it is. I am just need to get the CI to pass

@daniellehrner daniellehrner enabled auto-merge (squash) February 18, 2025 13:32
Signed-off-by: Daniel Lehrner <[email protected]>
@daniellehrner daniellehrner merged commit 18e3917 into hyperledger:main Feb 18, 2025
43 checks passed
@daniellehrner daniellehrner deleted the feat/issue-8245/gas_limit_36m branch February 18, 2025 14:16
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.

Increase gas limit for Mainnet and Sepolia to 36M
5 participants