fix: verify that ReissueShare match

We modify ReissueShare to facilitate verifying that common fields are
the same between all mint nodes. (All fields except sig-share must
match.)

While we are at it, we do a bit of cleanup/normalization of the API
such as renaming NodeSignature to IndexedSignatureShare.

* remove: MintNodeSignature
* remove: MintNodeSignatures
* modify ReissueShare fields
* NodeSignature --> IndexedSignatureShare
* add ReissueShare::to_common_bytes()
* fix: DbcBuilder now correctly checks that ReissueShares match
* Error::ReissueSharePublicKeySetMismatch --> ReissueShareMismatch
* update tests

src/builder.rs

@@ -9,15 +9,16 @@
 use blst_ringct::ringct::{RingCtMaterial, RingCtTransaction};
 pub use blst_ringct::{DecoyInput, MlsagMaterial, Output, RevealedCommitment, TrueInput};
 use blstrs::group::Curve;
-use blsttc::{PublicKeySet, SecretKey, SignatureShare};
+use blsttc::{SecretKey, SignatureShare};
 use bulletproofs::PedersenGens;
 use rand_core::RngCore;
-use std::collections::{BTreeMap, BTreeSet, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 
 use crate::{
-    Amount, AmountSecrets, Commitment, Dbc, DbcContent, Error, Hash, KeyImage, KeyManager,
-    NodeSignature, OwnerOnce, PublicKeyBlst, PublicKeyBlstMappable, ReissueRequest, ReissueShare,
-    Result, SecretKeyBlst, SpentProof, SpentProofContent, SpentProofShare, TransactionVerifier,
+    Amount, AmountSecrets, Commitment, Dbc, DbcContent, Error, Hash, IndexedSignatureShare,
+    KeyImage, KeyManager, OwnerOnce, PublicKeyBlst, PublicKeyBlstMappable, ReissueRequest,
+    ReissueShare, Result, SecretKeyBlst, SpentProof, SpentProofContent, SpentProofShare,
+    TransactionVerifier,
 };
 
 #[cfg(feature = "serde")]
@@ -277,7 +278,7 @@ impl ReissueRequestBuilder {
                     shares
                         .iter()
                         .map(SpentProofShare::spentbook_sig_share)
-                        .map(NodeSignature::threshold_crypto),
+                        .map(IndexedSignatureShare::threshold_crypto),
                 )?;
 
                 let public_commitments: Vec<Commitment> = any_share.public_commitments().clone();
@@ -352,48 +353,43 @@ impl DbcBuilder {
         self,
         verifier: &K,
     ) -> Result<Vec<(Dbc, OwnerOnce, AmountSecrets)>> {
-        let mut mint_sig_shares: Vec<NodeSignature> = Default::default();
-        let mut pk_set: HashSet<PublicKeySet> = Default::default();
+        let mut mint_sig_shares: Vec<IndexedSignatureShare> = Default::default();
+
+        // This is a map of ReissueShare that must all be exactly the same
+        // *except* for the SignatureShare.
+        let mut rs_map: HashMap<Vec<u8>, &ReissueShare> = Default::default();
 
         for rs in self.reissue_shares.iter() {
-            // Make a list of NodeSignature (sigshare from each Mint Node)
-            let mut node_shares: Vec<NodeSignature> = rs
-                .mint_node_signatures
-                .iter()
-                .map(|e| e.1 .1.clone())
-                .collect();
-            mint_sig_shares.append(&mut node_shares);
-
-            let pub_key_sets: HashSet<PublicKeySet> = rs
-                .mint_node_signatures
-                .iter()
-                .map(|e| e.1 .0.clone())
-                .collect();
-
-            // add pubkeyset to HashSet, so we can verify there is only one distinct PubKeySet
-            pk_set = &pk_set | &pub_key_sets; // union the sets together.
+            // Make a list of IndexedSignatureShare (sigshare from each Mint Node)
+            mint_sig_shares.push(rs.mint_signature_share.clone());
+
+            rs_map.insert(rs.to_common_bytes(), rs);
         }
 
-        // verify that PublicKeySet for all Dbc in all ReissueShare match.
-        if pk_set.len() != 1 {
-            return Err(Error::ReissueSharePublicKeySetMismatch);
+        if rs_map.len() != 1 {
+            return Err(Error::ReissueShareMismatch);
         }
-        let mint_public_key_set = match pk_set.iter().next() {
-            Some(pks) => pks,
-            None => return Err(Error::ReissueSharePublicKeySetMismatch),
+        // note: rs will be the last-inserted ReissueShare.
+        let rs = match rs_map.values().into_iter().next() {
+            Some(rs) => rs,
+            None => return Err(Error::ReissueShareMismatch),
         };
 
-        // Transform Vec<NodeSignature> to Vec<u64, &SignatureShare>
+        // note: we use only the fields that have been verified to be
+        // the same across mint nodes.
+        let ReissueShare {
+            transaction,
+            spent_proofs,
+            mint_public_key_set,
+            ..
+        } = rs;
+
+        // Transform Vec<IndexedSignatureShare> to Vec<u64, &SignatureShare>
         let mint_sig_shares_ref: Vec<(u64, &SignatureShare)> = mint_sig_shares
             .iter()
             .map(|e| e.threshold_crypto())
             .collect();
 
-        // Note: we can just use the first item because we already verified that
-        // all the ReissueShare match
-        let transaction = &self.reissue_shares[0].transaction;
-        let spent_proofs = &self.reissue_shares[0].spent_proofs;
-
         // Combine signatures from all the mint nodes to obtain Mint's Signature.
         let mint_sig = mint_public_key_set.combine_signatures(mint_sig_shares_ref)?;
 

src/dbc.rs

@@ -462,11 +462,9 @@ pub(crate) mod tests {
 
         assert_eq!(reissue_tx.hash(), reissue_share.transaction.hash());
 
-        let (mint_key_set, mint_sig_share) =
-            &reissue_share.mint_node_signatures.values().next().unwrap();
-
-        let mint_sig = mint_key_set
-            .combine_signatures(vec![mint_sig_share.threshold_crypto()])
+        let mint_sig = reissue_share
+            .mint_public_key_set
+            .combine_signatures(vec![reissue_share.mint_signature_share.threshold_crypto()])
             .unwrap();
 
         // We must obtain the RevealedCommitment for our output in order to
@@ -501,10 +499,11 @@ pub(crate) mod tests {
         let mint_pk = mint_node.key_manager().public_key_set()?.public_key();
         fuzzed_mint_sigs.extend(
             reissue_share
-                .mint_node_signatures
-                .into_keys()
+                .transaction
+                .mlsags
+                .iter()
                 .take(n_valid_sigs.coerce())
-                .map(|in_owner| (in_owner, (mint_pk, mint_sig.clone()))),
+                .map(|m| (m.key_image.into(), (mint_pk, mint_sig.clone()))),
         );
 
         let mut repeating_inputs = reissue_tx

src/error.rs

@@ -57,8 +57,8 @@ pub enum Error {
     #[error("We need at least one spent proof share for {0:?} to build a SpentProof")]
     ReissueRequestMissingSpentProofShare(KeyImage),
 
-    #[error("The PublicKeySet differs between ReissueShare entries")]
-    ReissueSharePublicKeySetMismatch,
+    #[error("ReissueShare do not match")]
+    ReissueShareMismatch,
 
     #[error("Decryption failed")]
     DecryptionBySecretKeyFailed,

src/key_manager.rs

@@ -16,33 +16,39 @@ use serde::{Deserialize, Serialize};
 
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
-pub struct NodeSignature {
+pub struct IndexedSignatureShare {
     index: u64,
-    sig: SignatureShare,
+    signature_share: SignatureShare,
 }
 
-impl NodeSignature {
-    pub fn new(index: u64, sig: SignatureShare) -> Self {
-        Self { index, sig }
+impl IndexedSignatureShare {
+    pub fn new(index: u64, signature_share: SignatureShare) -> Self {
+        Self {
+            index,
+            signature_share,
+        }
     }
 
     pub fn threshold_crypto(&self) -> (u64, &SignatureShare) {
-        (self.index, &self.sig)
+        (self.index, &self.signature_share)
     }
 
     pub fn to_bytes(&self) -> Vec<u8> {
         let mut bytes = self.index.to_le_bytes().to_vec();
-        bytes.extend(&self.sig.to_bytes());
+        bytes.extend(&self.signature_share.to_bytes());
         bytes
     }
 }
 
 pub trait KeyManager {
     type Error: std::error::Error;
     fn add_known_key(&mut self, key: PublicKey) -> Result<(), Self::Error>;
-    fn sign_with_child_key(&self, idx: &[u8], tx_hash: &Hash)
-        -> Result<NodeSignature, Self::Error>;
-    fn sign(&self, msg_hash: &Hash) -> Result<NodeSignature, Self::Error>;
+    fn sign_with_child_key(
+        &self,
+        idx: &[u8],
+        tx_hash: &Hash,
+    ) -> Result<IndexedSignatureShare, Self::Error>;
+    fn sign(&self, msg_hash: &Hash) -> Result<IndexedSignatureShare, Self::Error>;
     fn public_key_set(&self) -> Result<PublicKeySet, Self::Error>;
     fn verify(
         &self,
@@ -127,16 +133,16 @@ impl KeyManager for SimpleKeyManager {
         Ok(self.signer.public_key_set())
     }
 
-    fn sign_with_child_key(&self, index: &[u8], tx_hash: &Hash) -> Result<NodeSignature> {
+    fn sign_with_child_key(&self, index: &[u8], tx_hash: &Hash) -> Result<IndexedSignatureShare> {
         let child_signer = self.signer.derive_child(index);
-        Ok(NodeSignature::new(
+        Ok(IndexedSignatureShare::new(
             child_signer.index(),
             child_signer.sign(tx_hash),
         ))
     }
 
-    fn sign(&self, msg_hash: &Hash) -> Result<NodeSignature> {
-        Ok(NodeSignature::new(
+    fn sign(&self, msg_hash: &Hash) -> Result<IndexedSignatureShare> {
+        Ok(IndexedSignatureShare::new(
             self.signer.index(),
             self.signer.sign(msg_hash),
         ))

src/lib.rs

@@ -35,10 +35,10 @@ pub use crate::{
     error::{Error, Result},
     genesis::GenesisMaterial,
     key_manager::{
-        KeyManager, NodeSignature, PublicKey, PublicKeySet, Signature, SimpleKeyManager,
+        IndexedSignatureShare, KeyManager, PublicKey, PublicKeySet, Signature, SimpleKeyManager,
         SimpleSigner,
     },
-    mint::{MintNode, MintNodeSignatures, ReissueRequest, ReissueShare},
+    mint::{MintNode, ReissueRequest, ReissueShare},
     owner::{DerivationIndex, Owner, OwnerOnce},
     spent_proof::{SpentProof, SpentProofContent, SpentProofShare},
     spentbook::SpentBookNodeMock,

src/mint.rs

@@ -14,18 +14,15 @@
 // Outputs <= input value
 
 use crate::{
-    Error, Hash, KeyImage, KeyManager, NodeSignature, PublicKey, PublicKeySet, Result, SpentProof,
-    SpentProofShare, TransactionVerifier,
+    Error, Hash, IndexedSignatureShare, KeyImage, KeyManager, PublicKey, PublicKeySet, Result,
+    SpentProof, SpentProofShare, TransactionVerifier,
 };
 use blst_ringct::ringct::RingCtTransaction;
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeSet;
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
-pub type MintNodeSignature = (PublicKeySet, NodeSignature);
-pub type MintNodeSignatures = BTreeMap<KeyImage, MintNodeSignature>;
-
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Debug, Clone)]
 pub struct ReissueRequest {
@@ -38,7 +35,25 @@ pub struct ReissueRequest {
 pub struct ReissueShare {
     pub transaction: RingCtTransaction,
     pub spent_proofs: BTreeSet<SpentProof>,
-    pub mint_node_signatures: MintNodeSignatures,
+    pub mint_public_key_set: PublicKeySet,
+    pub mint_signature_share: IndexedSignatureShare,
+}
+
+impl ReissueShare {
+    /// Returns bytes of all fields that should be the same
+    /// across mint nodes.
+    ///
+    /// The mint_signature_share field is excluded because each node
+    /// will have a different signature share.
+    pub fn to_common_bytes(&self) -> Vec<u8> {
+        let mut bytes = self.transaction.to_bytes();
+        for sp in self.spent_proofs.iter() {
+            bytes.extend(&sp.to_bytes());
+        }
+        // public key set
+        bytes.extend(&self.mint_public_key_set.to_bytes());
+        bytes
+    }
 }
 
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -119,33 +134,33 @@ impl<K: KeyManager> MintNode<K> {
 
         TransactionVerifier::verify_without_sigs(self.key_manager(), &transaction, &spent_proofs)?;
 
-        let mint_node_signatures = self.sign_transaction(&transaction)?;
+        let (mint_public_key_set, mint_signature_share) = self.sign_transaction(&transaction)?;
 
         let reissue_share = ReissueShare {
             transaction,
             spent_proofs,
-            mint_node_signatures,
+            mint_public_key_set,
+            mint_signature_share,
         };
 
         Ok(reissue_share)
     }
 
-    fn sign_transaction(&self, transaction: &RingCtTransaction) -> Result<MintNodeSignatures> {
-        let sig = self
+    fn sign_transaction(
+        &self,
+        transaction: &RingCtTransaction,
+    ) -> Result<(PublicKeySet, IndexedSignatureShare)> {
+        let node_signature = self
             .key_manager
             .sign(&Hash::from(transaction.hash()))
             .map_err(|e| Error::Signing(e.to_string()))?;
 
-        let pks = self
+        let public_key_set = self
             .key_manager
             .public_key_set()
             .map_err(|e| Error::Signing(e.to_string()))?;
 
-        Ok(transaction
-            .mlsags
-            .iter()
-            .map(|m| (m.key_image.into(), (pks.clone(), sig.clone())))
-            .collect())
+        Ok((public_key_set, node_signature))
     }
 }
 
@@ -496,7 +511,7 @@ mod tests {
                             public_commitments: spent_proof_share.public_commitments().clone(),
                         },
                         spentbook_pks: spent_proof_share.spentbook_pks,
-                        spentbook_sig_share: NodeSignature::new(
+                        spentbook_sig_share: IndexedSignatureShare::new(
                             0,
                             SecretKeySet::random(1, &mut rng)
                                 .secret_key_share(1)

src/spent_proof.rs

@@ -7,8 +7,8 @@
 // permissions and limitations relating to use of the SAFE Network Software.
 
 use crate::{
-    Commitment, Error, Hash, KeyImage, KeyManager, NodeSignature, PublicKey, PublicKeySet, Result,
-    Signature,
+    Commitment, Error, Hash, IndexedSignatureShare, KeyImage, KeyManager, PublicKey, PublicKeySet,
+    Result, Signature,
 };
 
 use std::cmp::Ordering;
@@ -71,7 +71,7 @@ pub struct SpentProofShare {
 
     /// The Spentbook who notarized that this DBC was spent.
     pub spentbook_pks: PublicKeySet,
-    pub spentbook_sig_share: NodeSignature,
+    pub spentbook_sig_share: IndexedSignatureShare,
 }
 
 // impl manually to avoid clippy complaint about Hash conflict.
@@ -109,7 +109,7 @@ impl SpentProofShare {
     }
 
     /// get spentbook's signature share
-    pub fn spentbook_sig_share(&self) -> &NodeSignature {
+    pub fn spentbook_sig_share(&self) -> &IndexedSignatureShare {
         &self.spentbook_sig_share
     }