[WIP] add builder spec validator registration structs to the v1 package #19
Conversation
ciaranmcveigh5
changed the title
api/v1/validatorregistration.go
Outdated
| type ValidatorRegistration struct { | ||
| FeeRecipient bellatrix.ExecutionAddress | ||
| GasLimit uint64 | ||
| Timestamp uint64 |
There was a problem hiding this comment.
This should be a time.Time (as used in genesis.go).
| Version spec.DataVersion | ||
| Bellatrix *apiv1.SignedValidatorRegistration |
There was a problem hiding this comment.
This object isn't under the consensus specs versioning (this is stated explicitly in https://github.com/ethereum/builder-specs/blob/main/specs/builder.md#independently-versioned) so should use its own versioning (perhaps spec.BuilderVersion)
| }, | ||
| { | ||
| name: "TimestampInvalid", | ||
| input: []byte(`{"fee_recipient":"0x000102030405060708090a0b0c0d0e0f10111213","gas_limit":"100","timestamp":"invalid","pubkey":"0x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f"}`), |
There was a problem hiding this comment.
time.unix() is of type int64 not sure how you want to test for invalid data - for now i've just used "invalid"
|
Thanks for reviewing have made the requested updates, tests are passing - let me know if the builderVersion implementation isn't in line with what you were thinking |
| @@ -24,7 +25,7 @@ import ( | |||
|
|
|||
| // VersionedValidatorRegistration contains a versioned ValidatorRegistrationV1. | |||
| type VersionedValidatorRegistration struct { | |||
| Version spec.DataVersion | |||
| Version spec.BuilderVersion | |||
| Bellatrix *apiv1.ValidatorRegistration | |||
There was a problem hiding this comment.
This field should map to the builder version, so be called V1 rather than Bellatrix.
spec/builderversion.go
Outdated
|
|
||
| // MarshalJSON implements json.Marshaler. | ||
| func (d *BuilderVersion) MarshalJSON() ([]byte, error) { | ||
| return []byte(fmt.Sprintf("%q", responseVersionStrings[*d])), nil |
There was a problem hiding this comment.
Should be responseBuilderVersionStrings
spec/builderversion.go
Outdated
|
|
||
| // String returns a string representation of the | ||
| func (d BuilderVersion) String() string { | ||
| if int(d) >= len(responseVersionStrings) { |
There was a problem hiding this comment.
Should be responseBuilderVersionStrings
spec/builderversion.go
Outdated
| if int(d) >= len(responseVersionStrings) { | ||
| return "unknown" | ||
| } | ||
| return responseVersionStrings[d] |
There was a problem hiding this comment.
Should be responseBuilderVersionStrings
|
I have made some changes and successfully tested against Teku on Sepolia, please take a look at ciaranmcveigh5#1 if you can. Hopefully this can get merged to this repo eventually. |
(builder-specs) Encoding, bulk submission & tests
|
Need to address the above items where |
|
hey @mcdee I believe the above items where responseVersionStrings should be responseBuilderVersionStrings have been addressed in @moshe-blox commits currently builderversion.go has and dataversion.go has see file changes on spec/builderversion.go in this commit 0b866da#diff-c592b40af9139e1bbd99a5df3376d54b5a83bd0bc6b29525bfadacb5ea96a8cd Thanks for the reviews |
Jim has merged in the builder registration PR attestantio/go-eth2-client#19 so we are unblocked on that flow now - Add DutyBuilderRegistration - Update tests to include the registration duty category: feature ticket: #849
Resolves Issues
#18
Description
Adding builder api validator registration to the go-eth2-client