Prepare BumpTransactionEventHandler for async conversion

Convert methods that need to be async when the wallet traits become
async.

Author: joostjager

lightning/Cargo.toml

@@ -17,7 +17,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 [features]
 # Internal test utilities exposed to other repo crates
 _test_utils = ["regex", "bitcoin/bitcoinconsensus", "lightning-types/_test_utils"]
-_externalize_tests = ["inventory", "_test_utils"]
+_externalize_tests = ["inventory", "_test_utils", "tokio"]
 # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling).
 # This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
 unsafe_revoked_tx_signing = []
@@ -48,12 +48,14 @@ backtrace = { version = "0.3", optional = true }
 
 libm = { version = "0.2", default-features = false }
 inventory = { version = "0.3", optional = true  }
+tokio = { version = "1.35", features = [ "macros", "rt" ], default-features = false, optional = true }
 
 [dev-dependencies]
 regex = "1.5.6"
 lightning-types = { version = "0.3.0", path = "../lightning-types", features = ["_test_utils"] }
 lightning-macros = { path = "../lightning-macros" }
 parking_lot = { version = "0.12", default-features = false }
+tokio = { version = "1.35", features = [ "macros", "rt" ], default-features = false }
 
 [dev-dependencies.bitcoin]
 version = "0.32.2"

lightning/src/events/bump_transaction.rs

@@ -625,7 +625,7 @@ where
 	/// Handles a [`BumpTransactionEvent::ChannelClose`] event variant by producing a fully-signed
 	/// transaction spending an anchor output of the commitment transaction to bump its fee and
 	/// broadcasts them to the network as a package.
-	fn handle_channel_close(
+	async fn handle_channel_close(
 		&self, claim_id: ClaimId, package_target_feerate_sat_per_1000_weight: u32,
 		commitment_tx: &Transaction, commitment_tx_fee_sat: u64,
 		anchor_descriptor: &AnchorDescriptor,
@@ -770,7 +770,7 @@ where
 
 	/// Handles a [`BumpTransactionEvent::HTLCResolution`] event variant by producing a
 	/// fully-signed, fee-bumped HTLC transaction that is broadcast to the network.
-	fn handle_htlc_resolution(
+	async fn handle_htlc_resolution(
 		&self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32,
 		htlc_descriptors: &[HTLCDescriptor], tx_lock_time: LockTime,
 	) -> Result<(), ()> {
@@ -899,7 +899,7 @@ where
 	}
 
 	/// Handles all variants of [`BumpTransactionEvent`].
-	pub fn handle_event(&self, event: &BumpTransactionEvent) {
+	pub async fn handle_event(&self, event: &BumpTransactionEvent) {
 		match event {
 			BumpTransactionEvent::ChannelClose {
 				claim_id,
@@ -915,19 +915,21 @@ where
 					log_bytes!(claim_id.0),
 					commitment_tx.compute_txid()
 				);
-				if let Err(_) = self.handle_channel_close(
+				self.handle_channel_close(
 					*claim_id,
 					*package_target_feerate_sat_per_1000_weight,
 					commitment_tx,
 					*commitment_tx_fee_satoshis,
 					anchor_descriptor,
-				) {
+				)
+				.await
+				.unwrap_or_else(|_| {
 					log_error!(
 						self.logger,
 						"Failed bumping commitment transaction fee for {}",
 						commitment_tx.compute_txid()
 					);
-				}
+				});
 			},
 			BumpTransactionEvent::HTLCResolution {
 				claim_id,
@@ -942,18 +944,20 @@ where
 					log_bytes!(claim_id.0),
 					log_iter!(htlc_descriptors.iter().map(|d| d.outpoint()))
 				);
-				if let Err(_) = self.handle_htlc_resolution(
+				self.handle_htlc_resolution(
 					*claim_id,
 					*target_feerate_sat_per_1000_weight,
 					htlc_descriptors,
 					*tx_lock_time,
-				) {
+				)
+				.await
+				.unwrap_or_else(|_| {
 					log_error!(
 						self.logger,
 						"Failed bumping HTLC transaction fee for commitment {}",
 						htlc_descriptors[0].commitment_txid
 					);
-				}
+				});
 			},
 		}
 	}
@@ -1009,8 +1013,8 @@ mod tests {
 		}
 	}
 
-	#[test]
-	fn test_op_return_under_funds() {
+	#[tokio::test]
+	async fn test_op_return_under_funds() {
 		// Test what happens if we have to select coins but the anchor output value itself suffices
 		// to pay the required fee.
 		//
@@ -1069,7 +1073,7 @@ mod tests {
 		transaction_parameters.channel_type_features =
 			ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies();
 
-		handler.handle_event(&BumpTransactionEvent::ChannelClose {
+		let channel_close_event = &BumpTransactionEvent::ChannelClose {
 			channel_id: ChannelId([42; 32]),
 			counterparty_node_id: PublicKey::from_slice(&[2; 33]).unwrap(),
 			claim_id: ClaimId([42; 32]),
@@ -1085,6 +1089,7 @@ mod tests {
 				outpoint: OutPoint { txid: Txid::from_byte_array([42; 32]), vout: 0 },
 			},
 			pending_htlcs: Vec::new(),
-		});
+		};
+		handler.handle_event(channel_close_event).await;
 	}
 }

lightning/src/ln/async_signer_tests.rs

@@ -805,7 +805,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) {
 	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
-fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
+async fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
 	// Ensures that we can obtain holder signatures for commitment and HTLC transactions
 	// asynchronously by allowing their retrieval to fail and retrying via
 	// `ChannelMonitor::signer_unblocked`.
@@ -909,7 +909,7 @@ fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
 
 	// No HTLC transaction should be broadcast as the signer is not available yet.
 	if anchors && !remote_commitment {
-		handle_bump_htlc_event(&nodes[0], 1);
+		handle_bump_htlc_event(&nodes[0], 1).await;
 	}
 	let txn = nodes[0].tx_broadcaster.txn_broadcast();
 	assert!(txn.is_empty(), "expected no transaction to be broadcast, got {:?}", txn);
@@ -920,7 +920,7 @@ fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
 	get_monitor!(nodes[0], chan_id).signer_unblocked(nodes[0].tx_broadcaster, nodes[0].fee_estimator, &nodes[0].logger);
 
 	if anchors && !remote_commitment {
-		handle_bump_htlc_event(&nodes[0], 1);
+		handle_bump_htlc_event(&nodes[0], 1).await;
 	}
 	{
 		let txn = nodes[0].tx_broadcaster.txn_broadcast();
@@ -929,24 +929,24 @@ fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
 	}
 }
 
-#[test]
-fn test_async_holder_signatures_no_anchors() {
-	do_test_async_holder_signatures(false, false);
+#[tokio::test]
+async fn test_async_holder_signatures_no_anchors() {
+	do_test_async_holder_signatures(false, false).await;
 }
 
-#[test]
-fn test_async_holder_signatures_remote_commitment_no_anchors() {
-	do_test_async_holder_signatures(false, true);
+#[tokio::test]
+async fn test_async_holder_signatures_remote_commitment_no_anchors() {
+	do_test_async_holder_signatures(false, true).await;
 }
 
-#[test]
-fn test_async_holder_signatures_anchors() {
-	do_test_async_holder_signatures(true, false);
+#[tokio::test]
+async fn test_async_holder_signatures_anchors() {
+	do_test_async_holder_signatures(true, false).await;
 }
 
-#[test]
-fn test_async_holder_signatures_remote_commitment_anchors() {
-	do_test_async_holder_signatures(true, true);
+#[tokio::test]
+async fn test_async_holder_signatures_remote_commitment_anchors() {
+	do_test_async_holder_signatures(true, true).await;
 }
 
 #[test]

lightning/src/ln/functional_test_utils.rs

@@ -1846,15 +1846,15 @@ macro_rules! check_closed_event {
 	}
 }
 
-pub fn handle_bump_htlc_event(node: &Node, count: usize) {
+pub async fn handle_bump_htlc_event<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, count: usize) {
 	let events = node.chain_monitor.chain_monitor.get_and_clear_pending_events();
 	assert_eq!(events.len(), count);
 	for event in events {
 		match event {
 			Event::BumpTransaction(bump_event) => {
 				if let BumpTransactionEvent::HTLCResolution { .. } = &bump_event {}
 				else { panic!(); }
-				node.bump_tx_handler.handle_event(&bump_event);
+				node.bump_tx_handler.handle_event(&bump_event).await;
 			},
 			_ => panic!(),
 		}

lightning/src/ln/functional_tests.rs

@@ -1474,6 +1474,19 @@ pub fn claim_htlc_outputs() {
 // This is a regression test for https://github.com/lightningdevkit/rust-lightning/issues/3537.
 #[xtest(feature = "_externalize_tests")]
 pub fn test_multiple_package_conflicts() {
+	// #[tokio::test] can't be used here because it adds the #[test] attribute, which causes the external test runner to
+	// miss this test.
+	let body = Box::pin(async {
+		test_multiple_package_conflicts_internal().await;
+	});
+
+	return tokio::runtime::Builder::new_current_thread()
+		.build()
+		.expect("Failed building the Runtime")
+		.block_on(body);
+}
+
+async fn test_multiple_package_conflicts_internal() {
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 	let mut user_cfg = test_default_channel_config();
@@ -1626,21 +1639,17 @@ pub fn test_multiple_package_conflicts() {
 	check_closed_broadcast!(nodes[2], true);
 	check_added_monitors(&nodes[2], 1);
 
-	let process_bump_event = |node: &Node| {
-		let events = node.chain_monitor.chain_monitor.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		let bump_event = match &events[0] {
-			Event::BumpTransaction(bump_event) => bump_event,
-			_ => panic!("Unexepected event"),
-		};
-		node.bump_tx_handler.handle_event(bump_event);
-
-		let mut tx = node.tx_broadcaster.txn_broadcast();
-		assert_eq!(tx.len(), 1);
-		tx.pop().unwrap()
+	let events = nodes[2].chain_monitor.chain_monitor.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	let bump_event = match &events[0] {
+		Event::BumpTransaction(bump_event) => bump_event,
+		_ => panic!("Unexpected event"),
 	};
+	nodes[2].bump_tx_handler.handle_event(bump_event).await;
 
-	let conflict_tx = process_bump_event(&nodes[2]);
+	let mut tx = nodes[2].tx_broadcaster.txn_broadcast();
+	assert_eq!(tx.len(), 1);
+	let conflict_tx = tx.pop().unwrap();
 	assert_eq!(conflict_tx.input.len(), 3);
 	assert_eq!(conflict_tx.input[0].previous_output.txid, node2_commit_tx.compute_txid());
 	assert_eq!(conflict_tx.input[1].previous_output.txid, node2_commit_tx.compute_txid());