-
Notifications
You must be signed in to change notification settings - Fork 18
feat(spec-tests): typing & ssz infrastructure #416
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
base: spec-tests
Are you sure you want to change the base?
Conversation
This pull request has merge conflicts. Could you please resolve them @Zacholme7? 🙏 |
This pull request has merge conflicts. Could you please resolve them @Zacholme7? 🙏 |
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.
Nice! A few remarks, but otherwise I think this should go in soon to get a lot of testing in this release cycle.
impl TryFrom<u64> for PartialSignatureKind { | ||
type Error = (); | ||
|
||
fn try_from(value: u64) -> Result<Self, Self::Error> { | ||
impl From<u64> for PartialSignatureKind { | ||
fn from(value: u64) -> Self { | ||
match value { | ||
0 => Ok(PartialSignatureKind::PostConsensus), | ||
1 => Ok(PartialSignatureKind::RandaoPartialSig), | ||
2 => Ok(PartialSignatureKind::SelectionProofPartialSig), | ||
3 => Ok(PartialSignatureKind::ContributionProofs), | ||
4 => Ok(PartialSignatureKind::ValidatorRegistration), | ||
5 => Ok(PartialSignatureKind::VoluntaryExit), | ||
_ => Err(()), | ||
0 => PartialSignatureKind::PostConsensus, | ||
1 => PartialSignatureKind::RandaoPartialSig, | ||
2 => PartialSignatureKind::SelectionProofPartialSig, | ||
3 => PartialSignatureKind::ContributionProofs, | ||
4 => PartialSignatureKind::ValidatorRegistration, | ||
5 => PartialSignatureKind::VoluntaryExit, | ||
_ => panic!("Invalid PartialSignatureKind value: {value}"), | ||
} | ||
} | ||
} |
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.
Why is this change necessary?
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.
To derive Deserialize
on the PartialSignatureKind
for reading in the test files it requires From
not TryFrom
Did some cleanup and I think this is in a mergable place now. Merging into new |
Issue Addressed
This contains all parsing logic, ssz typing, and tree hashing needed for the current state of the spec tests. Contains the logic from #377 alongside some additions