search

NethermindEth / nethermind

A robust execution client for Ethereum node operators.
https://nethermind.io/nethermind-client
GNU General Public License v3.0
1.29k stars 449 forks source link

Add Labels to chainspec to activate hardforks at once. #7738

Open LukaszRozmej opened 3 weeks ago

LukaszRozmej commented 3 weeks ago

Right now to activate Dencun we need to add to chainspec:

    "eip4844TransitionTimestamp": "0x65687fd0",
    "eip4788TransitionTimestamp": "0x65687fd0",
    "eip1153TransitionTimestamp": "0x65687fd0",
    "eip5656TransitionTimestamp": "0x65687fd0",
    "eip6780TransitionTimestamp": "0x65687fd0"

We want to keep fine-grained eip transitions, but we would also like to specify it all at once with some label:

    "dencun": "0x65687fd0"

or 

    "cancun": "0x65687fd0"

It would be great if it worked both ways - so cancun flag could be checked, even if single eip timestamps would be set in config. If the activations conflict with each other some exception should be thrown.

It should work as that for all forks.

GradleD commented 3 weeks ago

Can I take care of this issue?

LukaszRozmej commented 3 weeks ago

Can I take care of this issue?

Sure, first approved PR wins! ;)

LuisDi98 commented 2 weeks ago

I understood the job can be simplified by adding the label with a timestamp, all good so creating a hashmap and grouping the eips is the solution, considering the exceptions if timestamps conflicts ✅ I would like to take this issue.

Also I am part of Dojo Coding community looking to contribute to new open source projects, it would be nice to work with Nethermind project.

LukaszRozmej commented 2 weeks ago

I understood the job can be simplified by adding the label with a timestamp, all good so creating a hashmap and grouping the eips is the solution, considering the exceptions if timestamps conflicts ✅ I would like to take this issue.

Also I am part of Dojo Coding community looking to contribute to new open source projects, it would be nice to work with Nethermind project.

So I think I would add a properties in ChainParameters class with custom getter's and setter's that would map to specific EIP's. (with setter set all of them, question is what to do with get? Options: check all of them with && or check any of them with ||)

The only problem is that this would only handle the actual domain logic class, and to parse JSON we would have to duplicate this logic in ChainSpecParamsJson as we have those 2 aspects separate.

mimisavage commented 6 hours ago

Can I handle this task?

LukaszRozmej commented 4 hours ago

Can I handle this task?

@LuisDi98 was already at least half done in https://github.com/NethermindEth/nethermind/pull/7764 , his latest changes though went in wrong direction. Not sure if he plans on finishing it. So feel free to build upon https://github.com/NethermindEth/nethermind/pull/7764/commits/2c3c96187036f1919b0bdb889d73728781ef2782 commit and read through the comments on the PR.