From 283c94727ff9d291bb188be94891b8427a75096f Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 5 Jun 2020 15:27:30 -0400 Subject: [PATCH 1/3] Refactor: move channel checks for HTLC adds into Channel This also includes adding a closure that creates a new pending HTLC status as a parameter for Channel's update_add_htlc. This will later be useful when we add the check for fee spike buffer violations, which will also result in changing an HTLC's pending status to failing. Co-authored-by: Jeffrey Czyz Co-authored-by: Valentine Wallace --- lightning/src/ln/channel.rs | 16 ++++++-- lightning/src/ln/channelmanager.rs | 59 +++++++++++++++++------------- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c1cdac3cecc..f792a0a5e39 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1663,8 +1663,16 @@ 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) { + 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 { @@ -1719,7 +1727,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 +1739,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(()) } 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)) } From c9926b95001996ec0730667a7f7e90381740fc09 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 6 May 2020 19:18:23 -0400 Subject: [PATCH 2/3] Add fee spike buffer + incl commit tx fee in chan reserve calculation When we receive an inbound HTLC from a peer on an inbound channel, make sure the funder can still cover the additional on-chain cost of the HTLC while maintaining their channel reserve. When we're sending an outbound HTLC, make sure the funder can still cover the additional on-chain cost of the HTLC while maintaining their channel reserve. + implement fee spike buffer for channel initiators sending payments. Also add an additional spec-deviating fee spike buffer on the receiving side (but don't close the channel if this reserve is violated, just fail the HTLC). From lightning-rfc PR #740. Co-authored-by: Matt Corallo Co-authored-by: Valentine Wallace --- lightning/src/ln/channel.rs | 168 ++++++++++++- lightning/src/ln/functional_tests.rs | 353 +++++++++++++++++++++------ 2 files changed, 431 insertions(+), 90 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f792a0a5e39..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,6 +1664,73 @@ impl Channel { cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64) } + // 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. @@ -1716,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")); } @@ -3044,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, } } @@ -3554,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, }); @@ -3584,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/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); } From 17ccab4f4a1003a74a127590b13089d060626f21 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 6 May 2020 19:18:51 -0400 Subject: [PATCH 3/3] Update chanmon fuzzer to include small payment actions. This change should allow the fuzzer to catch more edge cases, such as channel reserve checks that cut it close when sending payments. --- fuzz/src/chanmon_consistency.rs | 42 +++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 12 deletions(-) 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!(), }