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

Update geth.toml #139

Closed
wants to merge 2 commits into from
Closed

Conversation

thetroyharris
Copy link

Changing to match changes made by the geth team see: https://github.com/ethereum/go-ethereum/pull/29895/files

@Wolmin
Copy link
Contributor

Wolmin commented Jul 18, 2024

Hey @thetroyharris @JordyDutch - thank you for providing us this PR - to make sure that we are providing the values that are the same as default, we will need to first bump the versions across our services (LUKSO CLI, Docker repositories) - after we've done that, we will merge this PR as it is

@mxmar
Copy link
Contributor

mxmar commented Jul 18, 2024

Related: ethereum/go-ethereum@2613523

@JordyDutch
Copy link
Contributor

Related: ethereum/go-ethereum@2613523

@mxmar What if we just remove all GasPrice flags to always use the defaults? We also don't use it for Erigon client

@mxmar
Copy link
Contributor

mxmar commented Jul 23, 2024

Related: ethereum/go-ethereum@2613523

@mxmar What if we just remove all GasPrice flags to always use the defaults? We also don't use it for Erigon client

Thanks for your input @JordyDutch.

I was thinking the same, but I see that some 3rd parties rely on our network-configs, so if they don't see the flag, they won't make them configurable (Dappnode made gasprice configurable at my request). I'd prefer to leave the flags and just set them to the default value. This means that some 3rd parties (or devs) will make it configurable by default or will start speaking to the network team about it (because they see the flag in network-configs).

@JordyDutch
Copy link
Contributor

Related: ethereum/go-ethereum@2613523

@mxmar What if we just remove all GasPrice flags to always use the defaults? We also don't use it for Erigon client

Thanks for your input @JordyDutch.

I was thinking the same, but I see that some 3rd parties rely on our network-configs, so if they don't see the flag, they won't make them configurable (Dappnode made gasprice configurable at my request). I'd prefer to leave the flags and just set them to the default value. This means that some 3rd parties (or devs) will make it configurable by default or will start speaking to the network team about it (because they see the flag in network-configs).

Got it @mxmar, sounds good. But in this case it would also be good to add the flags and set to default for Erigon and Nethermind?

Because now we have 2 clients with and 2 clients without the flags.

@mxmar
Copy link
Contributor

mxmar commented Jul 23, 2024

Related: ethereum/go-ethereum@2613523

@mxmar What if we just remove all GasPrice flags to always use the defaults? We also don't use it for Erigon client

Thanks for your input @JordyDutch.
I was thinking the same, but I see that some 3rd parties rely on our network-configs, so if they don't see the flag, they won't make them configurable (Dappnode made gasprice configurable at my request). I'd prefer to leave the flags and just set them to the default value. This means that some 3rd parties (or devs) will make it configurable by default or will start speaking to the network team about it (because they see the flag in network-configs).

Got it @mxmar, sounds good. But in this case it would also be good to add the flags and set to default for Erigon and Nethermind?

Because now we have 2 clients with and 2 clients without the flags.

You are right, Nethermind is missing the default flag. @Wolmin can add in the next PR or you can add here. Erigon is still missing: erigontech/erigon#9945

@mxmar
Copy link
Contributor

mxmar commented Oct 3, 2024

@thetroyharris Please solve conflicts and we can merge this PR.

@mxmar
Copy link
Contributor

mxmar commented Oct 4, 2024

@thetroyharris @JordyDutch I'm closing this, because #143 PR has been merged ✔️

@mxmar mxmar closed this Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants