lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Consolidate non-configurable constants from chainspec

Open pawanjay176 opened this issue 1 year ago • 1 comments
trafficstars

Issue Addressed

Resolves #5631

Proposed Changes

We currently have a lot of spec constants listed under lighthouse's ChainSpec struct. Values in chainspec are meant to be configurable from a given config file. Hence, there is no point in having spec defined constants under chainspec.

This PR moves all values defined as constant in the spec to types/consts.rs. This is more consistent imo and also reduces the need to pass a ChainSpec in many places across the code. Most of the diff is renames from spec.x to X and importing the required constants.

Additional Info

This specific test is a confusing case. https://github.com/sigp/lighthouse/blob/70bcba1e6b7a7123b2f48e1c77b9afb9bbb11c26/consensus/types/src/beacon_state/iter.rs#L118

We are setting a custom value for genesis_slot which is defined as a constant in the spec I don't think this is required behaviour from lighthouse, not completely sure though.

Happy to delay this until electra and peerdas branches are merged.

pawanjay176 avatar Jul 01 '24 09:07 pawanjay176

Conflicts 🙌

dapplion avatar Jul 03 '24 08:07 dapplion

Closing this as the advantages don't seem that great and we'll loose some nice testing setups

pawanjay176 avatar Sep 03 '24 18:09 pawanjay176