Skip to content

params: make eip1559 configurable#25822

Closed
protolambda wants to merge 1 commit intoethereum:masterfrom
protolambda:eip1559-config
Closed

params: make eip1559 configurable#25822
protolambda wants to merge 1 commit intoethereum:masterfrom
protolambda:eip1559-config

Conversation

@protolambda
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

fjl commented Sep 20, 2022

I think we should accept this!

@karalabe
Copy link
Copy Markdown
Member

karalabe commented Oct 11, 2022 via email

@holiman
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@karalabe
Copy link
Copy Markdown
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
Copy Markdown
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