fix: verify Tx in DbcBuilder::build()

Previously DbcBuilder::build() was only doing some incomplete
validations.  This commit verifies the Tx which also requires
that DbcBuilder::build() accept a KeyManager param, which has
mintnode/spentbooknode trust implications/requirements that
are now documented.

* doc: add doc comment for TransactionVerifier::verify() and
       ::verify_without_sigs()
* rename mint_verifier param to verifier
* doc: add doc comment for Dbc::verify()
* optimization: iter/collect() mint sigs once per tx, not per Dbc
* add KeyManager arg to DbcBuilder::build()
* call TransactionVerifier::verify() within DbcBuilder::build()
* adapt tests, examples, benches to pass key_manager into ::build()

benches/reissue.rs

@@ -108,7 +108,7 @@ fn bench_reissue_100_to_1(c: &mut Criterion) {
     let reissue_share = mintnode.reissue(rr).unwrap();
 
     dbc_builder = dbc_builder.add_reissue_share(reissue_share);
-    let dbcs = dbc_builder.build().unwrap();
+    let dbcs = dbc_builder.build(mintnode.key_manager()).unwrap();
 
     let output_owner_once =
         OwnerOnce::from_owner_base(Owner::from_random_secret_key(&mut rng), &mut rng8);
@@ -187,7 +187,11 @@ fn generate_dbc_of_value(
 
     let reissue_share = mint_node.reissue(rr)?;
     dbc_builder = dbc_builder.add_reissue_share(reissue_share);
-    let (starting_dbc, ..) = dbc_builder.build()?.into_iter().next().unwrap();
+    let (starting_dbc, ..) = dbc_builder
+        .build(mint_node.key_manager())?
+        .into_iter()
+        .next()
+        .unwrap();
 
     Ok((mint_node, spentbook_node, starting_dbc))
 }

examples/mint-repl/mint-repl.rs

@@ -843,7 +843,7 @@ fn reissue_auto_cli(mintinfo: &mut MintInfo) -> Result<()> {
         for mint_node in mintinfo.mintnodes.iter() {
             dbc_builder = dbc_builder.add_reissue_share(mint_node.reissue(rr.clone())?);
         }
-        let outputs = dbc_builder.build()?;
+        let outputs = dbc_builder.build(mintinfo.mintnodes[0].key_manager())?;
         let output_dbcs: Vec<Dbc> = outputs.into_iter().map(|(dbc, ..)| dbc).collect();
 
         for dbc in input_dbcs.iter() {
@@ -895,7 +895,7 @@ fn reissue(mintinfo: &mut MintInfo, reissue_request: ReissueRequestRevealed) ->
         dbc_builder = dbc_builder.add_reissue_share(reissue_share);
     }
 
-    let output_dbcs = dbc_builder.build()?;
+    let output_dbcs = dbc_builder.build(mintinfo.mintnodes[0].key_manager())?;
 
     // for each output, construct Dbc and display
     for (dbc, _owner_once, _amount_secrets) in output_dbcs.iter() {

src/builder.rs

@@ -15,9 +15,9 @@ use rand_core::RngCore;
 use std::collections::{BTreeMap, BTreeSet, HashSet};
 
 use crate::{
-    Amount, AmountSecrets, Commitment, Dbc, DbcContent, Error, Hash, KeyImage, NodeSignature,
-    OwnerOnce, PublicKeyBlst, PublicKeyBlstMappable, ReissueRequest, ReissueShare, Result,
-    SecretKeyBlst, SpentProof, SpentProofContent, SpentProofShare,
+    Amount, AmountSecrets, Commitment, Dbc, DbcContent, Error, Hash, KeyImage, KeyManager,
+    NodeSignature, OwnerOnce, PublicKeyBlst, PublicKeyBlstMappable, ReissueRequest, ReissueShare,
+    Result, SecretKeyBlst, SpentProof, SpentProofContent, SpentProofShare, TransactionVerifier,
 };
 
 #[cfg(feature = "serde")]
@@ -342,7 +342,16 @@ impl DbcBuilder {
     }
 
     /// Build the output DBCs
-    pub fn build(self) -> Result<Vec<(Dbc, OwnerOnce, AmountSecrets)>> {
+    ///
+    /// 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.
+    pub fn build<K: KeyManager>(
+        self,
+        mint_verifier: &K,
+    ) -> Result<Vec<(Dbc, OwnerOnce, AmountSecrets)>> {
         let mut mint_sig_shares: Vec<NodeSignature> = Default::default();
         let mut pk_set: HashSet<PublicKeySet> = Default::default();
 
@@ -388,6 +397,22 @@ impl DbcBuilder {
         // 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)?;
 
+        // note: all output Dbcs share the same mint_sigs
+        let mint_sigs = transaction
+            .mlsags
+            .iter()
+            .map(|mlsag| {
+                (
+                    mlsag.key_image.into(),
+                    (mint_public_key_set.public_key(), mint_sig.clone()),
+                )
+            })
+            .collect();
+
+        // 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)?;
+
         let pc_gens = PedersenGens::default();
         let output_commitments: Vec<(Commitment, RevealedCommitment)> = self
             .revealed_commitments
@@ -425,16 +450,7 @@ impl DbcBuilder {
                         amount_secrets_list[0].clone(),
                     )),
                     transaction: transaction.clone(),
-                    mint_sigs: transaction
-                        .mlsags
-                        .iter()
-                        .map(|mlsag| {
-                            (
-                                mlsag.key_image.into(),
-                                (mint_public_key_set.public_key(), mint_sig.clone()),
-                            )
-                        })
-                        .collect(),
+                    mint_sigs: mint_sigs.clone(),
                     spent_proofs: spent_proofs.clone(),
                 };
                 (dbc, owner_once.clone(), amount_secrets_list[0].clone())
@@ -611,8 +627,15 @@ pub mod mock {
                 mint_nodes.push(mint_node);
             }
 
-            let (genesis_dbc, _owner_once, amount_secrets) =
-                dbc_builder.build()?.into_iter().next().unwrap();
+            // note: for our (mock) purposes, all mint nodes are verified to
+            // have the same public key.  (in the same section)
+            let mint_node_arbitrary = &mint_nodes[0];
+
+            let (genesis_dbc, _owner_once, amount_secrets) = dbc_builder
+                .build(mint_node_arbitrary.key_manager())?
+                .into_iter()
+                .next()
+                .unwrap();
             Ok((
                 mint_nodes,
                 self.spentbook_nodes,

src/dbc.rs

@@ -184,17 +184,19 @@ impl Dbc {
         hash
     }
 
-    // Check there exists a Transaction with the output containing this Dbc
-    // note: common verification logic is shared by MintNode::reissue() and Dbc::verify()
-    //
-    // note: for spent_proofs to verify, the mint_verifier must have/know the spentbook section's public key.
-    pub fn verify<K: KeyManager>(
-        &self,
-        base_sk: &SecretKey,
-        mint_verifier: &K,
-    ) -> Result<(), Error> {
+    /// Verifies that this Dbc is valid.
+    ///
+    /// important: this does not check if the Dbc has been spent.
+    /// For that, one must query the SpentBook.
+    ///
+    /// note: common verification logic is shared by MintNode::reissue(),
+    /// DbcBuilder::build() and Dbc::verify()
+    ///
+    /// see TransactionVerifier::verify() for a description of
+    /// verifier requirements.
+    pub fn verify<K: KeyManager>(&self, base_sk: &SecretKey, verifier: &K) -> Result<(), Error> {
         TransactionVerifier::verify(
-            mint_verifier,
+            verifier,
             &self.transaction,
             &self.mint_sigs,
             &self.spent_proofs,
@@ -423,7 +425,7 @@ pub(crate) mod tests {
         let split_reissue_share = mint_node.reissue(reissue_request)?;
 
         dbc_builder = dbc_builder.add_reissue_share(split_reissue_share);
-        let output_dbcs = dbc_builder.build()?;
+        let output_dbcs = dbc_builder.build(mint_node.key_manager())?;
 
         // The outputs become inputs for next reissue.
         let inputs: Vec<(Dbc, SecretKey, Vec<DecoyInput>)> = output_dbcs
@@ -675,7 +677,7 @@ pub(crate) mod tests {
         let reissue_share = mint_node.reissue(rr)?;
         dbc_builder = dbc_builder.add_reissue_share(reissue_share);
 
-        let mut iter = dbc_builder.build()?.into_iter();
+        let mut iter = dbc_builder.build(mint_node.key_manager())?.into_iter();
         let (starting_dbc, ..) = iter.next().unwrap();
         let (change_dbc, ..) = iter.next().unwrap();
 

src/mint.rs

@@ -262,7 +262,7 @@ mod tests {
 
         // Aggregate ReissueShare to build output DBCs
         dbc_builder = dbc_builder.add_reissue_share(reissue_share);
-        let output_dbcs = dbc_builder.build()?;
+        let output_dbcs = dbc_builder.build(mint_node.key_manager())?;
 
         for (dbc, owner_once, amount_secrets) in output_dbcs.iter() {
             let dbc_amount = amount_secrets.amount();
@@ -387,7 +387,7 @@ mod tests {
 
         // Aggregate ReissueShare to build output DBCs
         dbc_builder = dbc_builder.add_reissue_share(reissue_share);
-        let output_dbcs = dbc_builder.build()?;
+        let output_dbcs = dbc_builder.build(mint_node.key_manager())?;
 
         // The outputs become inputs for next reissue.
         let inputs_dbcs: Vec<(Dbc, SecretKey, Vec<DecoyInput>)> = output_dbcs
@@ -534,7 +534,7 @@ mod tests {
 
                 // Aggregate ReissueShare to build output DBCs
                 dbc_builder = dbc_builder.add_reissue_share(rs);
-                let output_dbcs = dbc_builder.build()?;
+                let output_dbcs = dbc_builder.build(mint_node.key_manager())?;
 
                 for (dbc, owner_once, _amount_secrets) in output_dbcs.iter() {
                     let dbc_confirm_result = dbc.verify(
@@ -703,7 +703,7 @@ mod tests {
 
         // Aggregate ReissueShare to build output DBCs
         dbc_builder = dbc_builder.add_reissue_share(reissue_share);
-        let output_dbcs = dbc_builder.build()?;
+        let output_dbcs = dbc_builder.build(mint_node.key_manager())?;
         let (b_dbc, ..) = &output_dbcs[0];
 
         // ----------

src/verification.rs

@@ -27,9 +27,20 @@ use std::collections::{BTreeMap, BTreeSet};
 pub struct TransactionVerifier {}
 
 impl TransactionVerifier {
-    // note: for spent_proofs to verify, the mint_verifier must have/know the spentbook section's public key.
+    /// Verifies a transaction including mint signatures and spent proofs.
+    ///
+    /// 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: for spent_proofs to verify, the verifier must have/know the
+    ///       public key of each spentbook section that recorded a tx input as spent.
+    /// note: for mint_sigs to verify, the verifier must have/know the
+    ///       public key of the mint section that performed the reissue and signed tx.
     pub fn verify<K: KeyManager>(
-        mint_verifier: &K,
+        verifier: &K,
         transaction: &RingCtTransaction,
         mint_sigs: &BTreeMap<KeyImage, (PublicKey, Signature)>,
         spent_proofs: &BTreeSet<SpentProof>,
@@ -42,6 +53,10 @@ impl TransactionVerifier {
 
         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
@@ -52,25 +67,35 @@ impl TransactionVerifier {
                 return Err(Error::UnknownInput);
             }
 
-            mint_verifier
+            verifier
                 .verify(&tx_hash, mint_key, mint_sig)
                 .map_err(|e| Error::Signing(e.to_string()))?;
         }
 
-        Self::verify_without_sigs_internal(mint_verifier, transaction, tx_hash, spent_proofs)
+        Self::verify_without_sigs_internal(verifier, transaction, tx_hash, spent_proofs)
     }
 
+    /// Verifies a transaction including including spent proofs and excluding mint signatures.
+    ///
+    /// This function relies/assumes that the caller (wallet/client) obtains
+    /// spentbook's public keys (held by KeyManager) in a
+    /// trustless/verified way.  ie, the caller should not simply obtain keys
+    /// from a SpentBookNode directly, but must somehow verify that the
+    /// SpentBookNode is a valid authority.
+    ///
+    /// note: for spent_proofs to verify, the verifier must have/know the
+    ///       public key of each spentbook section that recorded a tx input as spent.
     pub fn verify_without_sigs<K: KeyManager>(
-        mint_verifier: &K,
+        verifier: &K,
         transaction: &RingCtTransaction,
         spent_proofs: &BTreeSet<SpentProof>,
     ) -> Result<(), Error> {
         let tx_hash = Hash::from(transaction.hash());
-        Self::verify_without_sigs_internal(mint_verifier, transaction, tx_hash, spent_proofs)
+        Self::verify_without_sigs_internal(verifier, transaction, tx_hash, spent_proofs)
     }
 
     fn verify_without_sigs_internal<K: KeyManager>(
-        mint_verifier: &K,
+        verifier: &K,
         transaction: &RingCtTransaction,
         transaction_hash: Hash,
         spent_proofs: &BTreeSet<SpentProof>,
@@ -102,7 +127,7 @@ impl TransactionVerifier {
             {
                 return Err(Error::SpentProofInputMismatch);
             }
-            spent_proof.verify(transaction_hash, mint_verifier)?;
+            spent_proof.verify(transaction_hash, verifier)?;
         }
 
         // We must get the spent_proofs into the same order as mlsags