Merge pull request #5207 from stacks-network/test/stop-bitcoind

test: try stopping bitcoind within Rust, rather than spawning the CLI

Author: obycode

.github/workflows/bitcoin-tests.yml

@@ -75,6 +75,9 @@ jobs:
           - tests::neon_integrations::vote_for_aggregate_key_burn_op_test
           - tests::neon_integrations::mock_miner_replay
           - tests::neon_integrations::listunspent_max_utxos
+          - tests::neon_integrations::bitcoin_reorg_flap
+          - tests::neon_integrations::bitcoin_reorg_flap_with_follower
+          - tests::neon_integrations::start_stop_bitcoind
           - tests::epoch_25::microblocks_disabled
           - tests::should_succeed_handling_malformed_and_valid_txs
           - tests::nakamoto_integrations::simple_neon_integration
@@ -122,9 +125,6 @@ jobs:
           - tests::nakamoto_integrations::follower_bootup_across_multiple_cycles
           - tests::nakamoto_integrations::utxo_check_on_startup_panic
           - tests::nakamoto_integrations::utxo_check_on_startup_recover
-          # Do not run this one until we figure out why it fails in CI
-          # - tests::neon_integrations::bitcoin_reorg_flap
-          # - tests::neon_integrations::bitcoin_reorg_flap_with_follower
           # TODO: enable these once v1 signer is supported by a new nakamoto epoch
           # - tests::signer::v1::dkg
           # - tests::signer::v1::sign_request_rejected

testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs

@@ -2831,7 +2831,7 @@ impl BitcoinRPCRequest {
         Ok(())
     }
 
-    fn send(config: &Config, payload: BitcoinRPCRequest) -> RPCResult<serde_json::Value> {
+    pub fn send(config: &Config, payload: BitcoinRPCRequest) -> RPCResult<serde_json::Value> {
         let request = BitcoinRPCRequest::build_rpc_request(&config, &payload);
         let timeout = Duration::from_secs(u64::from(config.burnchain.timeout));
 

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

@@ -11,6 +11,7 @@ use stacks::core::StacksEpochId;
 use stacks_common::util::hash::hex_bytes;
 
 use super::PUBLISH_CONTRACT;
+use crate::burnchains::bitcoin_regtest_controller::BitcoinRPCRequest;
 use crate::config::InitialBalance;
 use crate::helium::RunLoop;
 use crate::tests::to_addr;
@@ -19,12 +20,14 @@ use crate::Config;
 #[derive(Debug)]
 pub enum BitcoinCoreError {
     SpawnFailed(String),
+    StopFailed(String),
 }
 
 impl std::fmt::Display for BitcoinCoreError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::SpawnFailed(msg) => write!(f, "bitcoind spawn failed: {msg}"),
+            Self::StopFailed(msg) => write!(f, "bitcoind stop failed: {msg}"),
         }
     }
 }
@@ -109,25 +112,25 @@ impl BitcoinCoreController {
 
     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");
-
-            self.add_rpc_cli_args(&mut command);
-
-            command.arg("stop");
-
-            let mut process = match command.spawn() {
-                Ok(child) => child,
-                Err(e) => return Err(BitcoinCoreError::SpawnFailed(format!("{e:?}"))),
+            let payload = BitcoinRPCRequest {
+                method: "stop".to_string(),
+                params: vec![],
+                id: "stacks".to_string(),
+                jsonrpc: "2.0".to_string(),
             };
 
-            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;
+            let res = BitcoinRPCRequest::send(&self.config, payload)
+                .map_err(|e| BitcoinCoreError::StopFailed(format!("{e:?}")))?;
+
+            if let Some(err) = res.get("error") {
+                if !err.is_null() {
+                    return Err(BitcoinCoreError::StopFailed(format!("{err}")));
                 }
-                eprintln!("{line}");
+            } else {
+                return Err(BitcoinCoreError::StopFailed(format!(
+                    "Invalid response: {:?}",
+                    res
+                )));
             }
         }
         Ok(())

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

@@ -12398,6 +12398,10 @@ fn bitcoin_reorg_flap() {
     channel.stop_chains_coordinator();
 }
 
+/// Advance the bitcoin chain and wait for the miner and any followers to
+/// process the next block.
+/// NOTE: This only works if the followers are mock-mining, or else the counter
+///       will not be updated.
 fn next_block_and_wait_all(
     btc_controller: &mut BitcoinRegtestController,
     miner_blocks_processed: &Arc<AtomicU64>,
@@ -12447,7 +12451,7 @@ fn bitcoin_reorg_flap_with_follower() {
     }
 
     let (conf, _miner_account) = neon_integration_test_conf();
-    let timeout = None;
+    let timeout = Some(Duration::from_secs(60));
 
     let mut btcd_controller = BitcoinCoreController::new(conf.clone());
     btcd_controller
@@ -12461,10 +12465,12 @@ fn bitcoin_reorg_flap_with_follower() {
     eprintln!("Chain bootstrapped...");
 
     let mut miner_run_loop = neon::RunLoop::new(conf.clone());
+    let run_loop_stopper = miner_run_loop.get_termination_switch();
     let miner_blocks_processed = miner_run_loop.get_blocks_processed_arc();
     let miner_channel = miner_run_loop.get_coordinator_channel().unwrap();
 
     let mut follower_conf = conf.clone();
+    follower_conf.node.mock_mining = true;
     follower_conf.events_observers.clear();
     follower_conf.node.working_dir = format!("{}-follower", &conf.node.working_dir);
     follower_conf.node.seed = vec![0x01; 32];
@@ -12483,7 +12489,7 @@ fn bitcoin_reorg_flap_with_follower() {
     follower_conf.node.data_url = format!("http://{}:{}", &localhost, rpc_port);
     follower_conf.node.p2p_address = format!("{}:{}", &localhost, p2p_port);
 
-    thread::spawn(move || miner_run_loop.start(None, 0));
+    let run_loop_thread = thread::spawn(move || miner_run_loop.start(None, 0));
     wait_for_runloop(&miner_blocks_processed);
 
     // figure out the started node's port
@@ -12499,23 +12505,20 @@ fn bitcoin_reorg_flap_with_follower() {
     );
 
     let mut follower_run_loop = neon::RunLoop::new(follower_conf.clone());
+    let follower_run_loop_stopper = follower_run_loop.get_termination_switch();
     let follower_blocks_processed = follower_run_loop.get_blocks_processed_arc();
     let follower_channel = follower_run_loop.get_coordinator_channel().unwrap();
 
-    thread::spawn(move || follower_run_loop.start(None, 0));
+    let follower_thread = thread::spawn(move || follower_run_loop.start(None, 0));
     wait_for_runloop(&follower_blocks_processed);
 
     eprintln!("Follower bootup complete!");
 
     // first block wakes up the run loop
-    next_block_and_wait_all(
-        &mut btc_regtest_controller,
-        &miner_blocks_processed,
-        &[],
-        timeout,
-    );
+    next_block_and_wait_with_timeout(&mut btc_regtest_controller, &miner_blocks_processed, 60);
 
-    // first block will hold our VRF registration
+    // next block will hold our VRF registration
+    // Note that the follower will not see its block processed counter bumped here
     next_block_and_wait_all(
         &mut btc_regtest_controller,
         &miner_blocks_processed,
@@ -12609,9 +12612,11 @@ fn bitcoin_reorg_flap_with_follower() {
     assert_eq!(miner_channel.get_sortitions_processed(), 225);
     assert_eq!(follower_channel.get_sortitions_processed(), 225);
 
-    btcd_controller.stop_bitcoind().unwrap();
-    miner_channel.stop_chains_coordinator();
-    follower_channel.stop_chains_coordinator();
+    run_loop_stopper.store(false, Ordering::SeqCst);
+    follower_run_loop_stopper.store(false, Ordering::SeqCst);
+
+    run_loop_thread.join().unwrap();
+    follower_thread.join().unwrap();
 }
 
 /// Tests the following:
@@ -12841,3 +12846,44 @@ fn listunspent_max_utxos() {
     let utxos = res.expect("Failed to get utxos");
     assert_eq!(utxos.num_utxos(), 10);
 }
+
+#[test]
+#[ignore]
+/// Test out stopping bitcoind and restarting it
+fn start_stop_bitcoind() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    let (mut conf, _miner_account) = neon_integration_test_conf();
+    let prom_bind = format!("{}:{}", "127.0.0.1", 6000);
+    conf.node.prometheus_bind = Some(prom_bind.clone());
+
+    conf.burnchain.max_rbf = 1000000;
+
+    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...");
+
+    btcd_controller
+        .stop_bitcoind()
+        .expect("Failed to stop bitcoind");
+
+    thread::sleep(Duration::from_secs(5));
+
+    btcd_controller
+        .start_bitcoind()
+        .expect("Failed to start bitcoind");
+
+    btcd_controller
+        .stop_bitcoind()
+        .expect("Failed to stop bitcoind");
+}