f drop TestNode changes

Author: TheBlueMatt

benches/payments.rs

@@ -8,7 +8,7 @@ use bitcoin::hex::DisplayHex;
 use bitcoin::Amount;
 use common::{
 	expect_channel_ready_event, generate_blocks_and_wait, premine_and_distribute_funds,
-	setup_bitcoind_and_electrsd, setup_two_nodes_with_store, TestChainSource, TestNode,
+	setup_bitcoind_and_electrsd, setup_two_nodes_with_store, TestChainSource,
 };
 use criterion::{criterion_group, criterion_main, Criterion};
 use ldk_node::{Event, Node};
@@ -18,7 +18,7 @@ use tokio::task::{self};
 
 use crate::common::open_channel_push_amt;
 
-fn spawn_payment(node_a: Arc<TestNode>, node_b: Arc<TestNode>, amount_msat: u64) {
+fn spawn_payment(node_a: Arc<Node>, node_b: Arc<Node>, amount_msat: u64) {
 	let mut preimage_bytes = [0u8; 32];
 	rand::rng().fill_bytes(&mut preimage_bytes);
 	let preimage = PaymentPreimage(preimage_bytes);
@@ -61,7 +61,7 @@ fn spawn_payment(node_a: Arc<TestNode>, node_b: Arc<TestNode>, amount_msat: u64)
 	});
 }
 
-async fn send_payments(node_a: Arc<TestNode>, node_b: Arc<TestNode>) -> std::time::Duration {
+async fn send_payments(node_a: Arc<Node>, node_b: Arc<Node>) -> std::time::Duration {
 	let start = Instant::now();
 
 	let total_payments = 1000;

src/lib.rs

@@ -1784,6 +1784,12 @@ impl Node {
 	}
 }
 
+impl Drop for Node {
+	fn drop(&mut self) {
+		let _ = self.stop();
+	}
+}
+
 /// Represents the status of the [`Node`].
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct NodeStatus {

src/logger.rs

@@ -171,14 +171,6 @@ impl LogWriter for Writer {
 	}
 }
 
-#[cfg(cycle_tests)]
-/// Our log writer
-pub struct Logger {
-	/// Specifies the logger's writer.
-	writer: Writer,
-}
-
-#[cfg(not(cycle_tests))]
 pub(crate) struct Logger {
 	/// Specifies the logger's writer.
 	writer: Writer,
@@ -203,12 +195,10 @@ impl Logger {
 		Ok(Self { writer: Writer::FileWriter { file_path, max_log_level } })
 	}
 
-	/// Creates a new logger writing to the `log` crate
 	pub fn new_log_facade() -> Self {
 		Self { writer: Writer::LogFacadeWriter }
 	}
 
-	/// Creates a new logger writing to an underlying [`LogWriter`]
 	pub fn new_custom_writer(log_writer: Arc<dyn LogWriter>) -> Self {
 		Self { writer: Writer::CustomWriter(log_writer) }
 	}

tests/common/mod.rs

@@ -13,7 +13,6 @@ pub(crate) mod logging;
 use std::collections::{HashMap, HashSet};
 use std::env;
 use std::future::Future;
-use std::ops::Deref;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 use std::time::Duration;
@@ -262,53 +261,10 @@ pub(crate) fn random_config(anchor_channels: bool) -> TestConfig {
 	TestConfig { node_config, ..Default::default() }
 }
 
-pub struct TestNode {
-	#[cfg(feature = "uniffi")]
-	inner: Option<Arc<Node>>,
-	#[cfg(not(feature = "uniffi"))]
-	inner: Option<Node>,
-}
-
 #[cfg(feature = "uniffi")]
-impl From<Arc<Node>> for TestNode {
-	fn from(inner: Arc<Node>) -> Self {
-		Self { inner: Some(inner) }
-	}
-}
-
+type TestNode = Arc<Node>;
 #[cfg(not(feature = "uniffi"))]
-impl From<Node> for TestNode {
-	fn from(inner: Node) -> Self {
-		Self { inner: Some(inner) }
-	}
-}
-
-impl Deref for TestNode {
-	type Target = Node;
-	fn deref(&self) -> &Node {
-		#[cfg(feature = "uniffi")]
-		{
-			self.inner.as_ref().unwrap().deref()
-		}
-		#[cfg(not(feature = "uniffi"))]
-		{
-			&self.inner.as_ref().unwrap()
-		}
-	}
-}
-
-#[cfg(cycle_tests)]
-impl Drop for TestNode {
-	fn drop(&mut self) {
-		if !std::thread::panicking() {
-			match self.inner.take().unwrap().stop() {
-				Ok(()) => {},
-				Err(e) if e == NodeError::NotRunning => {},
-				Err(e) => panic!("{e:?}"),
-			}
-		}
-	}
-}
+type TestNode = Node;
 
 #[derive(Clone)]
 pub(crate) enum TestChainSource<'a> {
@@ -474,7 +430,7 @@ pub(crate) fn setup_node_for_async_payments(
 	node.start().unwrap();
 	assert!(node.status().is_running);
 	assert!(node.status().latest_fee_rate_cache_update_timestamp.is_some());
-	node.into()
+	node
 }
 
 pub(crate) async fn generate_blocks_and_wait<E: ElectrumApi>(

tests/integration_tests_rust.rs

@@ -23,8 +23,8 @@ use common::{
 	expect_splice_pending_event, generate_blocks_and_wait, open_channel, open_channel_push_amt,
 	premine_and_distribute_funds, premine_blocks, prepare_rbf, random_config,
 	random_listening_addresses, setup_bitcoind_and_electrsd, setup_builder, setup_node,
-	setup_node_for_async_payments, setup_two_nodes, wait_for_tx, TestChainSource, TestNode,
-	TestStoreType, TestSyncStore,
+	setup_node_for_async_payments, setup_two_nodes, wait_for_tx, TestChainSource, TestStoreType,
+	TestSyncStore,
 };
 use ldk_node::config::{AsyncPaymentsRole, EsploraSyncConfig};
 use ldk_node::liquidity::LSPS2ServiceConfig;
@@ -154,15 +154,15 @@ async fn multi_hop_sending() {
 	let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap());
 
 	// Setup and fund 5 nodes
-	let mut nodes: Vec<TestNode> = Vec::new();
+	let mut nodes = Vec::new();
 	for _ in 0..5 {
 		let config = random_config(true);
 		let sync_config = EsploraSyncConfig { background_sync_config: None };
 		setup_builder!(builder, config.node_config);
 		builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
-		let node: TestNode = builder.build(config.node_entropy.into()).unwrap().into();
+		let node = builder.build(config.node_entropy.into()).unwrap();
 		node.start().unwrap();
-		nodes.push(node.into());
+		nodes.push(node);
 	}
 
 	let addresses = nodes.iter().map(|n| n.onchain_payment().new_address().unwrap()).collect();
@@ -1730,8 +1730,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 	setup_builder!(service_builder, service_config.node_config);
 	service_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	service_builder.set_liquidity_provider_lsps2(lsps2_service_config);
-	let service_node: TestNode =
-		service_builder.build(service_config.node_entropy.into()).unwrap().into();
+	let service_node = service_builder.build(service_config.node_entropy.into()).unwrap();
 	service_node.start().unwrap();
 
 	let service_node_id = service_node.node_id();
@@ -1741,15 +1740,13 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 	setup_builder!(client_builder, client_config.node_config);
 	client_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	client_builder.set_liquidity_source_lsps2(service_node_id, service_addr, None);
-	let client_node: TestNode =
-		client_builder.build(client_config.node_entropy.into()).unwrap().into();
+	let client_node = client_builder.build(client_config.node_entropy.into()).unwrap();
 	client_node.start().unwrap();
 
 	let payer_config = random_config(true);
 	setup_builder!(payer_builder, payer_config.node_config);
 	payer_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
-	let payer_node: TestNode =
-		payer_builder.build(payer_config.node_entropy.into()).unwrap().into();
+	let payer_node = payer_builder.build(payer_config.node_entropy.into()).unwrap();
 	payer_node.start().unwrap();
 
 	let service_addr = service_node.onchain_payment().new_address().unwrap();
@@ -2050,8 +2047,7 @@ async fn lsps2_client_trusts_lsp() {
 	setup_builder!(service_builder, service_config.node_config);
 	service_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	service_builder.set_liquidity_provider_lsps2(lsps2_service_config);
-	let service_node: TestNode =
-		service_builder.build(service_config.node_entropy.into()).unwrap().into();
+	let service_node = service_builder.build(service_config.node_entropy.into()).unwrap();
 	service_node.start().unwrap();
 	let service_node_id = service_node.node_id();
 	let service_addr = service_node.listening_addresses().unwrap().first().unwrap().clone();
@@ -2060,16 +2056,14 @@ async fn lsps2_client_trusts_lsp() {
 	setup_builder!(client_builder, client_config.node_config);
 	client_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	client_builder.set_liquidity_source_lsps2(service_node_id, service_addr.clone(), None);
-	let client_node: TestNode =
-		client_builder.build(client_config.node_entropy.into()).unwrap().into();
+	let client_node = client_builder.build(client_config.node_entropy.into()).unwrap();
 	client_node.start().unwrap();
 	let client_node_id = client_node.node_id();
 
 	let payer_config = random_config(true);
 	setup_builder!(payer_builder, payer_config.node_config);
 	payer_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
-	let payer_node: TestNode =
-		payer_builder.build(payer_config.node_entropy.into()).unwrap().into();
+	let payer_node = payer_builder.build(payer_config.node_entropy.into()).unwrap();
 	payer_node.start().unwrap();
 
 	let service_addr_onchain = service_node.onchain_payment().new_address().unwrap();
@@ -2226,8 +2220,7 @@ async fn lsps2_lsp_trusts_client_but_client_does_not_claim() {
 	setup_builder!(service_builder, service_config.node_config);
 	service_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	service_builder.set_liquidity_provider_lsps2(lsps2_service_config);
-	let service_node: TestNode =
-		service_builder.build(service_config.node_entropy.into()).unwrap().into();
+	let service_node = service_builder.build(service_config.node_entropy.into()).unwrap();
 	service_node.start().unwrap();
 
 	let service_node_id = service_node.node_id();
@@ -2237,17 +2230,15 @@ async fn lsps2_lsp_trusts_client_but_client_does_not_claim() {
 	setup_builder!(client_builder, client_config.node_config);
 	client_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
 	client_builder.set_liquidity_source_lsps2(service_node_id, service_addr.clone(), None);
-	let client_node: TestNode =
-		client_builder.build(client_config.node_entropy.into()).unwrap().into();
+	let client_node = client_builder.build(client_config.node_entropy.into()).unwrap();
 	client_node.start().unwrap();
 
 	let client_node_id = client_node.node_id();
 
 	let payer_config = random_config(true);
 	setup_builder!(payer_builder, payer_config.node_config);
 	payer_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
-	let payer_node: TestNode =
-		payer_builder.build(payer_config.node_entropy.into()).unwrap().into();
+	let payer_node = payer_builder.build(payer_config.node_entropy.into()).unwrap();
 	payer_node.start().unwrap();
 
 	let service_addr_onchain = service_node.onchain_payment().new_address().unwrap();

tests/integration_tests_vss.rs

@@ -24,30 +24,28 @@ async fn channel_full_cycle_with_vss_store() {
 	let mut builder_a = Builder::from_config(config_a.node_config);
 	builder_a.set_chain_source_esplora(esplora_url.clone(), None);
 	let vss_base_url = std::env::var("TEST_VSS_BASE_URL").unwrap();
-	let node_a: common::TestNode = builder_a
+	let node_a = builder_a
 		.build_with_vss_store_and_fixed_headers(
 			config_a.node_entropy,
 			vss_base_url.clone(),
 			"node_1_store".to_string(),
 			HashMap::new(),
 		)
-		.unwrap()
-		.into();
+		.unwrap();
 	node_a.start().unwrap();
 
 	println!("\n== Node B ==");
 	let config_b = common::random_config(true);
 	let mut builder_b = Builder::from_config(config_b.node_config);
 	builder_b.set_chain_source_esplora(esplora_url.clone(), None);
-	let node_b: common::TestNode = builder_b
+	let node_b = builder_b
 		.build_with_vss_store_and_fixed_headers(
 			config_b.node_entropy,
 			vss_base_url,
 			"node_2_store".to_string(),
 			HashMap::new(),
 		)
-		.unwrap()
-		.into();
+		.unwrap();
 	node_b.start().unwrap();
 
 	common::do_channel_full_cycle(