diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index c5c3c9f3a83..e0a4d654cca 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -408,7 +408,7 @@ pub fn do_test(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,7 +417,7 @@ pub fn do_test(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) { @@ -425,7 +425,7 @@ pub fn do_test(data: &[u8], out: Out) { 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(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(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(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(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!(), } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c1cdac3cecc..f2695a8ff77 100644 --- a/lightning/src/ln/channel.rs +++ b/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 Channel { 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(&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 Channel { 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::::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::::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 Channel { } 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 Channel { 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 Channel { 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,29 +3669,56 @@ impl Channel { 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::::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, }); @@ -3576,10 +3726,10 @@ impl Channel { 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; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3b17813749b..f064802e79b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2472,7 +2472,7 @@ impl //encrypted with the same key. It's not immediately obvious how to usefully exploit that, //but we should prevent it anyway. - let (mut pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg); + let (pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg); let channel_state = &mut *channel_state_lock; match channel_state.by_id.entry(msg.channel_id) { @@ -2480,39 +2480,46 @@ impl if chan.get().get_their_node_id() != *their_node_id { return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id)); } - if !chan.get().is_usable() { + + let create_pending_htlc_status = |chan: &Channel, pending_forward_info: PendingHTLCStatus, error_code: u16| { + // Ensure error_code has the UPDATE flag set, since by default we send a + // channel update along as part of failing the HTLC. + assert!((error_code & 0x1000) != 0); // If the update_add is completely bogus, the call will Err and we will close, // but if we've sent a shutdown and they haven't acknowledged it yet, we just // want to reject the new HTLC and fail it backwards instead of forwarding. - if let PendingHTLCStatus::Forward(PendingHTLCInfo { incoming_shared_secret, .. }) = pending_forward_info { - let chan_update = self.get_channel_update(chan.get()); - pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { - channel_id: msg.channel_id, - htlc_id: msg.htlc_id, - reason: if let Ok(update) = chan_update { - // TODO: Note that |20 is defined as "channel FROM the processing - // node has been disabled" (emphasis mine), which seems to imply - // that we can't return |20 for an inbound channel being disabled. - // This probably needs a spec update but should definitely be - // allowed. - onion_utils::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &{ + match pending_forward_info { + PendingHTLCStatus::Forward(PendingHTLCInfo { ref incoming_shared_secret, .. }) => { + let reason = if let Ok(upd) = self.get_channel_update(chan) { + onion_utils::build_first_hop_failure_packet(incoming_shared_secret, error_code, &{ let mut res = Vec::with_capacity(8 + 128); - res.extend_from_slice(&byte_utils::be16_to_array(update.contents.flags)); - res.extend_from_slice(&update.encode_with_len()[..]); + res.extend_from_slice(&byte_utils::be16_to_array(upd.contents.flags)); + res.extend_from_slice(&upd.encode_with_len()[..]); res }[..]) } else { - // This can only happen if the channel isn't in the fully-funded - // state yet, implying our counterparty is trying to route payments - // over the channel back to themselves (cause no one else should - // know the short_id is a lightning channel yet). We should have no - // problem just calling this unknown_next_peer - onion_utils::build_first_hop_failure_packet(&incoming_shared_secret, 0x4000|10, &[]) - }, - })); + // The only case where we'd be unable to + // successfully get a channel update is if the + // channel isn't in the fully-funded state yet, + // implying our counterparty is trying to route + // payments over the channel back to themselves + // (cause no one else should know the short_id + // is a lightning channel yet). We should have + // no problem just calling this + // unknown_next_peer (0x4000|10). + onion_utils::build_first_hop_failure_packet(incoming_shared_secret, 0x4000|10, &[]) + }; + let msg = msgs::UpdateFailHTLC { + channel_id: msg.channel_id, + htlc_id: msg.htlc_id, + reason + }; + PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msg)) + }, + _ => pending_forward_info } - } - try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info), channel_state, chan); + }; + try_chan_entry!(self, chan.get_mut().update_add_htlc(&msg, pending_forward_info, create_pending_htlc_status), channel_state, chan); }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id)) } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index bd1b1d4f029..6111720177c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1520,14 +1520,220 @@ fn test_duplicate_htlc_different_direction_onchain() { } } -fn do_channel_reserve_test(test_recv: bool) { +#[test] +fn test_basic_channel_reserve() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let logger = test_utils::TestLogger::new(); + + let chan_stat = get_channel_value_stat!(nodes[0], chan.2); + let channel_reserve = chan_stat.channel_reserve_msat; + + // The 2* and +1 are for the fee spike reserve. + let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]); + let commit_tx_fee = 2 * commit_tx_fee_msat(get_feerate!(nodes[0], chan.2), 1 + 1); + let max_can_send = 5000000 - channel_reserve - commit_tx_fee; + let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; + let route = get_route(&nodes[0].node.get_our_node_id(), net_graph_msg_handler, &nodes.last().unwrap().node.get_our_node_id(), None, &Vec::new(), max_can_send + 1, TEST_FINAL_CLTV, &logger).unwrap(); + let err = nodes[0].node.send_payment(&route, our_payment_hash, &None).err().unwrap(); + match err { + PaymentSendFailure::AllFailedRetrySafe(ref fails) => { + match fails[0] { + APIError::ChannelUnavailable{err} => + assert_eq!(err, "Cannot send value that would put us under local channel reserve value"), + _ => panic!("Unexpected error variant"), + } + }, + _ => panic!("Unexpected error variant"), + } + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 1); + + send_payment(&nodes[0], &vec![&nodes[1]], max_can_send, max_can_send); +} + +#[test] +fn test_chan_reserve_violation_outbound_htlc_inbound_chan() { + let mut chanmon_cfgs = create_chanmon_cfgs(2); + // Set the fee rate for the channel very high, to the point where the fundee + // sending any amount would result in a channel reserve violation. In this test + // we check that we would be prevented from sending an HTLC in this situation. + chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 }; + chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 }; + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let logger = test_utils::TestLogger::new(); + + macro_rules! get_route_and_payment_hash { + ($recv_value: expr) => {{ + let (payment_preimage, payment_hash) = get_payment_preimage_hash!(nodes[1]); + let net_graph_msg_handler = &nodes[1].net_graph_msg_handler; + let route = get_route(&nodes[1].node.get_our_node_id(), net_graph_msg_handler, &nodes.first().unwrap().node.get_our_node_id(), None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, &logger).unwrap(); + (route, payment_hash, payment_preimage) + }} + }; + + let (route, our_payment_hash, _) = get_route_and_payment_hash!(1000); + unwrap_send_err!(nodes[1].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot send value that would put them under remote channel reserve value")); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put them under remote channel reserve value".to_string(), 1); +} + +#[test] +fn test_chan_reserve_violation_inbound_htlc_outbound_channel() { + let mut chanmon_cfgs = create_chanmon_cfgs(2); + // Set the fee rate for the channel very high, to the point where the funder + // receiving 1 update_add_htlc would result in them closing the channel due + // to channel reserve violation. This close could also happen if the fee went + // up a more realistic amount, but many HTLCs were outstanding at the time of + // the update_add_htlc. + chanmon_cfgs[0].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 }; + chanmon_cfgs[1].fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 6000 }; + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let logger = test_utils::TestLogger::new(); + + macro_rules! get_route_and_payment_hash { + ($recv_value: expr) => {{ + let (payment_preimage, payment_hash) = get_payment_preimage_hash!(nodes[1]); + let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; + let route = get_route(&nodes[1].node.get_our_node_id(), net_graph_msg_handler, &nodes.first().unwrap().node.get_our_node_id(), None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, &logger).unwrap(); + (route, payment_hash, payment_preimage) + }} + }; + + let (route, payment_hash, _) = get_route_and_payment_hash!(1000); + // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc() + let secp_ctx = Secp256k1::new(); + let session_priv = SecretKey::from_slice(&{ + let mut session_key = [0; 32]; + let mut rng = thread_rng(); + rng.fill_bytes(&mut session_key); + session_key + }).expect("RNG is bad!"); + + let cur_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; + + let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap(); + let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 1000, &None, cur_height).unwrap(); + let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash); + let msg = msgs::UpdateAddHTLC { + channel_id: chan.2, + htlc_id: 1, + amount_msat: htlc_msat + 1, + payment_hash: payment_hash, + cltv_expiry: htlc_cltv, + onion_routing_packet: onion_packet, + }; + + nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &msg); + // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd. + nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot receive value that would put us under local channel reserve value".to_string(), 1); + assert_eq!(nodes[0].node.list_channels().len(), 0); + let err_msg = check_closed_broadcast!(nodes[0], true).unwrap(); + assert_eq!(err_msg.data, "Cannot receive value that would put us under local channel reserve value"); + check_added_monitors!(nodes[0], 1); +} + +#[test] +fn test_chan_reserve_violation_inbound_htlc_inbound_chan() { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); + let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let _ = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let logger = test_utils::TestLogger::new(); + + macro_rules! get_route_and_payment_hash { + ($recv_value: expr) => {{ + let (payment_preimage, payment_hash) = get_payment_preimage_hash!(nodes[0]); + let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; + let route = get_route(&nodes[0].node.get_our_node_id(), net_graph_msg_handler, &nodes.last().unwrap().node.get_our_node_id(), None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, &logger).unwrap(); + (route, payment_hash, payment_preimage) + }} + }; + + let feemsat = 239; + let total_routing_fee_msat = (nodes.len() - 2) as u64 * feemsat; + let chan_stat = get_channel_value_stat!(nodes[0], chan.2); + let feerate = get_feerate!(nodes[0], chan.2); + + // Add a 2* and +1 for the fee spike reserve. + let commit_tx_fee_2_htlc = 2*commit_tx_fee_msat(feerate, 2 + 1); + let recv_value_1 = (chan_stat.value_to_self_msat - chan_stat.channel_reserve_msat - total_routing_fee_msat - commit_tx_fee_2_htlc)/2; + let amt_msat_1 = recv_value_1 + total_routing_fee_msat; + + // Add a pending HTLC. + let (route_1, our_payment_hash_1, _) = get_route_and_payment_hash!(amt_msat_1); + let payment_event_1 = { + nodes[0].node.send_payment(&route_1, our_payment_hash_1, &None).unwrap(); + check_added_monitors!(nodes[0], 1); + + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event_1.msgs[0]); + // Attempt to trigger a channel reserve violation --> payment failure. + let commit_tx_fee_2_htlcs = commit_tx_fee_msat(feerate, 2); + let recv_value_2 = chan_stat.value_to_self_msat - amt_msat_1 - chan_stat.channel_reserve_msat - total_routing_fee_msat - commit_tx_fee_2_htlcs + 1; + let amt_msat_2 = recv_value_2 + total_routing_fee_msat; + let (route_2, _, _) = get_route_and_payment_hash!(amt_msat_2); + + // Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc() + let secp_ctx = Secp256k1::new(); + let session_priv = SecretKey::from_slice(&{ + let mut session_key = [0; 32]; + let mut rng = thread_rng(); + rng.fill_bytes(&mut session_key); + session_key + }).expect("RNG is bad!"); + + let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; + + let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route_2.paths[0], &session_priv).unwrap(); + let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route_2.paths[0], recv_value_2, &None, cur_height).unwrap(); + let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash_1); + let msg = msgs::UpdateAddHTLC { + channel_id: chan.2, + htlc_id: 1, + amount_msat: htlc_msat + 1, + payment_hash: our_payment_hash_1, + cltv_expiry: htlc_cltv, + onion_routing_packet: onion_packet, + }; + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg); + // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd. + nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Remote HTLC add would put them under remote reserve value".to_string(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + let err_msg = check_closed_broadcast!(nodes[1], true).unwrap(); + assert_eq!(err_msg.data, "Remote HTLC add would put them under remote reserve value"); + check_added_monitors!(nodes[1], 1); +} + +fn commit_tx_fee_msat(feerate: u64, num_htlcs: u64) -> u64 { + (COMMITMENT_TX_BASE_WEIGHT + num_htlcs * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate / 1000 * 1000 +} + +#[test] +fn test_channel_reserve_holding_cell_htlcs() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); - let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1900, 1001, InitFeatures::known(), InitFeatures::known()); - let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1900, 1001, InitFeatures::known(), InitFeatures::known()); + let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 190000, 1001, InitFeatures::known(), InitFeatures::known()); + let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 190000, 1001, InitFeatures::known(), InitFeatures::known()); let logger = test_utils::TestLogger::new(); let mut stat01 = get_channel_value_stat!(nodes[0], chan_1.2); @@ -1556,7 +1762,8 @@ fn do_channel_reserve_test(test_recv: bool) { } let feemsat = 239; // somehow we know? - let total_fee_msat = (nodes.len() - 2) as u64 * 239; + let total_fee_msat = (nodes.len() - 2) as u64 * feemsat; + let feerate = get_feerate!(nodes[0], chan_1.2); let recv_value_0 = stat01.their_max_htlc_value_in_flight_msat - total_fee_msat; @@ -1570,16 +1777,19 @@ fn do_channel_reserve_test(test_recv: bool) { nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over the max HTLC value in flight our peer will accept".to_string(), 1); } - let mut htlc_id = 0; // channel reserve is bigger than their_max_htlc_value_in_flight_msat so loop to deplete // nodes[0]'s wealth loop { let amt_msat = recv_value_0 + total_fee_msat; - if stat01.value_to_self_msat - amt_msat < stat01.channel_reserve_msat { + // 3 for the 3 HTLCs that will be sent, 2* and +1 for the fee spike reserve. + // Also, ensure that each payment has enough to be over the dust limit to + // ensure it'll be included in each commit tx fee calculation. + let commit_tx_fee_all_htlcs = 2*commit_tx_fee_msat(feerate, 3 + 1); + let ensure_htlc_amounts_above_dust_buffer = 3 * (stat01.their_dust_limit_msat + 1000); + if stat01.value_to_self_msat < stat01.channel_reserve_msat + commit_tx_fee_all_htlcs + ensure_htlc_amounts_above_dust_buffer + amt_msat { break; } send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_0, recv_value_0); - htlc_id += 1; let (stat01_, stat11_, stat12_, stat22_) = ( get_channel_value_stat!(nodes[0], chan_1.2), @@ -1595,18 +1805,19 @@ fn do_channel_reserve_test(test_recv: bool) { stat01 = stat01_; stat11 = stat11_; stat12 = stat12_; stat22 = stat22_; } - { - let recv_value = stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat; - // attempt to get channel_reserve violation - let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value + 1); - unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, - assert_eq!(err, "Cannot send value that would put us under local channel reserve value")); - assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 1); - } - - // adding pending output - let recv_value_1 = (stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat)/2; + // adding pending output. + // 2* and +1 HTLCs on the commit tx fee for the fee spike reserve. + // The reason we're dividing by two here is as follows: the dividend is the total outbound liquidity + // after fees, the channel reserve, and the fee spike buffer are removed. We eventually want to + // divide this quantity into 3 portions, that will each be sent in an HTLC. This allows us + // to test channel channel reserve policy at the edges of what amount is sendable, i.e. + // cases where 1 msat over X amount will cause a payment failure, but anything less than + // that can be sent successfully. So, dividing by two is a somewhat arbitrary way of getting + // the amount of the first of these aforementioned 3 payments. The reason we split into 3 payments + // is to test the behavior of the holding cell with respect to channel reserve and commit tx fee + // policy. + let commit_tx_fee_2_htlcs = 2*commit_tx_fee_msat(feerate, 2 + 1); + let recv_value_1 = (stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat - commit_tx_fee_2_htlcs)/2; let amt_msat_1 = recv_value_1 + total_fee_msat; let (route_1, our_payment_hash_1, our_payment_preimage_1) = get_route_and_payment_hash!(recv_value_1); @@ -1621,59 +1832,22 @@ fn do_channel_reserve_test(test_recv: bool) { nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event_1.msgs[0]); // channel reserve test with htlc pending output > 0 - let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat; + let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat - commit_tx_fee_2_htlcs; { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1); unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, assert_eq!(err, "Cannot send value that would put us under local channel reserve value")); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 2); - } - - { - // test channel_reserve test on nodes[1] side - let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1); - - // Need to manually create update_add_htlc message to go around the channel reserve check in send_htlc() - let secp_ctx = Secp256k1::new(); - let session_priv = SecretKey::from_slice(&{ - let mut session_key = [0; 32]; - let mut rng = thread_rng(); - rng.fill_bytes(&mut session_key); - session_key - }).expect("RNG is bad!"); - - let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; - let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap(); - let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], recv_value_2 + 1, &None, cur_height).unwrap(); - let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash); - let msg = msgs::UpdateAddHTLC { - channel_id: chan_1.2, - htlc_id, - amount_msat: htlc_msat, - payment_hash: our_payment_hash, - cltv_expiry: htlc_cltv, - onion_routing_packet: onion_packet, - }; - - if test_recv { - nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg); - // If we send a garbage message, the channel should get closed, making the rest of this test case fail. - assert_eq!(nodes[1].node.list_channels().len(), 1); - assert_eq!(nodes[1].node.list_channels().len(), 1); - let err_msg = check_closed_broadcast!(nodes[1], true).unwrap(); - assert_eq!(err_msg.data, "Remote HTLC add would put them under their reserve value"); - check_added_monitors!(nodes[1], 1); - return; - } } // split the rest to test holding cell - let recv_value_21 = recv_value_2/2; - let recv_value_22 = recv_value_2 - recv_value_21 - total_fee_msat; + let commit_tx_fee_3_htlcs = 2*commit_tx_fee_msat(feerate, 3 + 1); + let additional_htlc_cost_msat = commit_tx_fee_3_htlcs - commit_tx_fee_2_htlcs; + let recv_value_21 = recv_value_2/2 - additional_htlc_cost_msat/2; + let recv_value_22 = recv_value_2 - recv_value_21 - total_fee_msat - additional_htlc_cost_msat; { let stat = get_channel_value_stat!(nodes[0], chan_1.2); - assert_eq!(stat.value_to_self_msat - (stat.pending_outbound_htlcs_amount_msat + recv_value_21 + recv_value_22 + total_fee_msat + total_fee_msat), stat.channel_reserve_msat); + assert_eq!(stat.value_to_self_msat - (stat.pending_outbound_htlcs_amount_msat + recv_value_21 + recv_value_22 + total_fee_msat + total_fee_msat + commit_tx_fee_3_htlcs), stat.channel_reserve_msat); } // now see if they go through on both sides @@ -1690,7 +1864,7 @@ fn do_channel_reserve_test(test_recv: bool) { unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, assert_eq!(err, "Cannot send value that would put us under local channel reserve value")); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 3); + nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 2); } let (route_22, our_payment_hash_22, our_payment_preimage_22) = get_route_and_payment_hash!(recv_value_22); @@ -1705,6 +1879,7 @@ fn do_channel_reserve_test(test_recv: bool) { let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); check_added_monitors!(nodes[1], 1); + // the pending htlc should be promoted to committed nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_revoke_and_ack); check_added_monitors!(nodes[0], 1); let commitment_update_2 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); @@ -1765,19 +1940,35 @@ fn do_channel_reserve_test(test_recv: bool) { claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_21, recv_value_21); claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_22, recv_value_22); - let expected_value_to_self = stat01.value_to_self_msat - (recv_value_1 + total_fee_msat) - (recv_value_21 + total_fee_msat) - (recv_value_22 + total_fee_msat); + let commit_tx_fee_0_htlcs = 2*commit_tx_fee_msat(feerate, 1); + let recv_value_3 = commit_tx_fee_2_htlcs - commit_tx_fee_0_htlcs - total_fee_msat; + { + let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_3 + 1); + let err = nodes[0].node.send_payment(&route, our_payment_hash, &None).err().unwrap(); + match err { + PaymentSendFailure::AllFailedRetrySafe(ref fails) => { + match fails[0] { + APIError::ChannelUnavailable{err} => + assert_eq!(err, "Cannot send value that would put us under local channel reserve value"), + _ => panic!("Unexpected error variant"), + } + }, + _ => panic!("Unexpected error variant"), + } + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 3); + } + + send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_3, recv_value_3); + + let commit_tx_fee_1_htlc = 2*commit_tx_fee_msat(feerate, 1 + 1); + let expected_value_to_self = stat01.value_to_self_msat - (recv_value_1 + total_fee_msat) - (recv_value_21 + total_fee_msat) - (recv_value_22 + total_fee_msat) - (recv_value_3 + total_fee_msat); let stat0 = get_channel_value_stat!(nodes[0], chan_1.2); assert_eq!(stat0.value_to_self_msat, expected_value_to_self); - assert_eq!(stat0.value_to_self_msat, stat0.channel_reserve_msat); + assert_eq!(stat0.value_to_self_msat, stat0.channel_reserve_msat + commit_tx_fee_1_htlc); let stat2 = get_channel_value_stat!(nodes[2], chan_2.2); - assert_eq!(stat2.value_to_self_msat, stat22.value_to_self_msat + recv_value_1 + recv_value_21 + recv_value_22); -} - -#[test] -fn channel_reserve_test() { - do_channel_reserve_test(false); - do_channel_reserve_test(true); + assert_eq!(stat2.value_to_self_msat, stat22.value_to_self_msat + recv_value_1 + recv_value_21 + recv_value_22 + recv_value_3); } #[test] @@ -6244,23 +6435,31 @@ fn test_update_add_htlc_bolt2_receiver_sender_can_afford_amount_sent() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 95000000, InitFeatures::known(), InitFeatures::known()); + let logger = test_utils::TestLogger::new(); - let their_channel_reserve = get_channel_value_stat!(nodes[0], chan.2).channel_reserve_msat; + let chan_stat = get_channel_value_stat!(nodes[0], chan.2); + let channel_reserve = chan_stat.channel_reserve_msat; + let feerate = get_feerate!(nodes[0], chan.2); + // The 2* and +1 are for the fee spike reserve. + let commit_tx_fee_outbound = 2 * commit_tx_fee_msat(feerate, 1 + 1); + let max_can_send = 5000000 - channel_reserve - commit_tx_fee_outbound; let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]); let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; - let logger = test_utils::TestLogger::new(); - let route = get_route(&nodes[0].node.get_our_node_id(), net_graph_msg_handler, &nodes[1].node.get_our_node_id(), None, &[], 5000000-their_channel_reserve, TEST_FINAL_CLTV, &logger).unwrap(); + let route = get_route(&nodes[0].node.get_our_node_id(), net_graph_msg_handler, &nodes[1].node.get_our_node_id(), None, &[], max_can_send, TEST_FINAL_CLTV, &logger).unwrap(); nodes[0].node.send_payment(&route, our_payment_hash, &None).unwrap(); check_added_monitors!(nodes[0], 1); let mut updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); - updates.update_add_htlcs[0].amount_msat = 5000000-their_channel_reserve+1; + // Even though channel-initiator senders are required to respect the fee_spike_reserve, + // at this time channel-initiatee receivers are not required to enforce that senders + // respect the fee_spike_reserve. + updates.update_add_htlcs[0].amount_msat = max_can_send + commit_tx_fee_outbound + 1; nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]); assert!(nodes[1].node.list_channels().is_empty()); let err_msg = check_closed_broadcast!(nodes[1], true).unwrap(); - assert_eq!(err_msg.data, "Remote HTLC add would put them under their reserve value"); + assert_eq!(err_msg.data, "Remote HTLC add would put them under remote reserve value"); check_added_monitors!(nodes[1], 1); }