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

Add EIP: Add Controlled Gas Limit Increase Strategy #8933

Merged
merged 39 commits into from
Oct 16, 2024

Conversation

Giulio2002
Copy link
Contributor

@Giulio2002 Giulio2002 commented Oct 6, 2024

Adds the possibility to have a gradual increase in gas target over time:

  1. This EIP does not require an hard fork so in case of a DDOS vector, a fixed gas target can still be set
  2. It gives time to analyze network behaviour and bandwidth usage as the gas target increases, instead of setting the increasing the gas target in a cliff-like manner.
  3. It is less prone to cause issue when raising the gas target due to it being a gradual change

@Giulio2002 Giulio2002 requested a review from eth-bot as a code owner October 6, 2024 19:47
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-core labels Oct 6, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Oct 6, 2024

✅ All reviewers have approved.

@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Oct 6, 2024
@eth-bot eth-bot changed the title Add EIP7779: Controlled Gas Limit Increase Add EIP: Controlled Gas Limit Increase Oct 6, 2024
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Oct 6, 2024
@github-actions github-actions bot added w-ci Waiting on CI to pass and removed w-ci Waiting on CI to pass labels Oct 6, 2024
Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

Some comments.

In general I am not in favor of this approach. Because we currently have the freedom to let miners change the gas limit, this means that without a hardfork we can decide as community to scale up the gas limit. In case that a problem is found (for instance, a new DoS vector on blocks, like the Shanghai attacks https://ethos.dev/shanghai-attacks ) we can signal to miners that the gas limit should be scaled down (which can be done without a fork).

However, this EIP will remove this flexibility. If a problem is found and we need to decrease the gas limit as fast as possible, this means we have to fork, which will take a substantial amount of time.

As a general suggestion for this EIP: I would suggest linear increase of the gas limit, instead of an exponential increase. The exponential variant could lead to some rounding errors when rounding down to the nearest integer, which could lead to consensus bugs. The linear increase (constant gas limit bump per block) is also slightly easier to code.

EIPS/eip-7779.md Outdated Show resolved Hide resolved
EIPS/eip-7779.md Outdated Show resolved Hide resolved
EIPS/eip-7779.md Outdated Show resolved Hide resolved
EIPS/eip-7779.md Outdated Show resolved Hide resolved
@paulmillr
Copy link
Contributor

validators are already voicing their concern with increased bandwidth requirements. Increasing gas to 60M will make the situation worse.

Also, why would this be necessary with blob-based roadmap? Gas prices have been low since the last hardfork.

#8931 seems like a better solution.

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Oct 6, 2024
@Giulio2002
Copy link
Contributor Author

Giulio2002 commented Oct 6, 2024

validators are already voicing their concern with increased bandwidth requirements. Increasing gas to 60M will make the situation worse.

Also, why would this be necessary with blob-based roadmap? Gas prices have been low since the last hardfork.

#8931 seems like a better solution.

#8931 does not do anything for bandwidth. there is an ongoing conversation about this. also this is a gradual increase so the gas limit is unlikely to reach 60mn - maybe 40-45mn.

@Giulio2002
Copy link
Contributor Author

Some comments.

In general I am not in favor of this approach. Because we currently have the freedom to let miners change the gas limit, this means that without a hardfork we can decide as community to scale up the gas limit. In case that a problem is found (for instance, a new DoS vector on blocks, like the Shanghai attacks https://ethos.dev/shanghai-attacks ) we can signal to miners that the gas limit should be scaled down (which can be done without a fork).

However, this EIP will remove this flexibility. If a problem is found and we need to decrease the gas limit as fast as possible, this means we have to fork, which will take a substantial amount of time.

As a general suggestion for this EIP: I would suggest linear increase of the gas limit, instead of an exponential increase. The exponential variant could lead to some rounding errors when rounding down to the nearest integer, which could lead to consensus bugs. The linear increase (constant gas limit bump per block) is also slightly easier to code.

Can you re-comment on the updated version?

@eth-bot eth-bot changed the title Add EIP: Controlled Gas Limit Increase Add EIP: Add Controlled Gas Target Increase Strategy Oct 6, 2024
EIPS/eip-7779.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the t-core label Oct 6, 2024
@Giulio2002
Copy link
Contributor Author

Then I think you need to take into account what I mention about the cap and possible invalid blocks from honest nodes.

So actually - the CL only sends it to the builder API and not to the engine api so this does not affect the EIP

g11tech
g11tech previously approved these changes Oct 10, 2024
EIPS/eip-7783.md Outdated Show resolved Hide resolved
Co-authored-by: g11tech <develop@g11tech.io>
@Giulio2002
Copy link
Contributor Author

one job got cancelled :(

g11tech
g11tech previously approved these changes Oct 10, 2024
@g11tech
Copy link
Contributor

g11tech commented Oct 10, 2024

i think it requires an approval from @SamWilsn or @lightclient to merge

Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

Some comments and suggestions

EIPS/eip-7783.md Outdated Show resolved Hide resolved
EIPS/eip-7783.md Outdated Show resolved Hide resolved
EIPS/eip-7783.md Outdated Show resolved Hide resolved
EIPS/eip-7783.md Outdated Show resolved Hide resolved
EIPS/eip-7783.md Outdated Show resolved Hide resolved
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
Giulio2002 and others added 4 commits October 14, 2024 16:11
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

LGTM!

@g11tech g11tech closed this Oct 16, 2024
@g11tech g11tech reopened this Oct 16, 2024
@eth-bot eth-bot enabled auto-merge (squash) October 16, 2024 14:35
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 56d4bcc into ethereum:master Oct 16, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-informational
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants