Fix handling of reorgs that shorten the chain

Prior to this fix, `Indexer::update()` would panic on the
`assert_eq!(tip, *headers.tip())` assertion when handling reorgs that
shorten the existing chain without adding any blocks to replace them.

This should not normally happen, but might due to manual `invalidateblock`.
For example, this will reproduce the panic:
`bitcoin-cli invalidateblock $(bitcoin-cli getbestblockhash)`

Author: shesek

src/new_index/schema.rs

@@ -284,8 +284,8 @@ impl Indexer {
         tip: &BlockHash,
     ) -> Result<(Vec<HeaderEntry>, Option<usize>)> {
         let indexed_headers = self.store.indexed_headers.read().unwrap();
-        let raw_new_headers = daemon.get_new_headers(&indexed_headers, &tip)?;
-        let (new_headers, reorged_since) = indexed_headers.preprocess(raw_new_headers);
+        let raw_new_headers = daemon.get_new_headers(&indexed_headers, tip)?;
+        let (new_headers, reorged_since) = indexed_headers.preprocess(raw_new_headers, tip);
 
         if let Some(tip) = new_headers.last() {
             info!("{:?} ({} left to index)", tip, new_headers.len());

src/util/block.rs

@@ -2,9 +2,9 @@ use crate::chain::{BlockHash, BlockHeader};
 use crate::errors::*;
 use crate::new_index::BlockEntry;
 
+use itertools::Itertools;
 use std::collections::HashMap;
 use std::fmt;
-use std::iter::FromIterator;
 use std::slice;
 use time::format_description::well_known::Rfc3339;
 use time::OffsetDateTime as DateTime;
@@ -128,7 +128,7 @@ impl HeaderList {
         );
 
         let mut headers = HeaderList::empty();
-        headers.append(headers.preprocess(headers_chain).0);
+        headers.append(headers.preprocess(headers_chain, &tip_hash).0);
         headers
     }
 
@@ -138,43 +138,50 @@ impl HeaderList {
     /// Actually applying the headers requires to first pop() the reorged blocks (if any),
     /// then append() the new ones.
     #[trace]
-    pub fn preprocess(&self, new_headers: Vec<BlockHeader>) -> (Vec<HeaderEntry>, Option<usize>) {
+    pub fn preprocess(
+        &self,
+        new_headers: Vec<BlockHeader>,
+        new_tip: &BlockHash,
+    ) -> (Vec<HeaderEntry>, Option<usize>) {
         // header[i] -> header[i-1] (i.e. header.last() is the tip)
-        struct HashedHeader {
-            blockhash: BlockHash,
-            header: BlockHeader,
-        }
-        let hashed_headers =
-            Vec::<HashedHeader>::from_iter(new_headers.into_iter().map(|header| HashedHeader {
-                blockhash: header.block_hash(),
-                header,
-            }));
-        for i in 1..hashed_headers.len() {
-            assert_eq!(
-                hashed_headers[i].header.prev_blockhash,
-                hashed_headers[i - 1].blockhash
-            );
-        }
-        let prev_blockhash = match hashed_headers.first() {
-            Some(h) => h.header.prev_blockhash,
-            None => return (vec![], None), // hashed_headers is empty
-        };
-        let new_height: usize = if prev_blockhash == *DEFAULT_BLOCKHASH {
-            0
+        let (new_height, header_entries) = if !new_headers.is_empty() {
+            let hashed_headers = new_headers
+                .into_iter()
+                .map(|h| (h.block_hash(), h))
+                .collect::<Vec<_>>();
+            for ((curr_blockhash, _), (_, next_header)) in hashed_headers.iter().tuple_windows() {
+                assert_eq!(*curr_blockhash, next_header.prev_blockhash);
+            }
+            assert_eq!(hashed_headers.last().unwrap().0, *new_tip);
+
+            let prev_blockhash = &hashed_headers.first().unwrap().1.prev_blockhash;
+            let new_height = if *prev_blockhash == *DEFAULT_BLOCKHASH {
+                0
+            } else {
+                self.header_by_blockhash(prev_blockhash)
+                    .expect("headers do not connect")
+                    .height()
+                    + 1
+            };
+            let header_entries = (new_height..)
+                .zip(hashed_headers)
+                .map(|(height, (hash, header))| HeaderEntry {
+                    height,
+                    hash,
+                    header,
+                })
+                .collect();
+            (new_height, header_entries)
         } else {
-            self.header_by_blockhash(&prev_blockhash)
-                .unwrap_or_else(|| panic!("{} is not part of the blockchain", prev_blockhash))
+            // No new headers, but the new tip could potentially shorten the chain (or be a no-op if it matches the existing tip)
+            // This should not normally happen, but might due to manual `invalidateblock`
+            let new_height = self
+                .header_by_blockhash(new_tip)
+                .expect("new tip not in chain")
                 .height()
-                + 1
+                + 1;
+            (new_height, vec![])
         };
-        let header_entries = (new_height..)
-            .zip(hashed_headers.into_iter())
-            .map(|(height, hashed_header)| HeaderEntry {
-                height,
-                hash: hashed_header.blockhash,
-                header: hashed_header.header,
-            })
-            .collect();
         let reorged_since = (new_height < self.len()).then_some(new_height);
         (header_entries, reorged_since)
     }
@@ -200,12 +207,9 @@ impl HeaderList {
     #[trace]
     pub fn append(&mut self, new_headers: Vec<HeaderEntry>) {
         // new_headers[i] -> new_headers[i - 1] (i.e. new_headers.last() is the tip)
-        for i in 1..new_headers.len() {
-            assert_eq!(new_headers[i - 1].height() + 1, new_headers[i].height());
-            assert_eq!(
-                *new_headers[i - 1].hash(),
-                new_headers[i].header().prev_blockhash
-            );
+        for (curr_header, next_header) in new_headers.iter().tuple_windows() {
+            assert_eq!(curr_header.height() + 1, next_header.height());
+            assert_eq!(*curr_header.hash(), next_header.header().prev_blockhash);
         }
         let new_height = match new_headers.first() {
             Some(entry) => {

tests/rest.rs

@@ -399,15 +399,19 @@ fn test_rest() -> Result<()> {
         let c_spends = get_outspend(&tx_b.input[0].previous_output)?;
         assert_eq!(c_spends["spent"].as_bool(), Some(false));
 
+        // Invalidate the tip with no replacement, shortening the chain by one block
+        tester.invalidate_block(&tester.get_best_block_hash()?)?;
+        tester.sync()?;
+        assert_eq!(
+            get_plain("/blocks/tip/height")?,
+            (init_height + 20).to_string()
+        );
+
         // Reorg everything back to genesis
         tester.invalidate_block(&tester.get_block_hash(1)?)?;
-        tester.call::<Value>(
-            "generateblock",
-            &[miner_address.to_string().into(), Vec::<Value>::new().into()],
-        )?;
         tester.sync()?;
 
-        assert_eq!(get_plain("/blocks/tip/height")?, 1.to_string());
+        assert_eq!(get_plain("/blocks/tip/height")?, 0.to_string());
         assert_eq!(
             get_chain_stats(&address)?["funded_txo_sum"].as_u64(),
             Some(0)