From 0500cae148000f94359af80f492ecb2103852f0b Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 15 Dec 2020 19:13:39 -0300 Subject: [PATCH 01/61] add hint for port error --- zebra-network/src/peer_set/initialize.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index d2c8b9a39ad..dc2189677c4 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -211,7 +211,21 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - let listener = TcpListener::bind(addr).await?; + let check_listener = TcpListener::bind(addr).await; + + let listener = match check_listener { + Ok(l) => l, + Err(_) => { + error!( + "{} {} {}", + "Listener failed. Port already in use.", + "Is another instance of zebrad or zcashd running in your local machine ?", + "Consider changing the default port in the configuration file." + ); + std::process::exit(1); + } + }; + let local_addr = listener.local_addr()?; info!("Opened Zcash protocol endpoint at {}", local_addr); loop { From c11203f33c22ebb2e7ecca09eaa2c7ba8584c17b Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 10:25:12 -0300 Subject: [PATCH 02/61] change error to panic --- zebra-network/src/peer_set/initialize.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index dc2189677c4..24cadd8cfaa 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -216,13 +216,12 @@ where let listener = match check_listener { Ok(l) => l, Err(_) => { - error!( + panic!( "{} {} {}", "Listener failed. Port already in use.", "Is another instance of zebrad or zcashd running in your local machine ?", "Consider changing the default port in the configuration file." ); - std::process::exit(1); } }; From eb378b591e33d992b36fc699bad854659938f5bc Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 11:56:04 -0300 Subject: [PATCH 03/61] add issue filter for port panic --- zebrad/src/application.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 0fe458dc6ad..c83adea93c7 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -171,7 +171,13 @@ impl Application for ZebradApp { .panic_section(metadata_section) .issue_url(concat!(env!("CARGO_PKG_REPOSITORY"), "/issues/new")) .issue_filter(|kind| match kind { - color_eyre::ErrorKind::NonRecoverable(_) => true, + color_eyre::ErrorKind::NonRecoverable(error) => { + let error_str = match error.downcast_ref::() { + Some(as_string) => as_string, + None => return true, + }; + !error_str.contains("Port already in use") + } color_eyre::ErrorKind::Recoverable(error) => { // type checks should be faster than string conversions if error.is::() From a89426141e4546e2264ef3f6971711abe542a975 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 16:23:54 -0300 Subject: [PATCH 04/61] add lock file hint --- zebra-state/src/service/finalized_state.rs | 13 +++++++++++-- zebrad/src/application.rs | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 64397851468..f42634a174f 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -41,8 +41,17 @@ impl FinalizedState { rocksdb::ColumnFamilyDescriptor::new("sprout_nullifiers", db_options.clone()), rocksdb::ColumnFamilyDescriptor::new("sapling_nullifiers", db_options.clone()), ]; - let db = rocksdb::DB::open_cf_descriptors(&db_options, path, column_families) - .expect("database path and options are valid"); + let db_check = rocksdb::DB::open_cf_descriptors(&db_options, path, column_families); + + let db = match db_check { + Ok(d) => d, + Err(_) => { + panic!( + "{} {}", + "LOCK file in use.", "Check if there is antoher zebrad process running." + ); + } + }; let new_state = Self { queued_by_prev_hash: HashMap::new(), diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index c83adea93c7..c703a7489de 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -177,6 +177,7 @@ impl Application for ZebradApp { None => return true, }; !error_str.contains("Port already in use") + && !error_str.contains("LOCK file in use") } color_eyre::ErrorKind::Recoverable(error) => { // type checks should be faster than string conversions From 6f71772234635ddc9ac8e17c7cd2dde0d3f98680 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 16:40:05 -0300 Subject: [PATCH 05/61] remove non needed brackets --- zebra-network/src/peer_set/initialize.rs | 14 ++++++-------- zebra-state/src/service/finalized_state.rs | 10 ++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 24cadd8cfaa..fa7e0c58d59 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -215,14 +215,12 @@ where let listener = match check_listener { Ok(l) => l, - Err(_) => { - panic!( - "{} {} {}", - "Listener failed. Port already in use.", - "Is another instance of zebrad or zcashd running in your local machine ?", - "Consider changing the default port in the configuration file." - ); - } + Err(_) => panic!( + "{} {} {}", + "Listener failed. Port already in use.", + "Is another instance of zebrad or zcashd running in your local machine ?", + "Consider changing the default port in the configuration file." + ), }; let local_addr = listener.local_addr()?; diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index f42634a174f..c8623e29f43 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -45,12 +45,10 @@ impl FinalizedState { let db = match db_check { Ok(d) => d, - Err(_) => { - panic!( - "{} {}", - "LOCK file in use.", "Check if there is antoher zebrad process running." - ); - } + Err(_) => panic!( + "{} {}", + "LOCK file in use.", "Check if there is antoher zebrad process running." + ), }; let new_state = Self { From ef7744b21b5f5b63af8022891565f1356846d001 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 19:26:03 -0300 Subject: [PATCH 06/61] add metrics endpoint port conflict hint --- zebra-network/src/peer_set/initialize.rs | 2 +- zebra-state/src/service/finalized_state.rs | 2 +- zebrad/src/application.rs | 3 +-- zebrad/src/components/metrics.rs | 14 +++++++++++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index fa7e0c58d59..6490170e7bb 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -218,7 +218,7 @@ where Err(_) => panic!( "{} {} {}", "Listener failed. Port already in use.", - "Is another instance of zebrad or zcashd running in your local machine ?", + "Is another instance of zebrad or zcashd running in your local machine?", "Consider changing the default port in the configuration file." ), }; diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index c8623e29f43..f203b07e7e3 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -47,7 +47,7 @@ impl FinalizedState { Ok(d) => d, Err(_) => panic!( "{} {}", - "LOCK file in use.", "Check if there is antoher zebrad process running." + "LOCK file already in use.", "Check if there is antoher zebrad process running." ), }; diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index c703a7489de..163c5ba44a4 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -176,8 +176,7 @@ impl Application for ZebradApp { Some(as_string) => as_string, None => return true, }; - !error_str.contains("Port already in use") - && !error_str.contains("LOCK file in use") + !error_str.contains("already in use") } color_eyre::ErrorKind::Recoverable(error) => { // type checks should be faster than string conversions diff --git a/zebrad/src/components/metrics.rs b/zebrad/src/components/metrics.rs index 83963f3c731..04769d58c21 100644 --- a/zebrad/src/components/metrics.rs +++ b/zebrad/src/components/metrics.rs @@ -13,10 +13,18 @@ impl MetricsEndpoint { pub fn new(config: &ZebradConfig) -> Result { if let Some(addr) = config.metrics.endpoint_addr { info!("Initializing metrics endpoint at {}", addr); - metrics_exporter_prometheus::PrometheusBuilder::new() + let check_endpoint = metrics_exporter_prometheus::PrometheusBuilder::new() .listen_address(addr) - .install() - .expect("FIXME ERROR CONVERSION"); + .install(); + match check_endpoint { + Ok(endpoint) => endpoint, + Err(_) => panic!( + "{} {} {}", + "Port for metrics endpoint already in use by another process:", + addr, + "- You can change the metrics default endpoint in the config." + ), + } } Ok(Self {}) } From 8756236b6972e24ad5a7c256fa66348b353eb81e Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 17 Dec 2020 19:37:00 -0300 Subject: [PATCH 07/61] add hint for tracing endpoint port conflict --- zebrad/src/components/metrics.rs | 2 +- zebrad/src/components/tracing/endpoint.rs | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/zebrad/src/components/metrics.rs b/zebrad/src/components/metrics.rs index 04769d58c21..238a4b87ecf 100644 --- a/zebrad/src/components/metrics.rs +++ b/zebrad/src/components/metrics.rs @@ -22,7 +22,7 @@ impl MetricsEndpoint { "{} {} {}", "Port for metrics endpoint already in use by another process:", addr, - "- You can change the metrics default endpoint in the config." + "- You can change the metrics endpoint in the config." ), } } diff --git a/zebrad/src/components/tracing/endpoint.rs b/zebrad/src/components/tracing/endpoint.rs index 353bd0ba60a..425a8c599fd 100644 --- a/zebrad/src/components/tracing/endpoint.rs +++ b/zebrad/src/components/tracing/endpoint.rs @@ -55,11 +55,12 @@ impl TracingEndpoint { // need to construct it inside the task. let server = match Server::try_bind(&addr) { Ok(s) => s, - Err(e) => { - error!("Could not open tracing endpoint listener"); - error!("Error: {}", e); - return; - } + Err(_) => panic!( + "{} {} {}", + "Port for tracing endpoint already in use by another process:", + addr, + "- You can change the tracing endpoint port in the config." + ), } .serve(service); From 88e31eb8d1fa751a399c6d10f73ad5e1ab31cd8e Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Fri, 18 Dec 2020 10:15:26 -0300 Subject: [PATCH 08/61] apply all suggestions from code review Co-authored-by: teor --- zebra-network/src/peer_set/initialize.rs | 14 +++++++------- zebra-state/src/service/finalized_state.rs | 12 +++++++----- zebrad/src/application.rs | 13 ++++++++++++- zebrad/src/components/metrics.rs | 14 +++++++------- zebrad/src/components/tracing/endpoint.rs | 10 +++++----- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 6490170e7bb..678f10405ee 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -211,15 +211,15 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - let check_listener = TcpListener::bind(addr).await; + let listener_result = TcpListener::bind(addr).await; - let listener = match check_listener { + let listener = match listener_result { Ok(l) => l, - Err(_) => panic!( - "{} {} {}", - "Listener failed. Port already in use.", - "Is another instance of zebrad or zcashd running in your local machine?", - "Consider changing the default port in the configuration file." + Err(e) => panic!( + "Opening Zcash network protocol listener {:?} failed: {:?}. \ + Hint: Check if another zebrad or zcashd process is running. \ + Try changing the network listen_addr in the Zebra config.", + addr, e, ), }; diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index f203b07e7e3..2de11be7609 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -41,13 +41,15 @@ impl FinalizedState { rocksdb::ColumnFamilyDescriptor::new("sprout_nullifiers", db_options.clone()), rocksdb::ColumnFamilyDescriptor::new("sapling_nullifiers", db_options.clone()), ]; - let db_check = rocksdb::DB::open_cf_descriptors(&db_options, path, column_families); + let db_result = rocksdb::DB::open_cf_descriptors(&db_options, &path, column_families); - let db = match db_check { + let db = match db_result { Ok(d) => d, - Err(_) => panic!( - "{} {}", - "LOCK file already in use.", "Check if there is antoher zebrad process running." + Err(e) => panic!( + "Opening database {:?} failed: {:?}. \ + Hint: Check if another zebrad process is running. \ + Try changing the state cache_dir in the Zebra config.", + path, e, ), }; diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 163c5ba44a4..5fd3e48cbee 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -176,7 +176,18 @@ impl Application for ZebradApp { Some(as_string) => as_string, None => return true, }; - !error_str.contains("already in use") + // listener port conflicts + if error_str.contains("already in use") { + return false; + } + // RocksDB lock file conflicts + if error_str.contains("lock file") + && (error_str.contains("temporarily unavailable") + || error_str.contains("in use")) + { + return false; + } + true } color_eyre::ErrorKind::Recoverable(error) => { // type checks should be faster than string conversions diff --git a/zebrad/src/components/metrics.rs b/zebrad/src/components/metrics.rs index 238a4b87ecf..4f1b6ebfa63 100644 --- a/zebrad/src/components/metrics.rs +++ b/zebrad/src/components/metrics.rs @@ -13,16 +13,16 @@ impl MetricsEndpoint { pub fn new(config: &ZebradConfig) -> Result { if let Some(addr) = config.metrics.endpoint_addr { info!("Initializing metrics endpoint at {}", addr); - let check_endpoint = metrics_exporter_prometheus::PrometheusBuilder::new() + let endpoint_result = metrics_exporter_prometheus::PrometheusBuilder::new() .listen_address(addr) .install(); - match check_endpoint { + match endpoint_result { Ok(endpoint) => endpoint, - Err(_) => panic!( - "{} {} {}", - "Port for metrics endpoint already in use by another process:", - addr, - "- You can change the metrics endpoint in the config." + Err(e) => panic!( + "Opening metrics endpoint listener {:?} failed: {:?}. \ + Hint: Check if another zebrad or zcashd process is running. \ + Try changing the metrics endpoint_addr in the Zebra config.", + addr, e, ), } } diff --git a/zebrad/src/components/tracing/endpoint.rs b/zebrad/src/components/tracing/endpoint.rs index 425a8c599fd..37e04f0618d 100644 --- a/zebrad/src/components/tracing/endpoint.rs +++ b/zebrad/src/components/tracing/endpoint.rs @@ -55,11 +55,11 @@ impl TracingEndpoint { // need to construct it inside the task. let server = match Server::try_bind(&addr) { Ok(s) => s, - Err(_) => panic!( - "{} {} {}", - "Port for tracing endpoint already in use by another process:", - addr, - "- You can change the tracing endpoint port in the config." + Err(e) => panic!( + "Opening tracing endpoint listener {:?} failed: {:?}. \ + Hint: Check if another zebrad or zcashd process is running. \ + Try changing the tracing endpoint_addr in the Zebra config.", + addr, e, ), } .serve(service); From 02ed38623552727600b715bb72ddc0609c90fbdd Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Fri, 18 Dec 2020 12:47:15 -0300 Subject: [PATCH 09/61] add acceptance test for resource conflics --- zebra-test/src/command.rs | 18 ++++++++++++++ zebrad/tests/acceptance.rs | 51 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index e48bc993470..1173da4e493 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -369,6 +369,24 @@ impl TestOutput { fn was_killed(&self) -> bool { self.output.status.signal() != Some(9) } + + #[instrument(skip(self))] + pub fn stderr_contains(&self, regex: &str) -> Result<&Self> { + let re = regex::Regex::new(regex)?; + let stderr = String::from_utf8_lossy(&self.output.stderr); + + for line in stderr.lines() { + if re.is_match(line) { + return Ok(self); + } + } + + Err(eyre!( + "stderr of command did not contain any matches for the given regex" + )) + .context_from(self) + .with_section(|| format!("{:?}", regex).header("Match Regex:")) + } } /// Add context to an error report diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 1382a66a5bf..1434da14171 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1052,3 +1052,54 @@ async fn tracing_endpoint() -> Result<()> { Ok(()) } +/// Test will start 2 zebrad nodes one after the other using the same configuration. +/// It is expected that the first node spawned will block the network port and data dir. +/// The second node will panic with some of the conflicts added in #1535. +#[test] +fn resources_in_use_conflicts() -> Result<()> { + zebra_test::init(); + + // [Note on port conflict](#Note on port conflict) + let port = random_known_port(); + let listen_addr = format!("127.0.0.1:{}", port); + + // Write a configuration that has our created network listen_addr + let mut config = default_test_config()?; + config.network.listen_addr = listen_addr.parse().unwrap(); + let dir1 = TempDir::new("zebrad_tests")?; + fs::File::create(dir1.path().join("zebrad.toml"))? + .write_all(toml::to_string(&config)?.as_bytes())?; + + // Start the first node + let mut node1 = dir1.spawn_child(&["start"])?; + + // From another folder create the same configuration. + // `cache_dir` and `network.listen_addr` will be the same in the 2 nodes. + let dir2 = TempDir::new("zebrad_tests")?; + fs::File::create(dir2.path().join("zebrad.toml"))? + .write_all(toml::to_string(&config)?.as_bytes())?; + + // wait a bit to spawn the second node, we want the first fully started. + std::thread::sleep(LAUNCH_DELAY); + + // Spawn the second node + let mut node2 = dir2.spawn_child(&["start"])?; + + // Wait a few seconds and kill both nodes + std::thread::sleep(LAUNCH_DELAY); + node1.kill()?; + node2.kill()?; + + // In node1 we just want to check the network port was opened. + let output1 = node1.wait_with_output()?; + output1 + .stdout_contains(format!(r"Opened Zcash protocol endpoint at {}", listen_addr).as_str())?; + output1.assert_was_killed()?; + + // In the second node we look for any of our panics. + let output2 = node2.wait_with_output()?; + output2.stderr_contains("already in use|lock file|temporarily unavailable|in use")?; + output2.assert_was_not_killed()?; + + Ok(()) +} From 770ffb5e6e34ee8471146524a5d9d2abe03b741f Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Dec 2020 09:33:11 +1000 Subject: [PATCH 10/61] Use the correct regex for the port conflict acceptance test The test only causes a Zcash listener port conflict, so modify the regex to check for that conflict. (The previous regex was incorrect because it didn't use groups, but the new regex doesn't need groups.) --- zebrad/tests/acceptance.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 1434da14171..7b7edd3b70c 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -2,6 +2,7 @@ //! output for given argument combinations matches what is expected. //! //! ### Note on port conflict +//! //! If the test child has a cache or port conflict with another test, or a //! running zebrad or zcashd, then it will panic. But the acceptance tests //! expect it to run until it is killed. @@ -1052,11 +1053,12 @@ async fn tracing_endpoint() -> Result<()> { Ok(()) } -/// Test will start 2 zebrad nodes one after the other using the same configuration. -/// It is expected that the first node spawned will block the network port and data dir. -/// The second node will panic with some of the conflicts added in #1535. + +/// Test will start 2 zebrad nodes one after the other using the same Zcash listener. +/// It is expected that the first node spawned will get exclusive use of the port. +/// The second node will panic with the Zcash listener conflict hint added in #1535. #[test] -fn resources_in_use_conflicts() -> Result<()> { +fn zcash_listener_conflict() -> Result<()> { zebra_test::init(); // [Note on port conflict](#Note on port conflict) @@ -1073,8 +1075,9 @@ fn resources_in_use_conflicts() -> Result<()> { // Start the first node let mut node1 = dir1.spawn_child(&["start"])?; - // From another folder create the same configuration. - // `cache_dir` and `network.listen_addr` will be the same in the 2 nodes. + // From another folder create a configuration with the same listener. + // `network.listen_addr` will be the same in the 2 nodes. + // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?; fs::File::create(dir2.path().join("zebrad.toml"))? .write_all(toml::to_string(&config)?.as_bytes())?; @@ -1096,9 +1099,9 @@ fn resources_in_use_conflicts() -> Result<()> { .stdout_contains(format!(r"Opened Zcash protocol endpoint at {}", listen_addr).as_str())?; output1.assert_was_killed()?; - // In the second node we look for any of our panics. + // In the second node we look for the Zcash listener conflict let output2 = node2.wait_with_output()?; - output2.stderr_contains("already in use|lock file|temporarily unavailable|in use")?; + output2.stderr_contains("already in use")?; output2.assert_was_not_killed()?; Ok(()) From 09493efbe33b426a8d84c7fff392a3e4eb235db5 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Dec 2020 12:28:16 +1000 Subject: [PATCH 11/61] Simplify the test using helper functions --- zebrad/tests/acceptance.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 7b7edd3b70c..a69a60064f5 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1068,19 +1068,15 @@ fn zcash_listener_conflict() -> Result<()> { // Write a configuration that has our created network listen_addr let mut config = default_test_config()?; config.network.listen_addr = listen_addr.parse().unwrap(); - let dir1 = TempDir::new("zebrad_tests")?; - fs::File::create(dir1.path().join("zebrad.toml"))? - .write_all(toml::to_string(&config)?.as_bytes())?; - - // Start the first node - let mut node1 = dir1.spawn_child(&["start"])?; + let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; // From another folder create a configuration with the same listener. // `network.listen_addr` will be the same in the 2 nodes. // (But since the config is ephemeral, they will have different state paths.) - let dir2 = TempDir::new("zebrad_tests")?; - fs::File::create(dir2.path().join("zebrad.toml"))? - .write_all(toml::to_string(&config)?.as_bytes())?; + let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + + // Start the first node + let mut node1 = dir1.spawn_child(&["start"])?; // wait a bit to spawn the second node, we want the first fully started. std::thread::sleep(LAUNCH_DELAY); From 29cd0cd6be8fa011ac5b827b312b93f79403b82e Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Dec 2020 12:42:25 +1000 Subject: [PATCH 12/61] Split out common config conflict code into a function --- zebrad/tests/acceptance.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index a69a60064f5..a03a5880621 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1069,20 +1069,40 @@ fn zcash_listener_conflict() -> Result<()> { let mut config = default_test_config()?; config.network.listen_addr = listen_addr.parse().unwrap(); let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let regex1 = format!(r"Opened Zcash protocol endpoint at {}", listen_addr); // From another folder create a configuration with the same listener. // `network.listen_addr` will be the same in the 2 nodes. // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + + Ok(()) +} + +/// Launch a node in `first_dir`, wait a few seconds, then launch a node in +/// `second_dir`. Check that the first node's stdout contains +/// `first_stdout_regex`, and the second node's stderr contains +/// `second_stderr_regex`. +fn check_config_conflict( + first_dir: T, + first_stdout_regex: &str, + second_dir: U, + second_stderr_regex: &str, +) -> Result<()> +where + T: ZebradTestDirExt, + U: ZebradTestDirExt, +{ // Start the first node - let mut node1 = dir1.spawn_child(&["start"])?; + let mut node1 = first_dir.spawn_child(&["start"])?; // wait a bit to spawn the second node, we want the first fully started. std::thread::sleep(LAUNCH_DELAY); // Spawn the second node - let mut node2 = dir2.spawn_child(&["start"])?; + let mut node2 = second_dir.spawn_child(&["start"])?; // Wait a few seconds and kill both nodes std::thread::sleep(LAUNCH_DELAY); @@ -1091,13 +1111,12 @@ fn zcash_listener_conflict() -> Result<()> { // In node1 we just want to check the network port was opened. let output1 = node1.wait_with_output()?; - output1 - .stdout_contains(format!(r"Opened Zcash protocol endpoint at {}", listen_addr).as_str())?; + output1.stdout_contains(first_stdout_regex)?; output1.assert_was_killed()?; // In the second node we look for the Zcash listener conflict let output2 = node2.wait_with_output()?; - output2.stderr_contains("already in use")?; + output2.stderr_contains(second_stderr_regex)?; output2.assert_was_not_killed()?; Ok(()) From a2f97ab13e4fb7c66df68f6f590f7fc215e19348 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Dec 2020 13:19:58 +1000 Subject: [PATCH 13/61] Add state, metrics, and tracing conflict tests --- zebra-state/src/service/finalized_state.rs | 5 +- zebrad/src/components/metrics.rs | 6 +- zebrad/src/components/tracing/endpoint.rs | 6 +- zebrad/tests/acceptance.rs | 94 ++++++++++++++++++++-- 4 files changed, 100 insertions(+), 11 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 2de11be7609..98e63b84c3c 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -44,7 +44,10 @@ impl FinalizedState { let db_result = rocksdb::DB::open_cf_descriptors(&db_options, &path, column_families); let db = match db_result { - Ok(d) => d, + Ok(d) => { + tracing::info!("Opened Zebra state cache at: {:?}", path); + d + } Err(e) => panic!( "Opening database {:?} failed: {:?}. \ Hint: Check if another zebrad process is running. \ diff --git a/zebrad/src/components/metrics.rs b/zebrad/src/components/metrics.rs index 4f1b6ebfa63..15b6a22c7d1 100644 --- a/zebrad/src/components/metrics.rs +++ b/zebrad/src/components/metrics.rs @@ -12,12 +12,14 @@ impl MetricsEndpoint { /// Create the component. pub fn new(config: &ZebradConfig) -> Result { if let Some(addr) = config.metrics.endpoint_addr { - info!("Initializing metrics endpoint at {}", addr); let endpoint_result = metrics_exporter_prometheus::PrometheusBuilder::new() .listen_address(addr) .install(); match endpoint_result { - Ok(endpoint) => endpoint, + Ok(endpoint) => { + info!("Opened metrics endpoint at {}", addr); + endpoint + } Err(e) => panic!( "Opening metrics endpoint listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ diff --git a/zebrad/src/components/tracing/endpoint.rs b/zebrad/src/components/tracing/endpoint.rs index 37e04f0618d..fe087d3c2f7 100644 --- a/zebrad/src/components/tracing/endpoint.rs +++ b/zebrad/src/components/tracing/endpoint.rs @@ -41,7 +41,6 @@ impl TracingEndpoint { } else { return Ok(()); }; - info!("Initializing tracing endpoint at {}", addr); let service = make_service_fn(|_| async { Ok::<_, hyper::Error>(service_fn(request_handler)) }); @@ -54,7 +53,10 @@ impl TracingEndpoint { // try_bind uses the tokio runtime, so we // need to construct it inside the task. let server = match Server::try_bind(&addr) { - Ok(s) => s, + Ok(s) => { + info!("Opened tracing endpoint at {}", addr); + s + } Err(e) => panic!( "Opening tracing endpoint listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index a03a5880621..a607e6137de 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -976,7 +976,7 @@ async fn metrics_endpoint() -> Result<()> { let output = output.assert_failure()?; // Make sure metrics was started - output.stdout_contains(format!(r"Initializing metrics endpoint at {}", endpoint).as_str())?; + output.stdout_contains(format!(r"Opened metrics endpoint at {}", endpoint).as_str())?; // [Note on port conflict](#Note on port conflict) output @@ -1043,7 +1043,7 @@ async fn tracing_endpoint() -> Result<()> { let output = output.assert_failure()?; // Make sure tracing endpoint was started - output.stdout_contains(format!(r"Initializing tracing endpoint at {}", endpoint).as_str())?; + output.stdout_contains(format!(r"Opened tracing endpoint at {}", endpoint).as_str())?; // Todo: Match some trace level messages from output // [Note on port conflict](#Note on port conflict) @@ -1081,6 +1081,84 @@ fn zcash_listener_conflict() -> Result<()> { Ok(()) } +/// Start 2 zebrad nodes using the same metrics listener port, but different +/// state directories and Zcash listener ports. The first node should get +/// exclusive use of the port. The second node will panic with the Zcash metrics +/// conflict hint added in #1535. +#[test] +fn zcash_metrics_conflict() -> Result<()> { + zebra_test::init(); + + // [Note on port conflict](#Note on port conflict) + let port = random_known_port(); + let listen_addr = format!("127.0.0.1:{}", port); + + // Write a configuration that has our created metrics endpoint_addr + let mut config = default_test_config()?; + config.metrics.endpoint_addr = Some(listen_addr.parse().unwrap()); + let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let regex1 = format!(r"Opened metrics endpoint at {}", listen_addr); + + // From another folder create a configuration with the same endpoint. + // `metrics.endpoint_addr` will be the same in the 2 nodes. + // But they will have different Zcash listeners (auto port) and states (ephemeral) + let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + + check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + + Ok(()) +} + +/// Start 2 zebrad nodes using the same tracing listener port, but different +/// state directories and Zcash listener ports. The first node should get +/// exclusive use of the port. The second node will panic with the Zcash tracing +/// conflict hint added in #1535. +#[test] +fn zcash_tracing_conflict() -> Result<()> { + zebra_test::init(); + + // [Note on port conflict](#Note on port conflict) + let port = random_known_port(); + let listen_addr = format!("127.0.0.1:{}", port); + + // Write a configuration that has our created tracing endpoint_addr + let mut config = default_test_config()?; + config.tracing.endpoint_addr = Some(listen_addr.parse().unwrap()); + let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let regex1 = format!(r"Opened tracing endpoint at {}", listen_addr); + + // From another folder create a configuration with the same endpoint. + // `tracing.endpoint_addr` will be the same in the 2 nodes. + // But they will have different Zcash listeners (auto port) and states (ephemeral) + let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + + check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + + Ok(()) +} + +/// Start 2 zebrad nodes using the same state directory, but different Zcash +/// listener ports. The first node should get exclusive access to the database. +/// The second node will panic with the Zcash state conflict hint added in #1535. +#[test] +fn zcash_state_conflict() -> Result<()> { + zebra_test::init(); + + // A persistent config has a fixed temp state directory, but asks the OS to + // automatically choose an unused port + let config = persistent_test_config()?; + let dir_conflict = TempDir::new("zebrad_tests")?.with_config(config)?; + + check_config_conflict( + dir_conflict.path(), + "Opened Zebra state cache", + dir_conflict.path(), + "lock file", + )?; + + Ok(()) +} + /// Launch a node in `first_dir`, wait a few seconds, then launch a node in /// `second_dir`. Check that the first node's stdout contains /// `first_stdout_regex`, and the second node's stderr contains @@ -1109,15 +1187,19 @@ where node1.kill()?; node2.kill()?; - // In node1 we just want to check the network port was opened. + // In node1 we want to check for the success regex let output1 = node1.wait_with_output()?; output1.stdout_contains(first_stdout_regex)?; - output1.assert_was_killed()?; + output1 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - // In the second node we look for the Zcash listener conflict + // In the second node we look for the conflict regex let output2 = node2.wait_with_output()?; output2.stderr_contains(second_stderr_regex)?; - output2.assert_was_not_killed()?; + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; Ok(()) } From 4326b67a672f4f6dcbf853ab36fe748d010c0ae3 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Dec 2020 13:32:46 +1000 Subject: [PATCH 14/61] Add a full set of stderr acceptance test functions This change makes the stdout and stderr acceptance test interfaces identical. --- zebra-test/src/command.rs | 168 +++++++++++++++++++++++++++++--------- 1 file changed, 131 insertions(+), 37 deletions(-) diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index 1173da4e493..8395c03a37c 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -12,7 +12,7 @@ use std::{ io::BufRead, io::{BufReader, Lines, Read}, path::Path, - process::{Child, ChildStdout, Command, ExitStatus, Output, Stdio}, + process::{Child, ChildStderr, ChildStdout, Command, ExitStatus, Output, Stdio}, time::{Duration, Instant}, }; @@ -86,6 +86,7 @@ impl CommandExt for Command { dir, deadline: None, stdout: None, + stderr: None, bypass_test_capture: false, }) } @@ -152,6 +153,7 @@ pub struct TestChild { pub cmd: String, pub child: Child, pub stdout: Option>>, + pub stderr: Option>>, pub deadline: Option, bypass_test_capture: bool, } @@ -184,7 +186,7 @@ impl TestChild { }) } - /// Set a timeout for `expect_stdout`. + /// Set a timeout for `expect_stdout` or `expect_stderr`. /// /// Does not apply to `wait_with_output`. pub fn with_timeout(mut self, timeout: Duration) -> Self { @@ -192,17 +194,18 @@ impl TestChild { self } - /// Configures testrunner to forward stdout to the true stdout rather than - /// fakestdout used by cargo tests. + /// Configures testrunner to forward stdout and stderr to the true stdout, + /// rather than the fakestdout used by cargo tests. pub fn bypass_test_capture(mut self, cond: bool) -> Self { self.bypass_test_capture = cond; self } - /// Checks each line of the child's stdout against `regex`, and returns matching lines. + /// Checks each line of the child's stdout against `regex`, and returns Ok + /// if a line matches. /// /// Kills the child after the configured timeout has elapsed. - /// Note: the timeout is only checked after each line. + /// See `expect_line_matching` for details. #[instrument(skip(self))] pub fn expect_stdout(&mut self, regex: &str) -> Result<&mut Self> { if self.stdout.is_none() { @@ -214,12 +217,67 @@ impl TestChild { .map(BufRead::lines) } - let re = regex::Regex::new(regex).expect("regex must be valid"); let mut lines = self .stdout .take() .expect("child must capture stdout to call expect_stdout"); + match self.expect_line_matching(&mut lines, regex, "stdout") { + Ok(()) => { + self.stdout = Some(lines); + Ok(self) + } + Err(report) => Err(report), + } + } + + /// Checks each line of the child's stderr against `regex`, and returns Ok + /// if a line matches. + /// + /// Kills the child after the configured timeout has elapsed. + /// See `expect_line_matching` for details. + #[instrument(skip(self))] + pub fn expect_stderr(&mut self, regex: &str) -> Result<&mut Self> { + if self.stderr.is_none() { + self.stderr = self + .child + .stderr + .take() + .map(BufReader::new) + .map(BufRead::lines) + } + + let mut lines = self + .stderr + .take() + .expect("child must capture stderr to call expect_stderr"); + + match self.expect_line_matching(&mut lines, regex, "stderr") { + Ok(()) => { + self.stderr = Some(lines); + Ok(self) + } + Err(report) => Err(report), + } + } + + /// Checks each line in `lines` against `regex`, and returns Ok if a line + /// matches. Uses `stream_name` as the name for `lines` in error reports. + /// + /// Kills the child after the configured timeout has elapsed. + /// Note: the timeout is only checked after each full line is received from + /// the child. + #[instrument(skip(self, lines))] + pub fn expect_line_matching( + &mut self, + lines: &mut L, + regex: &str, + stream_name: &str, + ) -> Result<()> + where + L: Iterator>, + { + let re = regex::Regex::new(regex).expect("regex must be valid"); while !self.past_deadline() && self.is_running() { let line = if let Some(line) = lines.next() { line? @@ -227,20 +285,21 @@ impl TestChild { break; }; - // since we're about to discard this line write it to stdout so our - // test runner can capture it and display if the test fails, may - // cause weird reordering for stdout / stderr - if !self.bypass_test_capture { - println!("{}", line); - } else { + // Since we're about to discard this line write it to stdout, so it + // can be preserved. May cause weird reordering for stdout / stderr. + // Uses stdout even if the original lines were from stderr. + if self.bypass_test_capture { + // send lines to the terminal (or process stdout file redirect) use std::io::Write; #[allow(clippy::explicit_write)] writeln!(std::io::stdout(), "{}", line).unwrap(); + } else { + // if the test fails, the test runner captures and displays it + println!("{}", line); } if re.is_match(&line) { - self.stdout = Some(lines); - return Ok(self); + return Ok(()); } } @@ -251,9 +310,12 @@ impl TestChild { self.kill()?; } - let report = eyre!("stdout of command did not contain any matches for the given regex") - .context_from(self) - .with_section(|| format!("{:?}", regex).header("Match Regex:")); + let report = eyre!( + "{} of command did not contain any matches for the given regex", + stream_name + ) + .context_from(self) + .with_section(|| format!("{:?}", regex).header("Match Regex:")); Err(report) } @@ -340,6 +402,51 @@ impl TestOutput { .with_section(|| format!("{:?}", regex).header("Match Regex:")) } + #[instrument(skip(self))] + pub fn stderr_contains(&self, regex: &str) -> Result<&Self> { + let re = regex::Regex::new(regex)?; + let stderr = String::from_utf8_lossy(&self.output.stderr); + + for line in stderr.lines() { + if re.is_match(line) { + return Ok(self); + } + } + + Err(eyre!( + "stderr of command did not contain any matches for the given regex" + )) + .context_from(self) + .with_section(|| format!("{:?}", regex).header("Match Regex:")) + } + + #[instrument(skip(self))] + pub fn stderr_equals(&self, s: &str) -> Result<&Self> { + let stderr = String::from_utf8_lossy(&self.output.stderr); + + if stderr == s { + return Ok(self); + } + + Err(eyre!("stderr of command is not equal the given string")) + .context_from(self) + .with_section(|| format!("{:?}", s).header("Match String:")) + } + + #[instrument(skip(self))] + pub fn stderr_matches(&self, regex: &str) -> Result<&Self> { + let re = regex::Regex::new(regex)?; + let stderr = String::from_utf8_lossy(&self.output.stderr); + + if re.is_match(&stderr) { + return Ok(self); + } + + Err(eyre!("stderr of command is not equal to the given regex")) + .context_from(self) + .with_section(|| format!("{:?}", regex).header("Match Regex:")) + } + /// Returns Ok if the program was killed, Err(Report) if exit was by another /// reason. pub fn assert_was_killed(&self) -> Result<()> { @@ -369,24 +476,6 @@ impl TestOutput { fn was_killed(&self) -> bool { self.output.status.signal() != Some(9) } - - #[instrument(skip(self))] - pub fn stderr_contains(&self, regex: &str) -> Result<&Self> { - let re = regex::Regex::new(regex)?; - let stderr = String::from_utf8_lossy(&self.output.stderr); - - for line in stderr.lines() { - if re.is_match(line) { - return Ok(self); - } - } - - Err(eyre!( - "stderr of command did not contain any matches for the given regex" - )) - .context_from(self) - .with_section(|| format!("{:?}", regex).header("Match Regex:")) - } } /// Add context to an error report @@ -441,7 +530,12 @@ impl ContextFrom<&mut TestChild> for Report { let _ = stdout.read_to_string(&mut stdout_buf); } - if let Some(stderr) = &mut source.child.stderr { + if let Some(stderr) = &mut source.stderr { + for line in stderr { + let line = if let Ok(line) = line { line } else { break }; + let _ = writeln!(&mut stderr_buf, "{}", line); + } + } else if let Some(stderr) = &mut source.child.stderr { let _ = stderr.read_to_string(&mut stderr_buf); } From a4b83fac9d676eff174b8ac213cf8e25c6bbc1d6 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 13:56:02 -0300 Subject: [PATCH 15/61] print stderr --- zebra-test/src/command.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index 8395c03a37c..ff051fc7f94 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -406,6 +406,7 @@ impl TestOutput { pub fn stderr_contains(&self, regex: &str) -> Result<&Self> { let re = regex::Regex::new(regex)?; let stderr = String::from_utf8_lossy(&self.output.stderr); + dbg!(&stderr); for line in stderr.lines() { if re.is_match(line) { From fa1587431a8692b952b9394f4530a1eeb84d1511 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 15:53:50 -0300 Subject: [PATCH 16/61] fix the build --- zebrad/tests/acceptance.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index a607e6137de..22f21aff279 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1068,13 +1068,13 @@ fn zcash_listener_conflict() -> Result<()> { // Write a configuration that has our created network listen_addr let mut config = default_test_config()?; config.network.listen_addr = listen_addr.parse().unwrap(); - let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let dir1 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; let regex1 = format!(r"Opened Zcash protocol endpoint at {}", listen_addr); // From another folder create a configuration with the same listener. // `network.listen_addr` will be the same in the 2 nodes. // (But since the config is ephemeral, they will have different state paths.) - let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; @@ -1096,13 +1096,13 @@ fn zcash_metrics_conflict() -> Result<()> { // Write a configuration that has our created metrics endpoint_addr let mut config = default_test_config()?; config.metrics.endpoint_addr = Some(listen_addr.parse().unwrap()); - let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let dir1 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; let regex1 = format!(r"Opened metrics endpoint at {}", listen_addr); // From another folder create a configuration with the same endpoint. // `metrics.endpoint_addr` will be the same in the 2 nodes. // But they will have different Zcash listeners (auto port) and states (ephemeral) - let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; @@ -1124,13 +1124,13 @@ fn zcash_tracing_conflict() -> Result<()> { // Write a configuration that has our created tracing endpoint_addr let mut config = default_test_config()?; config.tracing.endpoint_addr = Some(listen_addr.parse().unwrap()); - let dir1 = TempDir::new("zebrad_tests")?.with_config(config.clone())?; + let dir1 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; let regex1 = format!(r"Opened tracing endpoint at {}", listen_addr); // From another folder create a configuration with the same endpoint. // `tracing.endpoint_addr` will be the same in the 2 nodes. // But they will have different Zcash listeners (auto port) and states (ephemeral) - let dir2 = TempDir::new("zebrad_tests")?.with_config(config)?; + let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; @@ -1146,8 +1146,8 @@ fn zcash_state_conflict() -> Result<()> { // A persistent config has a fixed temp state directory, but asks the OS to // automatically choose an unused port - let config = persistent_test_config()?; - let dir_conflict = TempDir::new("zebrad_tests")?.with_config(config)?; + let mut config = persistent_test_config()?; + let dir_conflict = TempDir::new("zebrad_tests")?.with_config(&mut config)?; check_config_conflict( dir_conflict.path(), From 7921547336ed3392cc5656c0765b1a14bacc429f Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 16:51:00 -0300 Subject: [PATCH 17/61] add a longer delay for port conflict --- zebrad/tests/acceptance.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 22f21aff279..d9ad3fe3e28 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -38,6 +38,8 @@ use zebrad::config::ZebradConfig; /// Previously, this value was 1 second, which caused occasional /// `tracing_endpoint` test failures on some machines. const LAUNCH_DELAY: Duration = Duration::from_secs(3); +const LAUNCH_DELAY_BIG: Duration = Duration::from_secs(100); + fn default_test_config() -> Result { let auto_port_ipv4_local = zebra_network::Config { @@ -1057,6 +1059,7 @@ async fn tracing_endpoint() -> Result<()> { /// Test will start 2 zebrad nodes one after the other using the same Zcash listener. /// It is expected that the first node spawned will get exclusive use of the port. /// The second node will panic with the Zcash listener conflict hint added in #1535. + #[test] fn zcash_listener_conflict() -> Result<()> { zebra_test::init(); @@ -1183,7 +1186,7 @@ where let mut node2 = second_dir.spawn_child(&["start"])?; // Wait a few seconds and kill both nodes - std::thread::sleep(LAUNCH_DELAY); + std::thread::sleep(LAUNCH_DELAY_BIG); node1.kill()?; node2.kill()?; From 0b20a698034731813d656c5f74b5ca63c814db61 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 21:03:38 -0300 Subject: [PATCH 18/61] change port conflict regex for windows --- zebrad/tests/acceptance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index d9ad3fe3e28..d5706b45967 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1079,7 +1079,7 @@ fn zcash_listener_conflict() -> Result<()> { // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; Ok(()) } @@ -1107,7 +1107,7 @@ fn zcash_metrics_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; Ok(()) } From abe5902f403d1b231a535081553d327c925cf819 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 21:08:54 -0300 Subject: [PATCH 19/61] rustfmt --- zebrad/tests/acceptance.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index d5706b45967..ac553aa87d4 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -40,7 +40,6 @@ use zebrad::config::ZebradConfig; const LAUNCH_DELAY: Duration = Duration::from_secs(3); const LAUNCH_DELAY_BIG: Duration = Duration::from_secs(100); - fn default_test_config() -> Result { let auto_port_ipv4_local = zebra_network::Config { listen_addr: "127.0.0.1:0".parse()?, From 803694105e852edb5b65c53df1197ba56aca2c18 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 19 Jan 2021 22:28:55 -0300 Subject: [PATCH 20/61] disable check --- zebrad/tests/acceptance.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index ac553aa87d4..bad97b06890 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1058,7 +1058,6 @@ async fn tracing_endpoint() -> Result<()> { /// Test will start 2 zebrad nodes one after the other using the same Zcash listener. /// It is expected that the first node spawned will get exclusive use of the port. /// The second node will panic with the Zcash listener conflict hint added in #1535. - #[test] fn zcash_listener_conflict() -> Result<()> { zebra_test::init(); @@ -1134,7 +1133,7 @@ fn zcash_tracing_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "already in use")?; + check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; Ok(()) } @@ -1186,6 +1185,7 @@ where // Wait a few seconds and kill both nodes std::thread::sleep(LAUNCH_DELAY_BIG); + node1.kill()?; node2.kill()?; @@ -1199,9 +1199,12 @@ where // In the second node we look for the conflict regex let output2 = node2.wait_with_output()?; output2.stderr_contains(second_stderr_regex)?; + // The following check fails on Windows so it is removed by now + /* output2 .assert_was_not_killed() .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + */ Ok(()) } From 0fa3781852eba29d9877f2cd8e5e88f677981bf5 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 20 Jan 2021 17:05:33 -0300 Subject: [PATCH 21/61] skip conflict tests on macos --- zebra-test/src/command.rs | 1 + zebrad/tests/acceptance.rs | 70 +++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index ff051fc7f94..ac3c5709d26 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -392,6 +392,7 @@ impl TestOutput { pub fn stdout_matches(&self, regex: &str) -> Result<&Self> { let re = regex::Regex::new(regex)?; let stdout = String::from_utf8_lossy(&self.output.stdout); + dbg!(&stdout); if re.is_match(&stdout) { return Ok(self); diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index bad97b06890..60e4339a507 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -38,7 +38,9 @@ use zebrad::config::ZebradConfig; /// Previously, this value was 1 second, which caused occasional /// `tracing_endpoint` test failures on some machines. const LAUNCH_DELAY: Duration = Duration::from_secs(3); -const LAUNCH_DELAY_BIG: Duration = Duration::from_secs(100); + +/// +const LAUNCH_DELAY_BIG: Duration = Duration::from_secs(25); fn default_test_config() -> Result { let auto_port_ipv4_local = zebra_network::Config { @@ -1174,37 +1176,41 @@ where T: ZebradTestDirExt, U: ZebradTestDirExt, { - // Start the first node - let mut node1 = first_dir.spawn_child(&["start"])?; - - // wait a bit to spawn the second node, we want the first fully started. - std::thread::sleep(LAUNCH_DELAY); - - // Spawn the second node - let mut node2 = second_dir.spawn_child(&["start"])?; - - // Wait a few seconds and kill both nodes - std::thread::sleep(LAUNCH_DELAY_BIG); - - node1.kill()?; - node2.kill()?; - - // In node1 we want to check for the success regex - let output1 = node1.wait_with_output()?; - output1.stdout_contains(first_stdout_regex)?; - output1 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - - // In the second node we look for the conflict regex - let output2 = node2.wait_with_output()?; - output2.stderr_contains(second_stderr_regex)?; - // The following check fails on Windows so it is removed by now - /* - output2 - .assert_was_not_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + // Only execute this test in windows or linux families + if !cfg!(target_os = "macos") { + // Start the first node + let mut node1 = first_dir.spawn_child(&["start"])?; + + // Wait a bit to spawn the second node, we want the first fully started. + std::thread::sleep(LAUNCH_DELAY_BIG); + + // Spawn the second node + let mut node2 = second_dir.spawn_child(&["start"])?; + + // Wait a few seconds and kill both nodes + std::thread::sleep(LAUNCH_DELAY_BIG); + + node1.kill()?; + node2.kill()?; + + // In node1 we want to check for the success regex + let output1 = node1.wait_with_output()?; + output1.stdout_contains(first_stdout_regex)?; + output1 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + + // In the second node we look for the conflict regex + let output2 = node2.wait_with_output()?; + output2.stderr_contains(second_stderr_regex)?; + + // Windows is marking the exit as killed + if !cfg!(windows) { + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + } + } - */ Ok(()) } From 2f4728d31fdfa059872a2cb3c2e7d8ba7031f393 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 21 Jan 2021 15:49:38 -0800 Subject: [PATCH 22/61] add timeout to listener bind --- zebra-network/src/peer_set/initialize.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 678f10405ee..1c3dd7bceb0 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -6,6 +6,7 @@ use std::{ net::SocketAddr, sync::{Arc, Mutex}, + time::Duration, }; use futures::{ @@ -211,10 +212,18 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - let listener_result = TcpListener::bind(addr).await; + let listener_fut = TcpListener::bind(addr); + let listener_fut = tokio::time::timeout(Duration::from_secs(3), listener_fut); + let listener_result = listener_fut.await; let listener = match listener_result { - Ok(l) => l, + Ok(Ok(l)) => l, + Ok(Err(e)) => panic!( + "Opening Zcash network protocol listener {:?} failed: {:?}. \ + Hint: Check if another zebrad or zcashd process is running. \ + Try changing the network listen_addr in the Zebra config.", + addr, e, + ), Err(e) => panic!( "Opening Zcash network protocol listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ From 16e61c25f23ed7c5d6609e1014797042ec4b5bc5 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 21 Jan 2021 16:17:12 -0800 Subject: [PATCH 23/61] test if output is being lost --- zebra-network/src/peer_set/initialize.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 1c3dd7bceb0..2663627d255 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -216,20 +216,20 @@ where let listener_fut = tokio::time::timeout(Duration::from_secs(3), listener_fut); let listener_result = listener_fut.await; - let listener = match listener_result { - Ok(Ok(l)) => l, - Ok(Err(e)) => panic!( - "Opening Zcash network protocol listener {:?} failed: {:?}. \ - Hint: Check if another zebrad or zcashd process is running. \ - Try changing the network listen_addr in the Zebra config.", - addr, e, - ), - Err(e) => panic!( + let panic_now = |e: &dyn std::fmt::Debug| { + println!("Panicking now"); + panic!( "Opening Zcash network protocol listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ Try changing the network listen_addr in the Zebra config.", addr, e, - ), + ); + }; + + let listener = match listener_result { + Ok(Ok(l)) => l, + Ok(Err(e)) => panic_now(&e), + Err(e) => panic_now(&e), }; let local_addr = listener.local_addr()?; From b19d8b360089f738e086db07ae1f03399d39d7ff Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 21 Jan 2021 16:19:48 -0800 Subject: [PATCH 24/61] add more tracing --- zebra-network/src/peer_set/initialize.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 2663627d255..76198c43d34 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -212,6 +212,7 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { + println!("attempting to open listener"); let listener_fut = TcpListener::bind(addr); let listener_fut = tokio::time::timeout(Duration::from_secs(3), listener_fut); let listener_result = listener_fut.await; From 6b4134a4304ee743c9cd607560020d4be0582eac Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 21 Jan 2021 16:50:25 -0800 Subject: [PATCH 25/61] add more tracing --- zebra-network/src/peer_set/initialize.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 76198c43d34..da7c3b8f5fc 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -67,6 +67,7 @@ where S: Service + Clone + Send + 'static, S::Future: Send + 'static, { + info!("checkpoint: {}", line!()); let (address_book, timestamp_collector) = TimestampCollector::spawn(); let (inv_sender, inv_receiver) = broadcast::channel(100); @@ -115,6 +116,7 @@ where inv_receiver, ); let peer_set = Buffer::new(BoxService::new(peer_set), constants::PEERSET_BUFFER_SIZE); + info!("checkpoint: {}", line!()); // Connect the tx end to the 3 peer sources: @@ -125,6 +127,7 @@ where peerset_tx.clone(), )); + info!("checkpoint: {}", line!()); // 2. Incoming peer connections, via a listener. // Warn if we're configured using the wrong network port. @@ -143,7 +146,9 @@ where ); } + info!("checkpoint: {}", line!()); let listen_guard = tokio::spawn(listen(config.listen_addr, listener, peerset_tx.clone())); + info!("checkpoint: {}", line!()); // 3. Outgoing peers we connect to in response to load. let mut candidates = CandidateSet::new(address_book.clone(), peer_set.clone()); From 8817a2c73acd35b84e5905196dc5e25092a428fe Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 21 Jan 2021 17:25:17 -0800 Subject: [PATCH 26/61] more printf debugging, MOREEE --- zebra-network/src/peer_set/initialize.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index da7c3b8f5fc..dca0bda2e45 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -118,14 +118,22 @@ where let peer_set = Buffer::new(BoxService::new(peer_set), constants::PEERSET_BUFFER_SIZE); info!("checkpoint: {}", line!()); - // Connect the tx end to the 3 peer sources: + let initial_peers_fut = { + let initial_peers = config.initial_peers(); + info!("checkpoint: {}", line!()); + let connector = connector.clone(); + info!("checkpoint: {}", line!()); + let tx = peerset_tx.clone(); + info!("checkpoint: {}", line!()); + + // Connect the tx end to the 3 peer sources: + add_initial_peers(initial_peers, connector, tx) + }; + + info!("checkpoint: {}", line!()); // 1. Initial peers, specified in the config. - let add_guard = tokio::spawn(add_initial_peers( - config.initial_peers(), - connector.clone(), - peerset_tx.clone(), - )); + let add_guard = tokio::spawn(initial_peers_fut); info!("checkpoint: {}", line!()); // 2. Incoming peer connections, via a listener. From 767b94b7082271322910c781c0f1c03e204fef26 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sat, 23 Jan 2021 09:09:36 -0300 Subject: [PATCH 27/61] move zcash listener opening --- zebra-network/src/peer_set/initialize.rs | 28 +++++++++++++++++++++++- zebrad/tests/acceptance.rs | 2 ++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index dca0bda2e45..1f3ffb4f240 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -94,7 +94,7 @@ where hs_timeout.layer(peer::Connector::new(hs)), ) }; - + // Create an mpsc channel for peer changes, with a generous buffer. let (peerset_tx, peerset_rx) = mpsc::channel::(100); // Create an mpsc channel for peerset demand signaling. @@ -118,6 +118,28 @@ where let peer_set = Buffer::new(BoxService::new(peer_set), constants::PEERSET_BUFFER_SIZE); info!("checkpoint: {}", line!()); + // 2. Incoming peer connections, via a listener. + + // Warn if we're configured using the wrong network port. + // TODO: use the right port if the port is unspecified + // split the address and port configs? + let (wrong_net, wrong_net_port) = match config.network { + Network::Mainnet => (Network::Testnet, 18233), + Network::Testnet => (Network::Mainnet, 8233), + }; + if config.listen_addr.port() == wrong_net_port { + warn!( + "We are configured with port {} for {:?}, but that port is the default port for {:?}", + config.listen_addr.port(), + config.network, + wrong_net + ); + } + + info!("checkpoint: {}", line!()); + let listen_guard = tokio::spawn(listen(config.listen_addr, listener, peerset_tx.clone())); + info!("checkpoint: {}", line!()); + let initial_peers_fut = { let initial_peers = config.initial_peers(); info!("checkpoint: {}", line!()); @@ -136,6 +158,8 @@ where let add_guard = tokio::spawn(initial_peers_fut); info!("checkpoint: {}", line!()); + + /* // 2. Incoming peer connections, via a listener. // Warn if we're configured using the wrong network port. @@ -158,6 +182,8 @@ where let listen_guard = tokio::spawn(listen(config.listen_addr, listener, peerset_tx.clone())); info!("checkpoint: {}", line!()); + */ + // 3. Outgoing peers we connect to in response to load. let mut candidates = CandidateSet::new(address_book.clone(), peer_set.clone()); diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 60e4339a507..0a8e5a41bf3 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1061,7 +1061,9 @@ async fn tracing_endpoint() -> Result<()> { /// It is expected that the first node spawned will get exclusive use of the port. /// The second node will panic with the Zcash listener conflict hint added in #1535. #[test] +#[ignore] fn zcash_listener_conflict() -> Result<()> { + zebra_test::init(); // [Note on port conflict](#Note on port conflict) From 6aa6abb6119367f93f1ce7b559659ffce31c8f14 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sat, 23 Jan 2021 12:39:15 -0300 Subject: [PATCH 28/61] remove listener bind timeout --- zebra-network/src/peer_set/initialize.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 1f3ffb4f240..08b713ff61a 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -6,7 +6,6 @@ use std::{ net::SocketAddr, sync::{Arc, Mutex}, - time::Duration, }; use futures::{ @@ -251,25 +250,16 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - println!("attempting to open listener"); - let listener_fut = TcpListener::bind(addr); - let listener_fut = tokio::time::timeout(Duration::from_secs(3), listener_fut); - let listener_result = listener_fut.await; - - let panic_now = |e: &dyn std::fmt::Debug| { - println!("Panicking now"); - panic!( + let listener_result = TcpListener::bind(addr).await; + + let listener = match listener_result { + Ok(l) => l, + Err(e) => panic!( "Opening Zcash network protocol listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ Try changing the network listen_addr in the Zebra config.", addr, e, - ); - }; - - let listener = match listener_result { - Ok(Ok(l)) => l, - Ok(Err(e)) => panic_now(&e), - Err(e) => panic_now(&e), + ), }; let local_addr = listener.local_addr()?; From 2e3cd158957ea78ff6a28f4061756adedd41d458 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sat, 23 Jan 2021 13:21:52 -0300 Subject: [PATCH 29/61] remove big delay --- zebrad/tests/acceptance.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 0a8e5a41bf3..1f714c633e6 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -39,9 +39,6 @@ use zebrad::config::ZebradConfig; /// `tracing_endpoint` test failures on some machines. const LAUNCH_DELAY: Duration = Duration::from_secs(3); -/// -const LAUNCH_DELAY_BIG: Duration = Duration::from_secs(25); - fn default_test_config() -> Result { let auto_port_ipv4_local = zebra_network::Config { listen_addr: "127.0.0.1:0".parse()?, @@ -1063,7 +1060,6 @@ async fn tracing_endpoint() -> Result<()> { #[test] #[ignore] fn zcash_listener_conflict() -> Result<()> { - zebra_test::init(); // [Note on port conflict](#Note on port conflict) @@ -1184,13 +1180,13 @@ where let mut node1 = first_dir.spawn_child(&["start"])?; // Wait a bit to spawn the second node, we want the first fully started. - std::thread::sleep(LAUNCH_DELAY_BIG); + std::thread::sleep(LAUNCH_DELAY); // Spawn the second node let mut node2 = second_dir.spawn_child(&["start"])?; // Wait a few seconds and kill both nodes - std::thread::sleep(LAUNCH_DELAY_BIG); + std::thread::sleep(LAUNCH_DELAY); node1.kill()?; node2.kill()?; From 7fc09834be3f57cfed3448ce573042a28dfefbaf Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sat, 23 Jan 2021 14:00:39 -0300 Subject: [PATCH 30/61] update platform conditionals docs --- zebrad/tests/acceptance.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 1f714c633e6..70396a25443 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1058,7 +1058,6 @@ async fn tracing_endpoint() -> Result<()> { /// It is expected that the first node spawned will get exclusive use of the port. /// The second node will panic with the Zcash listener conflict hint added in #1535. #[test] -#[ignore] fn zcash_listener_conflict() -> Result<()> { zebra_test::init(); @@ -1174,7 +1173,7 @@ where T: ZebradTestDirExt, U: ZebradTestDirExt, { - // Only execute this test in windows or linux families + // Don't execute this tests in macOS if !cfg!(target_os = "macos") { // Start the first node let mut node1 = first_dir.spawn_child(&["start"])?; @@ -1202,8 +1201,14 @@ where let output2 = node2.wait_with_output()?; output2.stderr_contains(second_stderr_regex)?; - // Windows is marking the exit as killed - if !cfg!(windows) { + // Panics on Windows exit with the same kill signal code(1) + if cfg!(target_os = "windows") { + output2 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + } + // Panics on Linux exit with a different kill signal code(9) + else { output2 .assert_was_not_killed() .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; From 09aab1d49a7ce5739652e70f0e8628e8eb7b7e98 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sat, 23 Jan 2021 18:18:32 -0300 Subject: [PATCH 31/61] test timeout in macOS zcash listener --- zebra-network/src/peer_set/initialize.rs | 24 +++++++-- zebrad/tests/acceptance.rs | 63 +++++++++++------------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 08b713ff61a..8f4c77ef0e4 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -6,6 +6,7 @@ use std::{ net::SocketAddr, sync::{Arc, Mutex}, + time::Duration, }; use futures::{ @@ -17,6 +18,7 @@ use futures::{ use tokio::{ net::{TcpListener, TcpStream}, sync::broadcast, + task, }; use tower::{ buffer::Buffer, discover::Change, layer::Layer, load::peak_ewma::PeakEwmaDiscover, @@ -250,16 +252,28 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - let listener_result = TcpListener::bind(addr).await; - let listener = match listener_result { - Ok(l) => l, - Err(e) => panic!( + let bind_fut = task::spawn_blocking(move || { + TcpListener::bind(addr) + }).await?; + + let listener_fut = tokio::time::timeout(Duration::from_secs(1), bind_fut); + let listener_result = listener_fut.await; + + let panic_now = |e: &dyn std::fmt::Debug| { + println!("Panicking now"); + panic!( "Opening Zcash network protocol listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ Try changing the network listen_addr in the Zebra config.", addr, e, - ), + ); + }; + + let listener = match listener_result { + Ok(Ok(l)) => l, + Ok(Err(e)) => panic_now(&e), + Err(e) => panic_now(&e), }; let local_addr = listener.local_addr()?; diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 70396a25443..907b237fbc3 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1173,46 +1173,43 @@ where T: ZebradTestDirExt, U: ZebradTestDirExt, { - // Don't execute this tests in macOS - if !cfg!(target_os = "macos") { - // Start the first node - let mut node1 = first_dir.spawn_child(&["start"])?; + // Start the first node + let mut node1 = first_dir.spawn_child(&["start"])?; - // Wait a bit to spawn the second node, we want the first fully started. - std::thread::sleep(LAUNCH_DELAY); + // Wait a bit to spawn the second node, we want the first fully started. + std::thread::sleep(LAUNCH_DELAY); - // Spawn the second node - let mut node2 = second_dir.spawn_child(&["start"])?; + // Spawn the second node + let mut node2 = second_dir.spawn_child(&["start"])?; - // Wait a few seconds and kill both nodes - std::thread::sleep(LAUNCH_DELAY); + // Wait a few seconds and kill both nodes + std::thread::sleep(LAUNCH_DELAY); - node1.kill()?; - node2.kill()?; + node1.kill()?; + node2.kill()?; - // In node1 we want to check for the success regex - let output1 = node1.wait_with_output()?; - output1.stdout_contains(first_stdout_regex)?; - output1 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + // In node1 we want to check for the success regex + let output1 = node1.wait_with_output()?; + output1.stdout_contains(first_stdout_regex)?; + output1 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - // In the second node we look for the conflict regex - let output2 = node2.wait_with_output()?; - output2.stderr_contains(second_stderr_regex)?; + // In the second node we look for the conflict regex + let output2 = node2.wait_with_output()?; + output2.stderr_contains(second_stderr_regex)?; - // Panics on Windows exit with the same kill signal code(1) - if cfg!(target_os = "windows") { - output2 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - } - // Panics on Linux exit with a different kill signal code(9) - else { - output2 - .assert_was_not_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - } + // Panics on Windows exit with the same kill signal code(1) + if cfg!(target_os = "windows") { + output2 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + } + // Panics on Linux exit with a different kill signal code(9) + else { + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; } Ok(()) From f11a3b6c04c3075b74d379b92ce68943dce6b8ab Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 12:25:59 -0300 Subject: [PATCH 32/61] remove commented out code --- zebra-network/src/peer_set/initialize.rs | 29 ++---------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 8f4c77ef0e4..d1e87b4dc72 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -119,7 +119,7 @@ where let peer_set = Buffer::new(BoxService::new(peer_set), constants::PEERSET_BUFFER_SIZE); info!("checkpoint: {}", line!()); - // 2. Incoming peer connections, via a listener. + // 1. Incoming peer connections, via a listener. // Warn if we're configured using the wrong network port. // TODO: use the right port if the port is unspecified @@ -155,36 +155,11 @@ where info!("checkpoint: {}", line!()); - // 1. Initial peers, specified in the config. + // 2. Initial peers, specified in the config. let add_guard = tokio::spawn(initial_peers_fut); info!("checkpoint: {}", line!()); - /* - // 2. Incoming peer connections, via a listener. - - // Warn if we're configured using the wrong network port. - // TODO: use the right port if the port is unspecified - // split the address and port configs? - let (wrong_net, wrong_net_port) = match config.network { - Network::Mainnet => (Network::Testnet, 18233), - Network::Testnet => (Network::Mainnet, 8233), - }; - if config.listen_addr.port() == wrong_net_port { - warn!( - "We are configured with port {} for {:?}, but that port is the default port for {:?}", - config.listen_addr.port(), - config.network, - wrong_net - ); - } - - info!("checkpoint: {}", line!()); - let listen_guard = tokio::spawn(listen(config.listen_addr, listener, peerset_tx.clone())); - info!("checkpoint: {}", line!()); - - */ - // 3. Outgoing peers we connect to in response to load. let mut candidates = CandidateSet::new(address_book.clone(), peer_set.clone()); From 6ca008c5182b8bd243f594eab8c5924bebbbb86a Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 13:09:55 -0300 Subject: [PATCH 33/61] add and use network contants --- zebra-network/src/constants.rs | 9 +++++++++ zebra-network/src/lib.rs | 2 +- zebra-network/src/peer_set/initialize.rs | 7 ++----- zebrad/src/application.rs | 6 +++++- zebrad/tests/acceptance.rs | 24 ++++++++++++++++++++---- 5 files changed, 37 insertions(+), 11 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 2e089442b03..087ab5fab6c 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -95,6 +95,15 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); +/// The maximum time to open a port if it haves a timeout. +pub const BIND_TIMEOUT: Duration = Duration::from_secs(1); + +/// Linux portion of OS error when the port attempting to be opened is already in use. +pub const PORT_IN_USE_LINUX: &str = "already in use"; + +/// Windows portion of OS error when the port attempting to be opened is already in use. +pub const PORT_IN_USE_WINDOWS: &str = "one usage"; + /// Magic numbers used to identify different Zcash networks. pub mod magics { use super::*; diff --git a/zebra-network/src/lib.rs b/zebra-network/src/lib.rs index 7bce06115a5..75f15ffdf35 100644 --- a/zebra-network/src/lib.rs +++ b/zebra-network/src/lib.rs @@ -66,7 +66,7 @@ pub type BoxError = Box; mod address_book; mod config; -mod constants; +pub mod constants; mod isolated; mod meta_addr; mod peer; diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index d1e87b4dc72..51637dcc32b 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -95,7 +95,7 @@ where hs_timeout.layer(peer::Connector::new(hs)), ) }; - + // Create an mpsc channel for peer changes, with a generous buffer. let (peerset_tx, peerset_rx) = mpsc::channel::(100); // Create an mpsc channel for peerset demand signaling. @@ -227,10 +227,7 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - - let bind_fut = task::spawn_blocking(move || { - TcpListener::bind(addr) - }).await?; + let bind_fut = task::spawn_blocking(move || TcpListener::bind(addr)).await?; let listener_fut = tokio::time::timeout(Duration::from_secs(1), bind_fut); let listener_result = listener_fut.await; diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 5fd3e48cbee..1d462133000 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -12,6 +12,8 @@ use abscissa_core::{ use application::fatal_error; use std::process; +use zebra_network::constants::{PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS}; + /// Application state pub static APPLICATION: AppCell = AppCell::new(); @@ -177,7 +179,9 @@ impl Application for ZebradApp { None => return true, }; // listener port conflicts - if error_str.contains("already in use") { + if error_str.contains(PORT_IN_USE_LINUX) + || error_str.contains(PORT_IN_USE_WINDOWS) + { return false; } // RocksDB lock file conflicts diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 907b237fbc3..94e164112f6 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -30,6 +30,7 @@ use zebra_chain::{ NetworkUpgrade, }, }; +use zebra_network::constants::{PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS}; use zebra_test::{command::TestDirExt, prelude::*}; use zebrad::config::ZebradConfig; @@ -1076,7 +1077,12 @@ fn zcash_listener_conflict() -> Result<()> { // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; + check_config_conflict( + dir1, + regex1.as_str(), + dir2, + format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + )?; Ok(()) } @@ -1104,7 +1110,12 @@ fn zcash_metrics_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; + check_config_conflict( + dir1, + regex1.as_str(), + dir2, + format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + )?; Ok(()) } @@ -1132,7 +1143,12 @@ fn zcash_tracing_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, "(already in use)|(one usage)")?; + check_config_conflict( + dir1, + regex1.as_str(), + dir2, + format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + )?; Ok(()) } @@ -1205,7 +1221,7 @@ where .assert_was_killed() .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; } - // Panics on Linux exit with a different kill signal code(9) + // Panics on Linux exit with a different kill signal code(9) else { output2 .assert_was_not_killed() From e99063ba817da45314eac0581924ffb598b3e145 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 13:16:09 -0300 Subject: [PATCH 34/61] use timeout constant in listener bind timeout --- zebra-network/src/peer_set/initialize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 51637dcc32b..283d985ed34 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -229,7 +229,7 @@ where { let bind_fut = task::spawn_blocking(move || TcpListener::bind(addr)).await?; - let listener_fut = tokio::time::timeout(Duration::from_secs(1), bind_fut); + let listener_fut = tokio::time::timeout(constants::BIND_TIMEOUT, bind_fut); let listener_result = listener_fut.await; let panic_now = |e: &dyn std::fmt::Debug| { From a0a3a61893c2a9a636ea133d5ed9012a8cb03788 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 13:53:15 -0300 Subject: [PATCH 35/61] remove unused import --- zebra-network/src/peer_set/initialize.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 283d985ed34..041d0587ce3 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -6,7 +6,6 @@ use std::{ net::SocketAddr, sync::{Arc, Mutex}, - time::Duration, }; use futures::{ From 9198f188df3ba4f7ad15972d8892749b7fcb216b Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 21:19:40 -0300 Subject: [PATCH 36/61] cleanup --- zebra-network/src/constants.rs | 5 +- zebra-network/src/peer_set/initialize.rs | 31 ++-------- zebra-test/src/command.rs | 2 - zebrad/src/application.rs | 4 +- zebrad/tests/acceptance.rs | 73 +++++++++++++----------- 5 files changed, 47 insertions(+), 68 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 087ab5fab6c..e8871b815eb 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -95,11 +95,8 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); -/// The maximum time to open a port if it haves a timeout. -pub const BIND_TIMEOUT: Duration = Duration::from_secs(1); - /// Linux portion of OS error when the port attempting to be opened is already in use. -pub const PORT_IN_USE_LINUX: &str = "already in use"; +pub const PORT_IN_USE_UNIX: &str = "already in use"; /// Windows portion of OS error when the port attempting to be opened is already in use. pub const PORT_IN_USE_WINDOWS: &str = "one usage"; diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 041d0587ce3..e6ad0cf09d7 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -17,7 +17,6 @@ use futures::{ use tokio::{ net::{TcpListener, TcpStream}, sync::broadcast, - task, }; use tower::{ buffer::Buffer, discover::Change, layer::Layer, load::peak_ewma::PeakEwmaDiscover, @@ -67,7 +66,6 @@ where S: Service + Clone + Send + 'static, S::Future: Send + 'static, { - info!("checkpoint: {}", line!()); let (address_book, timestamp_collector) = TimestampCollector::spawn(); let (inv_sender, inv_receiver) = broadcast::channel(100); @@ -116,7 +114,6 @@ where inv_receiver, ); let peer_set = Buffer::new(BoxService::new(peer_set), constants::PEERSET_BUFFER_SIZE); - info!("checkpoint: {}", line!()); // 1. Incoming peer connections, via a listener. @@ -136,29 +133,20 @@ where ); } - info!("checkpoint: {}", line!()); let listen_guard = tokio::spawn(listen(config.listen_addr, listener, peerset_tx.clone())); - info!("checkpoint: {}", line!()); let initial_peers_fut = { let initial_peers = config.initial_peers(); - info!("checkpoint: {}", line!()); let connector = connector.clone(); - info!("checkpoint: {}", line!()); let tx = peerset_tx.clone(); - info!("checkpoint: {}", line!()); // Connect the tx end to the 3 peer sources: add_initial_peers(initial_peers, connector, tx) }; - info!("checkpoint: {}", line!()); - // 2. Initial peers, specified in the config. let add_guard = tokio::spawn(initial_peers_fut); - info!("checkpoint: {}", line!()); - // 3. Outgoing peers we connect to in response to load. let mut candidates = CandidateSet::new(address_book.clone(), peer_set.clone()); @@ -226,25 +214,16 @@ where S: Service<(TcpStream, SocketAddr), Response = peer::Client, Error = BoxError> + Clone, S::Future: Send + 'static, { - let bind_fut = task::spawn_blocking(move || TcpListener::bind(addr)).await?; - - let listener_fut = tokio::time::timeout(constants::BIND_TIMEOUT, bind_fut); - let listener_result = listener_fut.await; + let listener_result = TcpListener::bind(addr).await; - let panic_now = |e: &dyn std::fmt::Debug| { - println!("Panicking now"); - panic!( + let listener = match listener_result { + Ok(l) => l, + Err(e) => panic!( "Opening Zcash network protocol listener {:?} failed: {:?}. \ Hint: Check if another zebrad or zcashd process is running. \ Try changing the network listen_addr in the Zebra config.", addr, e, - ); - }; - - let listener = match listener_result { - Ok(Ok(l)) => l, - Ok(Err(e)) => panic_now(&e), - Err(e) => panic_now(&e), + ), }; let local_addr = listener.local_addr()?; diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index ac3c5709d26..8395c03a37c 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -392,7 +392,6 @@ impl TestOutput { pub fn stdout_matches(&self, regex: &str) -> Result<&Self> { let re = regex::Regex::new(regex)?; let stdout = String::from_utf8_lossy(&self.output.stdout); - dbg!(&stdout); if re.is_match(&stdout) { return Ok(self); @@ -407,7 +406,6 @@ impl TestOutput { pub fn stderr_contains(&self, regex: &str) -> Result<&Self> { let re = regex::Regex::new(regex)?; let stderr = String::from_utf8_lossy(&self.output.stderr); - dbg!(&stderr); for line in stderr.lines() { if re.is_match(line) { diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 1d462133000..50c5bd1af9e 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -12,7 +12,7 @@ use abscissa_core::{ use application::fatal_error; use std::process; -use zebra_network::constants::{PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS}; +use zebra_network::constants::{PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS}; /// Application state pub static APPLICATION: AppCell = AppCell::new(); @@ -179,7 +179,7 @@ impl Application for ZebradApp { None => return true, }; // listener port conflicts - if error_str.contains(PORT_IN_USE_LINUX) + if error_str.contains(PORT_IN_USE_UNIX) || error_str.contains(PORT_IN_USE_WINDOWS) { return false; diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 94e164112f6..303b46f714a 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -30,7 +30,7 @@ use zebra_chain::{ NetworkUpgrade, }, }; -use zebra_network::constants::{PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS}; +use zebra_network::constants::{PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS}; use zebra_test::{command::TestDirExt, prelude::*}; use zebrad::config::ZebradConfig; @@ -1081,7 +1081,7 @@ fn zcash_listener_conflict() -> Result<()> { dir1, regex1.as_str(), dir2, - format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), )?; Ok(()) @@ -1114,7 +1114,7 @@ fn zcash_metrics_conflict() -> Result<()> { dir1, regex1.as_str(), dir2, - format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), )?; Ok(()) @@ -1147,7 +1147,7 @@ fn zcash_tracing_conflict() -> Result<()> { dir1, regex1.as_str(), dir2, - format!("({})|({})", PORT_IN_USE_LINUX, PORT_IN_USE_WINDOWS).as_str(), + format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), )?; Ok(()) @@ -1189,43 +1189,48 @@ where T: ZebradTestDirExt, U: ZebradTestDirExt, { - // Start the first node - let mut node1 = first_dir.spawn_child(&["start"])?; + // By DNS issues we want to skip all port conflict tests on macOS by now. + // They should be activated after https://github.com/ZcashFoundation/zebra/issues/1631 + if !cfg!(target_os = "macos") { + // Start the first node + let mut node1 = first_dir.spawn_child(&["start"])?; - // Wait a bit to spawn the second node, we want the first fully started. - std::thread::sleep(LAUNCH_DELAY); - - // Spawn the second node - let mut node2 = second_dir.spawn_child(&["start"])?; - - // Wait a few seconds and kill both nodes - std::thread::sleep(LAUNCH_DELAY); + // Wait a bit to spawn the second node, we want the first fully started. + std::thread::sleep(LAUNCH_DELAY); - node1.kill()?; - node2.kill()?; + // Spawn the second node + let mut node2 = second_dir.spawn_child(&["start"])?; - // In node1 we want to check for the success regex - let output1 = node1.wait_with_output()?; - output1.stdout_contains(first_stdout_regex)?; - output1 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + // Wait a few seconds and kill both nodes + std::thread::sleep(LAUNCH_DELAY); - // In the second node we look for the conflict regex - let output2 = node2.wait_with_output()?; - output2.stderr_contains(second_stderr_regex)?; + node1.kill()?; + node2.kill()?; - // Panics on Windows exit with the same kill signal code(1) - if cfg!(target_os = "windows") { - output2 + // In node1 we want to check for the success regex + let output1 = node1.wait_with_output()?; + output1.stdout_contains(first_stdout_regex)?; + output1 .assert_was_killed() .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - } - // Panics on Linux exit with a different kill signal code(9) - else { - output2 - .assert_was_not_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + + // In the second node we look for the conflict regex + let output2 = node2.wait_with_output()?; + output2.stderr_contains(second_stderr_regex)?; + + // Panics on Windows exit with the same kill signal code(1) + // https://github.com/ZcashFoundation/zebra/issues/1632 + if cfg!(target_os = "windows") { + output2 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + } + // Panics on Linux exit with a different kill signal code(9) + else { + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + } } Ok(()) From e11b976101f638f588243ef408aeba86bcfe52b1 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 25 Jan 2021 21:25:05 -0300 Subject: [PATCH 37/61] add todo about hint for disk full --- zebra-state/src/service/finalized_state.rs | 1 + zebrad/tests/acceptance.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 98e63b84c3c..94a83dcf0cb 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -48,6 +48,7 @@ impl FinalizedState { tracing::info!("Opened Zebra state cache at: {:?}", path); d } + // TODO: provide a different hint if the disk is full, see #1623 Err(e) => panic!( "Opening database {:?} failed: {:?}. \ Hint: Check if another zebrad process is running. \ diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 303b46f714a..340bb65111d 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1190,7 +1190,7 @@ where U: ZebradTestDirExt, { // By DNS issues we want to skip all port conflict tests on macOS by now. - // They should be activated after https://github.com/ZcashFoundation/zebra/issues/1631 + // Follow up at #1631 if !cfg!(target_os = "macos") { // Start the first node let mut node1 = first_dir.spawn_child(&["start"])?; @@ -1219,7 +1219,7 @@ where output2.stderr_contains(second_stderr_regex)?; // Panics on Windows exit with the same kill signal code(1) - // https://github.com/ZcashFoundation/zebra/issues/1632 + // Follow up at #1632 if cfg!(target_os = "windows") { output2 .assert_was_killed() From 925a4fbaef5645d2e91e1de6c674e1d21d9da464 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 26 Jan 2021 09:29:13 -0300 Subject: [PATCH 38/61] avoid the 2 constant names by os --- zebra-network/src/constants.rs | 8 +++++--- zebrad/src/application.rs | 6 ++---- zebrad/tests/acceptance.rs | 23 ++++------------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index e8871b815eb..c2fb959e96d 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -95,11 +95,13 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); -/// Linux portion of OS error when the port attempting to be opened is already in use. -pub const PORT_IN_USE_UNIX: &str = "already in use"; +/// Unix portion of OS error when the port attempting to be opened is already in use. +#[cfg(unix)] +pub const PORT_IN_USE_ERROR: &str = "already in use"; /// Windows portion of OS error when the port attempting to be opened is already in use. -pub const PORT_IN_USE_WINDOWS: &str = "one usage"; +#[cfg(not(unix))] +pub const PORT_IN_USE_ERROR: &str = "one usage"; /// Magic numbers used to identify different Zcash networks. pub mod magics { diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 50c5bd1af9e..eba18eded30 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -12,7 +12,7 @@ use abscissa_core::{ use application::fatal_error; use std::process; -use zebra_network::constants::{PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS}; +use zebra_network::constants::PORT_IN_USE_ERROR; /// Application state pub static APPLICATION: AppCell = AppCell::new(); @@ -179,9 +179,7 @@ impl Application for ZebradApp { None => return true, }; // listener port conflicts - if error_str.contains(PORT_IN_USE_UNIX) - || error_str.contains(PORT_IN_USE_WINDOWS) - { + if error_str.contains(PORT_IN_USE_ERROR) { return false; } // RocksDB lock file conflicts diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 340bb65111d..468c5dab37d 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -30,7 +30,7 @@ use zebra_chain::{ NetworkUpgrade, }, }; -use zebra_network::constants::{PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS}; +use zebra_network::constants::PORT_IN_USE_ERROR; use zebra_test::{command::TestDirExt, prelude::*}; use zebrad::config::ZebradConfig; @@ -1077,12 +1077,7 @@ fn zcash_listener_conflict() -> Result<()> { // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict( - dir1, - regex1.as_str(), - dir2, - format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), - )?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; Ok(()) } @@ -1110,12 +1105,7 @@ fn zcash_metrics_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict( - dir1, - regex1.as_str(), - dir2, - format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), - )?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; Ok(()) } @@ -1143,12 +1133,7 @@ fn zcash_tracing_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict( - dir1, - regex1.as_str(), - dir2, - format!("({})|({})", PORT_IN_USE_UNIX, PORT_IN_USE_WINDOWS).as_str(), - )?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; Ok(()) } From df0e38dc1ee702bc070d872aecf5425896ca165a Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 26 Jan 2021 10:09:00 -0300 Subject: [PATCH 39/61] remove one kill --- zebrad/tests/acceptance.rs | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 468c5dab37d..e974b87c591 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1184,13 +1184,13 @@ where std::thread::sleep(LAUNCH_DELAY); // Spawn the second node - let mut node2 = second_dir.spawn_child(&["start"])?; + let node2 = second_dir.spawn_child(&["start"])?; - // Wait a few seconds and kill both nodes + // Wait a few seconds and kill first node. + // Second node gets killed by the panic. std::thread::sleep(LAUNCH_DELAY); node1.kill()?; - node2.kill()?; // In node1 we want to check for the success regex let output1 = node1.wait_with_output()?; @@ -1202,20 +1202,9 @@ where // In the second node we look for the conflict regex let output2 = node2.wait_with_output()?; output2.stderr_contains(second_stderr_regex)?; - - // Panics on Windows exit with the same kill signal code(1) - // Follow up at #1632 - if cfg!(target_os = "windows") { - output2 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - } - // Panics on Linux exit with a different kill signal code(9) - else { - output2 - .assert_was_not_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - } + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; } Ok(()) From d349d219313852b7083133a4f2354d06d834c119 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 26 Jan 2021 15:30:43 -0300 Subject: [PATCH 40/61] add constant for lock file --- zebra-state/src/constants.rs | 5 +++++ zebra-state/src/lib.rs | 2 +- zebrad/src/application.rs | 3 ++- zebrad/tests/acceptance.rs | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index d99c14205a0..402c4d41668 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -1,3 +1,5 @@ +//! Definitions of constants. + /// The maturity threshold for transparent coinbase outputs. /// /// A transaction MUST NOT spend a transparent output of a coinbase transaction @@ -13,3 +15,6 @@ pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1; /// The database format version, incremented each time the database format changes. pub const DATABASE_FORMAT_VERSION: u32 = 4; + +/// Portion of OS error when the RocksDB lock file is already open. +pub const LOCK_FILE_ERROR: &str = "lock file"; diff --git a/zebra-state/src/lib.rs b/zebra-state/src/lib.rs index 545b7fe375f..dc302c77d04 100644 --- a/zebra-state/src/lib.rs +++ b/zebra-state/src/lib.rs @@ -16,7 +16,7 @@ #![allow(clippy::unnecessary_wraps)] mod config; -mod constants; +pub mod constants; mod error; mod request; mod response; diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index eba18eded30..dd03759ed4e 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -13,6 +13,7 @@ use application::fatal_error; use std::process; use zebra_network::constants::PORT_IN_USE_ERROR; +use zebra_state::constants::LOCK_FILE_ERROR; /// Application state pub static APPLICATION: AppCell = AppCell::new(); @@ -183,7 +184,7 @@ impl Application for ZebradApp { return false; } // RocksDB lock file conflicts - if error_str.contains("lock file") + if error_str.contains(LOCK_FILE_ERROR) && (error_str.contains("temporarily unavailable") || error_str.contains("in use")) { diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index e974b87c591..c07e51fc2ef 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -31,6 +31,7 @@ use zebra_chain::{ }, }; use zebra_network::constants::PORT_IN_USE_ERROR; +use zebra_state::constants::LOCK_FILE_ERROR; use zebra_test::{command::TestDirExt, prelude::*}; use zebrad::config::ZebradConfig; @@ -1154,7 +1155,7 @@ fn zcash_state_conflict() -> Result<()> { dir_conflict.path(), "Opened Zebra state cache", dir_conflict.path(), - "lock file", + LOCK_FILE_ERROR, )?; Ok(()) From 7797ef6df4fd103d1ca6818334786693c7e92795 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 26 Jan 2021 16:20:52 -0300 Subject: [PATCH 41/61] remove whitespace --- zebrad/tests/acceptance.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index c07e51fc2ef..538b92a85ec 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1188,9 +1188,8 @@ where let node2 = second_dir.spawn_child(&["start"])?; // Wait a few seconds and kill first node. - // Second node gets killed by the panic. + // Second node is terminated by panic, no need to kill. std::thread::sleep(LAUNCH_DELAY); - node1.kill()?; // In node1 we want to check for the success regex From 7ee9149c449f0b29127ef097e7973d89b2f5a267 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 16:12:45 -0300 Subject: [PATCH 42/61] early exit ignored macos test --- zebrad/tests/acceptance.rs | 58 ++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 538b92a85ec..461c035509e 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1177,35 +1177,37 @@ where { // By DNS issues we want to skip all port conflict tests on macOS by now. // Follow up at #1631 - if !cfg!(target_os = "macos") { - // Start the first node - let mut node1 = first_dir.spawn_child(&["start"])?; - - // Wait a bit to spawn the second node, we want the first fully started. - std::thread::sleep(LAUNCH_DELAY); - - // Spawn the second node - let node2 = second_dir.spawn_child(&["start"])?; - - // Wait a few seconds and kill first node. - // Second node is terminated by panic, no need to kill. - std::thread::sleep(LAUNCH_DELAY); - node1.kill()?; - - // In node1 we want to check for the success regex - let output1 = node1.wait_with_output()?; - output1.stdout_contains(first_stdout_regex)?; - output1 - .assert_was_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; - - // In the second node we look for the conflict regex - let output2 = node2.wait_with_output()?; - output2.stderr_contains(second_stderr_regex)?; - output2 - .assert_was_not_killed() - .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + if cfg!(target_os = "macos") { + return Ok(()); } + // Start the first node + let mut node1 = first_dir.spawn_child(&["start"])?; + + // Wait a bit to spawn the second node, we want the first fully started. + std::thread::sleep(LAUNCH_DELAY); + + // Spawn the second node + let node2 = second_dir.spawn_child(&["start"])?; + + // Wait a few seconds and kill first node. + // Second node is terminated by panic, no need to kill. + std::thread::sleep(LAUNCH_DELAY); + node1.kill()?; + + // In node1 we want to check for the success regex + let output1 = node1.wait_with_output()?; + output1.stdout_contains(first_stdout_regex)?; + output1 + .assert_was_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + + // In the second node we look for the conflict regex + let output2 = node2.wait_with_output()?; + output2.stderr_contains(second_stderr_regex)?; + output2 + .assert_was_not_killed() + .wrap_err("Possible port conflict. Are there other acceptance tests running?")?; + Ok(()) } From 1a8f724f9324b899481524d8c7419b8f40569cd5 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 21:48:20 -0300 Subject: [PATCH 43/61] make LOCK_FILE_ERROR a regex --- zebra-state/Cargo.toml | 1 + zebra-state/src/constants.rs | 9 +++++++-- zebrad/src/application.rs | 5 +---- zebrad/tests/acceptance.rs | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index 40571b5a486..9d9259f4e20 100644 --- a/zebra-state/Cargo.toml +++ b/zebra-state/Cargo.toml @@ -13,6 +13,7 @@ zebra-chain = { path = "../zebra-chain" } dirs = "3.0.1" hex = "0.4.2" lazy_static = "1.4.0" +regex = "1" serde = { version = "1", features = ["serde_derive"] } futures = "0.3.12" diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index 402c4d41668..f10b8b970ed 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -16,5 +16,10 @@ pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1; /// The database format version, incremented each time the database format changes. pub const DATABASE_FORMAT_VERSION: u32 = 4; -/// Portion of OS error when the RocksDB lock file is already open. -pub const LOCK_FILE_ERROR: &str = "lock file"; +use lazy_static::lazy_static; +use regex::Regex; + +lazy_static! { + /// Regex that matches the RocksDB error when its lock file is already open. + pub static ref LOCK_FILE_ERROR: Regex = Regex::new("(lock file).*(temporarily unavailable)|(in use)").expect("regex is valid"); +} diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index dd03759ed4e..d32d0af9f3d 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -184,10 +184,7 @@ impl Application for ZebradApp { return false; } // RocksDB lock file conflicts - if error_str.contains(LOCK_FILE_ERROR) - && (error_str.contains("temporarily unavailable") - || error_str.contains("in use")) - { + if LOCK_FILE_ERROR.is_match(error_str) { return false; } true diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 461c035509e..adfb50db29b 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1155,7 +1155,7 @@ fn zcash_state_conflict() -> Result<()> { dir_conflict.path(), "Opened Zebra state cache", dir_conflict.path(), - LOCK_FILE_ERROR, + LOCK_FILE_ERROR.as_str(), )?; Ok(()) From 6b9f2f6408b587664bdcf63f5a92fbfddbee214f Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 22:13:25 -0300 Subject: [PATCH 44/61] match path in state cache --- zebra-state/src/service/finalized_state.rs | 2 +- zebrad/tests/acceptance.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 94a83dcf0cb..55f09cc911b 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -45,7 +45,7 @@ impl FinalizedState { let db = match db_result { Ok(d) => { - tracing::info!("Opened Zebra state cache at: {:?}", path); + tracing::info!("Opened Zebra state cache at {:?}", path); d } // TODO: provide a different hint if the disk is full, see #1623 diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index adfb50db29b..e7ab7cf3a0f 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1153,7 +1153,11 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), - "Opened Zebra state cache", + format!( + "Opened Zebra state cache at \"{}/state/state/v4/mainnet\"", + dir_conflict.path().to_str().unwrap() + ) + .as_str(), dir_conflict.path(), LOCK_FILE_ERROR.as_str(), )?; From 14595037448aa44ccf310c69d797d653a4c913bd Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 22:17:17 -0300 Subject: [PATCH 45/61] changes to PORT_IN_USE_ERROR --- zebra-network/src/constants.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index c2fb959e96d..84f3c04750c 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -95,13 +95,8 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); -/// Unix portion of OS error when the port attempting to be opened is already in use. -#[cfg(unix)] -pub const PORT_IN_USE_ERROR: &str = "already in use"; - -/// Windows portion of OS error when the port attempting to be opened is already in use. -#[cfg(not(unix))] -pub const PORT_IN_USE_ERROR: &str = "one usage"; +/// OS-specific error when the port attempting to be opened is already in use. +pub const PORT_IN_USE_ERROR: &str = if cfg!(unix) { "already in use" } else { "one usage" }; /// Magic numbers used to identify different Zcash networks. pub mod magics { From a0dbca9b1347364d5d5b96ff02054ef9461ab493 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 22:19:19 -0300 Subject: [PATCH 46/61] rustfmt --- zebra-network/src/constants.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 84f3c04750c..6b5678096db 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -96,7 +96,11 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); /// OS-specific error when the port attempting to be opened is already in use. -pub const PORT_IN_USE_ERROR: &str = if cfg!(unix) { "already in use" } else { "one usage" }; +pub const PORT_IN_USE_ERROR: &str = if cfg!(unix) { + "already in use" +} else { + "one usage" +}; /// Magic numbers used to identify different Zcash networks. pub mod magics { From 2b32a2b551c0629d313f0e3b3f4dc160d3e6fd31 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 23:01:50 -0300 Subject: [PATCH 47/61] try a pathbuf --- zebrad/tests/acceptance.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index e7ab7cf3a0f..cc43bf6f129 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1151,11 +1151,18 @@ fn zcash_state_conflict() -> Result<()> { let mut config = persistent_test_config()?; let dir_conflict = TempDir::new("zebrad_tests")?.with_config(&mut config)?; + let mut dir_conflict_full = PathBuf::new(); + dir_conflict_full.push(dir_conflict.path()); + dir_conflict_full.push("state"); + dir_conflict_full.push("state"); + dir_conflict_full.push("v4"); + dir_conflict_full.push("mainnet"); + check_config_conflict( dir_conflict.path(), format!( - "Opened Zebra state cache at \"{}/state/state/v4/mainnet\"", - dir_conflict.path().to_str().unwrap() + "Opened Zebra state cache at \"{}\"", + dir_conflict_full.to_str().unwrap() ) .as_str(), dir_conflict.path(), From 11d3a61ed80ad1e108d3b39025344d65bab0fabc Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 27 Jan 2021 23:59:58 -0300 Subject: [PATCH 48/61] try escape backslash for windows --- zebrad/tests/acceptance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index cc43bf6f129..6afac21a050 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1162,7 +1162,7 @@ fn zcash_state_conflict() -> Result<()> { dir_conflict.path(), format!( "Opened Zebra state cache at \"{}\"", - dir_conflict_full.to_str().unwrap() + dir_conflict_full.to_str().unwrap().replace("\\", "\\\\") ) .as_str(), dir_conflict.path(), From ca20f22f80a1cd422416ad1babb5f147f77715e7 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 00:40:44 -0300 Subject: [PATCH 49/61] try string literal --- zebrad/tests/acceptance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 6afac21a050..68af6050638 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1161,8 +1161,8 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), format!( - "Opened Zebra state cache at \"{}\"", - dir_conflict_full.to_str().unwrap().replace("\\", "\\\\") + r#"Opened Zebra state cache at "{}""#, + dir_conflict_full.to_str().unwrap() ) .as_str(), dir_conflict.path(), From 1d0741c5aba4da780d607f7f8e6b491af67d8d15 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 10:32:25 -0300 Subject: [PATCH 50/61] another windows match try --- zebrad/tests/acceptance.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 68af6050638..9a1f9cc7a72 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1160,11 +1160,7 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), - format!( - r#"Opened Zebra state cache at "{}""#, - dir_conflict_full.to_str().unwrap() - ) - .as_str(), + format!(r"Opened Zebra state cache at {:?}", dir_conflict_full).as_str(), dir_conflict.path(), LOCK_FILE_ERROR.as_str(), )?; From d599ba8a824e88c7d66c1c3d413e152a83651c76 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 11:06:59 -0300 Subject: [PATCH 51/61] remove the string literal --- zebrad/tests/acceptance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 9a1f9cc7a72..8414d6e3cd3 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1160,7 +1160,7 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), - format!(r"Opened Zebra state cache at {:?}", dir_conflict_full).as_str(), + format!("Opened Zebra state cache at {:?}", dir_conflict_full).as_str(), dir_conflict.path(), LOCK_FILE_ERROR.as_str(), )?; From 6c875253d6fca95194a4ab021d3c8e6cd36ea832 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 13:13:18 -0300 Subject: [PATCH 52/61] change regex --- zebrad/tests/acceptance.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 8414d6e3cd3..6f75467b068 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1160,7 +1160,11 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), - format!("Opened Zebra state cache at {:?}", dir_conflict_full).as_str(), + format!( + "(Opened Zebra state cache at).*({})", + dir_conflict_full.to_str().unwrap() + ) + .as_str(), dir_conflict.path(), LOCK_FILE_ERROR.as_str(), )?; From cbdcddfdadeccc6a45320647ae0e199d0cda45a5 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 13:43:58 -0300 Subject: [PATCH 53/61] escape --- zebrad/Cargo.toml | 1 + zebrad/tests/acceptance.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/zebrad/Cargo.toml b/zebrad/Cargo.toml index e69e650f86c..1165f6db4ad 100644 --- a/zebrad/Cargo.toml +++ b/zebrad/Cargo.toml @@ -21,6 +21,7 @@ serde = { version = "1", features = ["serde_derive"] } toml = "0.5" chrono = "0.4" rand = "0.7" +regex = "1" hyper = { version = "0.14.0-dev", features = ["full"] } futures = "0.3" diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 6f75467b068..4b58e137ffd 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1161,8 +1161,8 @@ fn zcash_state_conflict() -> Result<()> { check_config_conflict( dir_conflict.path(), format!( - "(Opened Zebra state cache at).*({})", - dir_conflict_full.to_str().unwrap() + r"(Opened Zebra state cache at ).*({})", + regex::escape(dir_conflict_full.to_str().unwrap()) ) .as_str(), dir_conflict.path(), From 8f3c0f4663c74f2e8b562db1f438bf05a85c20a6 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 28 Jan 2021 21:06:53 -0300 Subject: [PATCH 54/61] dont match windows cache path --- zebrad/Cargo.toml | 1 - zebrad/tests/acceptance.rs | 31 ++++++++++++++++++++----------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/zebrad/Cargo.toml b/zebrad/Cargo.toml index 1165f6db4ad..e69e650f86c 100644 --- a/zebrad/Cargo.toml +++ b/zebrad/Cargo.toml @@ -21,7 +21,6 @@ serde = { version = "1", features = ["serde_derive"] } toml = "0.5" chrono = "0.4" rand = "0.7" -regex = "1" hyper = { version = "0.14.0-dev", features = ["full"] } futures = "0.3" diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 4b58e137ffd..47df5fadbf0 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1151,20 +1151,29 @@ fn zcash_state_conflict() -> Result<()> { let mut config = persistent_test_config()?; let dir_conflict = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - let mut dir_conflict_full = PathBuf::new(); - dir_conflict_full.push(dir_conflict.path()); - dir_conflict_full.push("state"); - dir_conflict_full.push("state"); - dir_conflict_full.push("v4"); - dir_conflict_full.push("mainnet"); + // Windows problems with this match will be worked on at #1654 + // We are matching the whole opened path only for unix by now. + let regex = if cfg!(unix) { + let mut dir_conflict_full = PathBuf::new(); + dir_conflict_full.push(dir_conflict.path()); + dir_conflict_full.push("state"); + dir_conflict_full.push("state"); + dir_conflict_full.push(format!( + "v{}", + zebra_state::constants::DATABASE_FORMAT_VERSION + )); + dir_conflict_full.push(config.network.network.to_string().to_lowercase()); + format!( + "Opened Zebra state cache at {:?}", + dir_conflict_full.to_str().unwrap() + ) + } else { + String::from("Opened Zebra state cache at ") + }; check_config_conflict( dir_conflict.path(), - format!( - r"(Opened Zebra state cache at ).*({})", - regex::escape(dir_conflict_full.to_str().unwrap()) - ) - .as_str(), + regex.as_str(), dir_conflict.path(), LOCK_FILE_ERROR.as_str(), )?; From a69d0618524213e21cfba900378a8cb799a606fc Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 10:55:42 +1000 Subject: [PATCH 55/61] Use Display for state path logs Avoids weird escaping on Windows when using Debug --- zebra-state/src/service/finalized_state.rs | 2 +- zebrad/tests/acceptance.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 55f09cc911b..b9932174be7 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -45,7 +45,7 @@ impl FinalizedState { let db = match db_result { Ok(d) => { - tracing::info!("Opened Zebra state cache at {:?}", path); + tracing::info!("Opened Zebra state cache at {}", path.display()); d } // TODO: provide a different hint if the disk is full, see #1623 diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 47df5fadbf0..b5f1d4efd26 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1164,8 +1164,8 @@ fn zcash_state_conflict() -> Result<()> { )); dir_conflict_full.push(config.network.network.to_string().to_lowercase()); format!( - "Opened Zebra state cache at {:?}", - dir_conflict_full.to_str().unwrap() + "Opened Zebra state cache at {}", + dir_conflict_full.display() ) } else { String::from("Opened Zebra state cache at ") From b792c7d436d9cf0f5a74b63c05641dd4cb3fc987 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 10:56:28 +1000 Subject: [PATCH 56/61] Add Windows conflict error messages --- zebra-network/src/constants.rs | 2 +- zebra-state/src/constants.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 6b5678096db..b6ef24b4470 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -99,7 +99,7 @@ pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); pub const PORT_IN_USE_ERROR: &str = if cfg!(unix) { "already in use" } else { - "one usage" + "access a socket in a way forbidden by its access permissions" }; /// Magic numbers used to identify different Zcash networks. diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index f10b8b970ed..e51ac559a0d 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -21,5 +21,5 @@ use regex::Regex; lazy_static! { /// Regex that matches the RocksDB error when its lock file is already open. - pub static ref LOCK_FILE_ERROR: Regex = Regex::new("(lock file).*(temporarily unavailable)|(in use)").expect("regex is valid"); + pub static ref LOCK_FILE_ERROR: Regex = Regex::new("(lock file).*(temporarily unavailable)|(in use)|(being used by another process)").expect("regex is valid"); } From 5d0c8da101e7fc8979191ae1bf28beed261b6986 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 13:28:44 +1000 Subject: [PATCH 57/61] Turn PORT_IN_USE_ERROR into a regex And add another Windows-specific port error alternative --- zebra-network/src/constants.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index b6ef24b4470..135d253351c 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -2,6 +2,9 @@ use std::time::Duration; +use lazy_static::lazy_static; +use regex::Regex; + // XXX should these constants be split into protocol also? use crate::protocol::external::types::*; @@ -95,11 +98,14 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); -/// OS-specific error when the port attempting to be opened is already in use. -pub const PORT_IN_USE_ERROR: &str = if cfg!(unix) { - "already in use" -} else { - "access a socket in a way forbidden by its access permissions" + +lazy_static! { + /// OS-specific error when the port attempting to be opened is already in use. + pub static ref PORT_IN_USE_ERROR: Regex = if cfg!(unix) { + Regex::new("already in use") + } else { + Regex::new("(access a socket in a way forbidden by its access permissions)|(Only one usage of each socket address)" + }.expect("regex is valid"); }; /// Magic numbers used to identify different Zcash networks. From 0e9b38ad0cebaca2aa0995c8c55ab1b29ce8e467 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 13:30:48 +1000 Subject: [PATCH 58/61] Use the new PORT_IN_USE_ERROR regex --- zebrad/src/application.rs | 2 +- zebrad/tests/acceptance.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index d32d0af9f3d..349ecdf7fc3 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -180,7 +180,7 @@ impl Application for ZebradApp { None => return true, }; // listener port conflicts - if error_str.contains(PORT_IN_USE_ERROR) { + if if PORT_IN_USE_ERROR.is_match(error_str) { return false; } // RocksDB lock file conflicts diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index b5f1d4efd26..30afd04f9d1 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1078,7 +1078,7 @@ fn zcash_listener_conflict() -> Result<()> { // (But since the config is ephemeral, they will have different state paths.) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR.as_str())?; Ok(()) } @@ -1106,7 +1106,7 @@ fn zcash_metrics_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR.as_str())?; Ok(()) } @@ -1134,7 +1134,7 @@ fn zcash_tracing_conflict() -> Result<()> { // But they will have different Zcash listeners (auto port) and states (ephemeral) let dir2 = TempDir::new("zebrad_tests")?.with_config(&mut config)?; - check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR)?; + check_config_conflict(dir1, regex1.as_str(), dir2, PORT_IN_USE_ERROR.as_str())?; Ok(()) } From 5274bad9d84f781cd65e99d9ae9e778f81ca7fea Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 13:33:33 +1000 Subject: [PATCH 59/61] Syntax fixes --- zebra-network/src/constants.rs | 2 +- zebrad/src/application.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 135d253351c..7ecf44c70f4 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -104,7 +104,7 @@ lazy_static! { pub static ref PORT_IN_USE_ERROR: Regex = if cfg!(unix) { Regex::new("already in use") } else { - Regex::new("(access a socket in a way forbidden by its access permissions)|(Only one usage of each socket address)" + Regex::new("(access a socket in a way forbidden by its access permissions)|(Only one usage of each socket address)") }.expect("regex is valid"); }; diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index 349ecdf7fc3..a6213df019a 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -180,7 +180,7 @@ impl Application for ZebradApp { None => return true, }; // listener port conflicts - if if PORT_IN_USE_ERROR.is_match(error_str) { + if PORT_IN_USE_ERROR.is_match(error_str) { return false; } // RocksDB lock file conflicts From 4c2522575e5961c74e9387fb6338468cfcdbb053 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 13:45:17 +1000 Subject: [PATCH 60/61] More syntax --- zebra-network/src/constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index 7ecf44c70f4..d418d430c19 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -106,7 +106,7 @@ lazy_static! { } else { Regex::new("(access a socket in a way forbidden by its access permissions)|(Only one usage of each socket address)") }.expect("regex is valid"); -}; +} /// Magic numbers used to identify different Zcash networks. pub mod magics { From 825db472a0f252652aa3dc19ec561289fc04f776 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Jan 2021 13:48:30 +1000 Subject: [PATCH 61/61] Add regex and lazy_static to zebra_network And silence a clippy warning --- Cargo.lock | 3 +++ zebra-network/Cargo.toml | 2 ++ zebra-network/src/constants.rs | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 67a39732198..36c6339a5a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3941,11 +3941,13 @@ dependencies = [ "futures", "hex", "indexmap", + "lazy_static", "metrics", "pin-project 0.4.27", "proptest", "proptest-derive", "rand 0.7.3", + "regex", "serde", "thiserror", "tokio 0.3.6", @@ -3991,6 +3993,7 @@ dependencies = [ "primitive-types", "proptest", "proptest-derive", + "regex", "rlimit", "rocksdb", "serde", diff --git a/zebra-network/Cargo.toml b/zebra-network/Cargo.toml index 05ca3134c17..6117372bd7a 100644 --- a/zebra-network/Cargo.toml +++ b/zebra-network/Cargo.toml @@ -16,8 +16,10 @@ hex = "0.4" # indexmap has rayon support for parallel iteration, # which we don't use, so disable it to drop the dependencies. indexmap = { version = "1.6", default-features = false } +lazy_static = "1.4.0" pin-project = "0.4" rand = "0.7" +regex = "1" serde = { version = "1", features = ["serde_derive"] } thiserror = "1" diff --git a/zebra-network/src/constants.rs b/zebra-network/src/constants.rs index d418d430c19..192a95cf22d 100644 --- a/zebra-network/src/constants.rs +++ b/zebra-network/src/constants.rs @@ -98,10 +98,10 @@ pub const EWMA_DEFAULT_RTT: Duration = Duration::from_secs(20 + 1); /// better peers when we restart the sync. pub const EWMA_DECAY_TIME: Duration = Duration::from_secs(200); - lazy_static! { /// OS-specific error when the port attempting to be opened is already in use. pub static ref PORT_IN_USE_ERROR: Regex = if cfg!(unix) { + #[allow(clippy::trivial_regex)] Regex::new("already in use") } else { Regex::new("(access a socket in a way forbidden by its access permissions)|(Only one usage of each socket address)")