Skip to content
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

Create a dedicated ssv_types crate. #58

Closed
wants to merge 27 commits into from
Closed

Conversation

Zacholme7
Copy link
Member

@Zacholme7 Zacholme7 commented Nov 27, 2024

Issue Addressed

N/A

Proposed Changes

This PR starts a ssv_types crate within anchor/common. It contains core SSV types in accordance with the spec. It does not contain related methods as we still need to determine what is needed, but just having these types defined helps. Validator related types are imported via the types crate in lighthouse.

Additional Info

This is just an initial draft to get the ball rolling. It will change as we see fit. Also waiting on re-export of ValidatorIndex in lighthouse into unstable.

@Zacholme7 Zacholme7 requested a review from jking-aus November 27, 2024 13:52
@Zacholme7 Zacholme7 changed the title Create a dedicated types module. Create a dedicated types crate. Nov 27, 2024
@Zacholme7 Zacholme7 changed the title Create a dedicated types crate. Create a dedicated ssv_types crate. Nov 27, 2024
@Zacholme7 Zacholme7 changed the base branch from stable to unstable November 27, 2024 14:09
Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I had a few comments

pub type OperatorID = u64;

/// Operator RSA public key
pub type OperatorPublicKey = [u8; 459];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using RSA, we probably should use a specific library and its associated public key type.

Each library will have an RSAPublicKey struct. This object will have been verified when created or deserialized. If we use this type, then there is no chance that we are carrying around invalid/malicious bytes in other types.

I think we want to deserialize any public key we get from the network instantly or reject the message, then we carry around only verified and valid keys throughout our types.

i.e our types shouldn't really be bytes anywhere, unless we have to have them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up from internal message. Pulled in the rsa crate. We are facing a decoding issue as the public key is formatted as a PKCS#8 key but is wrapped in a PKCS#1 header. If the header matched the key, we would be able to decode this fine but the rsa crate rejects the decoding as the header does not match the underlying key. The options to fix this is to replace the header as I have implemented, or I believe you can strip it and then parse the key. Thoughts on this approach?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing the header I think is the way to go, as an interop compatibility layer. In the future if they decide to move to be compliant we can just remove the interop layer (i.e replacing the header)

anchor/common/ssv_types/src/operator.rs Outdated Show resolved Hide resolved
anchor/common/ssv_types/src/share.rs Show resolved Hide resolved
anchor/common/ssv_types/src/committee.rs Outdated Show resolved Hide resolved
@jking-aus
Copy link
Contributor

good work but I think we'll go with your other idea -- closing this pr

@jking-aus jking-aus closed this Dec 12, 2024
@Zacholme7 Zacholme7 deleted the types branch December 12, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants