Generic FMD2 implementation #18
Conversation
fuzzy-message-detection/src/lib.rs
Outdated
| #[derive(Debug, Clone)] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| /// γ secret keys (scalars). For minimum false-positive rate p:=2^{-γ}. | ||
| pub struct SecretKey(pub(crate) Vec<Scalar>); | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| /// A subset of n-out-γ secret keys and the positions | ||
| /// they occupy in [SecretKey]. | ||
| pub struct DetectionKey { | ||
|
|
||
| pub(crate) keys: Vec<Scalar>, | ||
|
|
||
| pub(crate) indices: Vec<usize>, | ||
|
|
||
| } |
There was a problem hiding this comment.
Is there a specific reason to move the curve25519-based SecretKey and DetectionKey implementations here? In lib.rs, it appears you are defining the general FMD interfaces.
There was a problem hiding this comment.
Could the SecretKey and DetectionKey structs and methods be in the same file, or do you have other reasons to separate them?
There was a problem hiding this comment.
Is there a specific reason to move the curve25519-based SecretKey and DetectionKey implementations here? In lib.rs, it appears you are defining the general FMD interfaces.
I moved SecretKey and DetectionKey out of the vanilla FMD2 implementation because they will also be used by the Fmd2Poly implementation.
The trait FmdScheme uses these two structs, (as opposed to PublicKey, FlagCiphertext that are implementation-dependent.)
Could the SecretKey and DetectionKey structs and methods be in the same file, or do you have other reasons to separate them?
Yes, ideally everything in the same file. Perhaps we can find a better location than lib.rs? -- suggestions welcome.
There was a problem hiding this comment.
Should we move the SecretKey and DetectionKey along with their methods to separate files(keys.rs or secret_key.rs and detection_key.rs) instead of keeping them in lib.rs?
| } | ||
| } | ||
|
|
||
| /// FMD2 is proven to be IND-CCA secure in the [FMD paper](https://eprint.iacr.org/2021/089). | ||
| impl CcaSecure for Fmd2 {} | ||
| impl CcaSecure for Fmd2Params {} |
There was a problem hiding this comment.
Is CcaSecure a todo feature? I noticed it's empty
There was a problem hiding this comment.
It's just a marker trait to signal the scheme is IND-CCA. We can remove it if it is confusing.
| #[derive(Debug, Clone)] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| pub struct SecretKey(Vec<Scalar>); | ||
| use crate::{ |
There was a problem hiding this comment.
I'm going to suggest we use the same import format as Namada. There are three import blocks: First is from core, std, alloc, etc. The second is third party. The last is crate, super, etc. There should a single return between blocks, and no returns within a block. Each block should be alphabetized
| use crate::DetectionKey; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub(crate) struct GenericPublicKey { |
There was a problem hiding this comment.
nit: I would prefer to call this a DiversifiedPublicKey to match the terminology elsewhere.
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub(crate) struct GenericFlagCiphertexts { |
There was a problem hiding this comment.
Similar nit as before: DiversifiedFlagCiphertexts
|
|
||
| GenericFlagCiphertexts::generate_flag( | ||
| &gpk, | ||
| &TrapdoorBasepoint::new(&gpk, &Scalar::ONE), |
There was a problem hiding this comment.
Maybe we can have an impl From<GenericPublicKey> for this case. Or even a Default impl'd for TrapdoorBasepoint
| } | ||
|
|
||
| pub(crate) struct TrapdoorBasepoint { | ||
| b: RistrettoPoint, |
There was a problem hiding this comment.
Let's give these fields more descriptive names. Maybe something like base and exp. But one letter fields are starting to confuse me as I review other parts of the code.
| pub(crate) fn new(pk: &GenericPublicKey, trapdoor: &Scalar) -> TrapdoorBasepoint { | ||
| TrapdoorBasepoint { | ||
| b: pk.basepoint_eg * trapdoor, | ||
| t: *trapdoor, |
There was a problem hiding this comment.
Reading through penumbra's specs, they don't require this trapdoor. They use randomize the pk's basepoint with two different random scalars. Why can't we just do the same?
Refactor code. Re-uses internally the same FMD2 implementation for vanilla FMD2 and the polynomial-base FMD2. (Mainly, code just moved around.)
Needed for PR #19