Impl fee spike reserve + incl commit tx fee in channel reserve calculation

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.

+ implement fee spike reserve for channel initiators sending payments.
From lightning-rfc PR #740.

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,50 @@ 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 htlc_count_next_local_commit_tx(&self) -> u64 {
+		assert!(self.channel_outbound);
+
+		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,
+				_ => {},
+			}
+		}
+
+		their_acked_htlcs
+	}
+
+	fn htlc_count_next_remote_commit_tx(&self) -> u64 {
+		assert!(!self.channel_outbound);
+
+		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() {
+			// 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 the removal process hasn't been finalized.
+			match htlc.state {
+				OutboundHTLCState::Committed => their_acked_htlcs +=1,
+				OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs +=1,
+				_ => {},
+			}
+		}
+
+		their_acked_htlcs
+	}
+
 	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,8 +1746,10 @@ 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 {
-			return Err(ChannelError::Close("Remote HTLC add would put them over their reserve value"));
+		// The + 1 is for the HTLC that is currently being added to the commitment tx.
+		let remote_fee_cost_msat = if self.channel_outbound { 0 } else { (self.feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (self.htlc_count_next_remote_commit_tx() + 1) * COMMITMENT_TX_WEIGHT_PER_HTLC)) };
+		if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat + remote_fee_cost_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
+			return Err(ChannelError::Ignore("Remote HTLC add would put them over their reserve value"));
 		}
 		if self.next_remote_htlc_id != msg.htlc_id {
 			return Err(ChannelError::Close("Remote skipped HTLC ID"));
@@ -3031,6 +3078,9 @@ 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: if self.channel_outbound {
+				(self.feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (self.htlc_count_next_local_commit_tx() + 1) * COMMITMENT_TX_WEIGHT_PER_HTLC))
+			} else { 0 },
 		}
 	}
 
@@ -3534,9 +3584,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
 		}
 
+		// Add additional reserve that avoids stuck channels in the case of fee spikes.
+		let fee_spike_reserve = if self.channel_outbound { 2 * COMMITMENT_TX_WEIGHT_PER_HTLC * self.feerate_per_kw } else { 0 };
+
+		// The + 1 is for the HTLC currently being added to the commitment tx.
+		let local_fee_cost_msat = if self.channel_outbound { (self.feerate_per_kw * ((COMMITMENT_TX_BASE_WEIGHT + (self.htlc_count_next_local_commit_tx() + 1) * COMMITMENT_TX_WEIGHT_PER_HTLC))) } else { 0 };
+
 		// 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 + local_fee_cost_msat + fee_spike_reserve {
 			return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value"));
 		}