beacon-chain, config: get consensus values from beacon config #11798
Conversation
There was a problem hiding this comment.
I will need to think on this more, but the main reason it is a hardcoded value rather than something that can be configured at runtime is because presets are meant to be initialized at compile time. Since these presets determine how a state is hashed, for us currently it is only possible if there was a separate gnosis preset. Although I understand that this would prevent gnosis from running prysm via the default binaries/image. For context, you can take a look at this PR:
#10007
I understand, that this won't work with gnosis chain currently which requires it being changed at runtime. Will discuss with the team first on this change
|
That makes sense, I'll work on the gnosis preset then. |
There was a problem hiding this comment.
@gballet So we talked internally and while the change isn't great, we would be okay with this. Given that we are the only client that has this strict requirement, it seems that it has limited benefit to forcing this to be a compile time constant
| @@ -239,3 +239,15 @@ func configForkNames(b *BeaconChainConfig) map[[fieldparams.VersionLength]byte]s | |||
| fvn[bytesutil.ToBytes4(b.BellatrixForkVersion)] = "bellatrix" | |||
| return fvn | |||
| } | |||
|
|
|||
| func (b *BeaconChainConfig) Eth1DataVotesLength() uint64 { | |||
There was a problem hiding this comment.
Lets add a comment for each new method
|
@nisdas thanks for your feedback, I have added the required comments. Let me know if you would like other changes. |
gballet
force-pushed
the
parameters-from-chain-config
branch
from
f248d1a to
63126de
Compare
What type of PR is this?
Other
What does this PR do? Why is it needed?
This allows prysm to get some of its consensus parameters from the config file, instead of getting them from a hardcoded file. This is necessary to support e.g. the gnosis chain, that has a different slot time.
Another approach I considered was to store the hardcoded values for the gnosis chain inside a file in
config/fieldparams, protected by a tag. I think this PR is the better approach, however, as adding other chains would require adding a file each time, when the config file can be tweaked instead.