From 1860ca4b7a7758078139dc85e1f4fb3ee6bb984f Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 24 Mar 2023 10:03:18 +0100 Subject: [PATCH] Move test `bitcoind`/`electrsd` out of `OnceCell` `OnceCell` doesn't call `drop`, which makes the spawned `bitcoind`/`electrsd` instances linger around after our tests have finished. To fix this, we move them out of `OnceCell` and let every test that needs them spawn their own instances. This additional let us drop the `OnceCell` dev dependency. Additionally, we improve the test robustness by applying most of the changes from https://github.com/lightningdevkit/rust-lightning/pull/2033. --- Cargo.toml | 1 - src/test/functional_tests.rs | 157 +++++++++++++++++++---------------- 2 files changed, 84 insertions(+), 74 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b00a22a39..cdfadd5b9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,6 @@ libc = "0.2" [dev-dependencies] electrsd = { version = "0.22.0", features = ["legacy", "esplora_a33e97e1", "bitcoind_23_0"] } electrum-client = "0.12.0" -once_cell = "1.16.0" proptest = "1.0.0" [profile.release] diff --git a/src/test/functional_tests.rs b/src/test/functional_tests.rs index 1fe2e43fc..37acce432 100644 --- a/src/test/functional_tests.rs +++ b/src/test/functional_tests.rs @@ -7,93 +7,90 @@ use electrsd::bitcoind::bitcoincore_rpc::bitcoincore_rpc_json::AddressType; use electrsd::{bitcoind, bitcoind::BitcoinD, ElectrsD}; use electrum_client::ElectrumApi; -use once_cell::sync::OnceCell; - use std::env; -use std::sync::Mutex; use std::time::Duration; -static BITCOIND: OnceCell = OnceCell::new(); -static ELECTRSD: OnceCell = OnceCell::new(); -static PREMINE: OnceCell<()> = OnceCell::new(); -static MINER_LOCK: Mutex<()> = Mutex::new(()); - -fn get_bitcoind() -> &'static BitcoinD { - BITCOIND.get_or_init(|| { - let bitcoind_exe = - env::var("BITCOIND_EXE").ok().or_else(|| bitcoind::downloaded_exe_path().ok()).expect( - "you need to provide an env var BITCOIND_EXE or specify a bitcoind version feature", - ); - let mut conf = bitcoind::Conf::default(); - conf.network = "regtest"; - BitcoinD::with_conf(bitcoind_exe, &conf).unwrap() - }) -} - -fn get_electrsd() -> &'static ElectrsD { - ELECTRSD.get_or_init(|| { - let bitcoind = get_bitcoind(); - let electrs_exe = - env::var("ELECTRS_EXE").ok().or_else(electrsd::downloaded_exe_path).expect( - "you need to provide env var ELECTRS_EXE or specify an electrsd version feature", - ); - let mut conf = electrsd::Conf::default(); - conf.http_enabled = true; - conf.network = "regtest"; - ElectrsD::with_conf(electrs_exe, &bitcoind, &conf).unwrap() - }) +fn setup_bitcoind_and_electrsd() -> (BitcoinD, ElectrsD) { + let bitcoind_exe = + env::var("BITCOIND_EXE").ok().or_else(|| bitcoind::downloaded_exe_path().ok()).expect( + "you need to provide an env var BITCOIND_EXE or specify a bitcoind version feature", + ); + let mut bitcoind_conf = bitcoind::Conf::default(); + bitcoind_conf.network = "regtest"; + let bitcoind = BitcoinD::with_conf(bitcoind_exe, &bitcoind_conf).unwrap(); + + let electrs_exe = env::var("ELECTRS_EXE") + .ok() + .or_else(electrsd::downloaded_exe_path) + .expect("you need to provide env var ELECTRS_EXE or specify an electrsd version feature"); + let mut electrsd_conf = electrsd::Conf::default(); + electrsd_conf.http_enabled = true; + electrsd_conf.network = "regtest"; + let electrsd = ElectrsD::with_conf(electrs_exe, &bitcoind, &electrsd_conf).unwrap(); + (bitcoind, electrsd) } -fn generate_blocks_and_wait(num: usize) { - let _miner = MINER_LOCK.lock().unwrap(); - let cur_height = get_bitcoind().client.get_block_count().unwrap(); - let address = - get_bitcoind().client.get_new_address(Some("test"), Some(AddressType::Legacy)).unwrap(); - let _block_hashes = get_bitcoind().client.generate_to_address(num as u64, &address).unwrap(); - wait_for_block(cur_height as usize + num); +fn generate_blocks_and_wait(bitcoind: &BitcoinD, electrsd: &ElectrsD, num: usize) { + let cur_height = bitcoind.client.get_block_count().expect("failed to get current block height"); + let address = bitcoind + .client + .get_new_address(Some("test"), Some(AddressType::Legacy)) + .expect("failed to get new address"); + // TODO: expect this Result once the WouldBlock issue is resolved upstream. + let _block_hashes_res = bitcoind.client.generate_to_address(num as u64, &address); + wait_for_block(electrsd, cur_height as usize + num); } -fn wait_for_block(min_height: usize) { - let mut header = get_electrsd().client.block_headers_subscribe().unwrap(); +fn wait_for_block(electrsd: &ElectrsD, min_height: usize) { + let mut header = match electrsd.client.block_headers_subscribe() { + Ok(header) => header, + Err(_) => { + // While subscribing should succeed the first time around, we ran into some cases where + // it didn't. Since we can't proceed without subscribing, we try again after a delay + // and panic if it still fails. + std::thread::sleep(Duration::from_secs(1)); + electrsd.client.block_headers_subscribe().expect("failed to subscribe to block headers") + } + }; loop { if header.height >= min_height { break; } header = exponential_backoff_poll(|| { - get_electrsd().trigger().unwrap(); - get_electrsd().client.ping().unwrap(); - get_electrsd().client.block_headers_pop().unwrap() + electrsd.trigger().expect("failed to trigger electrsd"); + electrsd.client.ping().expect("failed to ping electrsd"); + electrsd.client.block_headers_pop().expect("failed to pop block header") }); } } -fn wait_for_tx(txid: Txid) { - let mut tx_res = get_electrsd().client.transaction_get(&txid); +fn wait_for_tx(electrsd: &ElectrsD, txid: Txid) { + let mut tx_res = electrsd.client.transaction_get(&txid); loop { if tx_res.is_ok() { break; } tx_res = exponential_backoff_poll(|| { - get_electrsd().trigger().unwrap(); - get_electrsd().client.ping().unwrap(); - Some(get_electrsd().client.transaction_get(&txid)) + electrsd.trigger().unwrap(); + electrsd.client.ping().unwrap(); + Some(electrsd.client.transaction_get(&txid)) }); } } -fn wait_for_outpoint_spend(outpoint: OutPoint) { - let tx = get_electrsd().client.transaction_get(&outpoint.txid).unwrap(); +fn wait_for_outpoint_spend(electrsd: &ElectrsD, outpoint: OutPoint) { + let tx = electrsd.client.transaction_get(&outpoint.txid).unwrap(); let txout_script = tx.output.get(outpoint.vout as usize).unwrap().clone().script_pubkey; - let mut is_spent = !get_electrsd().client.script_get_history(&txout_script).unwrap().is_empty(); + let mut is_spent = !electrsd.client.script_get_history(&txout_script).unwrap().is_empty(); loop { if is_spent { break; } is_spent = exponential_backoff_poll(|| { - get_electrsd().trigger().unwrap(); - get_electrsd().client.ping().unwrap(); - Some(!get_electrsd().client.script_get_history(&txout_script).unwrap().is_empty()) + electrsd.trigger().unwrap(); + electrsd.client.ping().unwrap(); + Some(!electrsd.client.script_get_history(&txout_script).unwrap().is_empty()) }); } } @@ -119,26 +116,27 @@ where } } -fn premine_and_distribute_funds(addrs: Vec
, amount: Amount) { - PREMINE.get_or_init(|| { - generate_blocks_and_wait(101); - }); +fn premine_and_distribute_funds( + bitcoind: &BitcoinD, electrsd: &ElectrsD, addrs: Vec
, amount: Amount, +) { + generate_blocks_and_wait(bitcoind, electrsd, 101); for addr in addrs { - let txid = get_bitcoind() + let txid = bitcoind .client .send_to_address(&addr, amount, None, None, None, None, None, None) .unwrap(); - wait_for_tx(txid); + wait_for_tx(electrsd, txid); } - generate_blocks_and_wait(1); + generate_blocks_and_wait(bitcoind, electrsd, 1); } #[test] fn channel_full_cycle() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); println!("== Node A =="); - let esplora_url = get_electrsd().esplora_url.as_ref().unwrap(); + let esplora_url = electrsd.esplora_url.as_ref().unwrap(); let config_a = random_config(esplora_url); let node_a = Builder::from_config(config_a).build(); node_a.start().unwrap(); @@ -150,7 +148,12 @@ fn channel_full_cycle() { node_b.start().unwrap(); let addr_b = node_b.new_funding_address().unwrap(); - premine_and_distribute_funds(vec![addr_a, addr_b], Amount::from_sat(100000)); + premine_and_distribute_funds( + &bitcoind, + &electrsd, + vec![addr_a, addr_b], + Amount::from_sat(100000), + ); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); assert_eq!(node_a.on_chain_balance().unwrap().get_spendable(), 100000); @@ -170,10 +173,10 @@ fn channel_full_cycle() { } }; - wait_for_tx(funding_txo.txid); + wait_for_tx(&electrsd, funding_txo.txid); println!("\n .. generating blocks, syncing wallets .. "); - generate_blocks_and_wait(6); + generate_blocks_and_wait(&bitcoind, &electrsd, 6); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -276,9 +279,9 @@ fn channel_full_cycle() { expect_event!(node_a, ChannelClosed); expect_event!(node_b, ChannelClosed); - wait_for_outpoint_spend(funding_txo.into_bitcoin_outpoint()); + wait_for_outpoint_spend(&electrsd, funding_txo.into_bitcoin_outpoint()); - generate_blocks_and_wait(1); + generate_blocks_and_wait(&bitcoind, &electrsd, 1); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -293,8 +296,9 @@ fn channel_full_cycle() { #[test] fn channel_open_fails_when_funds_insufficient() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); println!("== Node A =="); - let esplora_url = get_electrsd().esplora_url.as_ref().unwrap(); + let esplora_url = electrsd.esplora_url.as_ref().unwrap(); let config_a = random_config(&esplora_url); let node_a = Builder::from_config(config_a).build(); node_a.start().unwrap(); @@ -306,7 +310,12 @@ fn channel_open_fails_when_funds_insufficient() { node_b.start().unwrap(); let addr_b = node_b.new_funding_address().unwrap(); - premine_and_distribute_funds(vec![addr_a, addr_b], Amount::from_sat(100000)); + premine_and_distribute_funds( + &bitcoind, + &electrsd, + vec![addr_a, addr_b], + Amount::from_sat(100000), + ); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); assert_eq!(node_a.on_chain_balance().unwrap().get_spendable(), 100000); @@ -322,7 +331,8 @@ fn channel_open_fails_when_funds_insufficient() { #[test] fn connect_to_public_testnet_esplora() { - let esplora_url = get_electrsd().esplora_url.as_ref().unwrap(); + let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let esplora_url = electrsd.esplora_url.as_ref().unwrap(); let mut config = random_config(&esplora_url); config.esplora_server_url = "https://blockstream.info/testnet/api".to_string(); config.network = bitcoin::Network::Testnet; @@ -334,7 +344,8 @@ fn connect_to_public_testnet_esplora() { #[test] fn start_stop_reinit() { - let esplora_url = get_electrsd().esplora_url.as_ref().unwrap(); + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let esplora_url = electrsd.esplora_url.as_ref().unwrap(); let config = random_config(&esplora_url); let node = Builder::from_config(config.clone()).build(); let expected_node_id = node.node_id(); @@ -342,7 +353,7 @@ fn start_stop_reinit() { let funding_address = node.new_funding_address().unwrap(); let expected_amount = Amount::from_sat(100000); - premine_and_distribute_funds(vec![funding_address], expected_amount); + premine_and_distribute_funds(&bitcoind, &electrsd, vec![funding_address], expected_amount); assert_eq!(node.on_chain_balance().unwrap().get_total(), 0); node.start().unwrap();