Use HistoryEntry instead of {Confirmed,Mempool}Entry

Should simplify JSON serialization and height computation.

src/electrum.rs

@@ -256,7 +256,7 @@ impl Rpc {
                 self.tracker.get_history(&self.new_status(scripthash)?)
             }
         };
-        Ok(json!(history_entries.collect::<Vec<Value>>()))
+        Ok(json!(history_entries))
     }
 
     fn scripthash_subscribe(

src/status.rs

@@ -4,7 +4,7 @@ use bitcoin::{
     Amount, Block, BlockHash, OutPoint, SignedAmount, Transaction, Txid,
 };
 use rayon::prelude::*;
-use serde_json::{json, Value};
+use serde::ser::{Serialize, Serializer};
 
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::convert::TryFrom;
@@ -45,44 +45,74 @@ impl TxEntry {
     }
 }
 
-pub(crate) struct ConfirmedEntry {
+enum Height {
-    txid: Txid,
+    Confirmed { height: usize },
-    height: usize,
+    Unconfirmed { has_unconfirmed_inputs: bool },
 }
 
-impl ConfirmedEntry {
+impl Height {
-    pub fn hash(&self, engine: &mut sha256::HashEngine) {
+    fn as_i64(&self) -> i64 {
+        match self {
+            Self::Confirmed { height } => i64::try_from(*height).unwrap(),
+            Self::Unconfirmed {
+                has_unconfirmed_inputs: true,
+            } => -1,
+            Self::Unconfirmed {
+                has_unconfirmed_inputs: false,
+            } => 0,
+        }
+    }
+}
+
+impl Serialize for Height {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.serialize_i64(self.as_i64())
+    }
+}
+
+impl std::fmt::Display for Height {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.as_i64().fmt(f)
+    }
+}
+
+#[derive(Serialize)]
+pub(crate) struct HistoryEntry {
+    #[serde(rename = "tx_hash")]
+    txid: Txid,
+    height: Height,
+    #[serde(
+        skip_serializing_if = "Option::is_none",
+        with = "bitcoin::util::amount::serde::as_sat::opt"
+    )]
+    fee: Option<Amount>,
+}
+
+impl HistoryEntry {
+    fn hash(&self, engine: &mut sha256::HashEngine) {
         let s = format!("{}:{}:", self.txid, self.height);
         engine.input(s.as_bytes());
     }
 
-    pub fn value(&self) -> Value {
+    fn confirmed(txid: Txid, height: usize) -> Self {
-        json!({"tx_hash": self.txid, "height": self.height})
+        Self {
+            txid,
+            height: Height::Confirmed { height },
+            fee: None,
         }
     }
 
-pub(crate) struct MempoolEntry {
+    fn unconfirmed(txid: Txid, has_unconfirmed_inputs: bool, fee: Amount) -> Self {
-    txid: Txid,
+        Self {
-    has_unconfirmed_inputs: bool,
+            txid,
-    fee: Amount,
+            height: Height::Unconfirmed {
+                has_unconfirmed_inputs,
+            },
+            fee: Some(fee),
         }
-
-impl MempoolEntry {
-    fn height(&self) -> isize {
-        if self.has_unconfirmed_inputs {
-            -1
-        } else {
-            0
-        }
-    }
-
-    pub fn hash(&self, engine: &mut sha256::HashEngine) {
-        let s = format!("{}:{}:", self.txid, self.height());
-        engine.input(s.as_bytes());
-    }
-
-    pub fn value(&self) -> Value {
-        json!({"tx_hash": self.txid, "height": self.height(), "fee": self.fee.as_sat()})
     }
 }
 
@@ -204,20 +234,25 @@ impl Status {
         }
     }
 
-    pub(crate) fn get_confirmed(&self, chain: &Chain) -> Vec<ConfirmedEntry> {
+    pub(crate) fn get_history(&self, chain: &Chain, mempool: &Mempool) -> Vec<HistoryEntry> {
+        let mut result = self.get_confirmed(chain);
+        result.extend(self.get_mempool(mempool));
+        result
+    }
+
+    fn get_confirmed(&self, chain: &Chain) -> Vec<HistoryEntry> {
         self.confirmed_entries(chain)
             .collect::<BTreeMap<usize, &[TxEntry]>>()
             .into_iter()
             .flat_map(|(height, entries)| {
-                entries.iter().map(move |e| ConfirmedEntry {
+                entries
-                    txid: e.txid,
+                    .iter()
-                    height,
+                    .map(move |e| HistoryEntry::confirmed(e.txid, height))
-                })
             })
             .collect()
     }
 
-    pub(crate) fn get_mempool(&self, mempool: &Mempool) -> Vec<MempoolEntry> {
+    fn get_mempool(&self, mempool: &Mempool) -> Vec<HistoryEntry> {
         let mut entries = self
             .mempool
             .iter()
@@ -226,11 +261,7 @@ impl Status {
         entries.sort_by_key(|e| (e.has_unconfirmed_inputs, e.txid));
         entries
             .into_iter()
-            .map(|e| MempoolEntry {
+            .map(|e| HistoryEntry::unconfirmed(e.txid, e.has_unconfirmed_inputs, e.fee))
-                txid: e.txid,
-                has_unconfirmed_inputs: e.has_unconfirmed_inputs,
-                fee: e.fee,
-            })
             .collect()
     }
 
@@ -417,3 +448,33 @@ fn filter_inputs(tx: &Transaction, outpoints: &HashSet<OutPoint>) -> Vec<OutPoin
         })
         .collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::HistoryEntry;
+    use bitcoin::{hashes::hex::FromHex, Amount, Txid};
+    use serde_json::json;
+
+    #[test]
+    fn test_txinfo_json() {
+        let txid =
+            Txid::from_hex("5b75086dafeede555fc8f9a810d8b10df57c46f9f176ccc3dd8d2fa20edd685b")
+                .unwrap();
+        assert_eq!(
+            json!(HistoryEntry::confirmed(txid, 123456)),
+            json!({"tx_hash": "5b75086dafeede555fc8f9a810d8b10df57c46f9f176ccc3dd8d2fa20edd685b", "height": 123456})
+        );
+        assert_eq!(
+            json!(HistoryEntry::unconfirmed(txid, true, Amount::from_sat(123))),
+            json!({"tx_hash": "5b75086dafeede555fc8f9a810d8b10df57c46f9f176ccc3dd8d2fa20edd685b", "height": -1, "fee": 123})
+        );
+        assert_eq!(
+            json!(HistoryEntry::unconfirmed(
+                txid,
+                false,
+                Amount::from_sat(123)
+            )),
+            json!({"tx_hash": "5b75086dafeede555fc8f9a810d8b10df57c46f9f176ccc3dd8d2fa20edd685b", "height": 0, "fee": 123})
+        );
+    }
+}

src/tracker.rs

@@ -1,6 +1,5 @@
 use anyhow::{Context, Result};
 use bitcoin::{BlockHash, OutPoint, Txid};
-use serde_json::Value;
 
 use std::convert::TryInto;
 use std::path::Path;
@@ -14,7 +13,7 @@ use crate::{
     index::Index,
     mempool::{Histogram, Mempool},
     metrics::Metrics,
-    status::{Balance, Status},
+    status::{Balance, HistoryEntry, Status},
 };
 
 /// Electrum protocol subscriptions' tracker
@@ -52,16 +51,8 @@ impl Tracker {
         &self.metrics
     }
 
-    pub fn get_history(&self, status: &Status) -> impl Iterator<Item = Value> {
+    pub(crate) fn get_history(&self, status: &Status) -> Vec<HistoryEntry> {
-        let confirmed = status
+        status.get_history(self.index.chain(), &self.mempool)
-            .get_confirmed(self.index.chain())
-            .into_iter()
-            .map(|entry| entry.value());
-        let mempool = status
-            .get_mempool(&self.mempool)
-            .into_iter()
-            .map(|entry| entry.value());
-        confirmed.chain(mempool)
     }
 
     pub fn sync(&mut self, daemon: &Daemon) -> Result<()> {