Skip to content

Commit 45dcd31

Browse files
hstovehstove
committed
fix: properly wait for bitcoin to shut down
1 parent 167f4c6 commit 45dcd31

File tree

4 files changed

+85
-78
lines changed

4 files changed

+85
-78
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2822,6 +2822,17 @@ impl BitcoinRPCRequest {
28222822
Ok(())
28232823
}
28242824

2825+
pub fn stop_bitcoind(config: &Config) -> RPCResult<serde_json::Value> {
2826+
let payload = BitcoinRPCRequest {
2827+
method: "stop".to_string(),
2828+
params: vec![],
2829+
id: "stacks".to_string(),
2830+
jsonrpc: "2.0".to_string(),
2831+
};
2832+
2833+
BitcoinRPCRequest::send(config, payload)
2834+
}
2835+
28252836
pub fn send(config: &Config, payload: BitcoinRPCRequest) -> RPCResult<serde_json::Value> {
28262837
let request = BitcoinRPCRequest::build_rpc_request(config, &payload);
28272838
let timeout = Duration::from_secs(u64::from(config.burnchain.timeout));

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,23 @@ impl BitcoinCoreController {
103103
}
104104

105105
pub fn stop_bitcoind(&mut self) -> Result<(), BitcoinCoreError> {
106-
if self.bitcoind_process.take().is_some() {
107-
let payload = BitcoinRPCRequest {
108-
method: "stop".to_string(),
109-
params: vec![],
110-
id: "stacks".to_string(),
111-
jsonrpc: "2.0".to_string(),
112-
};
113-
114-
let res = BitcoinRPCRequest::send(&self.config, payload)
106+
if let Some(mut bitcoind_process) = self.bitcoind_process.take() {
107+
let res = BitcoinRPCRequest::stop_bitcoind(&self.config)
115108
.map_err(|e| BitcoinCoreError::StopFailed(format!("{e:?}")))?;
116109

117110
if let Some(err) = res.get("error") {
118111
if !err.is_null() {
119112
return Err(BitcoinCoreError::StopFailed(format!("{err}")));
120113
}
114+
} else if let Some(_result) = res.get("result") {
115+
// Expected, continue
121116
} else {
122117
return Err(BitcoinCoreError::StopFailed(format!(
123118
"Invalid response: {res:?}"
124119
)));
125120
}
121+
122+
bitcoind_process.wait().unwrap();
126123
}
127124
Ok(())
128125
}

‎testnet/stacks-node/src/tests/signer/mod.rs

Lines changed: 67 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ mod commands;
1616
mod v0;
1717

1818
use std::collections::HashSet;
19+
use std::fs::File;
1920
use std::path::PathBuf;
2021
use std::sync::atomic::{AtomicBool, Ordering};
2122
use std::sync::{Arc, Mutex};
22-
use std::time::{Duration, Instant};
23+
use std::time::{Duration, Instant, SystemTime};
2324
use std::{env, thread};
2425

2526
use clarity::boot_util::boot_code_id;
@@ -30,6 +31,7 @@ use libsigner::v0::messages::{
3031
};
3132
use libsigner::v0::signer_state::MinerState;
3233
use libsigner::{BlockProposal, SignerEntries, SignerEventTrait};
34+
use serde::{Deserialize, Serialize};
3335
use stacks::chainstate::coordinator::comm::CoordinatorChannels;
3436
use stacks::chainstate::nakamoto::signer_set::NakamotoSigners;
3537
use stacks::chainstate::nakamoto::NakamotoBlock;
@@ -44,9 +46,9 @@ use stacks::net::api::postblock_proposal::{
4446
};
4547
use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPublicKey};
4648
use stacks::types::PrivateKey;
49+
use stacks::util::get_epoch_time_secs;
4750
use stacks::util::hash::MerkleHashFunc;
4851
use stacks::util::secp256k1::{MessageSignature, Secp256k1PublicKey};
49-
use stacks::util::{get_epoch_time_secs, sleep_ms};
5052
use stacks_common::codec::StacksMessageCodec;
5153
use stacks_common::consts::SIGNER_SLOTS_PER_USER;
5254
use stacks_common::types::StacksEpochId;
@@ -64,7 +66,6 @@ use super::nakamoto_integrations::{
6466
use super::neon_integrations::{
6567
copy_dir_all, get_account, get_sortition_info_ch, submit_tx_fallible, Account,
6668
};
67-
use crate::burnchains::bitcoin_regtest_controller::BitcoinRPCRequest;
6869
use crate::neon::Counters;
6970
use crate::run_loop::boot_nakamoto;
7071
use crate::tests::bitcoin_regtest::BitcoinCoreController;
@@ -120,6 +121,11 @@ enum SetupSnapshotResult {
120121
NoSnapshot,
121122
}
122123

124+
#[derive(Serialize, Deserialize, Debug, Clone)]
125+
struct SnapshotMetadata {
126+
created_at: SystemTime,
127+
}
128+
123129
impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<SpawnedSigner<S, T>> {
124130
pub fn new(num_signers: usize, initial_balances: Vec<(StacksAddress, u64)>) -> Self {
125131
Self::new_with_config_modifications(
@@ -244,36 +250,6 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
244250

245251
let snapshot_setup_result = Self::setup_snapshot(snapshot_name, &naka_conf);
246252

247-
// let mut snapshot_exists = false;
248-
249-
// let snapshot_path = snapshot_name.map(|name| {
250-
// let working_dir = naka_conf.get_working_dir();
251-
252-
// let snapshot_path: PathBuf = format!("/tmp/stacks-node-tests/snapshots/{name}/")
253-
// .try_into()
254-
// .unwrap();
255-
256-
// info!("Snapshot path: {}", snapshot_path.clone().display());
257-
258-
// snapshot_exists = std::fs::metadata(snapshot_path.clone()).is_ok();
259-
260-
// if snapshot_exists {
261-
// info!(
262-
// "Snapshot directory already exists, copying to working dir";
263-
// "snapshot_path" => %snapshot_path.display(),
264-
// "working_dir" => %working_dir.display()
265-
// );
266-
// let err_msg = format!(
267-
// "Failed to copy snapshot dir to working dir: {} -> {}",
268-
// snapshot_path.display(),
269-
// working_dir.display()
270-
// );
271-
// copy_dir_all(snapshot_path.clone(), working_dir).expect(&err_msg);
272-
// }
273-
274-
// snapshot_path
275-
// });
276-
277253
let snapshot_exists = match &snapshot_setup_result {
278254
SetupSnapshotResult::WithSnapshot(info) => info.snapshot_exists,
279255
SetupSnapshotResult::NoSnapshot => false,
@@ -304,13 +280,6 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
304280
}
305281
}
306282

307-
pub fn snapshot_exists(&self) -> bool {
308-
self.snapshot_path
309-
.as_ref()
310-
.map(|p| std::fs::metadata(p).is_ok())
311-
.unwrap_or(false)
312-
}
313-
314283
/// Whether the snapshot needs to be created.
315284
///
316285
/// Returns `false` if not configured to snapshot.
@@ -345,6 +314,36 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
345314
let snapshot_exists = std::fs::metadata(snapshot_path.clone()).is_ok();
346315

347316
if snapshot_exists {
317+
let metadata_path = snapshot_path.join("metadata.json");
318+
if !metadata_path.clone().exists() {
319+
warn!("Snapshot metadata file does not exist, not restoring snapshot");
320+
return SetupSnapshotResult::NoSnapshot;
321+
}
322+
let Ok(metadata) = serde_json::from_reader::<_, SnapshotMetadata>(
323+
File::open(metadata_path.clone()).unwrap(),
324+
) else {
325+
warn!(
326+
"Invalid snapshot metadata file: {}",
327+
metadata_path.display()
328+
);
329+
return SetupSnapshotResult::NoSnapshot;
330+
};
331+
332+
let now = SystemTime::now();
333+
let created_at = metadata.created_at;
334+
let duration = now.duration_since(created_at).unwrap();
335+
// Regtest doesn't like if the last block is > 2 hours old, so
336+
// don't use this snapshot.
337+
if duration > Duration::from_secs(3600 * 1) {
338+
// Bitcoin regtest node is too old, act like no snapshot exists
339+
warn!("Bitcoin regtest node is too old, not restoring snapshot");
340+
std::fs::remove_dir_all(snapshot_path.clone()).unwrap();
341+
return SetupSnapshotResult::WithSnapshot(SnapshotSetupInfo {
342+
snapshot_path: snapshot_path.clone(),
343+
snapshot_exists: false,
344+
});
345+
}
346+
348347
info!(
349348
"Snapshot directory already exists, copying to working dir";
350349
"snapshot_path" => %snapshot_path.display(),
@@ -367,12 +366,12 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
367366
/// Make a snapshot of the current working directory.
368367
///
369368
/// This will stop the bitcoind node and copy the working directory to the snapshot path.
370-
pub fn make_snapshot(&self) {
371-
let snapshot_path = self.snapshot_path.as_ref().unwrap();
372-
373-
let working_dir = self.running_nodes.conf.get_working_dir();
369+
pub fn make_snapshot(working_dir: &PathBuf, snapshot_path: &Option<PathBuf>) {
370+
let Some(snapshot_path) = snapshot_path else {
371+
return;
372+
};
374373

375-
let snapshot_dir_exists = self.snapshot_exists();
374+
let snapshot_dir_exists = std::fs::metadata(snapshot_path).is_ok();
376375

377376
if snapshot_dir_exists {
378377
info!("Snapshot directory already exists, skipping snapshot";
@@ -388,18 +387,20 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
388387
"working_dir" => %working_dir.display()
389388
);
390389

391-
Self::stop_bitcoind(&self.running_nodes.conf);
392-
393-
sleep_ms(5000);
394-
395390
let err_msg = format!(
396391
"Failed to copy working dir to snapshot path: {} -> {}",
397392
working_dir.display(),
398393
snapshot_path.display()
399394
);
400395

401-
// Copy the working dir to the snapshot path
402396
copy_dir_all(working_dir, snapshot_path).expect(&err_msg);
397+
398+
let metadata_path = snapshot_path.join("metadata.json");
399+
let metadata = SnapshotMetadata {
400+
created_at: SystemTime::now(),
401+
};
402+
let metadata_file = File::create(metadata_path).unwrap();
403+
serde_json::to_writer_pretty(metadata_file, &metadata).unwrap();
403404
}
404405

405406
/// Send a status request to each spawned signer
@@ -1316,7 +1317,15 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
13161317
String::new()
13171318
}
13181319

1320+
pub fn shutdown_and_snapshot(self) {
1321+
self.shutdown_and_make_snapshot(true);
1322+
}
1323+
13191324
pub fn shutdown(self) {
1325+
self.shutdown_and_make_snapshot(false);
1326+
}
1327+
1328+
fn shutdown_and_make_snapshot(mut self, needs_snapshot: bool) {
13201329
check_nakamoto_empty_block_heuristics();
13211330

13221331
self.running_nodes
@@ -1325,34 +1334,25 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
13251334
.expect("Mutex poisoned")
13261335
.stop_chains_coordinator();
13271336

1337+
self.running_nodes.btcd_controller.stop_bitcoind().unwrap();
1338+
13281339
self.running_nodes
13291340
.run_loop_stopper
13301341
.store(false, Ordering::SeqCst);
13311342
self.running_nodes.run_loop_thread.join().unwrap();
13321343

1333-
Self::stop_bitcoind(&self.running_nodes.conf);
1344+
if needs_snapshot {
1345+
Self::make_snapshot(
1346+
&self.running_nodes.conf.get_working_dir(),
1347+
&self.snapshot_path,
1348+
);
1349+
}
13341350

13351351
for signer in self.spawned_signers {
13361352
assert!(signer.stop().is_none());
13371353
}
13381354
}
13391355

1340-
fn stop_bitcoind(config: &NeonConfig) {
1341-
info!("Stopping bitcoind...");
1342-
let _ = BitcoinRPCRequest::send(
1343-
config,
1344-
BitcoinRPCRequest {
1345-
method: "stop".to_string(),
1346-
params: vec![],
1347-
id: "stacks".to_string(),
1348-
jsonrpc: "2.0".to_string(),
1349-
},
1350-
)
1351-
.inspect_err(|e| {
1352-
error!("Failed to stop bitcoind: {e:?}");
1353-
});
1354-
}
1355-
13561356
/// Get the latest block response from the given slot
13571357
pub fn get_latest_block_response(&self, slot_id: u32) -> BlockResponse {
13581358
let mut stackerdb = StackerDB::new_normal(

‎testnet/stacks-node/src/tests/signer/v0.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,6 @@ impl SignerTest<SpawnedSigner> {
336336

337337
if self.needs_snapshot() {
338338
self.boot_to_epoch_3();
339-
self.make_snapshot();
340339
warn!("Snapshot created. Shutdown and try again.");
341340
return true;
342341
}
@@ -4507,7 +4506,7 @@ fn snapshot_test() {
45074506
let _http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
45084507

45094508
if signer_test.bootstrap_snapshot() {
4510-
signer_test.shutdown();
4509+
signer_test.shutdown_and_snapshot();
45114510
return;
45124511
}
45134512

0 commit comments

Comments
 (0)