Merge #752: bitcoind: don't crash on getblockheader failure

663fbd2c287791c608a056825ac103c1366b4c33 bitcoin: make get_block_stats fallible (Antoine Poinsot)
869779dd94b9f82d4c068c081c287df31ad78b1e bitcoin: make tip_time return an Option (Antoine Poinsot)

Pull request description:

  Fixes https://github.com/wizardsardine/liana/issues/728.

ACKs for top commit:
  darosior:
    ACK 663fbd2c287791c608a056825ac103c1366b4c33

Tree-SHA512: 072c4a4006e4f66eefd271ec2d65da991e0ddd2a09fa6a4329b5c4f08fc3bfd9dcb031710df47fd8bdfa02ccd615aaf55ae4411fbbf536705a22fcce918c408b

src/bitcoin/d/mod.rs

@@ -957,11 +957,17 @@ impl BitcoinD {
         None
     }
 
-    pub fn get_block_stats(&self, blockhash: bitcoin::BlockHash) -> BlockStats {
-        let res = self.make_node_request(
+    pub fn get_block_stats(&self, blockhash: bitcoin::BlockHash) -> Option<BlockStats> {
+        let res = match self.make_fallible_node_request(
             "getblockheader",
             &params!(Json::String(blockhash.to_string()),),
-        );
+        ) {
+            Ok(res) => res,
+            Err(e) => {
+                log::warn!("Error when fetching block header {}: {}", &blockhash, e);
+                return None;
+            }
+        };
         let confirmations = res
             .get("confirmations")
             .and_then(Json::as_i64)
@@ -989,14 +995,14 @@ impl BitcoinD {
             .and_then(Json::as_u64)
             .expect("Invalid median timestamp in `getblockheader` response: not an u64")
             as u32;
-        BlockStats {
+        Some(BlockStats {
             confirmations,
             previous_blockhash,
             height,
             blockhash,
             time,
             median_time_past,
-        }
+        })
     }
 
     pub fn broadcast_tx(&self, tx: &bitcoin::Transaction) -> Result<(), BitcoindError> {

src/bitcoin/d/utils.rs

@@ -28,15 +28,15 @@ pub fn block_before_date<Fh, Fs>(
 ) -> Option<BlockChainTip>
 where
     Fh: FnMut(i32) -> Option<bitcoin::BlockHash>,
-    Fs: FnMut(bitcoin::BlockHash) -> BlockStats,
+    Fs: FnMut(bitcoin::BlockHash) -> Option<BlockStats>,
 {
     log::debug!("Looking for the first block before {}", target_timestamp);
 
     let mut start_height = 0;
     let mut end_height = chain_tip.height;
 
-    let genesis_stats = get_stats(get_hash(0).expect("Genesis hash"));
-    let tip_stats = get_stats(chain_tip.hash);
+    let genesis_stats = get_stats(get_hash(0).expect("Genesis hash"))?;
+    let tip_stats = get_stats(chain_tip.hash)?;
     if !(genesis_stats.time..tip_stats.time).contains(&target_timestamp) {
         return None;
     }
@@ -47,7 +47,7 @@ where
         let current_height = start_height + delta.checked_div(2).unwrap();
         // We want the last block with a timestamp below, not the first with a higher one.
         let next_height = current_height.checked_add(1).unwrap();
-        let next_stats = get_stats(get_hash(next_height)?);
+        let next_stats = get_stats(get_hash(next_height)?)?;
         log::debug!("Current next block: {:?}", next_stats);
 
         if target_timestamp > next_stats.time {
@@ -85,13 +85,18 @@ mod tests {
     }
 
     // Inefficient dummy implementation of BitcoinD's self.get_block_stats
-    fn get_stats(chain: &[(BlockChainTip, BlockStats)], hash: bitcoin::BlockHash) -> BlockStats {
-        chain
-            .iter()
-            .find(|(tip, _)| tip.hash == hash)
-            .unwrap()
-            .1
-            .clone()
+    fn get_stats(
+        chain: &[(BlockChainTip, BlockStats)],
+        hash: bitcoin::BlockHash,
+    ) -> Option<BlockStats> {
+        Some(
+            chain
+                .iter()
+                .find(|(tip, _)| tip.hash == hash)
+                .unwrap()
+                .1
+                .clone(),
+        )
     }
 
     macro_rules! bh {

src/bitcoin/mod.rs

@@ -51,7 +51,7 @@ pub trait BitcoinInterface: Send {
     fn chain_tip(&self) -> BlockChainTip;
 
     /// Get the timestamp set in the best block's header.
-    fn tip_time(&self) -> u32;
+    fn tip_time(&self) -> Option<u32>;
 
     /// Check whether this former tip is part of the current best chain.
     fn is_in_chain(&self, tip: &BlockChainTip) -> bool;
@@ -272,11 +272,11 @@ impl BitcoinInterface for d::BitcoinD {
     }
 
     fn common_ancestor(&self, tip: &BlockChainTip) -> Option<BlockChainTip> {
-        let mut stats = self.get_block_stats(tip.hash);
+        let mut stats = self.get_block_stats(tip.hash)?;
         let mut ancestor = *tip;
 
         while stats.confirmations == -1 {
-            stats = self.get_block_stats(stats.previous_blockhash?);
+            stats = self.get_block_stats(stats.previous_blockhash?)?;
             ancestor = BlockChainTip {
                 hash: stats.blockhash,
                 height: stats.height,
@@ -316,9 +316,9 @@ impl BitcoinInterface for d::BitcoinD {
         self.tip_before_timestamp(timestamp)
     }
 
-    fn tip_time(&self) -> u32 {
+    fn tip_time(&self) -> Option<u32> {
         let tip = self.chain_tip();
-        self.get_block_stats(tip.hash).time
+        Some(self.get_block_stats(tip.hash)?.time)
     }
 
     fn wallet_transaction(
@@ -400,7 +400,7 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
         self.lock().unwrap().block_before_date(timestamp)
     }
 
-    fn tip_time(&self) -> u32 {
+    fn tip_time(&self) -> Option<u32> {
         self.lock().unwrap().tip_time()
     }
 

src/commands/mod.rs

@@ -694,7 +694,12 @@ impl DaemonControl {
     pub fn start_rescan(&self, timestamp: u32) -> Result<(), CommandError> {
         let mut db_conn = self.db.connection();
 
-        if timestamp < MAINNET_GENESIS_TIME || timestamp >= self.bitcoin.tip_time() {
+        let future_timestamp = self
+            .bitcoin
+            .tip_time()
+            .map(|t| timestamp >= t)
+            .unwrap_or(false);
+        if timestamp < MAINNET_GENESIS_TIME || future_timestamp {
             return Err(CommandError::InsaneRescanTimestamp(timestamp));
         }
         if db_conn.rescan_timestamp().is_some() || self.bitcoin.rescan_progress().is_some() {

src/testutils.rs

@@ -106,7 +106,7 @@ impl BitcoinInterface for DummyBitcoind {
         todo!()
     }
 
-    fn tip_time(&self) -> u32 {
+    fn tip_time(&self) -> Option<u32> {
         todo!()
     }