chore: remove derived Debug and Serialize from private key types
#696
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alxiong
reviewed
Oct 30, 2024
Comment on lines
+256
to
+258
| pub fn to_tagged_base64(&self) -> Result<TaggedBase64, Tb64Error> { | ||
| TaggedBase64::new(tag::BLS_SIGNING_KEY, &self.to_bytes()) | ||
| } |
There was a problem hiding this comment.
- should we explicit about
tag::BLS_OVER_BN254_SIGN_KEY? - add code doc on
SignKeythat if one wants to implement serde on outer struct that contains this key (which should be rare), need to exercise self-caution about the implication of that, (since we removed it), and manually implement them by calling one ofto_bytesorto_tagged_base64()
There was a problem hiding this comment.
I'm slightly hesitant on the first one. It's making a long tag.
Doc add in 5f85f10
ss-es
reviewed
Oct 30, 2024
signature/src/bls_over_bls12381.rs
Outdated
| fn serialized_size(&self, _compress: Compress) -> usize { | ||
| BLS_SIG_SK_SIZE | ||
| /// Deserialize `SignKey` from bytes. | ||
| pub fn from_bytes(bytes: &[u8]) -> Result<BLSSignKey, BLST_ERROR> { |
There was a problem hiding this comment.
is it possible for these to have explicit lengths? e.g. can from_bytes take a &[u8; 32] here (since to_bytes() returns one)?
similar comment for the other to_bytes/from_bytes
There was a problem hiding this comment.
Tried this: 8d25a8b
see if you like it or not
alxiong
reviewed
Oct 31, 2024
alxiong
previously approved these changes
Oct 31, 2024
alxiong
approved these changes
Oct 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #694
This PR:
In
jf-signaturecrateDebugfor private signing keys to not print the actual value.to/from_bytes()and the conversions to/fromTaggedBase64.This PR does not:
Private key type in
elgamalcrate still has derivations. Because of this ephemeral key in the ciphertext:jellyfish/elgamal/src/lib.rs
Line 147 in 5f06aa1
Private key type in
aeadcrate still has derivations. Because of this ephemeral key in the ciphertext:jellyfish/aead/src/lib.rs
Line 279 in 5f06aa1
jf-vrfcrate should be updated after to adopt these changes injf-signature.Key places to review:
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
CHANGELOG.mdof touched crates.Files changedin the GitHub PR explorer