Incl tx fee when calcing inbound+outbound HTLC limits on channels

When we receive an inbound HTLC from a peer on an inbound channel,
the peer needs to be able to pay for the additional commitment tx
fees as well.

When we're sending an outbound HTLC on an outbound channel, we need
to be able to pay for the additional commitment tx fees as well.

Co-authored-by: Matt Corallo <[email protected]>
Co-authored-by: Valentine Wallace <[email protected]>

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -46,6 +46,7 @@ pub struct ChannelValueStat {
 	pub pending_inbound_htlcs_amount_msat: u64,
 	pub holding_cell_outbound_amount_msat: u64,
 	pub their_max_htlc_value_in_flight_msat: u64, // outgoing
+	pub commit_tx_fee_outbound: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1656,6 +1657,54 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
 	}
 
+	fn commit_tx_fee_outbound_htlc(&self) -> u64 {
+		if !self.channel_outbound {
+			return 0;
+		} else {
+			let mut their_acked_htlcs = self.pending_inbound_htlcs.len() as u64;
+
+			for ref htlc in self.pending_outbound_htlcs.iter() {
+				match htlc.state {
+					OutboundHTLCState::Committed => their_acked_htlcs +=1,
+					OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs +=1,
+					OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+					_ => {},
+				}
+			}
+
+			for ref htlc in self.holding_cell_htlc_updates.iter() {
+				match htlc {
+					&&HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+					_ => {},
+				}
+			}
+
+			return self.feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + their_acked_htlcs * COMMITMENT_TX_WEIGHT_PER_HTLC);
+		}
+	}
+
+	fn commit_tx_fee_inbound_htlc(&self) -> u64 {
+		if self.channel_outbound {
+			return 0;
+		} else {
+			let mut their_acked_htlcs = self.pending_inbound_htlcs.len() as u64;
+
+			// When calculating the set of HTLCs which will be included in their next
+			// commitment_signed, all inbound HTLCs are included (as all states imply it will be
+			// included) and only committed outbound HTLCs, see below.
+			for ref htlc in self.pending_outbound_htlcs.iter() {
+				if let OutboundHTLCState::Committed = htlc.state {
+					// We only include outbound HTLCs if it will not be included in their next
+					// commitment_signed, ie if they've responded to us with an RAA after announcement
+					// and if they haven't yet started removal.
+					their_acked_htlcs += 1;
+				}
+			}
+
+			return self.feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + their_acked_htlcs * COMMITMENT_TX_WEIGHT_PER_HTLC);
+		}
+	}
+
 	pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError> {
 		if (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32) {
 			return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
@@ -1701,7 +1750,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				removed_outbound_total_msat += htlc.amount_msat;
 			}
 		}
-		if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
+		let remote_fee_cost = self.commit_tx_fee_inbound_htlc();
+		if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat - remote_fee_cost {
 			return Err(ChannelError::Close("Remote HTLC add would put them over their reserve value"));
 		}
 		if self.next_remote_htlc_id != msg.htlc_id {
@@ -3031,6 +3081,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				res
 			},
 			their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+			commit_tx_fee_outbound: self.commit_tx_fee_outbound_htlc(),
 		}
 	}
 
@@ -3536,7 +3587,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		// Check self.their_channel_reserve_satoshis (the amount we must keep as
 		// reserve for them to have something to claim if we misbehave)
-		if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+		if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat + self.commit_tx_fee_outbound_htlc() {
 			return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value"));
 		}
 

lightning/src/ln/functional_tests.rs

@@ -1577,7 +1577,7 @@ fn do_channel_reserve_test(test_recv: bool) {
 	// nodes[0]'s wealth
 	loop {
 		let amt_msat = recv_value_0 + total_fee_msat;
-		if stat01.value_to_self_msat - amt_msat < stat01.channel_reserve_msat {
+		if stat01.value_to_self_msat - amt_msat < stat01.channel_reserve_msat + stat01.commit_tx_fee_outbound {
 			break;
 		}
 		send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_0, recv_value_0);
@@ -1598,7 +1598,7 @@ fn do_channel_reserve_test(test_recv: bool) {
 	}
 
 	{
-		let recv_value = stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat;
+		let recv_value = stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat - stat01.commit_tx_fee_outbound;
 		// attempt to get channel_reserve violation
 		let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value + 1);
 		let err = nodes[0].node.send_payment(route.clone(), our_payment_hash).err().unwrap();
@@ -1611,7 +1611,8 @@ fn do_channel_reserve_test(test_recv: bool) {
 	}
 
 	// adding pending output
-	let recv_value_1 = (stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat)/2;
+	let commit_tx_fee_1_acked_htlc = get_feerate!(nodes[0], chan_1.2) * COMMITMENT_TX_WEIGHT_PER_HTLC;
+	let recv_value_1 = (stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat - stat01.commit_tx_fee_outbound - commit_tx_fee_1_acked_htlc)/2;
 	let amt_msat_1 = recv_value_1 + total_fee_msat;
 
 	let (route_1, our_payment_hash_1, our_payment_preimage_1) = get_route_and_payment_hash!(recv_value_1);
@@ -1626,7 +1627,7 @@ fn do_channel_reserve_test(test_recv: bool) {
 	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event_1.msgs[0]);
 
 	// channel reserve test with htlc pending output > 0
-	let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat;
+	let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat - stat01.commit_tx_fee_outbound - commit_tx_fee_1_acked_htlc;
 	{
 		let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1);
 		match nodes[0].node.send_payment(route, our_payment_hash).err().unwrap() {
@@ -1638,8 +1639,10 @@ fn do_channel_reserve_test(test_recv: bool) {
 	}
 
 	{
-		// test channel_reserve test on nodes[1] side
-		let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1);
+		// test channel_reserve test on nodes[1] side. The recv_value_1 HTLC hasn't been promoted to Committed yet,
+		// so it won't be included in the channel reserve check. Therefore, to trigger a channel close we'll have
+		// to add the extra HTLC commit tx fee back in.
+		let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + commit_tx_fee_1_acked_htlc + 1);
 
 		// Need to manually create update_add_htlc message to go around the channel reserve check in send_htlc()
 		let secp_ctx = Secp256k1::new();
@@ -1656,7 +1659,7 @@ fn do_channel_reserve_test(test_recv: bool) {
 		let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash);
 		let msg = msgs::UpdateAddHTLC {
 			channel_id: chan_1.2,
-			htlc_id,
+			htlc_id: htlc_id + 1,
 			amount_msat: htlc_msat,
 			payment_hash: our_payment_hash,
 			cltv_expiry: htlc_cltv,
@@ -1676,11 +1679,12 @@ fn do_channel_reserve_test(test_recv: bool) {
 	}
 
 	// split the rest to test holding cell
-	let recv_value_21 = recv_value_2/2;
-	let recv_value_22 = recv_value_2 - recv_value_21 - total_fee_msat;
+	let commit_tx_fee_1_holding_cell_htlc = get_feerate!(nodes[0], chan_1.2) * COMMITMENT_TX_WEIGHT_PER_HTLC;
+	let recv_value_21 = recv_value_2/2 - commit_tx_fee_1_holding_cell_htlc/2;
+	let recv_value_22 = recv_value_2 - recv_value_21 - total_fee_msat - commit_tx_fee_1_holding_cell_htlc;
 	{
 		let stat = get_channel_value_stat!(nodes[0], chan_1.2);
-		assert_eq!(stat.value_to_self_msat - (stat.pending_outbound_htlcs_amount_msat + recv_value_21 + recv_value_22 + total_fee_msat + total_fee_msat), stat.channel_reserve_msat);
+		assert_eq!(stat.value_to_self_msat - (stat.pending_outbound_htlcs_amount_msat + recv_value_21 + recv_value_22 + total_fee_msat + total_fee_msat + commit_tx_fee_1_holding_cell_htlc + commit_tx_fee_1_acked_htlc + stat01.commit_tx_fee_outbound), stat.channel_reserve_msat);
 	}
 
 	// now see if they go through on both sides
@@ -1714,6 +1718,7 @@ fn do_channel_reserve_test(test_recv: bool) {
 	let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
 	check_added_monitors!(nodes[1], 1);
 
+	// the pending htlc should be promoted to committed
 	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_revoke_and_ack);
 	check_added_monitors!(nodes[0], 1);
 	let commitment_update_2 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
@@ -1772,13 +1777,27 @@ fn do_channel_reserve_test(test_recv: bool) {
 	claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_21, recv_value_21);
 	claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_22, recv_value_22);
 
-	let expected_value_to_self = stat01.value_to_self_msat - (recv_value_1 + total_fee_msat) - (recv_value_21 + total_fee_msat) - (recv_value_22 + total_fee_msat);
+	let recv_value_3 = commit_tx_fee_1_acked_htlc + commit_tx_fee_1_holding_cell_htlc - total_fee_msat;
+	{
+		let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_3 + 1);
+		let err = nodes[0].node.send_payment(route.clone(), our_payment_hash).err().unwrap();
+		match err {
+			APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"),
+			_ => panic!("Unknown error variants"),
+		}
+		assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+		nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over their reserve value".to_string(), 4);
+	}
+
+	send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_3, recv_value_3);
+
+	let expected_value_to_self = stat01.value_to_self_msat - (recv_value_1 + total_fee_msat) - (recv_value_21 + total_fee_msat) - (recv_value_22 + total_fee_msat) - (recv_value_3 + total_fee_msat);
 	let stat0 = get_channel_value_stat!(nodes[0], chan_1.2);
 	assert_eq!(stat0.value_to_self_msat, expected_value_to_self);
-	assert_eq!(stat0.value_to_self_msat, stat0.channel_reserve_msat);
+	assert_eq!(stat0.value_to_self_msat, stat0.channel_reserve_msat + stat0.commit_tx_fee_outbound);
 
 	let stat2 = get_channel_value_stat!(nodes[2], chan_2.2);
-	assert_eq!(stat2.value_to_self_msat, stat22.value_to_self_msat + recv_value_1 + recv_value_21 + recv_value_22);
+	assert_eq!(stat2.value_to_self_msat, stat22.value_to_self_msat + recv_value_1 + recv_value_21 + recv_value_22 + recv_value_3);
 }
 
 #[test]
@@ -5951,15 +5970,18 @@ fn test_update_add_htlc_bolt2_receiver_sender_can_afford_amount_sent() {
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 	let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
 
-	let their_channel_reserve = get_channel_value_stat!(nodes[0], chan.2).channel_reserve_msat;
+	let chan_stat = get_channel_value_stat!(nodes[0], chan.2);
+	let their_channel_reserve = chan_stat.channel_reserve_msat;
+	let commit_tx_fee = chan_stat.commit_tx_fee_outbound;
 
-	let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 5000000-their_channel_reserve, TEST_FINAL_CLTV).unwrap();
+	let max_can_send = 5000000 - their_channel_reserve - commit_tx_fee;
+	let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], max_can_send, TEST_FINAL_CLTV).unwrap();
 	let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
 	nodes[0].node.send_payment(route, our_payment_hash).unwrap();
 	check_added_monitors!(nodes[0], 1);
 	let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
 
-	updates.update_add_htlcs[0].amount_msat = 5000000-their_channel_reserve+1;
+	updates.update_add_htlcs[0].amount_msat = max_can_send + 1;
 	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
 
 	assert!(nodes[1].node.list_channels().is_empty());
@@ -6043,7 +6065,7 @@ fn test_update_add_htlc_bolt2_receiver_check_cltv_expiry() {
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::supported(), InitFeatures::supported());
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 94000000, InitFeatures::supported(), InitFeatures::supported());
 	let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 3999999, TEST_FINAL_CLTV).unwrap();
 	let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
 	nodes[0].node.send_payment(route, our_payment_hash).unwrap();