fix: change BTreeSet<SpentProofShare> to HashSet

bug: The ReissueRequestBuilder was only storing 1 SpentProofShare per
KeyImage because the SpentProofShare were being ordered by KeyImage
which is the same for all items in the set for a given input.

The fix is to use a HashSet instead.

changes:

* use HashSet<SpentProofShare> instead of BTreeSet in ReissueRequestBuilder
* add NodeSignature::to_bytes()
* impl (PartialEq, Eq) manually on SpentProofShare
* remove Ord, PartialOrd impls on SpentProofShare
* impl Hash on SpentProofShare
* add SpentProofShare::to_bytes()

src/builder.rs

@@ -222,7 +222,7 @@ impl TransactionBuilder {
 #[derive(Debug)]
 pub struct ReissueRequestBuilder {
     pub transaction: RingCtTransaction,
-    pub spent_proof_shares: BTreeMap<KeyImage, BTreeSet<SpentProofShare>>,
+    pub spent_proof_shares: BTreeMap<KeyImage, HashSet<SpentProofShare>>,
 }
 
 impl ReissueRequestBuilder {
@@ -241,7 +241,6 @@ impl ReissueRequestBuilder {
             .entry(share.key_image().clone())
             .or_default();
         shares.insert(share);
-
         self
     }
 

src/key_manager.rs

@@ -29,6 +29,12 @@ impl NodeSignature {
     pub fn threshold_crypto(&self) -> (u64, &SignatureShare) {
         (self.index, &self.sig)
     }
+
+    pub fn to_bytes(&self) -> Vec<u8> {
+        let mut bytes = self.index.to_le_bytes().to_vec();
+        bytes.extend(&self.sig.to_bytes());
+        bytes
+    }
 }
 
 pub trait KeyManager {

src/spent_proof.rs

@@ -64,7 +64,7 @@ impl Ord for SpentProofContent {
 /// A share of a SpentProof, combine enough of these to form a
 /// SpentProof.
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone)]
 pub struct SpentProofShare {
     /// data to be signed
     pub content: SpentProofContent,
@@ -74,15 +74,21 @@ pub struct SpentProofShare {
     pub spentbook_sig_share: NodeSignature,
 }
 
-impl PartialOrd for SpentProofShare {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
+// impl manually to avoid clippy complaint about Hash conflict.
+impl PartialEq for SpentProofShare {
+    fn eq(&self, other: &Self) -> bool {
+        self.content == other.content
+            && self.spentbook_pks == other.spentbook_pks
+            && self.spentbook_sig_share == other.spentbook_sig_share
     }
 }
 
-impl Ord for SpentProofShare {
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.content.cmp(&other.content)
+impl Eq for SpentProofShare {}
+
+impl std::hash::Hash for SpentProofShare {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        let bytes = self.to_bytes();
+        bytes.hash(state);
     }
 }
 
@@ -111,6 +117,15 @@ impl SpentProofShare {
     pub fn spentbook_pks(&self) -> &PublicKeySet {
         &self.spentbook_pks
     }
+
+    /// represent this SpentProofContent as bytes
+    pub fn to_bytes(&self) -> Vec<u8> {
+        let mut bytes = self.content.to_bytes();
+
+        bytes.extend(&self.spentbook_pks.to_bytes());
+        bytes.extend(self.spentbook_sig_share.to_bytes());
+        bytes
+    }
 }
 
 /// SpentProof's are constructed when a DBC is logged to the spentbook.