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

params: make eip1559 configurable #25822

Closed
wants to merge 1 commit into from

Conversation

protolambda
Copy link
Contributor

This makes the EIP-1559 parameters BASE_FEE_MAX_CHANGE_DENOMINATOR and ELASTICITY_MULTIPLIER configurable, and changes the globals to function as defaults.

As an alternative chain or testnet with different (often faster) block time it's desirable to maintain an adjustment rate and gas consumption target as mainnet, while also increasing the elasticity to still be able to process rare transactions as large as the gas limit on mainnet.

Let me know if the config naming should be adjusted, or if defaulting the individual fields, instead of eip1559 config as a whole, is more desirable.

@karalabe
Copy link
Member

Hmm, my comment didn't make it from email from some reason.

Yeah, so this we won't really accept. We have made the commitment a long time ago that Geth is an Ethereum client and will be only focusing on that, explicitly not implementing configurability for various chain tweaks. The primary reason is that we see it as a slippery slope, eventually leading to all params being configurable ala Parity's chain spec. That however can introduce a lot of potential for bugs when some combination is not compatible with another, or accidentally enabling/disabling things on mainnet that are relevant for something else only.

@fjl
Copy link
Contributor

fjl commented Sep 20, 2022

I think we should accept this!

@karalabe
Copy link
Member

karalabe commented Oct 11, 2022 via email

@holiman
Copy link
Contributor

holiman commented Oct 11, 2022

Hmm, my comment didn't make it from email from some reason.

Three weeks later...

Hey Proto,

@karalabe did the email got stuck for three weeks in the out-folder?

As for this PR, we're leaning towards merging it. It's in @fjl's hands to get it merged now

@karalabe
Copy link
Member

We discussed it with proto 2 days ago, he'll make an alternative

@karalabe
Copy link
Member

They don't need it to be configurable from the chain config, they just need it to not be the params.xyz. He said he'll make a PR that is enough for Optimism without having to surface the field all the way out. That would still allow them to have a clean diff without the drawbacks / slippery slope / special status.

@fjl
Copy link
Contributor

fjl commented Oct 12, 2022

OK then!

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.

5 participants