From 80803c78a4579641b604c110e411556588ebdf24 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Wed, 9 Nov 2022 10:19:59 +0100
Subject: [PATCH] bitcoind: handle missing previousblockhash from
 getblockheader

It seems that internally bitcoind might temporarily not have a pprev
pointer for a block. This will result in the optional
"previousblockhash" field to be null and would previously make us crash.

Handle that gracefully.
---
 src/bitcoin/d/mod.rs         | 10 ++++++----
 src/bitcoin/mod.rs           | 10 +++++-----
 src/bitcoin/poller/looper.rs | 21 ++++++++++++++-------
 src/testutils.rs             |  2 +-
 4 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs
index d1427049..d3d29ca5 100644
--- a/src/bitcoin/d/mod.rs
+++ b/src/bitcoin/d/mod.rs
@@ -679,12 +679,14 @@ impl BitcoinD {
         let previous_blockhash = res
             .get("previousblockhash")
             .and_then(Json::as_str)
-            .and_then(|s| bitcoin::BlockHash::from_str(s).ok())
-            .expect("Invalid previousblockhash in `getblockheader` response");
+            .map(|s| {
+                bitcoin::BlockHash::from_str(s)
+                    .expect("Invalid previousblockhash in `getblockheader` response")
+            });
         let height = res
             .get("height")
             .and_then(Json::as_i64)
-            .expect("Invalid height in `getblockheader` response: not an u32")
+            .expect("Invalid height in `getblockheader` response: not an i64")
             as i32;
         BlockStats {
             confirmations,
@@ -848,7 +850,7 @@ impl From<Json> for GetTxRes {
 #[derive(Debug, Clone)]
 pub struct BlockStats {
     pub confirmations: i32,
-    pub previous_blockhash: bitcoin::BlockHash,
+    pub previous_blockhash: Option<bitcoin::BlockHash>,
     pub blockhash: bitcoin::BlockHash,
     pub height: i32,
 }
diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs
index ae7b2257..9c48ceb3 100644
--- a/src/bitcoin/mod.rs
+++ b/src/bitcoin/mod.rs
@@ -84,7 +84,7 @@ pub trait BitcoinInterface: Send {
     ) -> Vec<(bitcoin::OutPoint, bitcoin::Txid, i32, u32)>;
 
     /// Get the common ancestor between the Bitcoin backend's tip and the given tip.
-    fn common_ancestor(&self, tip: &BlockChainTip) -> BlockChainTip;
+    fn common_ancestor(&self, tip: &BlockChainTip) -> Option<BlockChainTip>;
 
     /// Broadcast this transaction to the Bitcoin P2P network
     fn broadcast_tx(&self, tx: &bitcoin::Transaction) -> Result<(), BitcoinError>;
@@ -260,19 +260,19 @@ impl BitcoinInterface for d::BitcoinD {
         spent
     }
 
-    fn common_ancestor(&self, tip: &BlockChainTip) -> BlockChainTip {
+    fn common_ancestor(&self, tip: &BlockChainTip) -> Option<BlockChainTip> {
         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,
             };
         }
 
-        ancestor
+        Some(ancestor)
     }
 
     fn broadcast_tx(&self, tx: &bitcoin::Transaction) -> Result<(), BitcoinError> {
@@ -335,7 +335,7 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
         self.lock().unwrap().spent_coins(outpoints)
     }
 
-    fn common_ancestor(&self, tip: &BlockChainTip) -> BlockChainTip {
+    fn common_ancestor(&self, tip: &BlockChainTip) -> Option<BlockChainTip> {
         self.lock().unwrap().common_ancestor(tip)
     }
 
diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs
index 7d39124c..77653a80 100644
--- a/src/bitcoin/poller/looper.rs
+++ b/src/bitcoin/poller/looper.rs
@@ -150,13 +150,20 @@ fn new_tip(bit: &impl BitcoinInterface, current_tip: &BlockChainTip) -> TipUpdat
     // block chain re-organisation. Find the common ancestor between our current chain and
     // the new chain and return that. The caller will take care of rewinding our state.
     log::info!("Block chain reorganization detected. Looking for common ancestor.");
-    let common_ancestor = bit.common_ancestor(current_tip);
-    log::info!(
-        "Common ancestor found: '{}'. Starting rescan from there. Old tip was '{}'.",
-        common_ancestor,
-        current_tip
-    );
-    TipUpdate::Reorged(common_ancestor)
+    if let Some(common_ancestor) = bit.common_ancestor(current_tip) {
+        log::info!(
+            "Common ancestor found: '{}'. Starting rescan from there. Old tip was '{}'.",
+            common_ancestor,
+            current_tip
+        );
+        TipUpdate::Reorged(common_ancestor)
+    } else {
+        log::error!(
+            "Failed to get common ancestor for tip '{}'. Starting over.",
+            current_tip
+        );
+        new_tip(bit, current_tip)
+    }
 }
 
 fn updates(
diff --git a/src/testutils.rs b/src/testutils.rs
index eff9da06..6add2785 100644
--- a/src/testutils.rs
+++ b/src/testutils.rs
@@ -67,7 +67,7 @@ impl BitcoinInterface for DummyBitcoind {
         Vec::new()
     }
 
-    fn common_ancestor(&self, _: &BlockChainTip) -> BlockChainTip {
+    fn common_ancestor(&self, _: &BlockChainTip) -> Option<BlockChainTip> {
         todo!()
     }