fix: refactor to include tx_hash in spentbook sig

* add SpentProofContent to represent all data to be signed by SpentBook
* SpentProof[Share] now includes transaction_hash in spentbook's
  signature
* refactor SpentProofShare and SpentProof to use SpentProofContent
* add doc comments for SpentProof{*]
* add Error variant InvalidTransactionHash for case when input tx does
  not match tx signed by spentbook
* change ReissueRequestBuilder::spent_proof_shares map value from HashSet
  to BTreeSet so we do not need to impl Hash for SpentProofShare

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, NodeSignature, OwnerOnce,
+    Amount, AmountSecrets, Commitment, Dbc, DbcContent, Error, Hash, NodeSignature, OwnerOnce,
     PublicKeyBlst, PublicKeyBlstMappable, ReissueRequest, ReissueShare, Result, SecretKeyBlst,
-    SpentProof, SpentProofShare,
+    SpentProof, SpentProofContent, SpentProofShare,
 };
 
 #[cfg(feature = "serde")]
@@ -222,7 +222,7 @@ impl TransactionBuilder {
 #[derive(Debug)]
 pub struct ReissueRequestBuilder {
     pub transaction: RingCtTransaction,
-    pub spent_proof_shares: BTreeMap<usize, HashSet<SpentProofShare>>,
+    pub spent_proof_shares: BTreeMap<usize, BTreeSet<SpentProofShare>>,
 }
 
 impl ReissueRequestBuilder {
@@ -274,27 +274,30 @@ impl ReissueRequestBuilder {
 
                 if shares
                     .iter()
-                    .map(|s| &s.public_commitments)
-                    .any(|pc| *pc != any_share.public_commitments)
+                    .map(|s| s.public_commitments())
+                    .any(|pc| pc != any_share.public_commitments())
                 {
                     return Err(Error::ReissueRequestPublicCommitmentMismatch);
                 }
 
-                let spentbook_pub_key = any_share.spentbook_public_key();
+                let spentbook_pub_key = any_share.spentbook_pks().public_key();
                 let spentbook_sig = any_share.spentbook_pks.combine_signatures(
                     shares
                         .iter()
                         .map(SpentProofShare::spentbook_sig_share)
                         .map(NodeSignature::threshold_crypto),
                 )?;
 
-                let public_commitments: Vec<Commitment> = any_share.public_commitments.clone();
+                let public_commitments: Vec<Commitment> = any_share.public_commitments().clone();
 
                 let spent_proof = SpentProof {
-                    key_image: any_share.key_image.clone(),
+                    content: SpentProofContent {
+                        key_image: any_share.key_image().clone(),
+                        transaction_hash: Hash::from(self.transaction.hash()),
+                        public_commitments,
+                    },
                     spentbook_pub_key,
                     spentbook_sig,
-                    public_commitments,
                 };
 
                 Ok(spent_proof)
@@ -477,8 +480,8 @@ impl DbcBuilder {
 // SpentBookNodeMock, SimpleKeyManager, SimpleSigner, GenesisBuilderMock
 pub mod mock {
     use crate::{
-        Amount, Dbc, GenesisDbcShare, KeyManager, MintNode, Result, SimpleKeyManager, SimpleSigner,
-        SpentBookNodeMock, SpentProof, SpentProofShare,
+        Amount, Dbc, GenesisDbcShare, Hash, KeyManager, MintNode, Result, SimpleKeyManager,
+        SimpleSigner, SpentBookNodeMock, SpentProof, SpentProofContent, SpentProofShare,
     };
     use blsttc::{SecretKeySet, SignatureShare};
     use std::collections::{BTreeMap, BTreeSet};
@@ -670,11 +673,16 @@ pub mod mock {
                 .spentbook_pks
                 .combine_signatures(spent_sigs)?;
 
+            let transaction_hash = Hash::from(genesis_dbc_share_arbitrary.transaction.hash());
+
             let spent_proofs = BTreeSet::from_iter([SpentProof {
-                key_image: spent_proof_share_arbitrary.key_image.clone(),
+                content: SpentProofContent {
+                    key_image: spent_proof_share_arbitrary.key_image().clone(),
+                    transaction_hash,
+                    public_commitments: spent_proof_share_arbitrary.public_commitments().clone(),
+                },
                 spentbook_pub_key: spent_proof_share_arbitrary.spentbook_pks.public_key(),
                 spentbook_sig,
-                public_commitments: spent_proof_share_arbitrary.public_commitments.clone(),
             }]);
 
             // Create the Genesis Dbc

src/dbc.rs

@@ -432,10 +432,9 @@ mod tests {
         )?;
 
         let sp = reissue_request.spent_proofs.iter().next().unwrap();
-        assert!(sp.spentbook_pub_key.verify(
-            &sp.spentbook_sig,
-            Hash::from(reissue_request.transaction.hash())
-        ));
+        assert!(sp
+            .spentbook_pub_key
+            .verify(&sp.spentbook_sig, sp.content.hash()));
 
         let split_reissue_share = mint_node.reissue(reissue_request)?;
 

src/error.rs

@@ -39,6 +39,9 @@ pub enum Error {
     #[error("Invalid SpentProof Signature for {0:?}")]
     InvalidSpentProofSignature(KeyImage),
 
+    #[error("Transaction hash does not match the transaction signed by spentbook")]
+    InvalidTransactionHash,
+
     #[error("The DBC transaction must have at least one input")]
     TransactionMustHaveAnInput,
 

src/lib.rs

@@ -38,7 +38,7 @@ pub use crate::{
     },
     mint::{GenesisDbcShare, MintNode, MintNodeSignatures, ReissueRequest, ReissueShare},
     owner::{DerivationIndex, Owner, OwnerOnce},
-    spent_proof::{SpentProof, SpentProofShare},
+    spent_proof::{SpentProof, SpentProofContent, SpentProofShare},
     spentbook::SpentBookNodeMock,
     validation::TransactionValidator,
 };

src/mint.rs

@@ -260,7 +260,7 @@ mod tests {
     use crate::{
         tests::{TinyInt, TinyVec},
         AmountSecrets, Dbc, DbcBuilder, GenesisBuilderMock, OwnerOnce, ReissueRequestBuilder,
-        SimpleKeyManager, SimpleSigner, SpentBookNodeMock,
+        SimpleKeyManager, SimpleSigner, SpentBookNodeMock, SpentProofContent,
     };
 
     #[test]
@@ -598,8 +598,11 @@ mod tests {
                             Err(e) => return check_error(e),
                         };
                     SpentProofShare {
-                        key_image: spent_proof_share.key_image,
-                        public_commitments: spent_proof_share.public_commitments,
+                        content: SpentProofContent {
+                            key_image: spent_proof_share.key_image().clone(),
+                            transaction_hash: spent_proof_share.transaction_hash(),
+                            public_commitments: spent_proof_share.public_commitments().clone(),
+                        },
                         spentbook_pks: spent_proof_share.spentbook_pks,
                         spentbook_sig_share: NodeSignature::new(
                             0,

src/spent_proof.rs

@@ -12,110 +12,169 @@ use crate::{
 };
 
 use std::cmp::Ordering;
-use std::hash;
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+/// Represents the data to be signed by the SpentBook in a SpentProof.
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct SpentProofContent {
+    /// KeyImage of input Dbc that this SpentProof is proving to be spent.
+    pub key_image: KeyImage,
+
+    /// Hash of transaction that input Dbc is being spent in.
+    pub transaction_hash: Hash,
+
+    /// public commitments for the transaction
+    pub public_commitments: Vec<Commitment>,
+}
+
+impl SpentProofContent {
+    /// represent this SpentProofContent as bytes
+    pub fn to_bytes(&self) -> Vec<u8> {
+        let mut bytes: Vec<u8> = Default::default();
+
+        bytes.extend(&self.key_image.to_bytes());
+        bytes.extend(self.transaction_hash.as_ref());
+        for pc in self.public_commitments.iter() {
+            bytes.extend(&pc.to_compressed());
+        }
+        bytes
+    }
+
+    /// represent this SpentProofContent as a Hash
+    pub fn hash(&self) -> Hash {
+        Hash::hash(&self.to_bytes())
+    }
+}
+
+impl PartialOrd for SpentProofContent {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for SpentProofContent {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.key_image.cmp(&other.key_image)
+    }
+}
+
 /// A share of a SpentProof, combine enough of these to form a
 /// SpentProof.
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct SpentProofShare {
-    pub key_image: KeyImage,
+    /// data to be signed
+    pub content: SpentProofContent,
 
     /// The Spentbook who notarized that this DBC was spent.
     pub spentbook_pks: PublicKeySet,
-
     pub spentbook_sig_share: NodeSignature,
-
-    pub public_commitments: Vec<Commitment>,
 }
 
-impl PartialEq for SpentProofShare {
-    fn eq(&self, other: &Self) -> bool {
-        self.spentbook_pks == other.spentbook_pks
-            && self.spentbook_sig_share == other.spentbook_sig_share
-            && self.public_commitments == other.public_commitments
+impl PartialOrd for SpentProofShare {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
     }
 }
 
-impl Eq for SpentProofShare {}
-
-impl hash::Hash for SpentProofShare {
-    fn hash<H: hash::Hasher>(&self, state: &mut H) {
-        self.key_image.hash(state);
-        self.spentbook_pks.hash(state);
-        self.spentbook_sig_share.hash(state);
-        for pc in self.public_commitments.iter() {
-            let bytes = pc.to_compressed();
-            bytes.hash(state);
-        }
+impl Ord for SpentProofShare {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.content.cmp(&other.content)
     }
 }
 
 impl SpentProofShare {
+    /// get KeyImage of input Dbc
+    pub fn key_image(&self) -> &KeyImage {
+        &self.content.key_image
+    }
+
+    /// get transaction hash
+    pub fn transaction_hash(&self) -> Hash {
+        self.content.transaction_hash
+    }
+
+    /// get public commitments
+    pub fn public_commitments(&self) -> &Vec<Commitment> {
+        &self.content.public_commitments
+    }
+
+    /// get spentbook's signature share
     pub fn spentbook_sig_share(&self) -> &NodeSignature {
         &self.spentbook_sig_share
     }
 
+    /// get spentbook's PublicKeySet
     pub fn spentbook_pks(&self) -> &PublicKeySet {
         &self.spentbook_pks
     }
-
-    pub fn spentbook_public_key(&self) -> PublicKey {
-        self.spentbook_pks.public_key()
-    }
 }
 
 /// SpentProof's are constructed when a DBC is logged to the spentbook.
-// #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-// #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
-
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub struct SpentProof {
-    pub key_image: KeyImage,
+    /// data to be signed
+    pub content: SpentProofContent,
 
     /// The Spentbook who notarized that this DBC was spent.
     pub spentbook_pub_key: PublicKey,
 
     /// The Spentbook's signature notarizing the DBC was spent.
-    /// signing over RingCtTransaction, spent_sig, and public_commitments.
+    /// signing over SpentProofContent. (KeyImage, RingCtTransaction, and public_commitments).
     pub spentbook_sig: Signature,
-
-    pub public_commitments: Vec<Commitment>,
 }
 
 impl SpentProof {
+    /// get KeyImage of input Dbc
+    pub fn key_image(&self) -> &KeyImage {
+        &self.content.key_image
+    }
+
+    /// get transaction hash
+    pub fn transaction_hash(&self) -> Hash {
+        self.content.transaction_hash
+    }
+
+    /// get public commitments
+    pub fn public_commitments(&self) -> &Vec<Commitment> {
+        &self.content.public_commitments
+    }
+
+    /// represent this SpentProof as bytes
     pub fn to_bytes(&self) -> Vec<u8> {
         let mut bytes: Vec<u8> = Default::default();
 
-        bytes.extend(&self.key_image.to_bytes());
+        bytes.extend(&self.content.to_bytes());
+        bytes.extend(&self.spentbook_pub_key.to_bytes());
         bytes.extend(&self.spentbook_sig.to_bytes());
 
-        for pc in self.public_commitments.iter() {
-            bytes.extend(&pc.to_compressed());
-        }
         bytes
     }
 
-    pub fn validate<K: KeyManager>(&self, tx: Hash, verifier: &K) -> Result<()> {
+    /// validate this SpentProof
+    ///
+    /// checks that the input transaction hash matches the tx_hash that was
+    /// signed by the spentbook and validates that spentbook signature is
+    /// valid for this SpentProof.
+    ///
+    /// note that the verifier must already hold (trust) the spentbook's public key.
+    pub fn validate<K: KeyManager>(&self, tx_hash: Hash, verifier: &K) -> Result<()> {
+        // verify input tx_hash matches our tx_hash which was signed by spentbook.
+        if tx_hash != self.content.transaction_hash {
+            return Err(Error::InvalidTransactionHash);
+        }
+
         verifier
-            .verify(&tx, &self.spentbook_pub_key, &self.spentbook_sig)
-            .map_err(|_| Error::InvalidSpentProofSignature(self.key_image.clone()))?;
+            .verify(
+                &self.content.hash(),
+                &self.spentbook_pub_key,
+                &self.spentbook_sig,
+            )
+            .map_err(|_| Error::InvalidSpentProofSignature(self.key_image().clone()))?;
         Ok(())
     }
 }
-
-impl PartialOrd for SpentProof {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for SpentProof {
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.key_image.cmp(&other.key_image)
-    }
-}