Implement into_model

For all `json` types implement an inherent `into_model` function and
remove the `TryInto` implementations. Done to improve ergonomics.

Patch includes a bunch of other cleanups and improvements on the way.

Author: tcharding

client/src/client_sync/error.rs

@@ -8,7 +8,8 @@ use bitcoin::{hex, secp256k1};
 #[derive(Debug)]
 pub enum Error {
     JsonRpc(jsonrpc::error::Error),
-    Hex(hex::HexToBytesError),
+    HexToArray(hex::HexToArrayError),
+    HexToBytes(hex::HexToBytesError),
     Json(serde_json::error::Error),
     BitcoinSerialization(bitcoin::consensus::encode::FromHexError),
     Secp256k1(secp256k1::Error),
@@ -29,8 +30,12 @@ impl From<jsonrpc::error::Error> for Error {
     fn from(e: jsonrpc::error::Error) -> Error { Error::JsonRpc(e) }
 }
 
+impl From<hex::HexToArrayError> for Error {
+    fn from(e: hex::HexToArrayError) -> Self { Self::HexToArray(e) }
+}
+
 impl From<hex::HexToBytesError> for Error {
-    fn from(e: hex::HexToBytesError) -> Error { Error::Hex(e) }
+    fn from(e: hex::HexToBytesError) -> Self { Self::HexToBytes(e) }
 }
 
 impl From<serde_json::error::Error> for Error {
@@ -59,7 +64,8 @@ impl fmt::Display for Error {
 
         match *self {
             JsonRpc(ref e) => write!(f, "JSON-RPC error: {}", e),
-            Hex(ref e) => write!(f, "hex decode error: {}", e),
+            HexToArray(ref e) => write!(f, "hex to array decode error: {}", e),
+            HexToBytes(ref e) => write!(f, "hex to bytes decode error: {}", e),
             Json(ref e) => write!(f, "JSON error: {}", e),
             BitcoinSerialization(ref e) => write!(f, "Bitcoin serialization error: {}", e),
             Secp256k1(ref e) => write!(f, "secp256k1 error: {}", e),
@@ -80,7 +86,8 @@ impl error::Error for Error {
 
         match *self {
             JsonRpc(ref e) => Some(e),
-            Hex(ref e) => Some(e),
+            HexToArray(ref e) => Some(e),
+            HexToBytes(ref e) => Some(e),
             Json(ref e) => Some(e),
             BitcoinSerialization(ref e) => Some(e),
             Secp256k1(ref e) => Some(e),

client/src/client_sync/v17/blockchain.rs

@@ -29,8 +29,7 @@ macro_rules! impl_client_v17__getbestblockhash {
             /// Gets the blockhash of the current chain tip.
             pub fn best_block_hash(&self) -> Result<bitcoin::BlockHash> {
                 let json = self.get_best_block_hash()?;
-                let concrete: $crate::json::model::GetBestBlockHash = json.try_into().unwrap();
-                Ok(concrete.0)
+                Ok(json.block_hash()?)
             }
 
             pub fn get_best_block_hash(&self) -> Result<GetBestBlockHash> {
@@ -48,8 +47,7 @@ macro_rules! impl_client_v17__getblock {
             /// Gets a block by blockhash.
             pub fn get_block(&self, hash: &BlockHash) -> Result<Block> {
                 let json = self.get_block_verbosity_zero(hash)?;
-                let concrete: $crate::json::model::GetBlockVerbosityZero = json.try_into()?;
-                Ok(concrete.0)
+                Ok(json.block()?)
             }
 
             // FIXME(getblock): This handling of optional args is ugly as hell but because the returned json

client/src/client_sync/v17/mod.rs

@@ -53,7 +53,7 @@ crate::impl_client_v17__gettransaction!();
 #[serde(rename_all = "kebab-case")]
 pub enum AddressType {
     Legacy,
-    P2ShSegwit,
+    P2shSegwit,
     Bech32,
 }
 
@@ -63,7 +63,7 @@ impl fmt::Display for AddressType {
 
         let s = match *self {
             Legacy => "legacy",
-            P2ShSegwit => "p2sh-segwit",
+            P2shSegwit => "p2sh-segwit",
             Bech32 => "bech32",
         };
         fmt::Display::fmt(s, f)

client/src/client_sync/v17/wallet.rs

@@ -102,7 +102,7 @@ macro_rules! impl_client_v17__sendtoaddress {
                 &self,
                 address: &Address<NetworkChecked>,
                 amount: Amount,
-            ) -> Result<bitcoin::Txid> {
+            ) -> Result<SendToAddress> {
                 let mut args = [address.to_string().into(), into_json(amount.to_btc())?];
                 self.call("sendtoaddress", handle_defaults(&mut args, &["".into(), "".into()]))
             }

client/src/client_sync/v23.rs

@@ -47,7 +47,7 @@ crate::impl_client_v17__gettransaction!();
 #[serde(rename_all = "kebab-case")]
 pub enum AddressType {
     Legacy,
-    P2ShSegwit,
+    P2shSegwit,
     Bech32,
     Bech32m,
 }
@@ -58,7 +58,7 @@ impl fmt::Display for AddressType {
 
         let s = match *self {
             Legacy => "legacy",
-            P2ShSegwit => "p2sh-segwit",
+            P2shSegwit => "p2sh-segwit",
             Bech32 => "bech32",
             Bech32m => "bech32m",
         };

integration_test/src/v17/blockchain.rs

@@ -12,7 +12,8 @@ macro_rules! impl_test_v17__getblockchaininfo {
         #[test]
         fn get_blockchain_info() {
             let bitcoind = $crate::bitcoind_no_wallet();
-            let _ = bitcoind.client.get_blockchain_info().expect("getblockchaininfo");
+            let json = bitcoind.client.get_blockchain_info().expect("getblockchaininfo");
+            assert!(json.into_model().is_ok());
         }
     };
 }
@@ -29,24 +30,53 @@ macro_rules! impl_test_v17__getbestblockhash {
         #[test]
         fn get_best_block_hash() {
             let bitcoind = $crate::bitcoind_no_wallet();
-            let _ = bitcoind.client.get_best_block_hash().expect("getbestblockhash");
+            let json = bitcoind.client.get_best_block_hash().expect("getbestblockhash");
+            assert!(json.into_model().is_ok());
+        }
+    };
+}
+
+/// Requires `Client` to be in scope and to implement `get_block 0`.
+#[macro_export]
+macro_rules! impl_test_v17__getblock_verbosity_0 {
+    () => {
+        #[test]
+        fn get_block_verbosity_0() {
+            let bitcoind = $crate::bitcoind_no_wallet();
+            let block_hash = best_block_hash();
+
+            let json = bitcoind.client.get_block_verbosity_zero(&block_hash).expect("getblock 0");
+            json.into_model().unwrap();
         }
     };
 }
 
 /// Requires `Client` to be in scope and to implement `get_block`.
 #[macro_export]
-macro_rules! impl_test_v17__getblock {
+macro_rules! impl_test_v17__getblock_verbosity_1 {
+    () => {
+        #[test]
+        fn get_block_verbosity_1() {
+            let bitcoind = $crate::bitcoind_no_wallet();
+            let block_hash = best_block_hash();
+
+            let json = bitcoind.client.get_block_verbosity_one(&block_hash).expect("getblock 1");
+            json.into_model().unwrap();
+        }
+    };
+}
+
+/// Requires `Client` to be in scope and to implement `get_block 2`.
+#[macro_export]
+macro_rules! impl_test_v17__getblock_verbosity_2 {
     () => {
         #[test]
-        fn get_block() {
+        fn get_block_verbosity_2() {
             let bitcoind = $crate::bitcoind_no_wallet();
             let block_hash = best_block_hash();
 
-            let _ = bitcoind.client.get_block_verbosity_zero(&block_hash).expect("getblock 0");
-            let _ = bitcoind.client.get_block_verbosity_one(&block_hash).expect("getblock 1");
-            // TODO: getblock 2
-            // let json = client.get_block_verbosity_two(&block_hash).expect("getblock 2");
+            let json = client.get_block_verbosity_two(&block_hash).expect("getblock 2");
+            json.into_model().unwrap();
         }
     };
 }

integration_test/src/v17/control.rs

@@ -12,6 +12,7 @@ macro_rules! impl_test_v17__stop {
         #[test]
         fn stop() {
             let bitcoind = $crate::bitcoind_no_wallet();
+            // There is no json object for `stop`, we just return a string.
             let _ = bitcoind.client.stop().expect("stop");
         }
     };

integration_test/src/v17/generating.rs

@@ -13,7 +13,8 @@ macro_rules! impl_test_v17__generatetoaddress {
         fn generate_to_address() {
             let bitcoind = $crate::bitcoind_with_default_wallet();
             let address = bitcoind.client.new_address().expect("failed to get new address");
-            let _ = bitcoind.client.generate_to_address(1, &address).expect("generatetoaddress");
+            let json = bitcoind.client.generate_to_address(1, &address).expect("generatetoaddress");
+            json.into_model().unwrap();
         }
     };
 }

integration_test/src/v17/network.rs

@@ -12,7 +12,9 @@ macro_rules! impl_test_v17__getnetworkinfo {
         #[test]
         fn get_network_info() {
             let bitcoind = $crate::bitcoind_no_wallet();
-            let _ = bitcoind.client.get_network_info().expect("getnetworkinfo");
+            let json = bitcoind.client.get_network_info().expect("getnetworkinfo");
+            json.into_model().unwrap();
+
             bitcoind.client.check_expected_server_version().expect("unexpected version");
         }
     };

integration_test/src/v17/wallet.rs

@@ -1,3 +1,10 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Macros for implementing test methods on a JSON-RPC client.
+//!
+//! Specifically this is methods found under the `== Wallet ==` section of the
+//! API docs of `bitcoind v0.17.1`.
+
 /// Requires `Client` to be in scope and to implement `createwallet`.
 #[macro_export]
 macro_rules! impl_test_v17__createwallet {
@@ -31,7 +38,8 @@ macro_rules! impl_test_v17__unloadwallet {
             let bitcoind = $crate::bitcoind_no_wallet();
             let wallet = format!("wallet-{}", rand::random::<u32>()).to_string();
             bitcoind.client.create_wallet(&wallet).expect("failed to create wallet");
-            let _ = bitcoind.client.unload_wallet(&wallet).expect("unloadwallet");
+            let json = bitcoind.client.unload_wallet(&wallet).expect("unloadwallet");
+            assert!(json.into_model().is_ok())
         }
     };
 }
@@ -45,13 +53,26 @@ macro_rules! impl_test_v17__getnewaddress {
             use bitcoind::AddressType;
 
             let bitcoind = $crate::bitcoind_with_default_wallet();
-            let _ = bitcoind.client.get_new_address().expect("getnewaddress");
 
-            let addr = bitcoind
+            let json = bitcoind.client.get_new_address().expect("getnewaddress");
+            assert!(json.into_model().is_ok());
+
+            // Test the helper as well just for good measure.
+            let _ = bitcoind.client.new_address().unwrap();
+
+            // Exhaustively test address types with helper.
+            let _ = bitcoind
+                .client
+                .new_address_with_type(AddressType::Legacy)
+                .unwrap();
+            let _ = bitcoind
+                .client
+                .new_address_with_type(AddressType::P2shSegwit)
+                .unwrap();
+            let _ = bitcoind
                 .client
                 .new_address_with_type(AddressType::Bech32)
                 .unwrap();
-
         }
     };
 }
@@ -66,7 +87,7 @@ macro_rules! impl_test_v17__getbalance {
 
             let bitcoind = $crate::bitcoind_with_default_wallet();
             let json = bitcoind.client.get_balance().expect("getbalance");
-            let _: model::GetBalance = json.try_into().unwrap();
+            assert!(json.into_model().is_ok())
         }
     };
 }
@@ -85,10 +106,11 @@ macro_rules! impl_test_v17__sendtoaddress {
             let address = bitcoind.client.new_address().expect("failed to create new address");
             let _ = bitcoind.client.generate_to_address(101, &address).expect("generatetoaddress");
 
-            let _ = bitcoind
+            let json = bitcoind
                 .client
                 .send_to_address(&address, Amount::from_sat(10_000))
-                .expect("sendtoaddress");
+                .expect("sendtddress");
+            json.into_model().unwrap();
         }
     };
 }
@@ -112,10 +134,12 @@ macro_rules! impl_test_v17__gettransaction {
             let txid = bitcoind
                 .client
                 .send_to_address(&address, Amount::from_sat(10_000))
-                .expect("sendtoaddress");
+                .expect("sendtoaddress")
+                .txid()
+                .unwrap();
 
             let json = bitcoind.client.get_transaction(txid).expect("gettransaction");
-            let _: model::GetTransaction = json.try_into().unwrap();
+            json.into_model().unwrap();
         }
     };
 }