From a15d8f78fefc711e5ddc0536b95ef71a6cca5139 Mon Sep 17 00:00:00 2001 From: Omer Yacine Date: Fri, 28 Jun 2024 22:35:04 +0200 Subject: [PATCH 1/4] hide the bitcoind stopper inside the carrier this avoids having exiplict handling of the bitcoind stopper and it being all over the place when the carrier (by the end of the test) the stopper will get dropped as well which will cause the mock server to shutdown --- teos/src/api/http.rs | 69 ++++++++++++++++---------------- teos/src/api/internal.rs | 64 +++++++++++++++--------------- teos/src/carrier.rs | 47 ++++++++++++++++------ teos/src/responder.rs | 49 +++++++++++------------ teos/src/test_utils.rs | 86 ++++++++++++++++++---------------------- teos/src/watcher.rs | 60 +++++++++++----------------- 6 files changed, 185 insertions(+), 190 deletions(-) diff --git a/teos/src/api/http.rs b/teos/src/api/http.rs index a041ee34..6cc613eb 100644 --- a/teos/src/api/http.rs +++ b/teos/src/api/http.rs @@ -335,7 +335,7 @@ mod test_helpers { use crate::api::internal::InternalAPI; use crate::protos::public_tower_services_server::PublicTowerServicesServer; - use crate::test_utils::{create_api_with_config, ApiConfig, BitcoindStopper}; + use crate::test_utils::{create_api_with_config, ApiConfig}; pub(crate) enum RequestBody<'a> { Jsonify(&'a str), @@ -346,8 +346,8 @@ mod test_helpers { pub(crate) async fn run_tower_in_background_with_config( api_config: ApiConfig, - ) -> (SocketAddr, Arc, BitcoindStopper) { - let (internal_rpc_api, bitcoind_stopper) = create_api_with_config(api_config).await; + ) -> (SocketAddr, Arc) { + let internal_rpc_api = create_api_with_config(api_config).await; let cloned = internal_rpc_api.clone(); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -361,14 +361,13 @@ mod test_helpers { .unwrap(); }); - (addr, cloned, bitcoind_stopper) + (addr, cloned) } - pub(crate) async fn run_tower_in_background() -> (SocketAddr, BitcoindStopper) { - let (sock_addr, _, bitcoind_stopper) = - run_tower_in_background_with_config(ApiConfig::default()).await; + pub(crate) async fn run_tower_in_background() -> SocketAddr { + let (sock_addr, _) = run_tower_in_background_with_config(ApiConfig::default()).await; - (sock_addr, bitcoind_stopper) + sock_addr } pub(crate) async fn check_api_error( @@ -447,7 +446,7 @@ mod tests_failures { #[tokio::test] async fn test_no_json_request_body() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error(Endpoint::Register, RequestBody::Body(""), server_addr).await; assert!(api_error.error.contains("EOF while parsing")); @@ -457,7 +456,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_json_request_body() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::DoNotJsonify(""), @@ -471,7 +470,7 @@ mod tests_failures { #[tokio::test] async fn test_empty_json_request_body() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::Jsonify(r#"{}"#), @@ -485,7 +484,7 @@ mod tests_failures { #[tokio::test] async fn test_empty_field() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::Jsonify(r#"{"user_id": ""}"#), @@ -499,7 +498,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_field_hex_encoding_odd() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::Jsonify(r#"{"user_id": "a"}"#), @@ -513,7 +512,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_hex_encoding_character() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error(Endpoint::Register, RequestBody::Jsonify(r#"{"user_id": "022fa2900ed7fc07b4e8ca3ea081e846245b0497944644aa78ea0b994ac22074dZ"}"#), @@ -527,7 +526,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_field_size() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::Jsonify(r#"{"user_id": "aa"}"#), @@ -542,7 +541,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_field_type() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::Register, RequestBody::DoNotJsonify(r#"{"user_id": 1}"#), @@ -557,7 +556,7 @@ mod tests_failures { #[tokio::test] async fn test_request_missing_field() { // We'll use a different endpoint here since we need a json object with more than one field - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (api_error, status) = check_api_error( Endpoint::AddAppointment, RequestBody::Jsonify(r#"{"signature": "aa"}"#), @@ -573,7 +572,7 @@ mod tests_failures { #[tokio::test] async fn test_empty_request_body() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let grpc_conn = PublicTowerServicesClient::connect(format!( "http://{}:{}", server_addr.ip(), @@ -593,7 +592,7 @@ mod tests_failures { #[tokio::test] async fn test_payload_too_large() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let grpc_conn = PublicTowerServicesClient::connect(format!( "http://{}:{}", server_addr.ip(), @@ -614,7 +613,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_endpoint() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let grpc_conn = PublicTowerServicesClient::connect(format!( "http://{}:{}", server_addr.ip(), @@ -634,7 +633,7 @@ mod tests_failures { #[tokio::test] async fn test_wrong_method() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let grpc_conn = PublicTowerServicesClient::connect(format!( "http://{}:{}", server_addr.ip(), @@ -671,7 +670,7 @@ mod tests_methods { #[tokio::test] async fn test_register() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let response = request_to_api::( Endpoint::Register, @@ -686,7 +685,7 @@ mod tests_methods { #[tokio::test] async fn test_register_max_slots() { - let (server_addr, _, _s) = + let (server_addr, _) = run_tower_in_background_with_config(ApiConfig::new(u32::MAX, DURATION)).await; let user_id = get_random_user_id(); @@ -723,7 +722,7 @@ mod tests_methods { #[tokio::test] async fn test_register_service_unavailable() { - let (server_addr, _, _s) = run_tower_in_background_with_config( + let (server_addr, _) = run_tower_in_background_with_config( ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable(), ) .await; @@ -751,7 +750,7 @@ mod tests_methods { #[tokio::test] async fn test_add_appointment() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // Register first let (user_sk, user_pk) = cryptography::get_random_keypair(); @@ -790,7 +789,7 @@ mod tests_methods { #[tokio::test] async fn test_add_appointment_non_registered() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; let (user_sk, _s) = cryptography::get_random_keypair(); let appointment = generate_dummy_appointment(None).inner; let signature = cryptography::sign(&appointment.to_vec(), &user_sk).unwrap(); @@ -818,7 +817,7 @@ mod tests_methods { #[tokio::test] async fn test_add_appointment_already_triggered() { // Get the InternalAPI so we can mess with the inner state - let (server_addr, internal_api, _s) = + let (server_addr, internal_api) = run_tower_in_background_with_config(ApiConfig::new(u32::MAX, DURATION)).await; // Register @@ -869,7 +868,7 @@ mod tests_methods { #[tokio::test] async fn test_add_appointment_service_unavailable() { - let (server_addr, _, _s) = run_tower_in_background_with_config( + let (server_addr, _) = run_tower_in_background_with_config( ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable(), ) .await; @@ -899,7 +898,7 @@ mod tests_methods { #[tokio::test] async fn test_get_appointment() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // Register first let (user_sk, user_pk) = cryptography::get_random_keypair(); @@ -954,7 +953,7 @@ mod tests_methods { #[tokio::test] async fn test_get_appointment_non_registered() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // User is not registered let (user_sk, _) = cryptography::get_random_keypair(); @@ -987,7 +986,7 @@ mod tests_methods { #[tokio::test] async fn test_get_appointment_not_found() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // Register first let (user_sk, user_pk) = cryptography::get_random_keypair(); @@ -1030,7 +1029,7 @@ mod tests_methods { #[tokio::test] async fn test_get_appointment_service_unavailable() { - let (server_addr, _, _s) = run_tower_in_background_with_config( + let (server_addr, _) = run_tower_in_background_with_config( ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable(), ) .await; @@ -1065,7 +1064,7 @@ mod tests_methods { #[tokio::test] async fn test_get_subscription_info() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // Register first let (user_sk, user_pk) = cryptography::get_random_keypair(); @@ -1101,7 +1100,7 @@ mod tests_methods { #[tokio::test] async fn test_get_subscription_info_non_registered() { - let (server_addr, _s) = run_tower_in_background().await; + let server_addr = run_tower_in_background().await; // User is not registered let (user_sk, _) = cryptography::get_random_keypair(); @@ -1129,7 +1128,7 @@ mod tests_methods { #[tokio::test] async fn test_get_subscription_info_service_unavailable() { let (user_sk, _) = cryptography::get_random_keypair(); - let (server_addr, _, _s) = run_tower_in_background_with_config( + let (server_addr, _) = run_tower_in_background_with_config( ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable(), ) .await; diff --git a/teos/src/api/internal.rs b/teos/src/api/internal.rs index fc2085d8..6dccbc20 100644 --- a/teos/src/api/internal.rs +++ b/teos/src/api/internal.rs @@ -446,7 +446,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_all_appointments() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let response = internal_api .get_all_appointments(Request::new(())) @@ -459,7 +459,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_all_appointments_watcher() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Add data to the Watcher so we can retrieve it later on let (user_sk, user_pk) = get_random_keypair(); @@ -487,7 +487,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_all_appointments_responder() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Add data to the Responser so we can retrieve it later on internal_api.watcher.add_random_tracker_to_responder(); @@ -507,7 +507,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_appointments() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let locator = Locator::new(get_random_tx().txid()).to_vec(); let response = internal_api @@ -521,7 +521,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_appointments_watcher() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; for i in 0..3 { // Create a dispute tx to be used for creating different dummy appointments with the same locator. @@ -571,7 +571,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_appointments_responder() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; for i in 0..3 { // Create a dispute tx to be used for creating different trackers. @@ -622,7 +622,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_tower_info_empty() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let response = internal_api .get_tower_info(Request::new(())) @@ -638,7 +638,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_tower_info() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Register a user let (user_sk, user_pk) = get_random_keypair(); @@ -675,7 +675,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_users() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let mut users = HashSet::new(); // Add a couple of users @@ -697,7 +697,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_users_empty() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let response = internal_api .get_users(Request::new(())) @@ -710,7 +710,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_user() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Register a user and get it back let (user_sk, user_pk) = get_random_keypair(); @@ -752,7 +752,7 @@ mod tests_private_api { #[tokio::test] async fn test_get_user_not_found() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Non-registered user let (_, user_pk) = get_random_keypair(); @@ -773,7 +773,7 @@ mod tests_private_api { #[tokio::test] async fn test_stop() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; assert!(!internal_api.shutdown_trigger.is_triggered()); internal_api.stop(Request::new(())).await.unwrap(); @@ -795,7 +795,7 @@ mod tests_public_api { #[tokio::test] async fn test_register() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let (_, user_pk) = get_random_keypair(); @@ -815,7 +815,7 @@ mod tests_public_api { #[tokio::test] async fn test_register_wrong_user_id() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let mut user_ids = Vec::new(); @@ -846,7 +846,7 @@ mod tests_public_api { #[tokio::test] async fn test_register_max_slots() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(u32::MAX, DURATION)).await; + let internal_api = create_api_with_config(ApiConfig::new(u32::MAX, DURATION)).await; let (_, user_pk) = get_random_keypair(); let user_id = UserId(user_pk).to_vec(); @@ -874,7 +874,7 @@ mod tests_public_api { #[tokio::test] async fn test_register_service_unavailable() { - let (internal_api, _s) = + let internal_api = create_api_with_config(ApiConfig::new(u32::MAX, DURATION).bitcoind_unreachable()).await; let (_, user_pk) = get_random_keypair(); @@ -894,7 +894,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // User must be registered let (user_sk, user_pk) = get_random_keypair(); @@ -920,7 +920,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment_non_registered() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // User is not registered this time let (user_sk, _) = get_random_keypair(); @@ -948,7 +948,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment_not_enough_slots() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(0, DURATION)).await; + let internal_api = create_api_with_config(ApiConfig::new(0, DURATION)).await; // User is registered but has no slots let (user_sk, user_pk) = get_random_keypair(); @@ -977,7 +977,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment_subscription_expired() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; // User is registered but subscription is expired let (user_sk, user_pk) = get_random_keypair(); @@ -1003,7 +1003,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment_already_triggered() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; let (user_sk, user_pk) = get_random_keypair(); let user_id = UserId(user_pk); @@ -1042,7 +1042,7 @@ mod tests_public_api { #[tokio::test] async fn test_add_appointment_service_unavailable() { - let (internal_api, _s) = + let internal_api = create_api_with_config(ApiConfig::new(u32::MAX, DURATION).bitcoind_unreachable()).await; let (user_sk, _) = get_random_keypair(); @@ -1066,7 +1066,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_appointment() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // The user must be registered let (user_sk, user_pk) = get_random_keypair(); @@ -1099,7 +1099,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_appointment_non_registered() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // Add a first user to link the appointment to him let (user_sk, user_pk) = get_random_keypair(); @@ -1127,7 +1127,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_appointment_non_existent() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // The user is registered but the appointment does not exist let (user_sk, user_pk) = get_random_keypair(); @@ -1154,7 +1154,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_appointment_subscription_expired() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; // Register the user let (user_sk, user_pk) = get_random_keypair(); @@ -1182,7 +1182,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_appointment_service_unavailable() { - let (internal_api, _s) = + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable()).await; let (user_sk, _) = get_random_keypair(); @@ -1205,7 +1205,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_subscription_info() { - let (internal_api, _s) = create_api().await; + let internal_api = create_api().await; // The user must be registered let (user_sk, user_pk) = get_random_keypair(); @@ -1229,7 +1229,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_subscription_info_non_registered() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; // The user is not registered let (user_sk, _) = get_random_keypair(); @@ -1252,7 +1252,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_subscription_info_expired() { - let (internal_api, _s) = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, 0)).await; // The user is registered but the subscription has expired let (user_sk, user_pk) = get_random_keypair(); @@ -1276,7 +1276,7 @@ mod tests_public_api { #[tokio::test] async fn test_get_subscription_info_service_unavailable() { - let (internal_api, _s) = + let internal_api = create_api_with_config(ApiConfig::new(SLOTS, DURATION).bitcoind_unreachable()).await; let (user_sk, _) = get_random_keypair(); diff --git a/teos/src/carrier.rs b/teos/src/carrier.rs index 15c9d835..f6c185e6 100644 --- a/teos/src/carrier.rs +++ b/teos/src/carrier.rs @@ -24,10 +24,14 @@ pub struct Carrier { issued_receipts: HashMap, /// The last known block height. block_height: u32, + #[cfg(test)] + /// A stopper that stops the mock bitcoind server in tests when the [`Carrier`] is dropped. + _stopper: Option, } impl Carrier { /// Creates a new [Carrier] instance. + #[cfg(not(test))] pub fn new( bitcoin_cli: Arc, bitcoind_reachable: Arc<(Mutex, Condvar)>, @@ -41,6 +45,23 @@ impl Carrier { } } + /// Creates a new [Carrier] instance. + #[cfg(test)] + pub fn new( + bitcoin_cli: Arc, + bitcoind_reachable: Arc<(Mutex, Condvar)>, + last_known_block_height: u32, + stopper: Option, + ) -> Self { + Carrier { + bitcoin_cli, + bitcoind_reachable, + issued_receipts: HashMap::new(), + block_height: last_known_block_height, + _stopper: stopper, + } + } + /// The last known block height. pub(crate) fn block_height(&self) -> u32 { self.block_height @@ -212,7 +233,7 @@ mod tests { let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); // Lets add some dummy data into the cache for i in 0..10 { @@ -236,7 +257,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -254,7 +275,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -274,7 +295,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -296,7 +317,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -319,7 +340,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -338,7 +359,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -358,7 +379,7 @@ mod tests { let bitcoind_reachable = Arc::new((Mutex::new(false), Condvar::new())); let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height); + let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height, None); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let delay = std::time::Duration::new(3, 0); @@ -388,7 +409,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(carrier.in_mempool(&txid)); } @@ -401,7 +422,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -416,7 +437,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -430,7 +451,7 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height); + let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -442,7 +463,7 @@ mod tests { let bitcoind_reachable = Arc::new((Mutex::new(false), Condvar::new())); let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height); + let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height, None); let txid = Txid::from_hex(TXID_HEX).unwrap(); let delay = std::time::Duration::new(3, 0); diff --git a/teos/src/responder.rs b/teos/src/responder.rs index b7412cca..025d31f7 100644 --- a/teos/src/responder.rs +++ b/teos/src/responder.rs @@ -481,8 +481,8 @@ mod tests { use crate::test_utils::{ create_carrier, generate_dummy_appointment, generate_dummy_appointment_with_user, generate_uuid, get_last_n_blocks, get_random_breach, get_random_tracker, get_random_tx, - store_appointment_and_its_user, BitcoindStopper, Blockchain, MockedServerQuery, DURATION, - EXPIRY_DELTA, SLOTS, START_HEIGHT, + store_appointment_and_its_user, Blockchain, MockedServerQuery, DURATION, EXPIRY_DELTA, + SLOTS, START_HEIGHT, }; use teos_common::constants::IRREVOCABLY_RESOLVED; @@ -553,27 +553,24 @@ mod tests { gatekeeper: Arc, dbm: Arc>, query: MockedServerQuery, - ) -> (Responder, BitcoindStopper) { + ) -> Responder { let height = if chain.tip().height < IRREVOCABLY_RESOLVED { chain.tip().height } else { IRREVOCABLY_RESOLVED }; + let carrier = create_carrier(query, chain.tip().height); let last_n_blocks = get_last_n_blocks(chain, height as usize).await; - let (carrier, bitcoind_stopper) = create_carrier(query, chain.tip().height); - ( - Responder::new(&last_n_blocks, chain.tip().height, carrier, gatekeeper, dbm), - bitcoind_stopper, - ) + Responder::new(&last_n_blocks, chain.tip().height, carrier, gatekeeper, dbm) } async fn init_responder_with_chain_and_dbm( mocked_query: MockedServerQuery, chain: &mut Blockchain, dbm: Arc>, - ) -> (Responder, BitcoindStopper) { + ) -> Responder { let gk = Gatekeeper::new( chain.get_block_count(), SLOTS, @@ -584,7 +581,7 @@ mod tests { create_responder(chain, Arc::new(gk), dbm, mocked_query).await } - async fn init_responder(mocked_query: MockedServerQuery) -> (Responder, BitcoindStopper) { + async fn init_responder(mocked_query: MockedServerQuery) -> Responder { let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); init_responder_with_chain_and_dbm(mocked_query, &mut chain, dbm).await @@ -629,7 +626,7 @@ mod tests { // A fresh responder has no associated data let mut chain = Blockchain::default().with_height(START_HEIGHT); let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); - let (responder, _s) = + let responder = init_responder_with_chain_and_dbm(MockedServerQuery::Regular, &mut chain, dbm.clone()) .await; assert!(responder.is_fresh()); @@ -648,7 +645,7 @@ mod tests { } // Create a new Responder reusing the same DB and check that the data is loaded - let (another_r, _) = + let another_r = init_responder_with_chain_and_dbm(MockedServerQuery::Regular, &mut chain, dbm).await; assert!(!responder.is_fresh()); assert_eq!(responder, another_r); @@ -657,7 +654,7 @@ mod tests { #[tokio::test] async fn test_handle_breach_accepted() { let start_height = START_HEIGHT as u32; - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; let (user_id, uuid) = responder.store_dummy_appointment_to_db(); let breach = get_random_breach(); @@ -689,7 +686,7 @@ mod tests { #[tokio::test] async fn test_handle_breach_accepted_in_mempool() { let start_height = START_HEIGHT as u32; - let (responder, _s) = init_responder(MockedServerQuery::InMempoool).await; + let responder = init_responder(MockedServerQuery::InMempoool).await; let (user_id, uuid) = responder.store_dummy_appointment_to_db(); let breach = get_random_breach(); @@ -707,7 +704,7 @@ mod tests { #[tokio::test] async fn test_handle_breach_accepted_in_txindex() { - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; let (user_id, uuid) = responder.store_dummy_appointment_to_db(); @@ -742,7 +739,7 @@ mod tests { #[tokio::test] async fn test_handle_breach_rejected() { - let (responder, _s) = init_responder(MockedServerQuery::Error( + let responder = init_responder(MockedServerQuery::Error( rpc_errors::RPC_VERIFY_ERROR as i64, )) .await; @@ -760,7 +757,7 @@ mod tests { #[tokio::test] async fn test_add_tracker() { - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; let start_height = START_HEIGHT as u32; let (user_id, uuid) = responder.store_dummy_appointment_to_db(); @@ -826,7 +823,7 @@ mod tests { // Has tracker should return true as long as the given tracker is held by the Responder. // As long as the tracker is in Responder.trackers and Responder.tx_tracker_map, the return // must be true. - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; // Add a new tracker let (user_id, uuid) = responder.store_dummy_appointment_to_db(); @@ -849,7 +846,7 @@ mod tests { async fn test_get_tracker() { // Should return a tracker as long as it exists let start_height = START_HEIGHT as u32; - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; // Store the user and the appointment in the database so we can add the tracker later on (due to FK restrictions) let (user_id, uuid) = responder.store_dummy_appointment_to_db(); @@ -881,7 +878,7 @@ mod tests { #[tokio::test] async fn test_check_confirmations() { - let (responder, _s) = init_responder(MockedServerQuery::Regular).await; + let responder = init_responder(MockedServerQuery::Regular).await; let target_height = (START_HEIGHT * 2) as u32; // Unconfirmed transactions that miss a confirmation will be added to missed_confirmations (if not there) or their missed confirmation count till be increased @@ -989,7 +986,7 @@ mod tests { #[tokio::test] async fn test_handle_reorged_txs() { - let (responder, _s) = init_responder(MockedServerQuery::InMempoool).await; + let responder = init_responder(MockedServerQuery::InMempoool).await; let mut trackers = Vec::new(); for _ in 0..10 { @@ -1022,7 +1019,7 @@ mod tests { #[tokio::test] async fn test_handle_reorged_txs_rejected() { - let (responder, _s) = init_responder(MockedServerQuery::Error( + let responder = init_responder(MockedServerQuery::Error( rpc_errors::RPC_VERIFY_REJECTED as i64, )) .await; @@ -1061,7 +1058,7 @@ mod tests { #[tokio::test] async fn test_rebroadcast_stale_txs_accepted() { - let (responder, _s) = init_responder(MockedServerQuery::InMempoool).await; + let responder = init_responder(MockedServerQuery::InMempoool).await; let mut statues = HashMap::new(); let height = 100; @@ -1104,7 +1101,7 @@ mod tests { #[tokio::test] async fn test_rebroadcast_stale_txs_rejected() { - let (responder, _s) = init_responder(MockedServerQuery::Error( + let responder = init_responder(MockedServerQuery::Error( rpc_errors::RPC_VERIFY_ERROR as i64, )) .await; @@ -1155,7 +1152,7 @@ mod tests { let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); let start_height = START_HEIGHT * 2; let mut chain = Blockchain::default().with_height(start_height); - let (responder, _s) = + let responder = init_responder_with_chain_and_dbm(MockedServerQuery::Regular, &mut chain, dbm).await; // filtered_block_connected is used to keep track of the confirmation received (or missed) by the trackers the Responder @@ -1419,7 +1416,7 @@ mod tests { async fn test_block_disconnected() { let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (responder, _s) = + let responder = init_responder_with_chain_and_dbm(MockedServerQuery::Regular, &mut chain, dbm).await; // Add user to the database diff --git a/teos/src/test_utils.rs b/teos/src/test_utils.rs index 4dba0a93..fc786cff 100644 --- a/teos/src/test_utils.rs +++ b/teos/src/test_utils.rs @@ -8,6 +8,7 @@ */ use rand::Rng; +use std::fmt::Debug; use std::sync::{Arc, Condvar, Mutex}; use std::thread; @@ -373,7 +374,7 @@ pub(crate) enum MockedServerQuery { Error(i64), } -pub(crate) fn create_carrier(query: MockedServerQuery, height: u32) -> (Carrier, BitcoindStopper) { +pub(crate) fn create_carrier(query: MockedServerQuery, height: u32) -> Carrier { let bitcoind_mock = match query { MockedServerQuery::Regular => BitcoindMock::new(MockOptions::default()), MockedServerQuery::InMempoool => BitcoindMock::new(MockOptions::in_mempool()), @@ -383,9 +384,11 @@ pub(crate) fn create_carrier(query: MockedServerQuery, height: u32) -> (Carrier, let bitcoind_reachable = Arc::new((Mutex::new(true), Condvar::new())); start_server(bitcoind_mock.server); - ( - Carrier::new(bitcoin_cli, bitcoind_reachable, height), - bitcoind_mock.stopper, + Carrier::new( + bitcoin_cli, + bitcoind_reachable, + height, + Some(bitcoind_mock.stopper), ) } @@ -393,18 +396,13 @@ pub(crate) async fn create_responder( chain: &mut Blockchain, gatekeeper: Arc, dbm: Arc>, - server_url: &str, ) -> Responder { let height = chain.tip().height; // For the local TxIndex logic to be sound, our index needs to have, at least, IRREVOCABLY_RESOLVED blocks debug_assert!(height >= IRREVOCABLY_RESOLVED); - + let carrier = create_carrier(MockedServerQuery::Regular, height); let last_n_blocks = get_last_n_blocks(chain, IRREVOCABLY_RESOLVED as usize).await; - let bitcoin_cli = Arc::new(BitcoindClient::new(server_url, Auth::None).unwrap()); - let bitcoind_reachable = Arc::new((Mutex::new(true), Condvar::new())); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, height); - Responder::new(&last_n_blocks, height, carrier, gatekeeper, dbm) } @@ -412,25 +410,21 @@ pub(crate) async fn create_watcher( chain: &mut Blockchain, responder: Arc, gatekeeper: Arc, - bitcoind_mock: BitcoindMock, dbm: Arc>, -) -> (Watcher, BitcoindStopper) { +) -> Watcher { let last_n_blocks = get_last_n_blocks(chain, 6).await; - start_server(bitcoind_mock.server); let (tower_sk, tower_pk) = get_random_keypair(); let tower_id = UserId(tower_pk); - ( - Watcher::new( - gatekeeper, - responder, - &last_n_blocks, - chain.get_block_count(), - tower_sk, - tower_id, - dbm, - ), - bitcoind_mock.stopper, + + Watcher::new( + gatekeeper, + responder, + &last_n_blocks, + chain.get_block_count(), + tower_sk, + tower_id, + dbm, ) } #[derive(Clone)] @@ -465,10 +459,7 @@ impl Default for ApiConfig { } } -pub(crate) async fn create_api_with_config( - api_config: ApiConfig, -) -> (Arc, BitcoindStopper) { - let bitcoind_mock = BitcoindMock::new(MockOptions::default()); +pub(crate) async fn create_api_with_config(api_config: ApiConfig) -> Arc { let mut chain = Blockchain::default().with_height(START_HEIGHT); let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); @@ -479,31 +470,21 @@ pub(crate) async fn create_api_with_config( EXPIRY_DELTA, dbm.clone(), )); - let responder = - create_responder(&mut chain, gk.clone(), dbm.clone(), bitcoind_mock.url()).await; - let (watcher, stopper) = create_watcher( - &mut chain, - Arc::new(responder), - gk.clone(), - bitcoind_mock, - dbm.clone(), - ) - .await; + let responder = create_responder(&mut chain, gk.clone(), dbm.clone()).await; + let watcher = create_watcher(&mut chain, Arc::new(responder), gk.clone(), dbm.clone()).await; let bitcoind_reachable = Arc::new((Mutex::new(api_config.bitcoind_reachable), Condvar::new())); let (shutdown_trigger, _) = triggered::trigger(); - ( - Arc::new(InternalAPI::new( - Arc::new(watcher), - vec![msgs::NetworkAddress::from_ipv4("address".to_string(), 21)], - bitcoind_reachable, - shutdown_trigger, - )), - stopper, - ) + + Arc::new(InternalAPI::new( + Arc::new(watcher), + vec![msgs::NetworkAddress::from_ipv4("address".to_string(), 21)], + bitcoind_reachable, + shutdown_trigger, + )) } -pub(crate) async fn create_api() -> (Arc, BitcoindStopper) { +pub(crate) async fn create_api() -> Arc { create_api_with_config(ApiConfig::default()).await } @@ -528,6 +509,15 @@ impl Drop for BitcoindStopper { } } +// Since [`CloseHandle`] doesn't implement debug, we can't derive it on [`BitcoindStopper`]. +// Implement a dummy one instead. This is just to be able to include [`BitcoindStopper`] +// as a field in debug deriving structs. +impl Debug for BitcoindStopper { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + Ok(()) + } +} + pub(crate) struct BitcoindMock { pub url: String, pub server: Server, diff --git a/teos/src/watcher.rs b/teos/src/watcher.rs index 037319d7..ef010bdc 100644 --- a/teos/src/watcher.rs +++ b/teos/src/watcher.rs @@ -546,8 +546,8 @@ mod tests { use crate::rpc_errors; use crate::test_utils::{ create_carrier, create_responder, create_watcher, generate_dummy_appointment, - generate_dummy_appointment_with_user, get_random_tx, BitcoindMock, BitcoindStopper, - Blockchain, MockOptions, MockedServerQuery, DURATION, EXPIRY_DELTA, SLOTS, START_HEIGHT, + generate_dummy_appointment_with_user, get_random_tx, Blockchain, MockedServerQuery, + DURATION, EXPIRY_DELTA, SLOTS, START_HEIGHT, }; use teos_common::cryptography::get_random_keypair; @@ -578,17 +578,12 @@ mod tests { } } - async fn init_watcher(chain: &mut Blockchain) -> (Watcher, BitcoindStopper) { + async fn init_watcher(chain: &mut Blockchain) -> Watcher { let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); init_watcher_with_db(chain, dbm).await } - async fn init_watcher_with_db( - chain: &mut Blockchain, - dbm: Arc>, - ) -> (Watcher, BitcoindStopper) { - let bitcoind_mock = BitcoindMock::new(MockOptions::default()); - + async fn init_watcher_with_db(chain: &mut Blockchain, dbm: Arc>) -> Watcher { let gk = Arc::new(Gatekeeper::new( chain.get_block_count(), SLOTS, @@ -596,15 +591,8 @@ mod tests { EXPIRY_DELTA, dbm.clone(), )); - let responder = create_responder(chain, gk.clone(), dbm.clone(), bitcoind_mock.url()).await; - create_watcher( - chain, - Arc::new(responder), - gk.clone(), - bitcoind_mock, - dbm.clone(), - ) - .await + let responder = create_responder(chain, gk.clone(), dbm.clone()).await; + create_watcher(chain, Arc::new(responder), gk.clone(), dbm.clone()).await } fn assert_appointment_added( @@ -629,7 +617,7 @@ mod tests { // A fresh watcher has no associated data let mut chain = Blockchain::default().with_height(START_HEIGHT); let dbm = Arc::new(Mutex::new(DBM::in_memory().unwrap())); - let (watcher, _s) = init_watcher_with_db(&mut chain, dbm.clone()).await; + let watcher = init_watcher_with_db(&mut chain, dbm.clone()).await; assert!(watcher.is_fresh()); let (user_sk, user_pk) = get_random_keypair(); @@ -647,7 +635,7 @@ mod tests { } // Create a new Responder reusing the same DB and check that the data is loaded - let (another_w, _as) = init_watcher_with_db(&mut chain, dbm).await; + let another_w = init_watcher_with_db(&mut chain, dbm).await; assert!(!another_w.is_fresh()); assert_eq!(watcher, another_w); } @@ -658,7 +646,7 @@ mod tests { // Not testing the update / rejection logic, since that's already covered in the Gatekeeper, just that the data makes // sense and the signature verifies. let mut chain = Blockchain::default().with_height(START_HEIGHT); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; let tower_pk = watcher.tower_id.0; let (_, user_pk) = get_random_keypair(); @@ -683,7 +671,7 @@ mod tests { async fn test_add_appointment() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); let tip_txs = chain.blocks.last().unwrap().txdata.clone(); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // add_appointment should add a given appointment to the Watcher given the following logic: // - if the appointment does not exist for a given user, add the appointment @@ -795,7 +783,7 @@ mod tests { // Transaction rejected // Update the Responder with a new Carrier - let (carrier, _as) = create_carrier( + let carrier = create_carrier( MockedServerQuery::Error(rpc_errors::RPC_VERIFY_ERROR as i64), chain.tip().deref().height, ); @@ -869,7 +857,7 @@ mod tests { #[tokio::test] async fn test_store_appointment() { let mut chain = Blockchain::default().with_height(START_HEIGHT); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Register the user let (_, user_pk) = get_random_keypair(); @@ -908,7 +896,7 @@ mod tests { #[tokio::test] async fn test_store_triggered_appointment() { let mut chain = Blockchain::default().with_height(START_HEIGHT); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Register the user let (_, user_pk) = get_random_keypair(); @@ -930,7 +918,7 @@ mod tests { // A properly formatted but invalid transaction should be rejected by the Responder // Update the Responder with a new Carrier that will reject the transaction - let (carrier, _as) = create_carrier( + let carrier = create_carrier( MockedServerQuery::Error(rpc_errors::RPC_VERIFY_ERROR as i64), chain.tip().deref().height, ); @@ -962,7 +950,7 @@ mod tests { #[tokio::test] async fn test_get_appointment() { let mut chain = Blockchain::default().with_height(START_HEIGHT); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; let dispute_tx = get_random_tx(); let appointment = generate_dummy_appointment(Some(&dispute_tx.txid())).inner; @@ -1050,7 +1038,7 @@ mod tests { #[tokio::test] async fn test_get_breaches() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Let's create some locators based on the transactions in the last block let locator_tx_map: HashMap<_, _> = (0..10) @@ -1081,7 +1069,7 @@ mod tests { #[tokio::test] async fn test_handle_breaches_accepted() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Let's create some locators based on the transactions in the last block let breaches: HashMap<_, _> = (0..10) @@ -1106,7 +1094,7 @@ mod tests { #[tokio::test] async fn test_handle_breaches_rejected_decryption() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Let's create some locators based on the transactions in the last block let breaches: HashMap<_, _> = (0..10) @@ -1142,10 +1130,10 @@ mod tests { #[tokio::test] async fn test_handle_breaches_rejected_by_responder_backend() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Replace the carrier with an erroneous one - let (carrier, _s) = create_carrier( + let carrier = create_carrier( MockedServerQuery::Error(rpc_errors::RPC_VERIFY_ERROR as i64), chain.tip().deref().height, ); @@ -1181,7 +1169,7 @@ mod tests { #[tokio::test] async fn test_handle_breaches_rejected_by_responder_malformed() { let mut chain = Blockchain::default().with_height_and_txs(START_HEIGHT, 10); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // Let's create some locators based on the transactions in the last block let breaches: HashMap<_, _> = (0..10) @@ -1217,7 +1205,7 @@ mod tests { #[tokio::test] async fn test_filtered_block_connected() { let mut chain = Blockchain::default().with_height(START_HEIGHT); - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // block_connected for the Watcher is used to keep track of what new transactions has been mined whose may be potential // channel breaches. @@ -1341,7 +1329,7 @@ mod tests { // Set the carrier response // Both non-decryptable blobs and blobs with invalid transactions will yield an invalid trigger. - let (carrier, _s) = create_carrier( + let carrier = create_carrier( MockedServerQuery::Error(rpc_errors::RPC_VERIFY_ERROR as i64), chain.tip().deref().height, ); @@ -1362,7 +1350,7 @@ mod tests { async fn test_block_disconnected() { let mut chain = Blockchain::default().with_height(START_HEIGHT); let start_height = START_HEIGHT as u32; - let (watcher, _s) = init_watcher(&mut chain).await; + let watcher = init_watcher(&mut chain).await; // block_disconnected for the Watcher fixes the locator cache by removing the disconnected block // and updates the last_known_block_height to the previous block height From 0a0a11811e31599ccca4841659a9e0056514a659 Mon Sep 17 00:00:00 2001 From: Omer Yacine Date: Fri, 28 Jun 2024 22:42:04 +0200 Subject: [PATCH 2/4] there were two duplicate `create_responder` test methods --- teos/src/responder.rs | 29 +++++------------------------ teos/src/test_utils.rs | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/teos/src/responder.rs b/teos/src/responder.rs index 025d31f7..aa650f6e 100644 --- a/teos/src/responder.rs +++ b/teos/src/responder.rs @@ -479,13 +479,12 @@ mod tests { use crate::dbm::DBM; use crate::rpc_errors; use crate::test_utils::{ - create_carrier, generate_dummy_appointment, generate_dummy_appointment_with_user, - generate_uuid, get_last_n_blocks, get_random_breach, get_random_tracker, get_random_tx, - store_appointment_and_its_user, Blockchain, MockedServerQuery, DURATION, EXPIRY_DELTA, - SLOTS, START_HEIGHT, + create_responder_with_query, generate_dummy_appointment, + generate_dummy_appointment_with_user, generate_uuid, get_random_breach, get_random_tracker, + get_random_tx, store_appointment_and_its_user, Blockchain, MockedServerQuery, DURATION, + EXPIRY_DELTA, SLOTS, START_HEIGHT, }; - use teos_common::constants::IRREVOCABLY_RESOLVED; use teos_common::test_utils::get_random_user_id; impl TransactionTracker { @@ -548,24 +547,6 @@ mod tests { } } - async fn create_responder( - chain: &mut Blockchain, - gatekeeper: Arc, - dbm: Arc>, - query: MockedServerQuery, - ) -> Responder { - let height = if chain.tip().height < IRREVOCABLY_RESOLVED { - chain.tip().height - } else { - IRREVOCABLY_RESOLVED - }; - - let carrier = create_carrier(query, chain.tip().height); - let last_n_blocks = get_last_n_blocks(chain, height as usize).await; - - Responder::new(&last_n_blocks, chain.tip().height, carrier, gatekeeper, dbm) - } - async fn init_responder_with_chain_and_dbm( mocked_query: MockedServerQuery, chain: &mut Blockchain, @@ -578,7 +559,7 @@ mod tests { EXPIRY_DELTA, dbm.clone(), ); - create_responder(chain, Arc::new(gk), dbm, mocked_query).await + create_responder_with_query(chain, Arc::new(gk), dbm, mocked_query).await } async fn init_responder(mocked_query: MockedServerQuery) -> Responder { diff --git a/teos/src/test_utils.rs b/teos/src/test_utils.rs index fc786cff..42b4c949 100644 --- a/teos/src/test_utils.rs +++ b/teos/src/test_utils.rs @@ -392,18 +392,28 @@ pub(crate) fn create_carrier(query: MockedServerQuery, height: u32) -> Carrier { ) } -pub(crate) async fn create_responder( +pub(crate) async fn create_responder_with_query( chain: &mut Blockchain, gatekeeper: Arc, dbm: Arc>, + query: MockedServerQuery, ) -> Responder { let height = chain.tip().height; // For the local TxIndex logic to be sound, our index needs to have, at least, IRREVOCABLY_RESOLVED blocks debug_assert!(height >= IRREVOCABLY_RESOLVED); - let carrier = create_carrier(MockedServerQuery::Regular, height); - let last_n_blocks = get_last_n_blocks(chain, IRREVOCABLY_RESOLVED as usize).await; - Responder::new(&last_n_blocks, height, carrier, gatekeeper, dbm) + let carrier = create_carrier(query, chain.tip().height); + let last_n_blocks = get_last_n_blocks(chain, height as usize).await; + + Responder::new(&last_n_blocks, chain.tip().height, carrier, gatekeeper, dbm) +} + +pub(crate) async fn create_responder( + chain: &mut Blockchain, + gatekeeper: Arc, + dbm: Arc>, +) -> Responder { + create_responder_with_query(chain, gatekeeper, dbm, MockedServerQuery::Regular).await } pub(crate) async fn create_watcher( From c42b29cf4a23c3945fa8a39080d0275f05f743f6 Mon Sep 17 00:00:00 2001 From: Omer Yacine Date: Fri, 28 Jun 2024 22:47:59 +0200 Subject: [PATCH 3/4] don't make two `new` methods, one is enough --- teos/src/carrier.rs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/teos/src/carrier.rs b/teos/src/carrier.rs index f6c185e6..207a597c 100644 --- a/teos/src/carrier.rs +++ b/teos/src/carrier.rs @@ -31,33 +31,18 @@ pub struct Carrier { impl Carrier { /// Creates a new [Carrier] instance. - #[cfg(not(test))] pub fn new( bitcoin_cli: Arc, bitcoind_reachable: Arc<(Mutex, Condvar)>, last_known_block_height: u32, + #[cfg(test)] stopper: Option, ) -> Self { Carrier { bitcoin_cli, bitcoind_reachable, issued_receipts: HashMap::new(), block_height: last_known_block_height, - } - } - - /// Creates a new [Carrier] instance. - #[cfg(test)] - pub fn new( - bitcoin_cli: Arc, - bitcoind_reachable: Arc<(Mutex, Condvar)>, - last_known_block_height: u32, - stopper: Option, - ) -> Self { - Carrier { - bitcoin_cli, - bitcoind_reachable, - issued_receipts: HashMap::new(), - block_height: last_known_block_height, + #[cfg(test)] _stopper: stopper, } } From ff8a3adec20dd2765737cf4d3c9a09f05d330081 Mon Sep 17 00:00:00 2001 From: Omer Yacine Date: Sat, 29 Jun 2024 11:05:27 +0200 Subject: [PATCH 4/4] make stopper non-optional in tests In some tests the carrier didn't need a stopper since it was already in the test scope so it would drop at the end of the test and do usual cleanup. This commit unifies stuff and still moves the stopper inside the carrier so that all the tests look alike (not having ones with Some(stopper) and ones with None). --- teos/src/carrier.rs | 95 +++++++++++++++++++++++++++++++++++------- teos/src/test_utils.rs | 4 +- 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/teos/src/carrier.rs b/teos/src/carrier.rs index 207a597c..c4b7689f 100644 --- a/teos/src/carrier.rs +++ b/teos/src/carrier.rs @@ -26,7 +26,7 @@ pub struct Carrier { block_height: u32, #[cfg(test)] /// A stopper that stops the mock bitcoind server in tests when the [`Carrier`] is dropped. - _stopper: Option, + _stopper: crate::test_utils::BitcoindStopper, } impl Carrier { @@ -35,7 +35,7 @@ impl Carrier { bitcoin_cli: Arc, bitcoind_reachable: Arc<(Mutex, Condvar)>, last_known_block_height: u32, - #[cfg(test)] stopper: Option, + #[cfg(test)] stopper: crate::test_utils::BitcoindStopper, ) -> Self { Carrier { bitcoin_cli, @@ -218,7 +218,12 @@ mod tests { let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); // Lets add some dummy data into the cache for i in 0..10 { @@ -242,7 +247,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -260,7 +270,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -280,7 +295,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -302,7 +322,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -325,7 +350,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -344,7 +374,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let r = carrier.send_transaction(&tx); @@ -364,7 +399,12 @@ mod tests { let bitcoind_reachable = Arc::new((Mutex::new(false), Condvar::new())); let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let mut carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height, None); + let mut carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable.clone(), + start_height, + bitcoind_mock.stopper, + ); let tx = consensus::deserialize(&Vec::from_hex(TX_HEX).unwrap()).unwrap(); let delay = std::time::Duration::new(3, 0); @@ -394,7 +434,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(carrier.in_mempool(&txid)); } @@ -407,7 +452,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -422,7 +472,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -436,7 +491,12 @@ mod tests { let start_height = START_HEIGHT as u32; start_server(bitcoind_mock.server); - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable, start_height, None); + let carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable, + start_height, + bitcoind_mock.stopper, + ); let txid = Txid::from_hex(TXID_HEX).unwrap(); assert!(!carrier.in_mempool(&txid)); } @@ -448,7 +508,12 @@ mod tests { let bitcoind_reachable = Arc::new((Mutex::new(false), Condvar::new())); let bitcoin_cli = Arc::new(BitcoindClient::new(bitcoind_mock.url(), Auth::None).unwrap()); let start_height = START_HEIGHT as u32; - let carrier = Carrier::new(bitcoin_cli, bitcoind_reachable.clone(), start_height, None); + let carrier = Carrier::new( + bitcoin_cli, + bitcoind_reachable.clone(), + start_height, + bitcoind_mock.stopper, + ); let txid = Txid::from_hex(TXID_HEX).unwrap(); let delay = std::time::Duration::new(3, 0); diff --git a/teos/src/test_utils.rs b/teos/src/test_utils.rs index 42b4c949..9f98bf0d 100644 --- a/teos/src/test_utils.rs +++ b/teos/src/test_utils.rs @@ -388,7 +388,7 @@ pub(crate) fn create_carrier(query: MockedServerQuery, height: u32) -> Carrier { bitcoin_cli, bitcoind_reachable, height, - Some(bitcoind_mock.stopper), + bitcoind_mock.stopper, ) } @@ -531,7 +531,7 @@ impl Debug for BitcoindStopper { pub(crate) struct BitcoindMock { pub url: String, pub server: Server, - stopper: BitcoindStopper, + pub stopper: BitcoindStopper, } #[derive(Default)]