Fix serialisation error with refresh share (#245)

Add serialisation test

frost-core/src/keys/refresh.rs

@@ -49,10 +49,19 @@ pub fn calculate_zero_key<C: Ciphersuite, R: RngCore + CryptoRng>(
     )?;
 
     let mut verifying_shares: BTreeMap<Identifier<C>, VerifyingShare<C>> = BTreeMap::new();
+    let mut zero_shares_minus_identity: Vec<SecretShare<C>> = Vec::new();
 
     for share in zero_shares.clone() {
         let signer_public = SigningShare::into(share.signing_share);
         verifying_shares.insert(share.identifier, signer_public);
+        let mut coefficients = share.commitment.0;
+        coefficients.remove(0);
+        zero_shares_minus_identity.push(SecretShare {
+            header: share.header,
+            identifier: share.identifier,
+            signing_share: share.signing_share,
+            commitment: VerifiableSecretSharingCommitment::new(coefficients),
+        });
     }
 
     let pub_key_package = PublicKeyPackage::<C> {
@@ -61,26 +70,36 @@ pub fn calculate_zero_key<C: Ciphersuite, R: RngCore + CryptoRng>(
         verifying_key: old_pub_key_package.verifying_key,
     };
 
-    Ok((zero_shares, pub_key_package))
+    Ok((zero_shares_minus_identity, pub_key_package))
 }
 
-/// Each participant refreshed their shares
+/// Each participant refreshes their shares
+/// This is done by taking the `zero_share` received from the trusted dealer and adding it to the original share
 pub fn refresh_share<C: Ciphersuite>(
     zero_share: SecretShare<C>,
     current_share: &SecretShare<C>,
 ) -> Result<SecretShare<C>, Error<C>> {
     let signing_share: Scalar<C> =
         zero_share.signing_share.to_scalar() + current_share.signing_share.to_scalar();
 
-    let zero_commitments = zero_share.commitment.0;
+    // The identity commitment needs to be added to the VSS commitment
+    let identity_commitment: Vec<CoefficientCommitment<C>> =
+        vec![(CoefficientCommitment(<C::Group>::identity()))];
+
+    let zero_commitments_without_id = zero_share.commitment.0;
     let old_commitments = current_share.commitment.0.clone();
 
-    let mut commitments: Vec<CoefficientCommitment<C>> = Vec::with_capacity(zero_commitments.len());
+    let zero_commitment: Vec<CoefficientCommitment<C>> = identity_commitment
+        .into_iter()
+        .chain(zero_commitments_without_id.clone())
+        .collect();
+
+    let mut commitments: Vec<CoefficientCommitment<C>> = Vec::with_capacity(zero_commitment.len());
 
-    if old_commitments.len() >= zero_commitments.len() {
-        for i in 0..zero_commitments.len() {
+    if old_commitments.len() >= zero_commitment.len() {
+        for i in 0..zero_commitment.len() {
             if let (Some(zero_commitment), Some(old_commitment)) =
-                (zero_commitments.get(i), old_commitments.get(i))
+                (zero_commitment.get(i), old_commitments.get(i))
             {
                 commitments.push(CoefficientCommitment::new(
                     zero_commitment.0 + old_commitment.0,

frost-core/src/tests/refresh.rs

@@ -7,7 +7,7 @@ use rand_core::{CryptoRng, RngCore};
 use crate::keys::refresh::{calculate_zero_key, refresh_share};
 use crate::{self as frost};
 use crate::{
-    keys::{PublicKeyPackage, SecretShare},
+    keys::{KeyPackage, PublicKeyPackage, SecretShare},
     Ciphersuite, Error, Identifier, SigningKey,
 };
 
@@ -140,3 +140,76 @@ fn build_old_shares<C: Ciphersuite, R: RngCore + CryptoRng>(
 
     (old_shares, pub_key_package)
 }
+
+/// Check serialisation
+pub fn check_refresh_shares_with_dealer_serialisation<C: Ciphersuite, R: RngCore + CryptoRng>(
+    mut rng: R,
+) {
+    // Compute shares
+
+    ////////////////////////////////////////////////////////////////////////////
+    // Old Key generation
+    ////////////////////////////////////////////////////////////////////////////
+
+    let max_signers = 5;
+    let min_signers = 3;
+    let (_old_shares, pub_key_package) = frost::keys::generate_with_dealer(
+        max_signers,
+        min_signers,
+        frost::keys::IdentifierList::Default,
+        &mut rng,
+    )
+    .unwrap();
+
+    ////////////////////////////////////////////////////////////////////////////
+    // New Key generation
+    //
+    // Zero key is calculated by trusted dealer
+    // Signer 2 will be removed and Signers 1, 3, 4 & 5 will remain
+    ////////////////////////////////////////////////////////////////////////////
+
+    let remaining_ids = vec![
+        Identifier::try_from(1).unwrap(),
+        Identifier::try_from(3).unwrap(),
+        Identifier::try_from(4).unwrap(),
+        Identifier::try_from(5).unwrap(),
+    ];
+
+    const NEW_MAX_SIGNERS: u16 = 4;
+
+    let (zero_shares, new_pub_key_package) = calculate_zero_key(
+        pub_key_package,
+        NEW_MAX_SIGNERS,
+        min_signers,
+        &remaining_ids,
+        &mut rng,
+    )
+    .unwrap();
+
+    // Trusted dealer serialises zero shares and key package
+
+    let zero_shares_serialised = SecretShare::<C>::serialize(&zero_shares[0]);
+
+    assert!(zero_shares_serialised.is_ok());
+
+    let new_pub_key_package_serialised = PublicKeyPackage::<C>::serialize(&new_pub_key_package);
+
+    assert!(new_pub_key_package_serialised.is_ok());
+
+    // Participant 1 deserialises zero share and key package
+
+    let zero_share = SecretShare::<C>::deserialize(&zero_shares_serialised.unwrap());
+
+    assert!(zero_share.is_ok());
+
+    let new_pub_key_package =
+        PublicKeyPackage::<C>::deserialize(&new_pub_key_package_serialised.unwrap());
+
+    assert!(new_pub_key_package.is_ok());
+
+    // Participant 1 checks Key Package can be created from Secret Share
+
+    let key_package = KeyPackage::<C>::try_from(zero_share.unwrap());
+
+    assert!(key_package.is_ok());
+}

frost-ed25519/tests/integration_tests.rs

@@ -71,6 +71,15 @@ fn check_refresh_shares_with_dealer() {
     frost_core::tests::refresh::check_refresh_shares_with_dealer::<Ed25519Sha512, _>(rng);
 }
 
+#[test]
+fn check_refresh_shares_with_dealer_serialisation() {
+    let rng = thread_rng();
+
+    frost_core::tests::refresh::check_refresh_shares_with_dealer_serialisation::<Ed25519Sha512, _>(
+        rng,
+    );
+}
+
 #[test]
 fn check_refresh_shares_with_dealer_fails_with_invalid_min_signers() {
     let rng = thread_rng();

frost-ed448/tests/integration_tests.rs

@@ -71,6 +71,15 @@ fn check_refresh_shares_with_dealer() {
     frost_core::tests::refresh::check_refresh_shares_with_dealer::<Ed448Shake256, _>(rng);
 }
 
+#[test]
+fn check_refresh_shares_with_dealer_serialisation() {
+    let rng = thread_rng();
+
+    frost_core::tests::refresh::check_refresh_shares_with_dealer_serialisation::<Ed448Shake256, _>(
+        rng,
+    );
+}
+
 #[test]
 fn check_refresh_shares_with_dealer_fails_with_invalid_min_signers() {
     let rng = thread_rng();

frost-p256/tests/integration_tests.rs

@@ -71,6 +71,15 @@ fn check_refresh_shares_with_dealer() {
     frost_core::tests::refresh::check_refresh_shares_with_dealer::<P256Sha256, _>(rng);
 }
 
+#[test]
+fn check_refresh_shares_with_dealer_serialisation() {
+    let rng = thread_rng();
+
+    frost_core::tests::refresh::check_refresh_shares_with_dealer_serialisation::<P256Sha256, _>(
+        rng,
+    );
+}
+
 #[test]
 fn check_refresh_shares_with_dealer_fails_with_invalid_min_signers() {
     let rng = thread_rng();

frost-ristretto255/tests/integration_tests.rs

@@ -71,6 +71,16 @@ fn check_refresh_shares_with_dealer() {
     frost_core::tests::refresh::check_refresh_shares_with_dealer::<Ristretto255Sha512, _>(rng);
 }
 
+#[test]
+fn check_refresh_shares_with_dealer_serialisation() {
+    let rng = thread_rng();
+
+    frost_core::tests::refresh::check_refresh_shares_with_dealer_serialisation::<
+        Ristretto255Sha512,
+        _,
+    >(rng);
+}
+
 #[test]
 fn check_refresh_shares_with_dealer_fails_with_invalid_min_signers() {
     let rng = thread_rng();

frost-secp256k1/tests/integration_tests.rs

@@ -71,6 +71,15 @@ fn check_refresh_shares_with_dealer() {
     frost_core::tests::refresh::check_refresh_shares_with_dealer::<Secp256K1Sha256, _>(rng);
 }
 
+#[test]
+fn check_refresh_shares_with_dealer_serialisation() {
+    let rng = thread_rng();
+
+    frost_core::tests::refresh::check_refresh_shares_with_dealer_serialisation::<Secp256K1Sha256, _>(
+        rng,
+    );
+}
+
 #[test]
 fn check_refresh_shares_with_dealer_fails_with_invalid_min_signers() {
     let rng = thread_rng();