fix: avoid unnecessary sig verifications

Previously we were performing duplicate signature verifications.

This occurred whenever two or more inputs to our tx (b) were each outputs of the
same source tx (a). In this case, KeyManager::verify() was being called using
the exact same tx_hash, mint_pk, and mint_sig for each input.

The change is to:
1. separate logic that verifies a mint sig is provided for each
tx from the logic that actually verifies each sig.
2. generate a list of unique pk+sig.
3. verify tx hash against each pk+sig.

src/builder.rs

@@ -343,14 +343,14 @@ impl DbcBuilder {
 
     /// Build the output DBCs
     ///
-    /// This function relies/assumes that the caller (wallet/client) obtains
-    /// the mint's and spentbook's public keys (held by KeyManager) in a
-    /// trustless/verified way.  ie, the caller should not simply obtain keys
-    /// from a MintNode directly, but must somehow verify that the MintNode is
-    /// a valid authority.
+    /// note: common tx verification logic is shared by MintNode::reissue(),
+    /// DbcBuilder::build() and Dbc::verify()
+    ///
+    /// see TransactionVerifier::verify() for a description of
+    /// verifier requirements.
     pub fn build<K: KeyManager>(
         self,
-        mint_verifier: &K,
+        verifier: &K,
     ) -> Result<Vec<(Dbc, OwnerOnce, AmountSecrets)>> {
         let mut mint_sig_shares: Vec<NodeSignature> = Default::default();
         let mut pk_set: HashSet<PublicKeySet> = Default::default();
@@ -411,7 +411,7 @@ impl DbcBuilder {
 
         // verify the Tx, along with mint sigs and spent proofs.
         // note that we do this just once for entire Tx, not once per output Dbc.
-        TransactionVerifier::verify(mint_verifier, transaction, &mint_sigs, spent_proofs)?;
+        TransactionVerifier::verify(verifier, transaction, &mint_sigs, spent_proofs)?;
 
         let pc_gens = PedersenGens::default();
         let output_commitments: Vec<(Commitment, RevealedCommitment)> = self

src/verification.rs

@@ -51,22 +51,23 @@ impl TransactionVerifier {
             return Err(Error::MissingSignatureForInput);
         }
 
-        let tx_hash = Hash::from(transaction.hash());
-
-        // todo: optimization.  avoid duplicate validations
-        // when multiple inputs share the same mint_key+mint_sig
-        // (because they were outputs of a single tx)
-
-        // Verify that each input has a corresponding valid mint signature.
-        for (key_image, (mint_key, mint_sig)) in mint_sigs.iter() {
-            if !transaction
-                .mlsags
-                .iter()
-                .any(|m| m.key_image == *key_image.as_ref())
-            {
+        // Verify that we received a mint pk/sig for each tx input.
+        for mlsag in transaction.mlsags.iter() {
+            if mint_sigs.get(&mlsag.key_image.into()).is_none() {
                 return Err(Error::UnknownInput);
             }
+        }
+
+        // Obtain unique list of pk/sig, to avoid duplicate verify() calls when
+        // 2 or more inputs to our tx (a) were outputs of the same source tx (b).
+        let mint_sigs_unique: BTreeMap<Vec<u8>, (&PublicKey, &Signature)> = mint_sigs
+            .iter()
+            .map(|(_k, (pk, s))| (Self::pk_sig_bytes(pk, s), (pk, s)))
+            .collect();
 
+        // Verify that each unique mint signature is valid.
+        let tx_hash = Hash::from(transaction.hash());
+        for (_, (mint_key, mint_sig)) in mint_sigs_unique.iter() {
             verifier
                 .verify(&tx_hash, mint_key, mint_sig)
                 .map_err(|e| Error::Signing(e.to_string()))?;
@@ -75,6 +76,13 @@ impl TransactionVerifier {
         Self::verify_without_sigs_internal(verifier, transaction, tx_hash, spent_proofs)
     }
 
+    fn pk_sig_bytes(pk: &PublicKey, sig: &Signature) -> Vec<u8> {
+        let mut bytes: Vec<u8> = Default::default();
+        bytes.extend(&pk.to_bytes());
+        bytes.extend(&sig.to_bytes());
+        bytes
+    }
+
     /// Verifies a transaction including including spent proofs and excluding mint signatures.
     ///
     /// This function relies/assumes that the caller (wallet/client) obtains