add proper blockchain io error handling

bitcoin_core_io/src/lib.rs

@@ -17,8 +17,40 @@ use bitcoin::{
     Block, BlockHeader, Transaction,
 };
 use bitcoin_hashes::sha256d::Hash as Sha256dHash;
-use bitcoin_rpc_client::{BitcoinCoreClient, BitcoinRpcApi, rpc::SerializedRawTransaction};
+use bitcoin_rpc_client::{
+    BitcoinCoreClient, BitcoinRpcApi,
+    rpc::SerializedRawTransaction, RpcError, ClientError,
+};
 use wallet::interface::BlockChainIO;
+use std::{fmt, error::Error};
+
+#[derive(Debug)]
+pub enum BitcoinCoreIoError {
+    ClientError(ClientError),
+    RpcError(RpcError),
+}
+
+impl fmt::Display for BitcoinCoreIoError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            &BitcoinCoreIoError::ClientError(ref e) => write!(f, "{:?}", e),
+            &BitcoinCoreIoError::RpcError(ref e) => write!(f, "{:?}", e),
+        }
+    }
+}
+
+impl Error for BitcoinCoreIoError {
+}
+
+impl BitcoinCoreIoError {
+    fn flatten<T>(r: Result<Result<T, RpcError>, ClientError>) -> Result<T, Self> {
+        match r {
+            Ok(Ok(t)) => Ok(t),
+            Err(a) => Err(BitcoinCoreIoError::ClientError(a)),
+            Ok(Err(b)) => Err(BitcoinCoreIoError::RpcError(b)),
+        }
+    }
+}
 
 pub struct BitcoinCoreIO(BitcoinCoreClient);
 
@@ -29,18 +61,22 @@ impl BitcoinCoreIO {
 }
 
 impl BlockChainIO for BitcoinCoreIO {
-    fn get_block_count(&self) -> u32 {
-        self.0.get_block_count().unwrap().unwrap().into()
+    type Error = BitcoinCoreIoError;
+
+    fn get_block_count(&self) -> Result<u32, Self::Error> {
+        Self::Error::flatten(self.0.get_block_count()).map(Into::into)
     }
 
-    fn get_block_hash(&self, height: u32) -> Sha256dHash {
-        self.0.get_block_hash(height).unwrap().unwrap()
+    fn get_block_hash(&self, height: u32) -> Result<Sha256dHash, Self::Error> {
+        Self::Error::flatten(self.0.get_block_hash(height))
     }
 
-    fn get_block(&self, header_hash: &Sha256dHash) -> Block {
+    fn get_block(&self, header_hash: &Sha256dHash) -> Result<Block, Self::Error> {
         use bitcoin_hashes::hex::FromHex;
 
-        let block = self.0.get_block(header_hash).unwrap().unwrap();
+        let &BitcoinCoreIO(ref client) = self;
+
+        let block = Self::Error::flatten(client.get_block(header_hash))?;
 
         // TODO(evg): review it
         let header = BlockHeader {
@@ -53,22 +89,17 @@ impl BlockChainIO for BitcoinCoreIO {
         };
         let mut txdata = Vec::new();
         for txid in &block.tx {
-            let tx_hex = self
-                .0
-                .get_raw_transaction_serialized(&txid)
-                .unwrap()
-                .unwrap();
-            let tx: Transaction = tx_hex.into();
-            txdata.push(tx);
+            let tx_hex = client
+                .get_raw_transaction_serialized(&txid);
+            txdata.push(Self::Error::flatten(tx_hex)?.into());
         }
 
-        Block { header, txdata }
+        Ok(Block { header, txdata })
     }
 
-    fn send_raw_transaction(&self, tx: &Transaction) {
-        self.0
-            .send_raw_transaction(SerializedRawTransaction::from(tx.clone()))
-            .unwrap()
-            .unwrap();
+    fn send_raw_transaction(&self, tx: &Transaction) -> Result<Sha256dHash, Self::Error> {
+        let v = self.0
+            .send_raw_transaction(SerializedRawTransaction::from(tx.clone()));
+        Self::Error::flatten(v)
     }
 }

rust-wallet-grpc/src/main.rs

@@ -125,11 +125,11 @@ fn main() {
         println!("{}", mnemonic.to_string());
         Box::new(electrumx_wallet)
     } else {
-        let bio = Box::new(BitcoinCoreIO::new(BitcoinCoreClient::new(
+        let bio = BitcoinCoreIO::new(BitcoinCoreClient::new(
             &cfg.url,
             &cfg.user,
             &cfg.password,
-        )));
+        ));
         let (default_wallet, mnemonic) = WalletWithTrustedFullNode::new(
             wc,
             bio,

rust-wallet-grpc/src/server.rs

@@ -225,9 +225,10 @@ impl Wallet for WalletImpl {
     ) -> grpc::SingleResponse<SyncWithTipResponse> {
         info!("manual(not ZMQ) sync with tip was requested");
 
-        let resp = SyncWithTipResponse::new();
-        self.af.lock().unwrap().sync_with_tip();
-        grpc::SingleResponse::completed(resp)
+        let resp = self.af.lock().unwrap()
+            .sync_with_tip()
+            .map(|()| SyncWithTipResponse::new());
+        grpc_error(resp)
     }
 
     fn make_tx(

rust-wallet-grpc/tests/tests.rs

@@ -68,11 +68,11 @@ fn launch_server_and_wait_new(
     thread::spawn(move || {
         let wallet: Box<dyn Wallet + Send> = match provider_copy {
             BlockChainProvider::TrustedFullNode => {
-                let bio = Box::new(BitcoinCoreIO::new(BitcoinCoreClient::new(
+                let bio = BitcoinCoreIO::new(BitcoinCoreClient::new(
                     &cfg.url,
                     &cfg.user,
                     &cfg.password,
-                )));
+                ));
                 let (default_wallet, _) =
                     WalletWithTrustedFullNode::new(WalletConfig::with_db_path(db_path), bio, mode)
                         .unwrap();

wallet/src/account.rs

@@ -369,6 +369,7 @@ mod test {
         Block, Transaction,
     };
     use bitcoin_hashes::sha256d::Hash as Sha256dHash;
+    use std::{fmt, error::Error};
 
     use crate::walletlibrary::{WalletConfigBuilder, WalletLibraryMode, KeyGenConfig};
     use crate::default::WalletWithTrustedFullNode;
@@ -377,21 +378,38 @@ mod test {
 
     struct FakeBlockChainIO;
 
+    #[derive(Debug)]
+    struct FakeBlockChainIoError;
+
+    impl fmt::Display for FakeBlockChainIoError {
+        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            write!(f, "")
+        }
+    }
+
+    impl Error for FakeBlockChainIoError {
+    }
+
     impl BlockChainIO for FakeBlockChainIO {
-        fn get_block_count(&self) -> u32 {
-            unimplemented!()
+        type Error = FakeBlockChainIoError;
+
+        fn get_block_count(&self) -> Result<u32, Self::Error> {
+            Err(FakeBlockChainIoError)
         }
-        #[allow(unused_variables)]
-        fn get_block_hash(&self, height: u32) -> Sha256dHash {
-            unimplemented!()
+
+        fn get_block_hash(&self, height: u32) -> Result<Sha256dHash, Self::Error> {
+            let _ = height;
+            Err(FakeBlockChainIoError)
         }
-        #[allow(unused_variables)]
-        fn get_block(&self, header_hash: &Sha256dHash) -> Block {
-            unimplemented!()
+
+        fn get_block(&self, header_hash: &Sha256dHash) -> Result<Block, Self::Error> {
+            let _ = header_hash;
+            Err(FakeBlockChainIoError)
         }
-        #[allow(unused_variables)]
-        fn send_raw_transaction(&self, tx: &Transaction) {
-            unimplemented!()
+
+        fn send_raw_transaction(&self, tx: &Transaction) -> Result<Sha256dHash, Self::Error> {
+            let _ = tx;
+            Err(FakeBlockChainIoError)
         }
     }
 
@@ -422,7 +440,7 @@ mod test {
             .finalize();
         let (mut af, _) = WalletWithTrustedFullNode::new(
             wc,
-            Box::new(FakeBlockChainIO),
+            FakeBlockChainIO,
             WalletLibraryMode::Create(KeyGenConfig::debug()),
         )
         .unwrap();
@@ -463,7 +481,7 @@ mod test {
             .finalize();
         let (mut af, _) = WalletWithTrustedFullNode::new(
             wc,
-            Box::new(FakeBlockChainIO),
+            FakeBlockChainIO,
             WalletLibraryMode::Create(KeyGenConfig::debug()),
         )
         .unwrap();

wallet/src/default.rs

@@ -22,12 +22,18 @@ use super::error::WalletError;
 use super::mnemonic::Mnemonic;
 
 // a factory for TREZOR (BIP44) compatible accounts
-pub struct WalletWithTrustedFullNode {
+pub struct WalletWithTrustedFullNode<IO>
+where
+    IO: BlockChainIO,
+{
     pub wallet_lib: Box<dyn WalletLibraryInterface + Send>,
-    bio: Box<dyn BlockChainIO + Send>,
+    bio: IO,
 }
 
-impl Wallet for WalletWithTrustedFullNode {
+impl<IO> Wallet for WalletWithTrustedFullNode<IO>
+where
+    IO: BlockChainIO,
+{
     fn wallet_lib(&self) -> &Box<dyn WalletLibraryInterface + Send> {
         &self.wallet_lib
     }
@@ -50,7 +56,7 @@ impl Wallet for WalletWithTrustedFullNode {
             .wallet_lib
             .send_coins(addr_str, amt, lock_coins, witness_only)?;
         if submit {
-            self.bio.send_raw_transaction(&tx);
+            self.bio.send_raw_transaction(&tx)?;
         }
         Ok((tx, lock_id))
     }
@@ -64,30 +70,36 @@ impl Wallet for WalletWithTrustedFullNode {
     ) -> Result<Transaction, Box<dyn Error>> {
         let tx = self.wallet_lib.make_tx(ops, addr_str, amt).unwrap();
         if submit {
-            self.bio.send_raw_transaction(&tx);
+            self.bio.send_raw_transaction(&tx)?;
         }
         Ok(tx)
     }
 
-    fn publish_tx(&mut self, tx: &Transaction) {
-        self.bio.send_raw_transaction(tx);
+    fn publish_tx(&mut self, tx: &Transaction) -> Result<(), Box<dyn Error>> {
+        self.bio.send_raw_transaction(tx)?;
+        Ok(())
     }
 
-    fn sync_with_tip(&mut self) {
-        let block_height = self.bio.get_block_count();
+    fn sync_with_tip(&mut self) -> Result<(), Box<dyn Error>> {
+        let block_height = self.bio.get_block_count()?;
 
         let start_from = self.wallet_lib.get_last_seen_block_height_from_memory() + 1;
-        self.process_block_range(start_from, block_height as usize);
+        self.process_block_range(start_from, block_height as usize)?;
+
+        Ok(())
     }
 }
 
-impl WalletWithTrustedFullNode {
+impl<IO> WalletWithTrustedFullNode<IO>
+where
+    IO: BlockChainIO,
+{
     /// initialize with new random master key
     pub fn new(
         wc: WalletConfig,
-        bio: Box<dyn BlockChainIO + Send>,
+        bio: IO,
         mode: WalletLibraryMode,
-    ) -> Result<(WalletWithTrustedFullNode, Mnemonic), WalletError> {
+    ) -> Result<(Self, Mnemonic), WalletError> {
         let (wallet_lib, mnemonic) = WalletLibrary::new(wc, mode).unwrap();
 
         Ok((
@@ -111,11 +123,13 @@ impl WalletWithTrustedFullNode {
             .update_last_seen_block_height_in_db(block_height);
     }
 
-    fn process_block_range(&mut self, left: usize, right: usize) {
+    fn process_block_range(&mut self, left: usize, right: usize) -> Result<(), IO::Error> {
         for i in left..right + 1 {
-            let block_hash = self.bio.get_block_hash(i as u32);
-            let block = self.bio.get_block(&block_hash);
+            let block_hash = self.bio.get_block_hash(i as u32)?;
+            let block = self.bio.get_block(&block_hash)?;
             self.process_block(i, &block);
         }
+
+        Ok(())
     }
 }

wallet/src/electrumx.rs

@@ -62,7 +62,7 @@ impl Wallet for ElectrumxWallet {
             .wallet_lib
             .send_coins(addr_str, amt, lock_coins, witness_only)?;
         if submit {
-            self.publish_tx(&tx);
+            self.publish_tx(&tx)?;
         }
         Ok((tx, lock_id))
     }
@@ -76,23 +76,24 @@ impl Wallet for ElectrumxWallet {
     ) -> Result<Transaction, Box<dyn Error>> {
         let tx = self.wallet_lib.make_tx(ops, addr_str, amt).unwrap();
         if submit {
-            self.publish_tx(&tx);
+            self.publish_tx(&tx)?;
         }
         Ok(tx)
     }
 
-    fn publish_tx(&mut self, tx: &Transaction) {
+    fn publish_tx(&mut self, tx: &Transaction) -> Result<(), Box<dyn Error>> {
         let tx = serialize_hex(tx);
-        self.electrumx_client.broadcast_transaction(tx).unwrap();
+        self.electrumx_client.broadcast_transaction(tx)?;
+        Ok(())
     }
 
     // TODO(evg): something better?
-    fn sync_with_tip(&mut self) {
+    fn sync_with_tip(&mut self) -> Result<(), Box<dyn Error>> {
         println!("******** SYNC_WITH_TIP_BEGIN ********");
         let mut all_wallet_related_txs = Vec::new();
         let btc_address_list = self.wallet_lib.get_full_address_list();
         for btc_address in btc_address_list {
-            let history = self.electrumx_client.get_history(&btc_address).unwrap();
+            let history = self.electrumx_client.get_history(&btc_address)?;
             for resp in history {
                 all_wallet_related_txs.push((resp.height, resp.tx_hash))
             }
@@ -116,8 +117,7 @@ impl Wallet for ElectrumxWallet {
             let tx_hash = wallet_related_tx.1;
             let tx_hex = self
                 .electrumx_client
-                .get_transaction(tx_hash.clone(), false, false)
-                .unwrap();
+                .get_transaction(tx_hash.clone(), false, false)?;
             let tx = hex::decode(tx_hex).unwrap();
 
             let tx: Transaction = deserialize(&tx).unwrap();
@@ -127,6 +127,8 @@ impl Wallet for ElectrumxWallet {
             to_skip.insert(tx_hash, ());
         }
         println!("******** SYNC_WITH_TIP_END ********\n\n\n");
+
+        Ok(())
     }
 }
 

wallet/src/interface.rs

@@ -40,8 +40,8 @@ pub trait Wallet {
         amt: u64,
         submit: bool,
     ) -> Result<Transaction, Box<dyn Error>>;
-    fn publish_tx(&mut self, tx: &Transaction);
-    fn sync_with_tip(&mut self);
+    fn publish_tx(&mut self, tx: &Transaction) -> Result<(), Box<dyn Error>>;
+    fn sync_with_tip(&mut self) -> Result<(), Box<dyn Error>>;
 }
 
 pub trait WalletLibraryInterface {
@@ -75,8 +75,10 @@ pub trait WalletLibraryInterface {
 }
 
 pub trait BlockChainIO {
-    fn get_block_count(&self) -> u32;
-    fn get_block_hash(&self, height: u32) -> Sha256dHash;
-    fn get_block(&self, header_hash: &Sha256dHash) -> Block;
-    fn send_raw_transaction(&self, tx: &Transaction);
+    type Error: Error + 'static;
+
+    fn get_block_count(&self) -> Result<u32, Self::Error>;
+    fn get_block_hash(&self, height: u32) -> Result<Sha256dHash, Self::Error>;
+    fn get_block(&self, header_hash: &Sha256dHash) -> Result<Block, Self::Error>;
+    fn send_raw_transaction(&self, tx: &Transaction) -> Result<Sha256dHash, Self::Error>;
 }