-
Notifications
You must be signed in to change notification settings - Fork 18
[zk-sdk-pod] Split out pod types in the zk-sdk into a separate zk-sdk-pod crate #135
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: main
Are you sure you want to change the base?
Conversation
d60c330 to
3730ad9
Compare
zk-sdk/src/encryption/pod/elgamal.rs
Outdated
| } | ||
| // // For proof verification, interpret pod::DecryptHandle as CompressedRistretto | ||
| // #[cfg(not(target_os = "solana"))] | ||
| // impl From<PodDecryptHandle> for CompressedRistretto { |
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.
In the pod crate, we had this direct conversion from Pod* types to CompressedRistretto.
We can't really to keep this in the zk-sdk because Pod* and CompressedRistretto types are now all foreign types. We can't keep this in the zk-sdk-pod either since that would add the curve25519-dalek dependency to the crate.
I think we can remove this. Given a pod_decrypt_handle: PodDecryptHandle, downstream can run CompressedRistretto(*bytes_of(&pod_decrypt_handle)) to do this conversion itself.
| /// The `AeCiphertext` type as a `Pod`. | ||
| #[derive(Clone, Copy, PartialEq, Eq)] | ||
| #[repr(transparent)] | ||
| pub struct PodAeCiphertext(pub [u8; AE_CIPHERTEXT_LEN]); |
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.
The code in zk-sdk-pod is mostly a copy of the original pod logic in the zk-sdk. The only main difference is that the enclosed byte array has a pub visibility while it used to have pub(crate) visibility inside the zk-sdk.
| assert_eq!(expected_ae_ciphertext, computed_ae_ciphertext); | ||
| } | ||
| } | ||
| pub use solana_zk_sdk_pod::encryption::auth_encryption::*; |
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.
It is not clear if we should re-export the pod types here. With the new dependence on zk-sdk-pod and also with some of the audit changes, we will need to do a major release. Maybe we can just remove the re-export here?
b013b52 to
773ec22
Compare
773ec22 to
55fa78b
Compare
55fa78b to
03a0521
Compare
Problem
Crates like
spl-token-2022-interfaceandspl-podjust uses the pod types in thesolana-zk-sdk, but has to rely entirely on the entire crate with all of its heavy dependencies.Summary of Changes
We want to ultimately export out the pod types into a separate
solana-zk-sdk-podcrate. This crate will only rely on crates like bytemuck and have no dependencies on heavy cryptography crates.The crates like
spl-token-2022-interfaceandspl-podtypes can depend onsolana-zk-sdk-poddirectly.I was planning on breaking up the migration of pod code from the
zk-sdkto thezk-sdk-podcrate in three PRs. This PR just focuses on the pod types for the encryption submodule.On a follow-up, I will migrate the pod types for the proofs. I will then re-organize the rest of the pod types in a final follow-up.