fix: make prop_dbc_validation pass

This test was intermittently failing in CI.  I found and fixed a
few problems with it, including some problematic logic in
TransactionVerifier.

* verification: improve logic for MissingSignatureForInput error.
* verification: verify number of mint sigs equals number of inputs
* verification: replace UnknownInput with MintSignatureInputMismatch
* error: replace UnknownInput with MintSignatureInputMismatch
* test: generated fuzzed_amount_secrets with correct blinding factor
  for cases when extra_output_amount is 0.
* test: remove ugly commitments_match case when verification succeeds.
* test: check for MintSignatureInputMismatch error instead of UnknownInput
* test: check for AmountCommitmentsDoNotMatch error

src/dbc.rs

@@ -268,8 +268,6 @@ impl Dbc {
 pub(crate) mod tests {
     use super::*;
 
-    use std::iter::FromIterator;
-
     use quickcheck_macros::quickcheck;
 
     use crate::tests::{NonZeroTinyInt, TinyInt};
@@ -442,7 +440,7 @@ pub(crate) mod tests {
         let owner_once =
             OwnerOnce::from_owner_base(Owner::from_random_secret_key(&mut rng), &mut rng8);
 
-        let (mut rr_builder, _dbc_builder, _material) = crate::TransactionBuilder::default()
+        let (mut rr_builder, dbc_builder, _material) = crate::TransactionBuilder::default()
             .add_inputs_dbc(inputs, &mut rng8)?
             .add_output(
                 Output {
@@ -471,8 +469,25 @@ pub(crate) mod tests {
             .combine_signatures(vec![mint_sig_share.threshold_crypto()])
             .unwrap();
 
-        let fuzzed_amt_secrets =
-            AmountSecrets::from_amount(amount + extra_output_amount.coerce::<Amount>(), &mut rng8);
+        // We must obtain the RevealedCommitment for our output in order to
+        // know the correct blinding factor when creating fuzzed_amt_secrets.
+        let output = reissue_tx.outputs.get(0).unwrap();
+        let pc_gens = bulletproofs::PedersenGens::default();
+        let output_commitments: Vec<(crate::Commitment, RevealedCommitment)> = dbc_builder
+            .revealed_commitments
+            .iter()
+            .map(|r| (r.commit(&pc_gens).to_affine(), *r))
+            .collect();
+        let amount_secrets_list: Vec<AmountSecrets> = output_commitments
+            .iter()
+            .filter(|(c, _)| *c == output.commitment())
+            .map(|(_, r)| AmountSecrets::from(*r))
+            .collect();
+
+        let fuzzed_amt_secrets = AmountSecrets::from((
+            amount + extra_output_amount.coerce::<Amount>(),
+            amount_secrets_list[0].blinding_factor(),
+        ));
         let dbc_amount = fuzzed_amt_secrets.amount();
 
         let fuzzed_content = DbcContent::from((
@@ -559,8 +574,6 @@ pub(crate) mod tests {
         let key_manager = mint_node.key_manager();
         let verification_res = dbc.verify(&owner_once.owner_base().secret_key()?, key_manager);
 
-        let commitments_match = matches!(verification_res, Err(Error::AmountCommitmentsDoNotMatch));
-
         let dbc_owner = dbc
             .owner_once(&owner_once.owner_base().secret_key()?)?
             .public_key_blst();
@@ -578,29 +591,19 @@ pub(crate) mod tests {
                 assert_eq!(n_wrong_signer_sigs.coerce::<u8>(), 0);
                 assert_eq!(n_wrong_msg_sigs.coerce::<u8>(), 0);
 
-                // note: reissue can succeed even though amount_secrets_cipher amount does not
-                // match tx commited amount.
-                assert!(dbc_amount == amount || !commitments_match);
-                assert!(extra_output_amount.coerce::<u8>() == 0 || !commitments_match);
+                assert_eq!(dbc_amount, amount);
+                assert_eq!(extra_output_amount.coerce::<u8>(), 0);
             }
             Err(Error::MissingSignatureForInput) => {
                 assert!(n_valid_sigs.coerce::<u8>() < n_inputs.coerce::<u8>());
             }
+            Err(Error::MintSignatureInputMismatch) => {
+                assert_ne!(dbc.mint_sigs.len(), n_inputs.coerce::<usize>());
+            }
             Err(Error::SpentProofInputMismatch) => {
                 // todo: fuzz spent proofs.
                 assert!(dbc.spent_proofs.len() < dbc.transaction.mlsags.len());
             }
-            Err(Error::UnknownInput) => {
-                assert!(n_extra_input_sigs.coerce::<u8>() > 0);
-                assert_ne!(
-                    Vec::from_iter(dbc.mint_sigs.keys().cloned()),
-                    dbc.transaction
-                        .mlsags
-                        .iter()
-                        .map(|m| m.key_image.into())
-                        .collect::<Vec<KeyImage>>()
-                );
-            }
             Err(Error::DbcContentNotPresentInTransactionOutput) => {
                 assert!(!dbc
                     .transaction
@@ -631,6 +634,10 @@ pub(crate) mod tests {
                     .values()
                     .any(|(pk, _)| key_manager.verify_known_key(pk).is_err()));
             }
+            Err(Error::AmountCommitmentsDoNotMatch) => {
+                assert_ne!(amount, dbc_amount);
+                assert_ne!(extra_output_amount, TinyInt(0));
+            }
             res => panic!("Unexpected verification result {:?}", res),
         }
 

src/error.rs

@@ -24,9 +24,6 @@ pub enum Error {
     #[error("An error occured when signing {0}")]
     Signing(String),
 
-    #[error("This input has a signature, but it doesn't appear in the transaction")]
-    UnknownInput,
-
     #[error("Failed signature check.")]
     FailedSignature,
 
@@ -36,6 +33,9 @@ pub enum Error {
     #[error("At least one transaction input is missing a signature.")]
     MissingSignatureForInput,
 
+    #[error("The number of mint signatures does not match the number of transaction inputs.")]
+    MintSignatureInputMismatch,
+
     #[error("Invalid SpentProof Signature for {0:?}")]
     InvalidSpentProofSignature(KeyImage),
 

src/verification.rs

@@ -47,14 +47,14 @@ impl TransactionVerifier {
     ) -> Result<(), Error> {
         // Do quick checks first to reduce potential DOS vectors.
 
-        if mint_sigs.len() < transaction.mlsags.len() {
-            return Err(Error::MissingSignatureForInput);
+        if mint_sigs.len() != transaction.mlsags.len() {
+            return Err(Error::MintSignatureInputMismatch);
         }
 
         // 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);
+                return Err(Error::MissingSignatureForInput);
             }
         }