-
Notifications
You must be signed in to change notification settings - Fork 10
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
Conversation
types
module. types
crate.
types
crate. ssv_types
crate.
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! I had a few comments
pub type OperatorID = u64; | ||
|
||
/// Operator RSA public key | ||
pub type OperatorPublicKey = [u8; 459]; |
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.
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.
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.
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?
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.
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)
good work but I think we'll go with your other idea -- closing this pr |
Issue Addressed
N/A
Proposed Changes
This PR starts a
ssv_types
crate withinanchor/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 thetypes
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.