From ded11462caa00b635ec2cd095d6af11b3e0a61c5 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Tue, 23 May 2023 17:12:02 -0400 Subject: [PATCH 01/10] SerDe for QC proof. --- Cargo.toml | 1 + src/quorum_certificate.rs | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d641e6f..cac7648 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ ark-ff = "0.4.0" ark-poly = "0.4.0" ark-serialize = "0.4.0" ark-std = { version = "0.4.0", default-features = false } +bincode = { version = "1.1.3" } bitvec = { version = "1.0.1", default-features = false, features = ["alloc", "atomic", "serde"] } derivative = "2.2.0" digest = { version = "0.10" } diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index dba496a..e05b112 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -1,3 +1,4 @@ +use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use bitvec::prelude::*; use core::marker::PhantomData; use typenum::U32; @@ -13,6 +14,7 @@ use generic_array::{ArrayLength, GenericArray}; use jf_primitives::errors::PrimitivesError; use jf_primitives::errors::PrimitivesError::ParameterError; use jf_primitives::signatures::AggregateableSignatureSchemes; +use serde::{Deserialize, Serialize}; /// Trait for validating a QC built from different signatures on the same message pub trait QuorumCertificateValidation { @@ -26,7 +28,7 @@ pub trait QuorumCertificateValidation { /// Extra value to check the aggregated signature of the QC /// E.g: snark proof, bitmap corresponding to the public keys involved in signing - type Proof; + type Proof: Serialize + for<'a> Deserialize<'a>; /// Allows to fix the size of the message at compilation time. type MessageLength: ArrayLength; @@ -73,8 +75,10 @@ pub trait QuorumCertificateValidation { ) -> Result; } -// TODO: add CanonicalSerialize/Deserialize -pub struct BitvectorQuorumCertificate(PhantomData); +#[derive(CanonicalSerialize, CanonicalDeserialize)] +pub struct BitvectorQuorumCertificate( + PhantomData, +); pub struct StakeTableEntry { pub stake_key: A::VerificationKey, @@ -93,7 +97,7 @@ pub struct QCParams { impl QuorumCertificateValidation for BitvectorQuorumCertificate where - A: AggregateableSignatureSchemes, + A: AggregateableSignatureSchemes + Sync + Send, { type QCProverParams = QCParams; @@ -270,6 +274,12 @@ mod tests { ) .is_ok()); + // Check the QC can be serialized / deserialized + assert_eq!( + qc, + bincode::deserialize(&bincode::serialize(&qc).unwrap()).unwrap() + ); + // bad paths // number of signatures unmatch assert!(BitvectorQuorumCertificate::<$aggsig>::assemble( @@ -332,9 +342,4 @@ mod tests { fn test_quorum_certificate() { test_quorum_certificate!(BLSOverBN254CurveSignatureScheme); } - - // #[test] - // fn test_serde() { - // // TODO - // } } From f0e6cf7fa3892a241231de6d0f9813859f3225b0 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Wed, 24 May 2023 16:27:54 -0400 Subject: [PATCH 02/10] Remove Sync/Send trait bounds. --- Cargo.toml | 6 +++--- src/quorum_certificate.rs | 24 +++++++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cac7648..7a1e758 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,8 +24,8 @@ digest = { version = "0.10" } displaydoc = { version = "0.2.3", default-features = false } ethereum-types = { version = "0.14.1", features = ["impl-serde"] } generic-array = "0.14.7" -jf-primitives = { git = "https://github.com/espressosystems/jellyfish" } # , tag = "0.3.0" -jf-utils = { git = "https://github.com/espressosystems/jellyfish" } # , tag = "0.3.0" +jf-primitives = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde" } # , tag = "0.3.0" +jf-utils = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde" } # , tag = "0.3.0" serde = { version = "1.0", default-features = false, features = ["derive", "rc"] } sha3 = "0.10.7" tagged-base64 = { git = "https://github.com/espressosystems/tagged-base64", tag = "0.3.0" } @@ -33,7 +33,7 @@ thiserror = "1.0" typenum = { version = "1.16.0", no_sstd = true } [dev-dependencies] -jf-primitives = { git = "https://github.com/espressosystems/jellyfish", features = ["test-srs"] } # , tag = "0.3.0" +jf-primitives = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde", features = ["test-srs"] } # , tag = "0.3.0" sha2 = { version = "0.10" } [features] diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index e05b112..ad9ea9b 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -1,6 +1,6 @@ -use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use bitvec::prelude::*; use core::marker::PhantomData; +//use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use typenum::U32; use ark_std::{ @@ -17,14 +17,17 @@ use jf_primitives::signatures::AggregateableSignatureSchemes; use serde::{Deserialize, Serialize}; /// Trait for validating a QC built from different signatures on the same message -pub trait QuorumCertificateValidation { +pub trait QuorumCertificateValidation< + A: AggregateableSignatureSchemes + Serialize + for<'a> Deserialize<'a>, +> +{ /// Public parameters for generating the QC /// E.g: snark proving/verifying keys, list of (or pointer to) public keys stored in the smart contract. - type QCProverParams; + type QCProverParams; //: Serialize + for<'a> Deserialize<'a>; /// Public parameters for validating the QC /// E.g: verifying keys, stake table commitment - type QCVerifierParams; + type QCVerifierParams; //: Serialize + for<'a> Deserialize<'a>; /// Extra value to check the aggregated signature of the QC /// E.g: snark proof, bitmap corresponding to the public keys involved in signing @@ -75,19 +78,22 @@ pub trait QuorumCertificateValidation { ) -> Result; } -#[derive(CanonicalSerialize, CanonicalDeserialize)] -pub struct BitvectorQuorumCertificate( - PhantomData, -); +#[derive(Serialize, Deserialize)] +pub struct BitvectorQuorumCertificate< + A: AggregateableSignatureSchemes + Serialize + for<'a> Deserialize<'a>, +>(PhantomData); +#[derive(Serialize, Deserialize)] pub struct StakeTableEntry { pub stake_key: A::VerificationKey, pub stake_amount: U256, } +//#[derive(Serialize, Deserialize)] pub struct StakeTableDigest(Vec); // TODO: refactor +//#[derive(CanonicalSerialize, CanonicalDeserialize)] pub struct QCParams { pub stake_table_digest: StakeTableDigest, pub stake_entries: Vec>, @@ -97,7 +103,7 @@ pub struct QCParams { impl QuorumCertificateValidation for BitvectorQuorumCertificate where - A: AggregateableSignatureSchemes + Sync + Send, + A: AggregateableSignatureSchemes, { type QCProverParams = QCParams; From cba190f77b1f0126a13d8c8a98c978fa65c174c9 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Wed, 24 May 2023 16:32:46 -0400 Subject: [PATCH 03/10] Remove stake_table_digest for now as it creates complications. --- src/quorum_certificate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index ad9ea9b..bff73a9 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -95,7 +95,7 @@ pub struct StakeTableDigest(Vec { - pub stake_table_digest: StakeTableDigest, + // pub stake_table_digest: StakeTableDigest, pub stake_entries: Vec>, pub threshold: U256, pub agg_sig_pp: A::PublicParameter, @@ -236,7 +236,7 @@ mod tests { stake_amount: U256::from(7u8), }; let qc_pp = QCParams::<$aggsig> { - stake_table_digest: StakeTableDigest::<$aggsig>(vec![12u8, 2u8, 7u8, 8u8]), + //stake_table_digest: StakeTableDigest::<$aggsig>(vec![12u8, 2u8, 7u8, 8u8]), stake_entries: vec![entry1, entry2, entry3], threshold: U256::from(10u8), agg_sig_pp, From ee7e7207ce1a6fb13506b4d2989b54f82e70917c Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Wed, 24 May 2023 16:46:35 -0400 Subject: [PATCH 04/10] Remove trait bounds on parameters for QCParams struct due to https://github.com/serde-rs/serde/issues/2418#issuecomment-1485533880 --- src/quorum_certificate.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index bff73a9..948c547 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -23,11 +23,11 @@ pub trait QuorumCertificateValidation< { /// Public parameters for generating the QC /// E.g: snark proving/verifying keys, list of (or pointer to) public keys stored in the smart contract. - type QCProverParams; //: Serialize + for<'a> Deserialize<'a>; + type QCProverParams: Serialize; //: Serialize + for<'a> Deserialize<'a>; /// Public parameters for validating the QC /// E.g: verifying keys, stake table commitment - type QCVerifierParams; //: Serialize + for<'a> Deserialize<'a>; + type QCVerifierParams: Serialize; //: Serialize + for<'a> Deserialize<'a>; /// Extra value to check the aggregated signature of the QC /// E.g: snark proof, bitmap corresponding to the public keys involved in signing @@ -84,8 +84,8 @@ pub struct BitvectorQuorumCertificate< >(PhantomData); #[derive(Serialize, Deserialize)] -pub struct StakeTableEntry { - pub stake_key: A::VerificationKey, +pub struct StakeTableEntry { + pub stake_key: V, pub stake_amount: U256, } @@ -93,22 +93,22 @@ pub struct StakeTableEntry { pub struct StakeTableDigest(Vec); // TODO: refactor -//#[derive(CanonicalSerialize, CanonicalDeserialize)] -pub struct QCParams { +#[derive(Serialize, Deserialize)] //, CanonicalDeserialize)] +pub struct QCParams { // pub stake_table_digest: StakeTableDigest, - pub stake_entries: Vec>, + pub stake_entries: Vec>, pub threshold: U256, - pub agg_sig_pp: A::PublicParameter, + pub agg_sig_pp: P, } impl QuorumCertificateValidation for BitvectorQuorumCertificate where - A: AggregateableSignatureSchemes, + A: AggregateableSignatureSchemes + Serialize + for<'a> Deserialize<'a>, { - type QCProverParams = QCParams; + type QCProverParams = QCParams; // TODO: later with SNARKs we'll use a smaller verifier parameter - type QCVerifierParams = QCParams; + type QCVerifierParams = QCParams; type Proof = BitVec; type MessageLength = U32; @@ -223,19 +223,19 @@ mod tests { let key_pair1 = KeyPair::generate(&mut rng); let key_pair2 = KeyPair::generate(&mut rng); let key_pair3 = KeyPair::generate(&mut rng); - let entry1 = StakeTableEntry::<$aggsig> { + let entry1 = StakeTableEntry { stake_key: key_pair1.ver_key(), stake_amount: U256::from(3u8), }; - let entry2 = StakeTableEntry::<$aggsig> { + let entry2 = StakeTableEntry { stake_key: key_pair2.ver_key(), stake_amount: U256::from(5u8), }; - let entry3 = StakeTableEntry::<$aggsig> { + let entry3 = StakeTableEntry { stake_key: key_pair3.ver_key(), stake_amount: U256::from(7u8), }; - let qc_pp = QCParams::<$aggsig> { + let qc_pp = QCParams { //stake_table_digest: StakeTableDigest::<$aggsig>(vec![12u8, 2u8, 7u8, 8u8]), stake_entries: vec![entry1, entry2, entry3], threshold: U256::from(10u8), From a5249c78363e206ea020dc6d83eb7d3c6f78d8f1 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Wed, 24 May 2023 17:20:07 -0400 Subject: [PATCH 05/10] Add test to ensure that the QCParams struct can be (de)serialized. --- src/quorum_certificate.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index 948c547..1b030aa 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -1,6 +1,6 @@ +use ark_std::fmt::Debug; use bitvec::prelude::*; use core::marker::PhantomData; -//use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use typenum::U32; use ark_std::{ @@ -83,18 +83,21 @@ pub struct BitvectorQuorumCertificate< A: AggregateableSignatureSchemes + Serialize + for<'a> Deserialize<'a>, >(PhantomData); -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, PartialEq, Debug)] pub struct StakeTableEntry { pub stake_key: V, pub stake_amount: U256, } +// TODO remove this //#[derive(Serialize, Deserialize)] -pub struct StakeTableDigest(Vec); +//pub struct StakeTableDigest(Vec); -// TODO: refactor -#[derive(Serialize, Deserialize)] //, CanonicalDeserialize)] +// TODO: refactor: @binyi was this the refactor you had in mind (i.e removing the trait bound on the QCParams parameters as they create troubles for SerDe)? +// See commit ee7e7207ce1a6fb13506b4d2989b54f82e70917c +#[derive(Serialize, Deserialize, PartialEq, Debug)] pub struct QCParams { + // TODO remove stake_table_digest? It is not used and creates complications for serde in Jellyfish with the Schnorr signature due to A::MessageUnit // pub stake_table_digest: StakeTableDigest, pub stake_entries: Vec>, pub threshold: U256, @@ -280,12 +283,17 @@ mod tests { ) .is_ok()); - // Check the QC can be serialized / deserialized + // Check the QC and the QCParams can be serialized / deserialized assert_eq!( qc, bincode::deserialize(&bincode::serialize(&qc).unwrap()).unwrap() ); + assert_eq!( + qc_pp, + bincode::deserialize(&bincode::serialize(&qc_pp).unwrap()).unwrap() + ); + // bad paths // number of signatures unmatch assert!(BitvectorQuorumCertificate::<$aggsig>::assemble( From 55ce8541573c6a34246f35e18d9ff1725ddd0a7a Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Wed, 24 May 2023 17:25:13 -0400 Subject: [PATCH 06/10] Add some TODO. --- src/quorum_certificate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index 1b030aa..8cf803d 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -239,6 +239,7 @@ mod tests { stake_amount: U256::from(7u8), }; let qc_pp = QCParams { + // TODO remove //stake_table_digest: StakeTableDigest::<$aggsig>(vec![12u8, 2u8, 7u8, 8u8]), stake_entries: vec![entry1, entry2, entry3], threshold: U256::from(10u8), From bb7dd734c01b7b0da472e5236409f00513ae13fd Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Thu, 25 May 2023 10:35:24 -0400 Subject: [PATCH 07/10] Enforce the deserialization trait for associated types QCProverParams and QCVerifierParams. --- src/quorum_certificate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index 8cf803d..5129a14 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -23,11 +23,11 @@ pub trait QuorumCertificateValidation< { /// Public parameters for generating the QC /// E.g: snark proving/verifying keys, list of (or pointer to) public keys stored in the smart contract. - type QCProverParams: Serialize; //: Serialize + for<'a> Deserialize<'a>; + type QCProverParams: Serialize + for<'a> Deserialize<'a>; /// Public parameters for validating the QC /// E.g: verifying keys, stake table commitment - type QCVerifierParams: Serialize; //: Serialize + for<'a> Deserialize<'a>; + type QCVerifierParams: Serialize + for<'a> Deserialize<'a>; /// Extra value to check the aggregated signature of the QC /// E.g: snark proof, bitmap corresponding to the public keys involved in signing From c96b86594f273f6ade801fa7cd60c1909c34a345 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Thu, 25 May 2023 10:37:10 -0400 Subject: [PATCH 08/10] Remove TODO comment. --- src/quorum_certificate.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index 5129a14..0a1797e 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -93,8 +93,6 @@ pub struct StakeTableEntry { //#[derive(Serialize, Deserialize)] //pub struct StakeTableDigest(Vec); -// TODO: refactor: @binyi was this the refactor you had in mind (i.e removing the trait bound on the QCParams parameters as they create troubles for SerDe)? -// See commit ee7e7207ce1a6fb13506b4d2989b54f82e70917c #[derive(Serialize, Deserialize, PartialEq, Debug)] pub struct QCParams { // TODO remove stake_table_digest? It is not used and creates complications for serde in Jellyfish with the Schnorr signature due to A::MessageUnit From 2506abcc9c66799d46132e2d1165bd1b295ea973 Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Thu, 25 May 2023 10:40:14 -0400 Subject: [PATCH 09/10] Remove StakeTableDigest and add a comment regarding how to compute the message before hashing it. --- src/quorum_certificate.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/quorum_certificate.rs b/src/quorum_certificate.rs index 0a1797e..748ca37 100644 --- a/src/quorum_certificate.rs +++ b/src/quorum_certificate.rs @@ -40,6 +40,7 @@ pub trait QuorumCertificateValidation< type CheckedType; /// Produces a partial signature on a message with a single user signing key + /// NOTE: the original message (vote) should be prefixed with the hash of the stake table. /// * `agg_sig_pp` - public parameters for aggregate signature /// * `message` - message to be signed /// * `signing_keys` - user signing key @@ -89,14 +90,8 @@ pub struct StakeTableEntry { pub stake_amount: U256, } -// TODO remove this -//#[derive(Serialize, Deserialize)] -//pub struct StakeTableDigest(Vec); - #[derive(Serialize, Deserialize, PartialEq, Debug)] pub struct QCParams { - // TODO remove stake_table_digest? It is not used and creates complications for serde in Jellyfish with the Schnorr signature due to A::MessageUnit - // pub stake_table_digest: StakeTableDigest, pub stake_entries: Vec>, pub threshold: U256, pub agg_sig_pp: P, @@ -237,8 +232,6 @@ mod tests { stake_amount: U256::from(7u8), }; let qc_pp = QCParams { - // TODO remove - //stake_table_digest: StakeTableDigest::<$aggsig>(vec![12u8, 2u8, 7u8, 8u8]), stake_entries: vec![entry1, entry2, entry3], threshold: U256::from(10u8), agg_sig_pp, From 8106affb6b7aab1300a80f281356475d79493fca Mon Sep 17 00:00:00 2001 From: Philippe Camacho Date: Thu, 25 May 2023 11:50:45 -0400 Subject: [PATCH 10/10] Point to main branch of Jellyfish. --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7a1e758..cac7648 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,8 +24,8 @@ digest = { version = "0.10" } displaydoc = { version = "0.2.3", default-features = false } ethereum-types = { version = "0.14.1", features = ["impl-serde"] } generic-array = "0.14.7" -jf-primitives = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde" } # , tag = "0.3.0" -jf-utils = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde" } # , tag = "0.3.0" +jf-primitives = { git = "https://github.com/espressosystems/jellyfish" } # , tag = "0.3.0" +jf-utils = { git = "https://github.com/espressosystems/jellyfish" } # , tag = "0.3.0" serde = { version = "1.0", default-features = false, features = ["derive", "rc"] } sha3 = "0.10.7" tagged-base64 = { git = "https://github.com/espressosystems/tagged-base64", tag = "0.3.0" } @@ -33,7 +33,7 @@ thiserror = "1.0" typenum = { version = "1.16.0", no_sstd = true } [dev-dependencies] -jf-primitives = { git = "https://github.com/espressosystems/jellyfish", branch = "fix/signature-scheme-serde", features = ["test-srs"] } # , tag = "0.3.0" +jf-primitives = { git = "https://github.com/espressosystems/jellyfish", features = ["test-srs"] } # , tag = "0.3.0" sha2 = { version = "0.10" } [features]