refactor!: reducing spent proof verification public API scope to only…

… check it is known key

src/dbc.rs

@@ -638,15 +638,15 @@ pub(crate) mod tests {
             if let Some(spent_proof) = repeating_inputs.next() {
                 let id = crate::bls_dkg_id(&mut rng);
                 let key_manager = mock::KeyManager::from(mock::Signer::from(id));
-                let sig_share = key_manager.sign(&spent_proof.content.hash()).unwrap();
+                let sig_share = key_manager.sign(&spent_proof.content.hash());
                 let sig = key_manager
-                    .public_key_set()?
+                    .public_key_set()
                     .combine_signatures(vec![sig_share.threshold_crypto()])
                     .unwrap();
 
                 let fuzzed_sp = SpentProof {
                     content: spent_proof.content.clone(),
-                    spentbook_pub_key: key_manager.public_key_set()?.public_key(),
+                    spentbook_pub_key: key_manager.public_key_set().public_key(),
                     spentbook_sig: sig,
                 };
                 // note: existing items may be replaced.
@@ -658,11 +658,10 @@ pub(crate) mod tests {
         // Valid spentbook signatures BUT signing wrong message
         for _ in 0..n_wrong_msg_sigs.coerce() {
             if let Some(spent_proof) = repeating_inputs.next() {
-                let wrong_msg_sig_share =
-                    spentbook_node.key_manager.sign(&Hash([0u8; 32])).unwrap();
+                let wrong_msg_sig_share = spentbook_node.key_manager.sign(&Hash([0u8; 32]));
                 let wrong_msg_sig = spentbook_node
                     .key_manager
-                    .public_key_set()?
+                    .public_key_set()
                     .combine_signatures(vec![wrong_msg_sig_share.threshold_crypto()])
                     .unwrap();
 
@@ -689,10 +688,10 @@ pub(crate) mod tests {
                     public_commitments: spent_proof.public_commitments().clone(),
                 };
 
-                let sig_share = spentbook_node.key_manager.sign(&content.hash()).unwrap();
+                let sig_share = spentbook_node.key_manager.sign(&content.hash());
                 let sig = spentbook_node
                     .key_manager
-                    .public_key_set()?
+                    .public_key_set()
                     .combine_signatures(vec![sig_share.threshold_crypto()])
                     .unwrap();
 

src/mock/error.rs

@@ -11,7 +11,7 @@ use thiserror::Error;
 use serde::{Deserialize, Serialize};
 
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Error, Debug, Clone, PartialEq)]
+#[derive(Error, Debug, Clone, PartialEq, Eq)]
 /// Mock error variants.
 pub enum Error {
     #[error("Key image has already been spent")]

src/mock/genesis_builder.rs

@@ -56,15 +56,10 @@ impl GenesisBuilder {
         if !self.spentbook_nodes.is_empty() {
             // we only support a single mock spentbook section.  pubkeys must match.
             assert_eq!(
-                spentbook_node
-                    .key_manager
-                    .public_key_set()
-                    .unwrap()
-                    .public_key(),
+                spentbook_node.key_manager.public_key_set().public_key(),
                 self.spentbook_nodes[0]
                     .key_manager
                     .public_key_set()
-                    .unwrap()
                     .public_key()
             );
         }

src/mock/key_manager.rs

@@ -62,89 +62,47 @@ impl Signer {
 #[derive(Debug, Clone)]
 pub struct KeyManager {
     signer: Signer,
-    cache: Keys,
+    cache: HashSet<PublicKey>,
 }
 
 impl From<Signer> for KeyManager {
     fn from(signer: Signer) -> Self {
         let public_key_set = signer.public_key_set();
-        let mut cache = Keys::default();
-        cache.add_known_key(public_key_set.public_key());
-        Self { signer, cache }
+        let cache = HashSet::default();
+        let mut key_manager = Self { signer, cache };
+        key_manager.add_known_key(public_key_set.public_key());
+
+        key_manager
     }
 }
 
 impl crate::SpentProofKeyVerifier for KeyManager {
     type Error = crate::Error;
 
-    fn verify(&self, msg_hash: &Hash, key: &PublicKey, signature: &Signature) -> Result<()> {
-        self.cache.verify(msg_hash, key, signature)
+    fn verify_known_key(&self, key: &PublicKey) -> Result<()> {
+        if self.cache.contains(key) {
+            Ok(())
+        } else {
+            Err(Error::UnrecognisedAuthority)
+        }
     }
 }
 
 impl KeyManager {
-    pub fn add_known_key(&mut self, key: PublicKey) -> Result<()> {
-        self.cache.add_known_key(key);
-        Ok(())
+    pub fn add_known_key(&mut self, key: PublicKey) {
+        self.cache.insert(key);
     }
 
-    pub fn public_key_set(&self) -> Result<PublicKeySet> {
-        Ok(self.signer.public_key_set())
+    pub fn public_key_set(&self) -> PublicKeySet {
+        self.signer.public_key_set()
     }
 
-    pub fn sign_with_child_key(
-        &self,
-        index: &[u8],
-        tx_hash: &Hash,
-    ) -> Result<IndexedSignatureShare> {
+    pub fn sign_with_child_key(&self, index: &[u8], tx_hash: &Hash) -> IndexedSignatureShare {
         let child_signer = self.signer.derive_child(index);
-        Ok(IndexedSignatureShare::new(
-            child_signer.index(),
-            child_signer.sign(tx_hash),
-        ))
-    }
-
-    pub fn sign(&self, msg_hash: &Hash) -> Result<IndexedSignatureShare> {
-        Ok(IndexedSignatureShare::new(
-            self.signer.index(),
-            self.signer.sign(msg_hash),
-        ))
-    }
-
-    pub fn verify_known_key(&self, key: &PublicKey) -> Result<()> {
-        self.cache.verify_known_key(key)
-    }
-}
-
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Debug, Default, Clone)]
-struct Keys(HashSet<PublicKey>);
-
-impl From<Vec<PublicKey>> for Keys {
-    fn from(keys: Vec<PublicKey>) -> Self {
-        Self(keys.into_iter().collect())
-    }
-}
-
-impl Keys {
-    pub fn add_known_key(&mut self, key: PublicKey) {
-        self.0.insert(key);
+        IndexedSignatureShare::new(child_signer.index(), child_signer.sign(tx_hash))
     }
 
-    fn verify(&self, msg: &Hash, key: &PublicKey, sig: &Signature) -> Result<()> {
-        self.verify_known_key(key)?;
-        if key.verify(sig, msg) {
-            Ok(())
-        } else {
-            Err(Error::FailedSignature)
-        }
-    }
-
-    fn verify_known_key(&self, key: &PublicKey) -> Result<()> {
-        if self.0.contains(key) {
-            Ok(())
-        } else {
-            Err(Error::UnrecognisedAuthority)
-        }
+    pub fn sign(&self, msg_hash: &Hash) -> IndexedSignatureShare {
+        IndexedSignatureShare::new(self.signer.index(), self.signer.sign(msg_hash))
     }
 }

src/mock/spentbook.rs

@@ -199,8 +199,8 @@ impl SpentBookNode {
                 public_commitments,
             };
 
-            let spentbook_pks = self.key_manager.public_key_set()?;
-            let spentbook_sig_share = self.key_manager.sign(&sp_content.hash())?;
+            let spentbook_pks = self.key_manager.public_key_set();
+            let spentbook_sig_share = self.key_manager.sign(&sp_content.hash());
 
             Ok(SpentProofShare {
                 content: sp_content,

src/owner.rs

@@ -139,7 +139,7 @@ impl From<OwnerOnce> for Owner {
 /// The one-time-use Owner key(pair) is derived from a reusable
 /// base Owner key(pair) using the DerivationIndex.
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct OwnerOnce {
     pub owner_base: Owner,
     pub derivation_index: DerivationIndex,

src/spent_proof.rs

@@ -158,12 +158,7 @@ impl SpentProofShare {
 /// the pubkey of the spentbook section that signed each of the proofs.
 pub trait SpentProofKeyVerifier {
     type Error: std::error::Error;
-    fn verify(
-        &self,
-        msg_hash: &Hash,
-        key: &PublicKey,
-        signature: &Signature,
-    ) -> Result<(), Self::Error>;
+    fn verify_known_key(&self, key: &PublicKey) -> Result<(), Self::Error>;
 }
 
 /// SpentProof's are constructed when a DBC is logged to the spentbook.
@@ -257,13 +252,20 @@ impl SpentProof {
             return Err(Error::InvalidTransactionHash);
         }
 
+        let pub_key = &self.spentbook_pub_key;
+
+        if !pub_key.verify(&self.spentbook_sig, &self.content.hash()) {
+            return Err(Error::InvalidSpentProofSignature(
+                *self.key_image(),
+                format!(
+                    "Failed to verify SpentProof signature with key: {}",
+                    pub_key.to_hex()
+                ),
+            ));
+        }
+
         proof_key_verifier
-            .verify(
-                &self.content.hash(),
-                &self.spentbook_pub_key,
-                &self.spentbook_sig,
-            )
-            .map_err(|e| Error::InvalidSpentProofSignature(*self.key_image(), e.to_string()))?;
-        Ok(())
+            .verify_known_key(pub_key)
+            .map_err(|err| Error::InvalidSpentProofSignature(*self.key_image(), err.to_string()))
     }
 }