fix: handle burn chain flapping

This test (from Jude) can reproduce the problematic behavior when
the burnchain flaps between two branches.

- We get blocks at height 211 - 213
- Then we get a fork, with different blocks at 211-213, as well as 214
  and 215.
- We then flap back to the original fork, so it goes back to the common
  ancestor, 210, and tries to download 211-215
- When it gets block 211, it is already in the database, so it fails,
  cancelling the download of the rest of the blocks, leaving 214 and 215
  in this branch not stored
- Then we try to store 216, but its parent 215 is not stored yet, so we
  cannot continue.

The fix for this is to ignore attempts to store duplicate blocks.

Author: obycode

stackslib/src/burnchains/bitcoin/indexer.rs

@@ -837,7 +837,11 @@ impl BitcoinIndexer {
                 }
             } else {
                 // ignore the reorg
-                test_debug!("Reorg chain does not overtake original Bitcoin chain");
+                test_debug!(
+                    "Reorg chain does not overtake original Bitcoin chain ({} >= {})",
+                    orig_total_work,
+                    reorg_total_work
+                );
                 new_tip = orig_spv_client.get_headers_height()?;
             }
         }

stackslib/src/burnchains/db.rs

@@ -313,7 +313,7 @@ impl<'a> BurnchainDBTransaction<'a> {
         &self,
         header: &BurnchainBlockHeader,
     ) -> Result<i64, BurnchainError> {
-        let sql = "INSERT INTO burnchain_db_block_headers
+        let sql = "INSERT OR IGNORE INTO burnchain_db_block_headers
                    (block_height, block_hash, parent_block_hash, num_txs, timestamp)
                    VALUES (?, ?, ?, ?, ?)";
         let args: &[&dyn ToSql] = &[
@@ -323,9 +323,17 @@ impl<'a> BurnchainDBTransaction<'a> {
             &u64_to_sql(header.num_txs)?,
             &u64_to_sql(header.timestamp)?,
         ];
-        match self.sql_tx.execute(sql, args) {
-            Ok(_) => Ok(self.sql_tx.last_insert_rowid()),
-            Err(e) => Err(e.into()),
+        let affected_rows = self.sql_tx.execute(sql, args)?;
+        if affected_rows == 0 {
+            // This means a duplicate entry was found and the insert operation was ignored
+            debug!(
+                "Duplicate entry for block_hash: {}, insert operation ignored.",
+                header.block_hash
+            );
+            Ok(-1)
+        } else {
+            // A new row was inserted successfully
+            Ok(self.sql_tx.last_insert_rowid())
         }
     }
 

stackslib/src/chainstate/coordinator/mod.rs

@@ -2216,7 +2216,7 @@ impl<
                 BurnchainDB::get_burnchain_block(&self.burnchain_blocks_db.conn(), &cursor)
                     .map_err(|e| {
                         warn!(
-                            "ChainsCoordinator: could not retrieve  block burnhash={}",
+                            "ChainsCoordinator: could not retrieve block burnhash={}",
                             &cursor
                         );
                         Error::NonContiguousBurnchainBlock(e)

testnet/stacks-node/src/tests/bitcoin_regtest.rs

@@ -16,6 +16,7 @@ use crate::helium::RunLoop;
 use crate::tests::to_addr;
 use crate::Config;
 
+#[derive(Debug)]
 pub enum BitcoinCoreError {
     SpawnFailed(String),
 }
@@ -75,7 +76,6 @@ impl BitcoinCoreController {
             Err(e) => return Err(BitcoinCoreError::SpawnFailed(format!("{:?}", e))),
         };
 
-        eprintln!("bitcoind spawned, waiting for startup");
         let mut out_reader = BufReader::new(process.stdout.take().unwrap());
 
         let mut line = String::new();
@@ -97,6 +97,34 @@ impl BitcoinCoreController {
         Ok(())
     }
 
+    pub fn stop_bitcoind(&mut self) -> Result<(), BitcoinCoreError> {
+        if let Some(_) = self.bitcoind_process.take() {
+            let mut command = Command::new("bitcoin-cli");
+            command
+                .stdout(Stdio::piped())
+                .arg("-rpcconnect=127.0.0.1")
+                .arg("-rpcport=8332")
+                .arg("-rpcuser=neon-tester")
+                .arg("-rpcpassword=neon-tester-pass")
+                .arg("stop");
+
+            let mut process = match command.spawn() {
+                Ok(child) => child,
+                Err(e) => return Err(BitcoinCoreError::SpawnFailed(format!("{:?}", e))),
+            };
+
+            let mut out_reader = BufReader::new(process.stdout.take().unwrap());
+            let mut line = String::new();
+            while let Ok(bytes_read) = out_reader.read_line(&mut line) {
+                if bytes_read == 0 {
+                    break;
+                }
+                eprintln!("{}", &line);
+            }
+        }
+        Ok(())
+    }
+
     pub fn kill_bitcoind(&mut self) {
         if let Some(mut bitcoind_process) = self.bitcoind_process.take() {
             bitcoind_process.kill().unwrap();

testnet/stacks-node/src/tests/neon_integrations.rs

@@ -4,7 +4,7 @@ use std::path::Path;
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{mpsc, Arc};
 use std::time::{Duration, Instant};
-use std::{cmp, env, fs, thread};
+use std::{cmp, env, fs, io, thread};
 
 use clarity::vm::ast::stack_depth_checker::AST_CALL_STACK_DEPTH_BUFFER;
 use clarity::vm::ast::ASTRules;
@@ -9302,7 +9302,11 @@ fn test_problematic_blocks_are_not_relayed_or_stored() {
     let tip_info = get_chain_info(&conf);
 
     // all blocks were processed
-    assert!(tip_info.stacks_tip_height >= old_tip_info.stacks_tip_height + 5);
+    info!(
+        "tip_info.stacks_tip_height = {}, old_tip_info.stacks_tip_height = {}",
+        tip_info.stacks_tip_height, old_tip_info.stacks_tip_height
+    );
+    assert!(tip_info.stacks_tip_height > old_tip_info.stacks_tip_height);
     // one was problematic -- i.e. the one that included tx_high
     assert_eq!(all_new_files.len(), 1);
 
@@ -11174,3 +11178,133 @@ fn filter_txs_by_origin() {
 
     test_observer::clear();
 }
+
+// https://stackoverflow.com/questions/26958489/how-to-copy-a-folder-recursively-in-rust
+fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
+    fs::create_dir_all(&dst)?;
+    for entry in fs::read_dir(src)? {
+        let entry = entry?;
+        let ty = entry.file_type()?;
+        if ty.is_dir() {
+            copy_dir_all(entry.path(), dst.as_ref().join(entry.file_name()))?;
+        } else {
+            fs::copy(entry.path(), dst.as_ref().join(entry.file_name()))?;
+        }
+    }
+    Ok(())
+}
+
+#[test]
+#[ignore]
+fn bitcoin_reorg_flap() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    let (conf, _miner_account) = neon_integration_test_conf();
+
+    let mut btcd_controller = BitcoinCoreController::new(conf.clone());
+    btcd_controller
+        .start_bitcoind()
+        .map_err(|_e| ())
+        .expect("Failed starting bitcoind");
+
+    let mut btc_regtest_controller = BitcoinRegtestController::new(conf.clone(), None);
+
+    btc_regtest_controller.bootstrap_chain(201);
+
+    eprintln!("Chain bootstrapped...");
+
+    let mut run_loop = neon::RunLoop::new(conf.clone());
+    let blocks_processed = run_loop.get_blocks_processed_arc();
+
+    let channel = run_loop.get_coordinator_channel().unwrap();
+
+    thread::spawn(move || run_loop.start(None, 0));
+
+    // give the run loop some time to start up!
+    wait_for_runloop(&blocks_processed);
+
+    // first block wakes up the run loop
+    next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
+
+    // first block will hold our VRF registration
+    next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
+
+    let mut sort_height = channel.get_sortitions_processed();
+    eprintln!("Sort height: {}", sort_height);
+
+    while sort_height < 210 {
+        next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
+        sort_height = channel.get_sortitions_processed();
+        eprintln!("Sort height: {}", sort_height);
+    }
+
+    // stop bitcoind and copy its DB to simulate a chain flap
+    btcd_controller.stop_bitcoind().unwrap();
+    thread::sleep(Duration::from_secs(5));
+
+    let btcd_dir = conf.get_burnchain_path_str();
+    let mut new_conf = conf.clone();
+    new_conf.node.working_dir = format!("{}.new", &conf.node.working_dir);
+    fs::create_dir_all(&new_conf.node.working_dir).unwrap();
+
+    copy_dir_all(&btcd_dir, &new_conf.get_burnchain_path_str()).unwrap();
+
+    // resume
+    let mut btcd_controller = BitcoinCoreController::new(conf.clone());
+    btcd_controller
+        .start_bitcoind()
+        .map_err(|_e| ())
+        .expect("Failed starting bitcoind");
+
+    let btc_regtest_controller = BitcoinRegtestController::new(conf.clone(), None);
+    thread::sleep(Duration::from_secs(5));
+
+    info!("\n\nBegin fork A\n\n");
+
+    // make fork A
+    for _i in 0..3 {
+        btc_regtest_controller.build_next_block(1);
+        thread::sleep(Duration::from_secs(5));
+    }
+
+    btcd_controller.stop_bitcoind().unwrap();
+
+    info!("\n\nBegin reorg flap from A to B\n\n");
+
+    // carry out the flap to fork B -- new_conf's state was the same as before the reorg
+    let mut btcd_controller = BitcoinCoreController::new(new_conf.clone());
+    let btc_regtest_controller = BitcoinRegtestController::new(new_conf.clone(), None);
+
+    btcd_controller
+        .start_bitcoind()
+        .map_err(|_e| ())
+        .expect("Failed starting bitcoind");
+
+    for _i in 0..5 {
+        btc_regtest_controller.build_next_block(1);
+        thread::sleep(Duration::from_secs(5));
+    }
+
+    btcd_controller.stop_bitcoind().unwrap();
+
+    info!("\n\nBegin reorg flap from B to A\n\n");
+
+    let mut btcd_controller = BitcoinCoreController::new(conf.clone());
+    let btc_regtest_controller = BitcoinRegtestController::new(conf.clone(), None);
+    btcd_controller
+        .start_bitcoind()
+        .map_err(|_e| ())
+        .expect("Failed starting bitcoind");
+
+    // carry out the flap back to fork A
+    for _i in 0..7 {
+        btc_regtest_controller.build_next_block(1);
+        thread::sleep(Duration::from_secs(5));
+    }
+
+    assert_eq!(channel.get_sortitions_processed(), 225);
+    btcd_controller.stop_bitcoind().unwrap();
+    channel.stop_chains_coordinator();
+}