Claim HTLC output on-chain if preimage is recv'd after force-close

If we receive a preimage for an outgoing HTLC on a force-closed channel,
we need to claim the HTLC output on-chain.

Co-authored-by: Antoine Riard <[email protected]>
Co-authored-by: Valentine Wallace <[email protected]>

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -128,7 +128,7 @@ impl chain::Watch for TestChainMonitor {
 		};
 		let mut deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 			read(&mut Cursor::new(&map_entry.get().1)).unwrap().1;
-		deserialized_monitor.update_monitor(&update, &&TestBroadcaster {}, &self.logger).unwrap();
+		deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap();
 		let mut ser = VecWriter(Vec::new());
 		deserialized_monitor.serialize_for_disk(&mut ser).unwrap();
 		map_entry.insert((update.update_id, ser.0));

lightning/src/chain/chainmonitor.rs

@@ -198,7 +198,7 @@ where C::Target: chain::Filter,
 			},
 			Some(orig_monitor) => {
 				log_trace!(self.logger, "Updating Channel Monitor for channel {}", log_funding_info!(orig_monitor));
-				let update_res = orig_monitor.update_monitor(&update, &self.broadcaster, &self.logger);
+				let update_res = orig_monitor.update_monitor(&update, &self.broadcaster, &self.fee_estimator, &self.logger);
 				if let Err(e) = &update_res {
 					log_error!(self.logger, "Failed to update channel monitor: {:?}", e);
 				}

lightning/src/chain/channelmonitor.rs

@@ -1150,6 +1150,25 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		    L::Target: Logger,
 	{
 		self.payment_preimages.insert(payment_hash.clone(), payment_preimage.clone());
+		// If the channel is force closed, try to claim the output from this preimage
+		if self.lockdown_from_offchain || self.holder_tx_signed {
+			if let Some(txid) = self.current_counterparty_commitment_txid {
+				if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
+					let (htlc_claim_reqs, set_script) = self.get_counterparty_htlc_output_claim_reqs(*commitment_number, txid, None);
+					if set_script {
+						self.counterparty_payment_script = {
+							// Note that the Network here is ignored as we immediately drop the address for the
+							// script_pubkey version
+							let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
+							Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
+						};
+					}
+					for req in htlc_claim_reqs {
+						self.onchain_tx_handler.claim_counterparty_htlc(req, broadcaster, fee_estimator, logger);
+					}
+				}
+			}
+		}
 	}
 
 	pub(crate) fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &L)
@@ -1166,11 +1185,16 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// itself.
 	///
 	/// panics if the given update is not the next update by update_id.
-	pub fn update_monitor<B: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, logger: &L) -> Result<(), MonitorUpdateError>
-		where B::Target: BroadcasterInterface,
-					L::Target: Logger,
+	pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError>
+	where B::Target: BroadcasterInterface,
+		    F::Target: FeeEstimator,
+		    L::Target: Logger,
 	{
-		if self.latest_update_id + 1 != updates.update_id {
+		// ChannelMonitor updates may be applied after force close if we receive a
+		// preimage for a broadcasted counterparty HTLC output that we'd like to
+		// claim on-chain. If this is the case, we no longer have guaranteed access
+		// to the monitor's update ID, so we use a sentinel value instead.
+		if updates.update_id != std::u64::MAX && self.latest_update_id + 1 != updates.update_id {
 			panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!");
 		}
 		for update in updates.updates.iter() {
@@ -1438,39 +1462,61 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 				check_htlc_fails!(txid, "previous", 'prev_loop);
 			}
 
+			let (htlc_claim_reqs, set_script) = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx));
+			if set_script {
+				self.counterparty_payment_script = {
+					// Note that the Network here is ignored as we immediately drop the address for the
+					// script_pubkey version
+					let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
+					Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
+				};
+			}
+			for req in htlc_claim_reqs {
+				claimable_outpoints.push(req);
+			}
+
+		}
+		(claimable_outpoints, (commitment_txid, watch_outputs))
+	}
+
+	fn get_counterparty_htlc_output_claim_reqs(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>) -> (Vec<ClaimRequest>, bool) {
+		let mut claims = Vec::new();
+		let mut set_counterparty_payment_script = false;
+		if let Some(htlc_outputs) = self.counterparty_claimable_outpoints.get(&commitment_txid) {
 			if let Some(revocation_points) = self.their_cur_revocation_points {
 				let revocation_point_option =
 					if revocation_points.0 == commitment_number { Some(&revocation_points.1) }
-					else if let Some(point) = revocation_points.2.as_ref() {
-						if revocation_points.0 == commitment_number + 1 { Some(point) } else { None }
-					} else { None };
+				else if let Some(point) = revocation_points.2.as_ref() {
+					if revocation_points.0 == commitment_number + 1 { Some(point) } else { None }
+				} else { None };
 				if let Some(revocation_point) = revocation_point_option {
-					self.counterparty_payment_script = {
-						// Note that the Network here is ignored as we immediately drop the address for the
-						// script_pubkey version
-						let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
-						Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
-					};
+					set_counterparty_payment_script = true;
 
 					// Then, try to find htlc outputs
-					for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() {
+					for (_, &(ref htlc, _)) in htlc_outputs.iter().enumerate() {
 						if let Some(transaction_output_index) = htlc.transaction_output_index {
-							if transaction_output_index as usize >= tx.output.len() ||
-									tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
-								return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user
+							if let Some(transaction) = tx {
+								if transaction_output_index as usize >= transaction.output.len() ||
+									transaction.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
+										return (claims, set_counterparty_payment_script) // Corrupted per_commitment_data, fuck this user
+									}
 							}
-							let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
+							let preimage = if htlc.offered {
+								if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) {
+									Some(*p)
+								} else { None }
+							} else { None };
 							let aggregable = if !htlc.offered { false } else { true };
 							if preimage.is_some() || !htlc.offered {
 								let witness_data = InputMaterial::CounterpartyHTLC { per_commitment_point: *revocation_point, counterparty_delayed_payment_base_key: self.counterparty_tx_cache.counterparty_delayed_payment_base_key, counterparty_htlc_base_key: self.counterparty_tx_cache.counterparty_htlc_base_key, preimage, htlc: htlc.clone() };
-								claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
+								claims.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
 							}
 						}
 					}
 				}
 			}
 		}
-		(claimable_outpoints, (commitment_txid, watch_outputs))
+		(claims, set_counterparty_payment_script)
 	}
 
 	/// Attempts to claim a counterparty HTLC-Success/HTLC-Timeout's outputs using the revocation key
@@ -2499,16 +2545,18 @@ mod tests {
 	use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 	use ln::chan_utils;
 	use ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction};
-	use util::test_utils::TestLogger;
+	use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
 	use bitcoin::secp256k1::key::{SecretKey,PublicKey};
 	use bitcoin::secp256k1::Secp256k1;
-	use std::sync::Arc;
+	use std::sync::{Arc, Mutex};
 	use chain::keysinterface::InMemoryChannelKeys;
 
 	#[test]
 	fn test_prune_preimages() {
 		let secp_ctx = Secp256k1::new();
 		let logger = Arc::new(TestLogger::new());
+		let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
+		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 });
 
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
@@ -2584,7 +2632,7 @@ mod tests {
 		monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
 		monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
 		for &(ref preimage, ref hash) in preimages.iter() {
-			monitor.provide_payment_preimage(hash, preimage);
+			monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger);
 		}
 
 		// Now provide a secret, pruning preimages 10-15

lightning/src/ln/functional_tests.rs

@@ -3523,7 +3523,7 @@ fn test_force_close_fail_back() {
 	{
 		let mut monitors = nodes[2].chain_monitor.chain_monitor.monitors.lock().unwrap();
 		monitors.get_mut(&OutPoint{ txid: Txid::from_slice(&payment_event.commitment_msg.channel_id[..]).unwrap(), index: 0 }).unwrap()
-			.provide_payment_preimage(&our_payment_hash, &our_payment_preimage);
+			.provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &node_cfgs[2].fee_estimator, &&logger);
 	}
 	connect_block(&nodes[2], &block, 1);
 	let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap();
@@ -8503,3 +8503,119 @@ fn test_htlc_no_detection() {
         connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1, 201, true, header_201.block_hash());
         expect_payment_failed!(nodes[0], our_payment_hash, true);
 }
+
+#[test]
+fn test_unordered_onchain_htlc_settlement() {
+	// If we route an HTLC, then learn the HTLC's preimage after the upstream
+	// channel has been force-closed by our counterparty, we must claim that HTLC
+	// on-chain. (Given an HTLC forwarded from Alice --> Bob --> Carol, Alice
+	// would be the upstream node, and Carol the downstream.)
+	//
+	// Steps of the test:
+	// 1) Alice sends a HTLC to Carol through Bob.
+	// 2) Carol doesn't settle the HTLC.
+	// 3) Alice force-closes her channel with Bob.
+	// 4) Bob sees the Alice's commitment on his chain. An offered output is present but can't
+	//    be claimed as Bob doesn't have yet knowledge of the preimage.
+	// 5) Carol release the preimage to Bob off-chain.
+	// 6) Bob claims the offered output on Alice's commitment.
+	// TODO: reverse the test with Bob's commitment reaching the chain instead of Alice's one.
+
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+	let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	// Create some initial channels
+	let chan_ab = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001, InitFeatures::known(), InitFeatures::known());
+	create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 10001, InitFeatures::known(), InitFeatures::known());
+
+	// Steps (1) and (2):
+	// Send an HTLC Alice --> Bob --> Carol, but Carol doesn't settle the HTLC back.
+	let (payment_preimage, _payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3_000_000);
+
+	// Check that Alice's commitment transaction now contains an output for this HTLC.
+	let alice_txn = get_local_commitment_txn!(nodes[0], chan_ab.2);
+	check_spends!(alice_txn[0], chan_ab.3);
+	assert_eq!(alice_txn[0].output.len(), 2);
+	check_spends!(alice_txn[1], alice_txn[0]); // 2nd transaction is a non-final HTLC-timeout
+	assert_eq!(alice_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+	assert_eq!(alice_txn.len(), 2);
+
+	// Steps (3) and (4):
+	// Broadcast Alice's commitment transaction and check that Bob responds by (1) broadcasting
+	// a channel update and (2) adding a new ChannelMonitor.
+	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
+	connect_block(&nodes[1], &Block { header, txdata: vec![alice_txn[0].clone()]}, 1);
+	check_closed_broadcast!(nodes[1], false);
+	check_added_monitors!(nodes[1], 1);
+	{
+		let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); // ChannelManager : 1 (commitment tx)
+		assert_eq!(bob_txn.len(), 1);
+		check_spends!(bob_txn[0], chan_ab.3);
+		bob_txn.clear();
+	}
+
+	// Step (5):
+	// Carol then claims the funds and sends an update_fulfill message to Bob, and they go
+	// through the process of removing the HTLC from their commitment transactions.
+	assert!(nodes[2].node.claim_funds(payment_preimage, &None, 3_000_000));
+	check_added_monitors!(nodes[2], 1);
+	let carol_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
+	assert!(carol_updates.update_add_htlcs.is_empty());
+	assert!(carol_updates.update_fail_htlcs.is_empty());
+	assert!(carol_updates.update_fail_malformed_htlcs.is_empty());
+	assert!(carol_updates.update_fee.is_none());
+	assert_eq!(carol_updates.update_fulfill_htlcs.len(), 1);
+
+	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &carol_updates.update_fulfill_htlcs[0]);
+	nodes[1].node.handle_commitment_signed(&nodes[2].node.get_our_node_id(), &carol_updates.commitment_signed);
+	// One monitor update for the preimage, one monitor update for the new commitment tx info
+	check_added_monitors!(nodes[1], 2);
+
+	let events = nodes[1].node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 2);
+	let bob_revocation = match events[0] {
+		MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+			assert_eq!(*node_id, nodes[2].node.get_our_node_id());
+			(*msg).clone()
+		},
+		_ => panic!("Unexpected event"),
+	};
+	let bob_updates = match events[1] {
+		MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+			assert_eq!(*node_id, nodes[2].node.get_our_node_id());
+			(*updates).clone()
+		},
+		_ => panic!("Unexpected event"),
+	};
+
+	nodes[2].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bob_revocation);
+	check_added_monitors!(nodes[2], 1);
+	nodes[2].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bob_updates.commitment_signed);
+	check_added_monitors!(nodes[2], 1);
+
+	let events = nodes[2].node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 1);
+	let carol_revocation = match events[0] {
+		MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+			assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+			(*msg).clone()
+		},
+		_ => panic!("Unexpected event"),
+	};
+	nodes[1].node.handle_revoke_and_ack(&nodes[2].node.get_our_node_id(), &carol_revocation);
+	check_added_monitors!(nodes[1], 1);
+
+	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+	connect_block(&nodes[1], &Block { header, txdata: vec![] }, 1);
+	// Step (6):
+	// Finally, check that Bob broadcasted a preimage-claiming transaction for the HTLC
+	// output on Alice's broadcasted commitment transaction.
+	{
+		let bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelMonitor : 1 (htlc-preimage tx)
+		assert_eq!(bob_txn.len(), 1);
+		check_spends!(bob_txn[0], alice_txn[0]);
+		assert_eq!(bob_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+	}
+}