Add signed validator registrations type - #38
Conversation
|
And for reference, this PR allows SSZ support for the validator registration endpoint: Without this new type, there's no easy way to SSZ encode/decode these registrations. |
|
Hi @jtraglia, thanks for raising this. I took a look and think maybe it would be better to follow the pattern that BlobSidecars has, where there is an array type defined for multiple. I think this would require requesting changes to the PR you mentioned to create the new array type, e.g.
Looking at the BlobSidecars implementation I think it makes sense to use a more descriptive name, like Please let me know your thoughts, I'll add some comments to the code too. |
| ) | ||
|
|
||
| // SignedValidatorRegistrationList represents a list of SignedValidatorRegistration. | ||
| type SignedValidatorRegistrationList struct { |
There was a problem hiding this comment.
I think we should rename this to SignedValidatorRegistrations to be consistent with other naming.
|
|
||
| // Need to `go install github.com/ferranbt/fastssz/sszgen@latest` for this to work. | ||
| //go:generate rm -f signedvalidatorregistration_encoding.go validatorregistration_encoding.go | ||
| //go:generate rm -f signedvalidatorregistrationlist_encoding.go signedvalidatorregistration_encoding.go validatorregistration_encoding.go |
There was a problem hiding this comment.
We should rename the file to signedvalidatorregistrations and similar for the other files to be consistent with other naming conventions.
|
Hey @Bez625, thanks for the review. I agree with the feedback. Will make these changes soon! |
jtraglia
changed the title
|
Alright @Bez625, should be ready now. Thanks again! |
|
Thanks @jtraglia. Have you started a discussion about adding the array return type to Also I noticed this PR is to merge in to the |
Sort of. The PR introduces a new And it's up to you. I would like to preempt that PR so that support can be included in the upcoming mev-boost release. I will note that we've recently started forwarding validator registrations without decoding them which works with SSZ. So it's not an urgent need, but it would allow us to add proper tests.
Oops, I didn't realize that electra had been merged into master. No problem, fixed. |
2 participants
To allow SSZ support in mev-boost, I believe we need a new
SignedValidatorRegistrationstype.The max size is
VALIDATOR_REGISTRY_LIMIT:https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#state-list-lengths