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

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;
 

fuzz/src/chanmon_consistency.rs

@@ -408,7 +408,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
 	loop {
  macro_rules! send_payment {
- ($source: expr, $dest: expr) => { {
+ ($source: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -417,15 +417,15 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
  // Probably ran out of funds
  test_return!();
  }
  } };
- ($source: expr, $middle: expr, $dest: expr) => { {
+ ($source: expr, $middle: expr, $dest: expr, $amt: expr) => { {
  let payment_hash = Sha256::hash(&[payment_id; 1]);
  payment_id = payment_id.wrapping_add(1);
  if let Err(_) = $source.send_payment(&Route {
@@ -441,7 +441,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  node_features: NodeFeatures::empty(),
  short_channel_id: $dest.1,
  channel_features: ChannelFeatures::empty(),
- fee_msat: 5000000,
+ fee_msat: $amt,
  cltv_expiry_delta: 200,
  }]],
  }, PaymentHash(payment_hash.into_inner()), &None) {
@@ -644,12 +644,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  });
  for event in events.drain(..) {
  match event {
- events::Event::PaymentReceived { payment_hash, payment_secret, .. } => {
+ events::Event::PaymentReceived { payment_hash, payment_secret, amt } => {
  if claim_set.insert(payment_hash.0) {
  if $fail {
  assert!(nodes[$node].fail_htlc_backwards(&payment_hash, &payment_secret));
  } else {
- assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, 5_000_000));
+ assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), &payment_secret, amt));
  }
  }
  },
@@ -691,12 +691,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  nodes[2].channel_monitor_updated(&chan_2_funding, *id);
  }
  },
- 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a)),
- 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a)),
- 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b)),
- 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b)),
- 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
- 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x09 => send_payment!(nodes[0], (&nodes[1], chan_a), 5_000_000),
+ 0x0a => send_payment!(nodes[1], (&nodes[0], chan_a), 5_000_000),
+ 0x0b => send_payment!(nodes[1], (&nodes[2], chan_b), 5_000_000),
+ 0x0c => send_payment!(nodes[2], (&nodes[1], chan_b), 5_000_000),
+ 0x0d => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 5_000_000),
+ 0x0e => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 5_000_000),
  0x0f => {
  if !chan_a_disconnected {
  nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
@@ -781,6 +781,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
  },
  0x22 => send_payment_with_secret!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b)),
  0x23 => send_payment_with_secret!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a)),
+ 0x25 => send_payment!(nodes[0], (&nodes[1], chan_a), 10),
+ 0x26 => send_payment!(nodes[1], (&nodes[0], chan_a), 10),
+ 0x27 => send_payment!(nodes[1], (&nodes[2], chan_b), 10),
+ 0x28 => send_payment!(nodes[2], (&nodes[1], chan_b), 10),
+ 0x29 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 10),
+ 0x2a => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 10),
+ 0x2b => send_payment!(nodes[0], (&nodes[1], chan_a), 1_000),
+ 0x2c => send_payment!(nodes[1], (&nodes[0], chan_a), 1_000),
+ 0x2d => send_payment!(nodes[1], (&nodes[2], chan_b), 1_000),
+ 0x2e => send_payment!(nodes[2], (&nodes[1], chan_b), 1_000),
+ 0x2f => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 1_000),
+ 0x30 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 1_000),
+ 0x31 => send_payment!(nodes[0], (&nodes[1], chan_a), 100_000),
+ 0x32 => send_payment!(nodes[1], (&nodes[0], chan_a), 100_000),
+ 0x33 => send_payment!(nodes[1], (&nodes[2], chan_b), 100_000),
+ 0x34 => send_payment!(nodes[2], (&nodes[1], chan_b), 100_000),
+ 0x35 => send_payment!(nodes[0], (&nodes[1], chan_a), (&nodes[2], chan_b), 100_000),
+ 0x36 => send_payment!(nodes[2], (&nodes[1], chan_b), (&nodes[0], chan_a), 100_000),
  // 0x24 defined above
  _ => test_return!(),
  }

lightning/src/ln/channel.rs

@@ -44,6 +44,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 their_dust_limit_msat: u64,
 }
 
 enum InboundHTLCRemovalReason {
@@ -1663,8 +1664,83 @@ 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)
 	}
 
-	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) {
+	// Get the fee cost of a commitment tx with a given number of HTLC outputs.
+	// Note that num_htlcs should not include dust HTLCs.
+	fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+ // Note that we need to divide before multiplying to round properly,
+ // since the lowest denomination of bitcoin on-chain is the satoshi.
+ (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000
+	}
+
+	// Get the commitment tx fee for the local (i.e our) next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in our next
+	// commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust
+	// HTLCs are excluded.
+	fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(self.channel_outbound);
+
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis {
+ continue
+ }
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ for htlc in self.holding_cell_htlc_updates.iter() {
+ match htlc {
+ &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	// Get the commitment tx fee for the remote's next commitment transaction
+	// based on the number of pending HTLCs that are on track to be in their
+	// next commitment tx. `addl_htcs` is an optional parameter allowing the caller
+	// to add a number of additional HTLCs to the calculation. Note that dust HTLCs
+	// are excluded.
+	fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
+ assert!(!self.channel_outbound);
+
+ // 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.
+ let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
+ for ref htlc in self.pending_outbound_htlcs.iter() {
+ if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis {
+ continue
+ }
+ // We only include outbound HTLCs if it will not be included in their next
+ // commitment_signed, i.e. if they've responded to us with an RAA after announcement.
+ match htlc.state {
+ OutboundHTLCState::Committed => their_acked_htlcs += 1,
+ OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1,
+ _ => {},
+ }
+ }
+
+ self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs)
+	}
+
+	pub fn update_add_htlc<F>(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F) -> Result<(), ChannelError>
+	where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus {
+ // We can't accept HTLCs sent after we've sent a shutdown.
+ let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if local_sent_shutdown {
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20);
+ }
+ // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
+ let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32);
+ if remote_sent_shutdown {
  return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state"));
  }
  if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -1708,9 +1784,55 @@ 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_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
- return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
+
+ let pending_value_to_self_msat =
+ self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat;
+ let pending_remote_value_msat =
+ self.channel_value_satoshis * 1000 - pending_value_to_self_msat;
+ if pending_remote_value_msat < msg.amount_msat {
+ return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds"));
+ }
+
+ // Check that the remote can afford to pay for this HTLC on-chain at the current
+ // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
+ let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+ // +1 for this HTLC.
+ self.next_remote_commit_tx_fee_msat(1)
+ };
+ if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees"));
+ };
+
+ let chan_reserve_msat =
+ Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+ if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+ return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value"));
+ }
+
+ if !self.channel_outbound {
+ // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
+ // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
+ // only on the sender's.
+ // Note that when we eventually remove support for fee updates and switch to anchor output fees,
+ // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1
+ // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of
+ // being sensitive to fee spikes.
+ let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1);
+ if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+ // Note that if the pending_forward_status is not updated here, then it's because we're already failing
+ // the HTLC, i.e. its status is already set to failing.
+ pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7);
+ }
+ } else {
+ // Check that they won't violate our local required channel reserve by adding this HTLC.
+
+ // +1 for this HTLC.
+ let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1);
+ if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+ return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value"));
+ }
  }
+
  if self.next_remote_htlc_id != msg.htlc_id {
  return Err(ChannelError::Close("Remote skipped HTLC ID"));
  }
@@ -1719,7 +1841,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  }
 
  if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
- if let PendingHTLCStatus::Forward(_) = pending_forward_state {
+ if let PendingHTLCStatus::Forward(_) = pending_forward_status {
  panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");
  }
  }
@@ -1731,7 +1853,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  amount_msat: msg.amount_msat,
  payment_hash: msg.payment_hash,
  cltv_expiry: msg.cltv_expiry,
- state: InboundHTLCState::RemoteAnnounced(pending_forward_state),
+ state: InboundHTLCState::RemoteAnnounced(pending_forward_status),
  });
  Ok(())
 	}
@@ -3036,6 +3158,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
  res
  },
  their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+ their_dust_limit_msat: self.their_dust_limit_satoshis * 1000,
  }
 	}
 
@@ -3546,40 +3669,67 @@ 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"));
  }
 
+ if !self.channel_outbound {
+ // Check that we won't violate the remote channel reserve by adding this HTLC.
+
+ let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
+ let remote_chan_reserve_msat = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
+ // 1 additional HTLC corresponding to this HTLC.
+ let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1);
+ if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value"));
+ }
+ }
+
+ let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat;
+ if pending_value_to_self_msat < amount_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds"));
+ }
+
+ // The `+1` is for the HTLC currently being added to the commitment tx and
+ // the `2 *` and `+1` are for the fee spike buffer.
+ let local_commit_tx_fee_msat = if self.channel_outbound {
+ 2 * self.next_local_commit_tx_fee_msat(1 + 1)
+ } else { 0 };
+ if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat {
+ return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees"));
+ }
+
  // Check self.local_channel_reserve_satoshis (the amount we must keep as
  // reserve for the remote to have something to claim if we misbehave)
- if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
+ let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000;
+ if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat {
  return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
  }
 
  // Now update local state:
  if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) {
  self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC {
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
  source,
- onion_routing_packet: onion_routing_packet,
+ onion_routing_packet,
  });
  return Ok(None);
  }
 
  self.pending_outbound_htlcs.push(OutboundHTLCOutput {
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
+ amount_msat,
  payment_hash: payment_hash.clone(),
- cltv_expiry: cltv_expiry,
+ cltv_expiry,
  state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())),
  source,
  });
 
  let res = msgs::UpdateAddHTLC {
  channel_id: self.channel_id,
  htlc_id: self.next_local_htlc_id,
- amount_msat: amount_msat,
- payment_hash: payment_hash,
- cltv_expiry: cltv_expiry,
- onion_routing_packet: onion_routing_packet,
+ amount_msat,
+ payment_hash,
+ cltv_expiry,
+ onion_routing_packet,
  };
  self.next_local_htlc_id += 1;