-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
eth2util: clone go-eth2-client data version #2462
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2462 +/- ##
==========================================
- Coverage 53.70% 53.57% -0.13%
==========================================
Files 198 198
Lines 26506 26619 +113
==========================================
+ Hits 14234 14261 +27
- Misses 10510 10582 +72
- Partials 1762 1776 +14
☔ View full report in Codecov by Sentry. |
core/ssz.go
Outdated
@@ -42,7 +43,7 @@ func (b VersionedSignedBeaconBlock) MarshalSSZ() ([]byte, error) { | |||
|
|||
// MarshalSSZTo ssz marshals the VersionedSignedBeaconBlock object to a target array. | |||
func (b VersionedSignedBeaconBlock) MarshalSSZTo(buf []byte) ([]byte, error) { | |||
return marshalSSZVersionedTo(buf, b.Version, b.sszValFromVersion) | |||
return marshalSSZVersionedTo(buf, eth2util.DataVersion(b.Version), b.sszValFromVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that casting isn't enough, we need to ensure the numbers we are working with are aligned with pre-v0.18.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has been refactored to convert instead of cast.
return b, nil | ||
} | ||
|
||
// UnmarshalJSON unmarshals the DataVersion from strings or a number equaled to the go-eth2-client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// UnmarshalJSON unmarshals the DataVersion from strings or a number equaled to the go-eth2-client | |
// UnmarshalJSON unmarshals the DataVersion from string or a number equaled to the go-eth2-client |
} | ||
|
||
// ToETH2 returns a eth2spec.DataVersion equivalent to the DataVersion. | ||
func (v DataVersion) ToETH2() eth2spec.DataVersion { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method will return different values once we update go-eth2-client to v0.18, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup
Clones the go-eth2-client
github.com/attestantio/go-eth2-client/spec.DataVersion
type aligning with pre-v0.18 values.This is required since we serialise these version numbers over the wire so they MUST remain constant to ensure backwards compatibility between charon versions. go-eth2-client changed the
DataVersion
numbers in the recent v0.18.0 upgrade which we had to roll-back.This change allows the following:
The migration will look something like:
category: refactor
ticket: #2433