attestantio / go-eth2-client

Apache License 2.0
102 stars 59 forks source link

Start to add support for Electra #131

Closed jtraglia closed 2 months ago

jtraglia commented 2 months ago

This PR makes the necessary changes (adds new structures and updates versioned structures) for Electra. Sorry, it contains a lot of new files and it lacks tests. I would like to pull in these changes into an electra branch, so that multiple people (myself included) can add tests later, in smaller PRs. After this PR gets merged, I'll make the equivalent PR for go-builder-client. We need to wait because go-eth2-client is a dependency and it currently needs a replace directive.

jtraglia commented 2 months ago

I added two nolint:gocyclo comments for these:

spec/versionedsignedbeaconblock.go:272:1: cyclomatic complexity 31 of func `(*VersionedSignedBeaconBlock).Attestations` is high (> 30) (gocyclo)
func (v *VersionedSignedBeaconBlock) Attestations() ([]VersionedAttestation, error) {
^
spec/electra/beaconblockbody_json.go:71:1: cyclomatic complexity 31 of func `(*BeaconBlockBody).UnmarshalJSON` is high (> 30) (gocyclo)
func (b *BeaconBlockBody) UnmarshalJSON(input []byte) error {
^

Not really sure how I would make these less complex.

mcdee commented 2 months ago

The Electra changes themselves look good.

There are a fair number of non-Electra changes in here. Would it be possible to remove these from this PR, to avoid additional pain when we come to merge this branch back into master? Anything non-cosmetic can be pushed to a separate PR to be included directly into master. Thanks.

jtraglia commented 2 months ago

Yes, good suggestion. I'll make those changes first thing tomorrow morning.

jtraglia commented 2 months ago

Yes, good suggestion. I'll make those changes first thing tomorrow morning.

I've undone the non-electra changes except for the new Epoch marshaling functions in spec/phase0/types.go.

I made these PRs to master: