chore(crypto): CRP-1470 CRP-2305 Support compact CBOR encoding for commitment openings #8179
Conversation
…mmitment openings
|
Since now commitment openings are only stored in memory it's quite likely we don't really need a transition strategy here, but to me it still feels a little safer. The PR changes the default encoding for P-256 to use bytestrings (since we don't have a P256 key yet) and uses the old encoding for K256 and Ed25519. There are two todos that would be addressed
We could alternately just do it all at once on the assumption that EccScalarBytes is really not serialized to disk anywhere. This would be a rather simpler patch. |
fspreiss
left a comment
fspreiss
left a comment
There was a problem hiding this comment.
Thanks, @randombit! Even though we assume that CspSecretKey::IDkgCommitmentOpening(CommitmentOpeningBytes) are only stored in the canister secret key store, which is only ever persisted in memory and not on disk, and thus this backwards-compatible de-serialization should not ever be needed, I'm OK with going the fail-safe way and adding it anyway.
|
|
||
| #[test] | ||
| fn test_scalarbytes_deserialization_compact_cbor_format() { | ||
| let old_ser = [ |
There was a problem hiding this comment.
Maybe rename to compact_ser or so?
| } | ||
| } | ||
|
|
||
| /// DeserializeScalar to use deserialize_any for the inner value |
There was a problem hiding this comment.
Is there maybe a space missing in DeserializeScalar?
| // Use deserialize_any to handle both bytes and seq | ||
| let bytes: Box<[u8; 32]> = map.next_value_seed(BytesOrArrayScalar)?; |
There was a problem hiding this comment.
The comment only makes sense knowing that BytesOrArrayScalar uses deserialize_any, so maybe say "Use BytesOrArrayScalar, which uses deserialize_any, to handle both bytes and seq".
|
Please add a PR description before merging: maybe all that is needed is to copy/paste most/parts of this comment. |
2 participants
No description provided.