[wip] Implement Holder HTLC claim chunking for 0FC channels

Author: tankyleo

lightning/src/events/bump_transaction/mod.rs

@@ -41,6 +41,8 @@ use crate::util::logger::Logger;
 use bitcoin::amount::Amount;
 use bitcoin::consensus::Encodable;
 use bitcoin::constants::WITNESS_SCALE_FACTOR;
+use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::locktime::absolute::LockTime;
 use bitcoin::secp256k1;
 use bitcoin::secp256k1::ecdsa::Signature;
@@ -983,7 +985,7 @@ where
 			// Our estimate should be within a 1% error margin of the actual weight and we should
 			// never underestimate.
 			assert!(expected_signed_tx_weight >= signed_tx_weight);
-			assert!(expected_signed_tx_weight * 99 / 100 <= signed_tx_weight);
+			assert!(expected_signed_tx_weight * 98 / 100 <= signed_tx_weight);
 
 			let expected_signed_tx_fee =
 				fee_for_weight(target_feerate_sat_per_1000_weight, signed_tx_weight);
@@ -1045,20 +1047,63 @@ where
 					log_bytes!(claim_id.0),
 					log_iter!(htlc_descriptors.iter().map(|d| d.outpoint()))
 				);
-				self.handle_htlc_resolution(
-					*claim_id,
-					*target_feerate_sat_per_1000_weight,
-					htlc_descriptors,
-					*tx_lock_time,
-				)
-				.await
-				.unwrap_or_else(|_| {
-					log_error!(
+				let channel_type = &htlc_descriptors[0]
+					.channel_derivation_parameters
+					.transaction_parameters
+					.channel_type_features;
+				let htlc_chunks_size = if channel_type.supports_anchor_zero_fee_commitments() {
+					// Cap the size of transactions claiming `HolderHTLCOutput` in 0FC channels.
+					// Otherwise, we could hit the max 10_000vB size limit on V3 transactions
+					// (BIP 431 rule 4).
+					25
+				} else {
+					htlc_descriptors.len()
+				};
+				for htlc_descriptors in htlc_descriptors.chunks(htlc_chunks_size) {
+					let utxo_id = if channel_type.supports_anchor_zero_fee_commitments() {
+						// Generate a new claim_id to map a user-provided utxo to this
+						// particular set of HTLCs via `select_confirmed_utxos`; matches the
+						// scheme used in `onchain`.
+						//
+						// In rare cases, a HTLC set can change if a counterparty claims a
+						// subset of the HTLCs in the set. In that case we regenerate a new set
+						// of HTLCs, a new claim_id, and ask the user for a new UTXO to confirm
+						// this new set. Users should be able to provide the UTXO that was
+						// previously assigned to the now double-spent transaction to confirm
+						// this new set.
+						let mut engine = Sha256::engine();
+						for htlc in htlc_descriptors {
+							engine.input(&htlc.commitment_txid.to_byte_array());
+							engine
+								.input(&htlc.htlc.transaction_output_index.unwrap().to_be_bytes());
+						}
+						ClaimId(Sha256::from_engine(engine).to_byte_array())
+					} else {
+						// Non-0FC channels batch the full set of HTLCs in the claim into a
+						// single transaction, so we re-use the claim_id as the UTXO id here.
+						*claim_id
+					};
+					log_info!(
 						self.logger,
-						"Failed bumping HTLC transaction fee for commitment {}",
-						htlc_descriptors[0].commitment_txid
+						"Batch transaction assigned to UTXO id {} contains these HTLCs: {}",
+						log_bytes!(utxo_id.0),
+						log_iter!(htlc_descriptors.iter().map(|d| d.outpoint()))
 					);
-				});
+					self.handle_htlc_resolution(
+						utxo_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
+						);
+					});
+				}
 			},
 		}
 	}

lightning/src/ln/zero_fee_commitment_tests.rs

@@ -1,5 +1,7 @@
+use crate::events::{ClosureReason, Event};
 use crate::ln::chan_utils::shared_anchor_script_pubkey;
 use crate::ln::functional_test_utils::*;
+use crate::ln::msgs::BaseMessageHandler;
 
 #[test]
 fn test_p2a_anchor_values_under_trims_and_rounds() {
@@ -92,3 +94,128 @@ fn test_p2a_anchor_values_under_trims_and_rounds() {
 	p2a_value_test!([353_000], [353_001], 240);
 	p2a_value_test!([353_001], [353_001], 240);
 }
+
+#[test]
+fn test_htlc_claim_chunking() {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let mut user_cfg = test_default_channel_config();
+	user_cfg.channel_handshake_config.our_htlc_minimum_msat = 1;
+	user_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true;
+	user_cfg.manually_accept_inbound_channels = true;
+
+	let configs = [Some(user_cfg.clone()), Some(user_cfg)];
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &configs);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let coinbase_tx = provide_anchor_reserves(&nodes);
+
+	let _node_a_id = nodes[0].node.get_our_node_id();
+	let _node_b_id = nodes[1].node.get_our_node_id();
+
+	const CHAN_CAPACITY: u64 = 10_000_000;
+	let (_, _, chan_id, _funding_tx) = create_announced_chan_between_nodes_with_value(
+		&nodes,
+		0,
+		1,
+		CHAN_CAPACITY,
+		(CHAN_CAPACITY / 2) * 1000,
+	);
+
+	let mut node_1_preimages = Vec::new();
+	const NONDUST_HTLC_AMT_MSAT: u64 = 1_000_000;
+	for _ in 0..26 {
+		let (preimage, payment_hash, _, _) =
+			route_payment(&nodes[0], &[&nodes[1]], NONDUST_HTLC_AMT_MSAT);
+		node_1_preimages.push((preimage, payment_hash));
+	}
+	let node_0_commit_tx = get_local_commitment_txn!(nodes[0], chan_id);
+	assert_eq!(node_0_commit_tx.len(), 1);
+	assert_eq!(node_0_commit_tx[0].output.len(), 26 + 2 + 1);
+	let node_1_commit_tx = get_local_commitment_txn!(nodes[1], chan_id);
+	assert_eq!(node_1_commit_tx.len(), 1);
+	assert_eq!(node_1_commit_tx[0].output.len(), 26 + 2 + 1);
+
+	for (preimage, payment_hash) in node_1_preimages {
+		nodes[1].node.claim_funds(preimage);
+		check_added_monitors!(nodes[1], 1);
+		expect_payment_claimed!(nodes[1], payment_hash, NONDUST_HTLC_AMT_MSAT);
+	}
+	nodes[0].node.get_and_clear_pending_msg_events();
+	nodes[1].node.get_and_clear_pending_msg_events();
+
+	mine_transaction(&nodes[0], &node_1_commit_tx[0]);
+	mine_transaction(&nodes[1], &node_1_commit_tx[0]);
+
+	let mut events = nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events.pop().unwrap() {
+		Event::BumpTransaction(bump_event) => {
+			nodes[1].bump_tx_handler.handle_event(&bump_event);
+		},
+		_ => panic!("Unexpected event"),
+	}
+
+	let htlc_claims = nodes[1].tx_broadcaster.txn_broadcast();
+	assert_eq!(htlc_claims.len(), 2);
+
+	check_spends!(htlc_claims[0], node_1_commit_tx[0], coinbase_tx);
+	check_spends!(htlc_claims[1], node_1_commit_tx[0], coinbase_tx);
+
+	assert_eq!(htlc_claims[0].input.len(), 26);
+	assert_eq!(htlc_claims[0].output.len(), 26);
+	assert_eq!(htlc_claims[1].input.len(), 2);
+	assert_eq!(htlc_claims[1].output.len(), 2);
+
+	check_closed_broadcast!(nodes[0], true);
+	check_added_monitors!(nodes[0], 1);
+	check_closed_event!(
+		nodes[0],
+		1,
+		ClosureReason::CommitmentTxConfirmed,
+		[nodes[1].node.get_our_node_id()],
+		CHAN_CAPACITY
+	);
+	assert!(nodes[0].node.list_channels().is_empty());
+	check_closed_broadcast!(nodes[1], true);
+	check_added_monitors!(nodes[1], 1);
+	check_closed_event!(
+		nodes[1],
+		1,
+		ClosureReason::CommitmentTxConfirmed,
+		[nodes[0].node.get_our_node_id()],
+		CHAN_CAPACITY
+	);
+	assert!(nodes[1].node.list_channels().is_empty());
+	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+	assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+
+	mine_transaction(&nodes[1], &htlc_claims[0]);
+
+	let mut events = nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events.pop().unwrap() {
+		Event::BumpTransaction(bump_event) => {
+			nodes[1].bump_tx_handler.handle_event(&bump_event);
+		},
+		_ => panic!("Unexpected event"),
+	}
+
+	let fresh_htlc_claims = nodes[1].tx_broadcaster.txn_broadcast();
+	assert_eq!(fresh_htlc_claims.len(), 1);
+	check_spends!(fresh_htlc_claims[0], node_1_commit_tx[0], htlc_claims[0]);
+	assert_eq!(fresh_htlc_claims[0].input.len(), 2);
+	assert_eq!(fresh_htlc_claims[0].output.len(), 2);
+
+	// Assert when we handled the second `BumpTransaction::HTLCResolution` event, we
+	// assigned the same UTXO id to that set of HTLCs
+	let log_entries = &nodes[1].logger.lines.lock().unwrap();
+	let keys: Vec<_> = log_entries
+		.keys()
+		.filter(|key| key.1.contains("Batch transaction assigned to UTXO id"))
+		.collect();
+	assert_eq!(keys.len(), 2);
+	let values = [*log_entries.get(keys[0]).unwrap(), *log_entries.get(keys[1]).unwrap()];
+	assert!(values.contains(&1));
+	assert!(values.contains(&2));
+}