params: Config2#32224
Conversation
It's not great to call scheduleAtTime so many times. The function is much more expensive now since it has to establish the fork order every time. A later refactoring needs to turn it around to compute the order only once.
lightclient
left a comment
There was a problem hiding this comment.
This is looking really good, just catching up a bit on the changes. Saw a TODO for the TTD and BPOs for checking compatible - are there other major todo items?
|
|
||
| var ConfigParam = params.Define(params.T[Config]{ | ||
| Name: "clique", | ||
| Optional: true, // optional says |
There was a problem hiding this comment.
missing part of the comment here?
| param map[int]any | ||
| } | ||
|
|
||
| func NewConfig2(activations Activations, param ...ParamValue) *Config2 { |
| return ok | ||
| } | ||
|
|
||
| // SetActivations returns a new configuration with the given forks activated. |
There was a problem hiding this comment.
| // SetActivations returns a new configuration with the given forks activated. | |
| // SetActivations returns a new configuration with the given forks activations. |
I think it should say "activations" since the way it currently reads is that that it will make the new config have the passed in forks be active.
There was a problem hiding this comment.
This method is a little confusing to me in general though, it seems like it should set the forks for the current Config2 object (even overwriting the existing) but instead it returns a new object that has both the existing activations and the new activations.
| func (cfg *Config2) Rules(blockNum uint64, blockTime uint64) Rules2 { | ||
| r := Rules2{ | ||
| active: make(map[forks.Fork]bool, len(cfg.activation)), | ||
| } | ||
| return r | ||
| } |
There was a problem hiding this comment.
Seems like this still needs to fill in the active map?
~Will probably be mostly supplanted by #32224, but this should do for now for devnet 3.~ Seems like #32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
~Will probably be mostly supplanted by ethereum#32224, but this should do for now for devnet 3.~ Seems like ethereum#32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
~Will probably be mostly supplanted by ethereum#32224, but this should do for now for devnet 3.~ Seems like ethereum#32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This is a refactoring of the fork configuration system. The design goals for this change are as follows:
Forks
to make this simpler, so that ideally a new enum value has to be added and that's it.
Parameters
handled by a global registry, and can be implemented across different packages. For example, the EIP-1559 configuration can be implemented in the eip1559 package, where the config is ultimately used.
Config/Rules object
map[Fork]bool.