diff --git a/CHANGELOG.md b/CHANGELOG.md index f2621a166..d82d5e79b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE - Use the current burnchain tip to lookup UTXOs (Issue #3733) +- The node now gracefully shuts down even if it is in the middle of a handshake with + bitcoind. Fixes issue #3734. + ## [2.4.0.0.0] This is a **consensus-breaking** release to revert consensus to PoX, and is the second fork proposed in SIP-022. diff --git a/src/burnchains/bitcoin/indexer.rs b/src/burnchains/bitcoin/indexer.rs index be395ef9e..ebcbb820a 100644 --- a/src/burnchains/bitcoin/indexer.rs +++ b/src/burnchains/bitcoin/indexer.rs @@ -58,6 +58,8 @@ use crate::core::{ StacksEpoch, STACKS_EPOCHS_MAINNET, STACKS_EPOCHS_REGTEST, STACKS_EPOCHS_TESTNET, }; use std::convert::TryFrom; +use std::sync::atomic::AtomicBool; +use std::sync::Arc; pub const USER_AGENT: &'static str = "Stacks/2.1"; @@ -140,6 +142,7 @@ pub struct BitcoinIndexerRuntime { pub struct BitcoinIndexer { pub config: BitcoinIndexerConfig, pub runtime: BitcoinIndexerRuntime, + pub should_keep_running: Option>, } impl BitcoinIndexerConfig { @@ -211,10 +214,16 @@ impl BitcoinIndexerRuntime { } impl BitcoinIndexer { - pub fn new(config: BitcoinIndexerConfig, runtime: BitcoinIndexerRuntime) -> BitcoinIndexer { + #[cfg(test)] + pub fn new( + config: BitcoinIndexerConfig, + runtime: BitcoinIndexerRuntime, + should_keep_running: Option>, + ) -> BitcoinIndexer { BitcoinIndexer { - config: config, - runtime: runtime, + config, + runtime, + should_keep_running, } } @@ -243,6 +252,7 @@ impl BitcoinIndexer { working_dir_path.to_str().unwrap().to_string(), ), runtime: BitcoinIndexerRuntime::new(BitcoinNetworkType::Regtest), + should_keep_running: None, } } @@ -250,6 +260,7 @@ impl BitcoinIndexer { BitcoinIndexer { config: self.config.clone(), runtime: BitcoinIndexerRuntime::new(self.runtime.network_id), + should_keep_running: self.should_keep_running.clone(), } } @@ -1203,7 +1214,8 @@ mod test { use stacks_common::util::get_epoch_time_secs; use stacks_common::util::uint::Uint256; - use std::env; + use std::sync::atomic::Ordering; + use std::{env, thread}; #[test] fn test_indexer_find_bitcoin_reorg_genesis() { @@ -1353,6 +1365,7 @@ mod test { let mut indexer = BitcoinIndexer::new( BitcoinIndexerConfig::test_default(path_1.to_string()), BitcoinIndexerRuntime::new(BitcoinNetworkType::Regtest), + None, ); let common_ancestor_height = indexer .inner_find_bitcoin_reorg( @@ -1527,6 +1540,7 @@ mod test { let mut indexer = BitcoinIndexer::new( BitcoinIndexerConfig::test_default(path_1.to_string()), BitcoinIndexerRuntime::new(BitcoinNetworkType::Regtest), + None, ); let common_ancestor_height = indexer .inner_find_bitcoin_reorg( @@ -1610,7 +1624,7 @@ mod test { fs::remove_file(&indexer_conf.spv_headers_path).unwrap(); } - let mut indexer = BitcoinIndexer::new(indexer_conf, BitcoinIndexerRuntime::new(mode)); + let mut indexer = BitcoinIndexer::new(indexer_conf, BitcoinIndexerRuntime::new(mode), None); let last_block = indexer.sync_headers(0, None).unwrap(); eprintln!("sync'ed to block {}", last_block); @@ -3181,6 +3195,7 @@ mod test { let mut indexer = BitcoinIndexer::new( BitcoinIndexerConfig::test_default(db_path.to_string()), BitcoinIndexerRuntime::new(BitcoinNetworkType::Mainnet), + None, ); let mut inserted_bad_header = false; @@ -3341,6 +3356,7 @@ mod test { let mut indexer = BitcoinIndexer::new( BitcoinIndexerConfig::test_default(db_path.to_string()), BitcoinIndexerRuntime::new(BitcoinNetworkType::Mainnet), + None, ); let mut inserted_good_header = false; @@ -3481,6 +3497,7 @@ mod test { let mut indexer = BitcoinIndexer::new( BitcoinIndexerConfig::test_default(db_path.to_string()), BitcoinIndexerRuntime::new(BitcoinNetworkType::Regtest), + None, ); if let Err(burnchain_error::TrySyncAgain) = indexer.check_chain_tip_timestamp() { @@ -3493,4 +3510,26 @@ mod test { assert!(indexer.check_chain_tip_timestamp().is_ok()); assert_eq!(spv_client.get_highest_header_height().unwrap(), 1); } + + /// This test ensures that setting `should_keep_running` to false halts the handshake function. + #[test] + fn test_should_keep_running_halts_handshake() { + let db_path = "/tmp/test_should_keep_running.dat".to_string(); + + if fs::metadata(&db_path).is_ok() { + fs::remove_file(&db_path).unwrap(); + } + + let should_keep_running = Arc::new(AtomicBool::new(true)); + let mut indexer = BitcoinIndexer::new( + BitcoinIndexerConfig::test_default(db_path.to_string()), + BitcoinIndexerRuntime::new(BitcoinNetworkType::Mainnet), + Some(should_keep_running.clone()), + ); + + thread::spawn(move || indexer.connect_handshake_backoff()); + thread::sleep(Duration::from_millis(10_000)); + + should_keep_running.store(false, Ordering::SeqCst); + } } diff --git a/src/burnchains/bitcoin/network.rs b/src/burnchains/bitcoin/network.rs index f633ede19..124430018 100644 --- a/src/burnchains/bitcoin/network.rs +++ b/src/burnchains/bitcoin/network.rs @@ -18,6 +18,7 @@ use std::io; use std::io::Write; use std::net::SocketAddr; use std::ops::Deref; +use std::sync::atomic::Ordering; use std::thread; use std::time; use std::time::{SystemTime, UNIX_EPOCH}; @@ -224,6 +225,12 @@ impl BitcoinIndexer { warn!("Connection broken; retrying in {} sec...", backoff); } + if let Some(ref should_keep_running) = self.should_keep_running { + if !should_keep_running.load(Ordering::SeqCst) { + return Err(btc_error::TimedOut); + } + } + let duration = time::Duration::from_millis((backoff * 1_000.0) as u64); thread::sleep(duration); } diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 15c3bc1d2..c87dfee07 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -132,7 +132,10 @@ pub fn addr2str(btc_addr: &BitcoinAddress) -> String { } /// Helper method to create a BitcoinIndexer -pub fn make_bitcoin_indexer(config: &Config) -> BitcoinIndexer { +pub fn make_bitcoin_indexer( + config: &Config, + should_keep_running: Option>, +) -> BitcoinIndexer { let (network, _) = config.burnchain.get_bitcoin_network(); let burnchain_params = BurnchainParameters::from_params(&config.burnchain.chain, &network) .expect("Bitcoin network unsupported"); @@ -158,6 +161,7 @@ pub fn make_bitcoin_indexer(config: &Config) -> BitcoinIndexer { let burnchain_indexer = BitcoinIndexer { config: indexer_config.clone(), runtime: indexer_runtime, + should_keep_running: should_keep_running, }; burnchain_indexer } @@ -307,6 +311,7 @@ impl BitcoinRegtestController { let burnchain_indexer = BitcoinIndexer { config: indexer_config.clone(), runtime: indexer_runtime, + should_keep_running: should_keep_running.clone(), }; Self { @@ -352,6 +357,7 @@ impl BitcoinRegtestController { let burnchain_indexer = BitcoinIndexer { config: indexer_config.clone(), runtime: indexer_runtime, + should_keep_running: None, }; Self { diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index aead48f4e..032413814 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -4060,7 +4060,11 @@ impl StacksNode { /// Main loop of the p2p thread. /// Runs in a separate thread. /// Continuously receives, until told otherwise. - pub fn p2p_main(mut p2p_thread: PeerThread, event_dispatcher: EventDispatcher) { + pub fn p2p_main( + mut p2p_thread: PeerThread, + event_dispatcher: EventDispatcher, + should_keep_running: Arc, + ) { let (mut dns_resolver, mut dns_client) = DNSResolver::new(10); // spawn a daemon thread that runs the DNS resolver. @@ -4087,7 +4091,7 @@ impl StacksNode { .make_cost_metric() .unwrap_or_else(|| Box::new(UnitMetric)); - let indexer = make_bitcoin_indexer(&p2p_thread.config); + let indexer = make_bitcoin_indexer(&p2p_thread.config, Some(should_keep_running)); // receive until we can't reach the receiver thread loop { @@ -4187,6 +4191,7 @@ impl StacksNode { }) .expect("FATAL: failed to start relayer thread"); + let should_keep_running_clone = globals.should_keep_running.clone(); let p2p_event_dispatcher = runloop.get_event_dispatcher(); let p2p_thread = PeerThread::new(runloop, p2p_net); let p2p_thread_handle = thread::Builder::new() @@ -4197,7 +4202,7 @@ impl StacksNode { )) .spawn(move || { debug!("p2p thread ID is {:?}", thread::current().id()); - Self::p2p_main(p2p_thread, p2p_event_dispatcher); + Self::p2p_main(p2p_thread, p2p_event_dispatcher, should_keep_running_clone); }) .expect("FATAL: failed to start p2p thread"); diff --git a/testnet/stacks-node/src/node.rs b/testnet/stacks-node/src/node.rs index 4f29e94e0..41404fb22 100644 --- a/testnet/stacks-node/src/node.rs +++ b/testnet/stacks-node/src/node.rs @@ -163,6 +163,7 @@ pub fn get_names(use_test_chainstate_data: bool) -> Box, - ) -> Node { - let burnchain_tip = burnchain_controller.get_chain_tip(); - - let keychain = Keychain::default(config.node.seed.clone()); - - let mut event_dispatcher = EventDispatcher::new(); - - for observer in &config.events_observers { - event_dispatcher.register_observer(observer); - } - - let chainstate_path = config.get_chainstate_path_str(); - let sortdb_path = config.get_burn_db_file_path(); - - let (chain_state, _) = match StacksChainState::open( - config.is_mainnet(), - config.burnchain.chain_id, - &chainstate_path, - Some(config.node.get_marf_opts()), - ) { - Ok(x) => x, - Err(_e) => panic!(), - }; - - let mut node = Node { - active_registered_key: None, - bootstraping_chain: false, - chain_state, - chain_tip: None, - keychain, - last_sortitioned_block: None, - config, - burnchain_tip: None, - nonce: 0, - event_dispatcher, - leader_key_registers: HashSet::new(), - block_commits: HashSet::new(), - }; - - node.spawn_peer_server(); - - let pox_constants = burnchain_controller.sortdb_ref().pox_constants.clone(); - loop { - let sortdb = SortitionDB::open(&sortdb_path, false, pox_constants.clone()) - .expect("BUG: failed to open burn database"); - if let Ok(Some(ref chain_tip)) = node.chain_state.get_stacks_chain_tip(&sortdb) { - if chain_tip.consensus_hash == burnchain_tip.block_snapshot.consensus_hash { - info!("Syncing Stacks blocks - completed"); - break; - } else { - info!( - "Syncing Stacks blocks - received block #{}", - chain_tip.height - ); - } - } else { - info!("Syncing Stacks blocks - unable to progress"); - } - thread::sleep(time::Duration::from_secs(5)); - } - node - } - fn make_atlas_config() -> AtlasConfig { AtlasConfig::new(false) } @@ -458,6 +393,7 @@ impl Node { .unwrap() } + // This function is used for helium and mocknet. pub fn spawn_peer_server(&mut self) { // we can call _open_ here rather than _connect_, since connect is first called in // make_genesis_block diff --git a/testnet/stacks-node/src/run_loop/neon.rs b/testnet/stacks-node/src/run_loop/neon.rs index 5182ceddd..ffcd15288 100644 --- a/testnet/stacks-node/src/run_loop/neon.rs +++ b/testnet/stacks-node/src/run_loop/neon.rs @@ -525,7 +525,8 @@ impl RunLoop { true, ) .expect("Failed to connect Atlas DB during startup"); - let coordinator_indexer = make_bitcoin_indexer(&self.config); + let coordinator_indexer = + make_bitcoin_indexer(&self.config, Some(self.should_keep_running.clone())); let coordinator_thread_handle = thread::Builder::new() .name(format!( @@ -652,7 +653,7 @@ impl RunLoop { let sn = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) .expect("FATAL: could not read sortition DB"); - let indexer = make_bitcoin_indexer(config); + let indexer = make_bitcoin_indexer(config, Some(globals.should_keep_running.clone())); let heaviest_affirmation_map = match static_get_heaviest_affirmation_map( &burnchain, @@ -799,7 +800,7 @@ impl RunLoop { } }; - let indexer = make_bitcoin_indexer(config); + let indexer = make_bitcoin_indexer(config, Some(globals.should_keep_running.clone())); let heaviest_affirmation_map = match static_get_heaviest_affirmation_map( &burnchain,