Merge #359: Add Deserialize macro to commands response

24eef7d Add Clone to config::Config (edouard)
ac38367 Fix command get_spend_tx for consistency (edouard)
7091708 Fix command get_history for consistency (edouard)
0162c3b Fix command list_vaults for consistency (edouard)
23eefed Implem Display for HistoryEventKind (edouard)
fb2fcf3 Add Clone macro to commands structs (edouard)
f2b3446 commands: expose ServersStatuses fields (edouard)
903aaad Add Deserialize macro to commands response (edouard)

Pull request description:

  Clients importing the revaultd lib and consuming
  the jsonrpc API need to deserialize the commands
  responses.

  This commit introduces new deserialization helpers
  and adds them to commands structs fields.

ACKs for top commit:
  darosior:
    utACK 24eef7d

Tree-SHA512: 30062d885332539e83fbbe2fa454c872432f08b2eae9837ab14262449d92aac1ebdc580f917b89ba4aa9c3ca9504b1336e801d358d478334209eab2741c44115

Author: darosior

src/bitcoind/interface.rs

@@ -22,7 +22,8 @@ use jsonrpc::{
     client::Client,
     simple_http::{Error as HttpError, SimpleHttpTransport},
 };
-use serde::Serialize;
+
+use serde::{Deserialize, Serialize};
 use serde_json::Value as Json;
 
 // The minimum deposit value according to revault_tx depends also on the unvault's
@@ -920,7 +921,7 @@ impl BitcoinD {
     }
 }
 
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct WalletTransaction {
     pub hex: String,
     #[serde(rename = "received_at")]

src/commands/mod.rs

@@ -32,8 +32,8 @@ use crate::{
     DaemonControl, VERSION,
 };
 use utils::{
-    finalized_emer_txs, gethistory, listvaults_from_db, presigned_txs, ser_amount, ser_to_string,
-    serialize_option_tx_hex, vaults_from_deposits,
+    deser_amount_from_sats, deser_from_str, finalized_emer_txs, gethistory, listvaults_from_db,
+    presigned_txs, ser_amount, ser_to_string, serialize_option_tx_hex, vaults_from_deposits,
 };
 
 use revault_tx::{
@@ -198,7 +198,6 @@ impl DaemonControl {
         } = db_tip(&revaultd.db_file()).expect("Database must not be dead");
         let number_of_vaults = self
             .list_vaults(None, None)
-            .vaults
             .iter()
             .filter(|l| {
                 l.status != VaultStatus::Spent
@@ -226,14 +225,12 @@ impl DaemonControl {
     /// List the current vaults, optionally filtered by status and/or deposit outpoints.
     pub fn list_vaults(
         &self,
-        statuses: Option<Vec<VaultStatus>>,
-        deposit_outpoints: Option<Vec<OutPoint>>,
-    ) -> ListVaultsResult {
+        statuses: Option<&[VaultStatus]>,
+        deposit_outpoints: Option<&[OutPoint]>,
+    ) -> Vec<ListVaultsEntry> {
         let revaultd = self.revaultd.read().unwrap();
-        ListVaultsResult {
-            vaults: listvaults_from_db(&revaultd, statuses, deposit_outpoints)
-                .expect("Database must be available"),
-        }
+        listvaults_from_db(&revaultd, statuses, deposit_outpoints)
+            .expect("Database must be available")
     }
 
     /// Get the deposit address at the lowest still unused derivation index
@@ -863,7 +860,7 @@ impl DaemonControl {
     pub fn get_spend_tx(
         &self,
         outpoints: &[OutPoint],
-        destinations: BTreeMap<Address, u64>,
+        destinations: &BTreeMap<Address, u64>,
         feerate_vb: u64,
     ) -> Result<SpendTransaction, CommandError> {
         let revaultd = self.revaultd.read().unwrap();
@@ -897,11 +894,11 @@ impl DaemonControl {
         }
 
         let txos: Vec<SpendTxOut> = destinations
-            .into_iter()
+            .iter()
             .map(|(addr, value)| {
                 let script_pubkey = addr.script_pubkey();
                 SpendTxOut::new(TxOut {
-                    value,
+                    value: *value,
                     script_pubkey,
                 })
             })
@@ -1344,26 +1341,26 @@ impl DaemonControl {
         start: u32,
         end: u32,
         limit: u64,
-        kind: Vec<HistoryEventKind>,
+        kind: &[HistoryEventKind],
     ) -> Result<Vec<HistoryEvent>, CommandError> {
         let revaultd = self.revaultd.read().unwrap();
         gethistory(&revaultd, &self.bitcoind_conn, start, end, limit, kind)
     }
 }
 
 /// Descriptors the daemon was configured with
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct GetInfoDescriptors {
-    #[serde(serialize_with = "ser_to_string")]
+    #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_from_str")]
     pub deposit: DepositDescriptor,
-    #[serde(serialize_with = "ser_to_string")]
+    #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_from_str")]
     pub unvault: UnvaultDescriptor,
-    #[serde(serialize_with = "ser_to_string")]
+    #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_from_str")]
     pub cpfp: CpfpDescriptor,
 }
 
 /// Information about the current state of the daemon
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct GetInfoResult {
     pub version: String,
     pub network: Network,
@@ -1375,12 +1372,15 @@ pub struct GetInfoResult {
 }
 
 /// Information about a vault.
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ListVaultsEntry {
-    #[serde(serialize_with = "ser_amount")]
+    #[serde(
+        serialize_with = "ser_amount",
+        deserialize_with = "deser_amount_from_sats"
+    )]
     pub amount: Amount,
     pub blockheight: u32,
-    #[serde(serialize_with = "ser_to_string")]
+    #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_from_str")]
     pub status: VaultStatus,
     pub txid: Txid,
     pub vout: u32,
@@ -1392,14 +1392,8 @@ pub struct ListVaultsEntry {
     pub moved_at: Option<u32>,
 }
 
-/// Information about all our current vaults.
-#[derive(Debug, Clone, Serialize)]
-pub struct ListVaultsResult {
-    pub vaults: Vec<ListVaultsEntry>,
-}
-
 /// Revocation transactions for a given vault
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct RevocationTransactions {
     pub cancel_tx: CancelTransaction,
     pub emergency_tx: EmergencyTransaction,
@@ -1408,7 +1402,7 @@ pub struct RevocationTransactions {
 }
 
 /// A vault's presigned transaction.
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct VaultPresignedTransaction<T: RevaultTransaction> {
     pub psbt: T,
     // FIXME: is it really necessary?.. It's mostly contained in the PSBT already
@@ -1417,7 +1411,7 @@ pub struct VaultPresignedTransaction<T: RevaultTransaction> {
 }
 
 /// Information about a vault's presigned transactions.
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ListPresignedTxEntry {
     pub vault_outpoint: OutPoint,
     pub unvault: VaultPresignedTransaction<UnvaultTransaction>,
@@ -1429,7 +1423,7 @@ pub struct ListPresignedTxEntry {
 }
 
 /// Information about a vault's onchain transactions.
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ListOnchainTxEntry {
     pub vault_outpoint: OutPoint,
     pub deposit: WalletTransaction,
@@ -1443,7 +1437,7 @@ pub struct ListOnchainTxEntry {
 }
 
 /// Status of a Spend transaction
-#[derive(Debug, Deserialize, PartialEq)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "snake_case")]
 pub enum ListSpendStatus {
     NonFinal,
@@ -1452,7 +1446,7 @@ pub enum ListSpendStatus {
 }
 
 /// Information about a Spend transaction
-#[derive(Debug, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ListSpendEntry {
     pub deposit_outpoints: Vec<OutPoint>,
     pub psbt: SpendTransaction,
@@ -1461,15 +1455,15 @@ pub struct ListSpendEntry {
 }
 
 /// Information about the configured servers.
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ServersStatuses {
-    coordinator: ServerStatus,
-    cosigners: Vec<ServerStatus>,
-    watchtowers: Vec<ServerStatus>,
+    pub coordinator: ServerStatus,
+    pub cosigners: Vec<ServerStatus>,
+    pub watchtowers: Vec<ServerStatus>,
 }
 
 /// The type of an accounting event.
-#[derive(Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub enum HistoryEventKind {
     #[serde(rename = "cancel")]
     Cancel,
@@ -1479,8 +1473,18 @@ pub enum HistoryEventKind {
     Spend,
 }
 
+impl fmt::Display for HistoryEventKind {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::Cancel => write!(f, "Cancel"),
+            Self::Deposit => write!(f, "Deposit"),
+            Self::Spend => write!(f, "Spend"),
+        }
+    }
+}
+
 /// An accounting event.
-#[derive(Debug, Serialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct HistoryEvent {
     pub kind: HistoryEventKind,
     pub date: u32,

src/commands/utils.rs

@@ -32,9 +32,10 @@ use revault_tx::{
 use std::{
     collections::{HashMap, HashSet},
     fmt,
+    str::FromStr,
 };
 
-use serde::Serializer;
+use serde::{de, Deserialize, Deserializer, Serializer};
 
 fn serialize_tx_hex<S>(tx: &BitcoinTransaction, s: S) -> Result<S::Ok, S::Error>
 where
@@ -61,17 +62,39 @@ pub fn ser_to_string<T: fmt::Display, S: Serializer>(field: T, s: S) -> Result<S
     s.serialize_str(&field.to_string())
 }
 
+/// Deserialize a type `S` by deserializing a string, then using the `FromStr`
+/// impl of `S` to create the result. The generic type `S` is not required to
+/// implement `Deserialize`.
+pub fn deser_from_str<'de, S, D>(deserializer: D) -> Result<S, D::Error>
+where
+    S: FromStr,
+    S::Err: fmt::Display,
+    D: Deserializer<'de>,
+{
+    let s: String = Deserialize::deserialize(deserializer)?;
+    S::from_str(&s).map_err(de::Error::custom)
+}
+
 /// Serialize an amount as sats
 pub fn ser_amount<S: Serializer>(amount: &Amount, s: S) -> Result<S::Ok, S::Error> {
     s.serialize_u64(amount.as_sat())
 }
 
+/// Deserialize an amount from sats
+pub fn deser_amount_from_sats<'de, D>(deserializer: D) -> Result<Amount, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let a = u64::deserialize(deserializer)?;
+    Ok(Amount::from_sat(a))
+}
+
 /// List the vaults from DB, and filter out the info the RPC wants
 // FIXME: we could make this more efficient with smarter SQL queries
 pub fn listvaults_from_db(
     revaultd: &RevaultD,
-    statuses: Option<Vec<VaultStatus>>,
-    outpoints: Option<Vec<OutPoint>>,
+    statuses: Option<&[VaultStatus]>,
+    outpoints: Option<&[OutPoint]>,
 ) -> Result<Vec<ListVaultsEntry>, DatabaseError> {
     db_vaults(&revaultd.db_file()).map(|db_vaults| {
         db_vaults
@@ -257,7 +280,7 @@ pub fn gethistory<T: BitcoindThread>(
     start: u32,
     end: u32,
     limit: u64,
-    kind: Vec<HistoryEventKind>,
+    kind: &[HistoryEventKind],
 ) -> Result<Vec<HistoryEvent>, CommandError> {
     let db_path = revaultd.db_file();
     // All vaults which have one transaction (either the funding, the canceling, the unvaulting, the spending)
@@ -873,8 +896,8 @@ mod tests {
         for v in &vaults {
             let res = &listvaults_from_db(
                 &revaultd,
-                Some(vec![v.db_vault.status]),
-                Some(vec![v.db_vault.deposit_outpoint]),
+                Some(&[v.db_vault.status]),
+                Some(&[v.db_vault.deposit_outpoint]),
             )
             .unwrap()[0];
             assert_eq!(res.amount, v.db_vault.amount);
@@ -891,16 +914,16 @@ mod tests {
         // Checking that filters work
         assert_eq!(listvaults_from_db(&revaultd, None, None).unwrap().len(), 4);
         assert_eq!(
-            listvaults_from_db(&revaultd, Some(vec![VaultStatus::Unconfirmed]), None)
+            listvaults_from_db(&revaultd, Some(&[VaultStatus::Unconfirmed]), None)
                 .unwrap()
                 .len(),
             1
         );
         assert_eq!(
             listvaults_from_db(
                 &revaultd,
-                Some(vec![VaultStatus::Unconfirmed]),
-                Some(vec![vaults[1].db_vault.deposit_outpoint])
+                Some(&[VaultStatus::Unconfirmed]),
+                Some(&[vaults[1].db_vault.deposit_outpoint])
             )
             .unwrap()
             .len(),
@@ -910,7 +933,7 @@ mod tests {
             listvaults_from_db(
                 &revaultd,
                 None,
-                Some(vec![
+                Some(&[
                     vaults[0].db_vault.deposit_outpoint,
                     vaults[1].db_vault.deposit_outpoint
                 ])
@@ -922,7 +945,7 @@ mod tests {
         assert_eq!(
             listvaults_from_db(
                 &revaultd,
-                Some(vec![
+                Some(&[
                     VaultStatus::Unconfirmed,
                     VaultStatus::Funded,
                     VaultStatus::Secured
@@ -936,7 +959,7 @@ mod tests {
         assert_eq!(
             listvaults_from_db(
                 &revaultd,
-                Some(vec![
+                Some(&[
                     VaultStatus::Unconfirmed,
                     VaultStatus::Funded,
                     VaultStatus::Secured,
@@ -1469,7 +1492,7 @@ mod tests {
             0,
             4,
             20,
-            vec![
+            &[
                 HistoryEventKind::Deposit,
                 HistoryEventKind::Cancel,
                 HistoryEventKind::Spend,
@@ -1516,7 +1539,7 @@ mod tests {
             0,
             2,
             20,
-            vec![
+            &[
                 HistoryEventKind::Deposit,
                 HistoryEventKind::Cancel,
                 HistoryEventKind::Spend,

src/communication.rs

@@ -18,7 +18,7 @@ use revault_tx::{
 
 use std::{collections::BTreeMap, fmt};
 
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 /// The kind of signature the WT refused
 #[derive(Debug)]
@@ -306,7 +306,7 @@ pub fn get_presigs(
     Ok(resp.signatures)
 }
 
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct ServerStatus {
     pub host: String,
     pub reachable: bool,