From b2c9941015007702a87dbc9a8ea00fa0f2a45a74 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 6 Jan 2020 20:29:33 -0500 Subject: [PATCH] Implement multipath sends using payment_secret. This rather dramatically changes the return type of send_payment making it much clearer when resending is safe and allowing us to return a list of Results since different paths may have different return values. --- lightning/src/ln/chanmon_update_fail_tests.rs | 11 +- lightning/src/ln/channelmanager.rs | 262 ++++++++++++------ lightning/src/ln/functional_test_utils.rs | 32 ++- lightning/src/ln/functional_tests.rs | 94 ++----- lightning/src/ln/onion_utils.rs | 4 +- 5 files changed, 236 insertions(+), 167 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 289fd8067..a52d1ca03 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -4,7 +4,7 @@ //! here. See also the chanmon_fail_consistency fuzz test. use chain::transaction::OutPoint; -use ln::channelmanager::{RAACommitmentOrder, PaymentPreimage, PaymentHash}; +use ln::channelmanager::{RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSendFailure}; use ln::channelmonitor::ChannelMonitorUpdateErr; use ln::features::InitFeatures; use ln::msgs; @@ -30,7 +30,7 @@ fn test_simple_monitor_permanent_update_fail() { let (_, payment_hash_1) = get_payment_preimage_hash!(&nodes[0]); *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure); - if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1, &None) {} else { panic!(); } + unwrap_send_err!(nodes[0].node.send_payment(route, payment_hash_1, &None), true, APIError::ChannelUnavailable {..}, {}); check_added_monitors!(nodes[0], 2); let events_1 = nodes[0].node.get_and_clear_pending_msg_events(); @@ -63,7 +63,8 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { let (payment_preimage_1, payment_hash_1) = get_payment_preimage_hash!(&nodes[0]); *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure); - if let Err(APIError::MonitorUpdateFailed) = nodes[0].node.send_payment(route.clone(), payment_hash_1, &None) {} else { panic!(); } + + unwrap_send_err!(nodes[0].node.send_payment(route.clone(), payment_hash_1, &None), false, APIError::MonitorUpdateFailed, {}); check_added_monitors!(nodes[0], 1); assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); @@ -106,7 +107,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { // Now set it to failed again... let (_, payment_hash_2) = get_payment_preimage_hash!(&nodes[0]); *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure); - if let Err(APIError::MonitorUpdateFailed) = nodes[0].node.send_payment(route, payment_hash_2, &None) {} else { panic!(); } + unwrap_send_err!(nodes[0].node.send_payment(route, payment_hash_2, &None), false, APIError::MonitorUpdateFailed, {}); check_added_monitors!(nodes[0], 1); assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); @@ -169,7 +170,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let (payment_preimage_2, payment_hash_2) = get_payment_preimage_hash!(nodes[0]); *nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::TemporaryFailure); - if let Err(APIError::MonitorUpdateFailed) = nodes[0].node.send_payment(route.clone(), payment_hash_2, &None) {} else { panic!(); } + unwrap_send_err!(nodes[0].node.send_payment(route.clone(), payment_hash_2, &None), false, APIError::MonitorUpdateFailed, {}); check_added_monitors!(nodes[0], 1); assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 918f8b28f..f6293e04d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -446,15 +446,6 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_P #[allow(dead_code)] const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; -macro_rules! secp_call { - ( $res: expr, $err: expr ) => { - match $res { - Ok(key) => key, - Err(_) => return Err($err), - } - }; -} - /// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels pub struct ChannelDetails { /// The channel's ID (prior to funding transaction generation, this is a random 32 bytes, @@ -491,6 +482,42 @@ pub struct ChannelDetails { pub is_live: bool, } +/// If a payment fails to send, it can be in one of several states. This enum is returned as the +/// Err() type describing which state the payment is in, see the description of individual enum +/// states for more. +#[derive(Debug)] +pub enum PaymentSendFailure { + /// A parameter which was passed to send_payment was invalid, preventing us from attempting to + /// send the payment at all. No channel state has been changed or messages sent to peers, and + /// once you've changed the parameter at error, you can freely retry the payment in full. + ParameterError(APIError), + /// A parameter in a single path which was passed to send_payment was invalid, preventing us + /// from attempting to send the payment at all. No channel state has been changed or messages + /// sent to peers, and once you've changed the parameter at error, you can freely retry the + /// payment in full. + /// + /// The results here are ordered the same as the paths in the route object which was passed to + /// send_payment. + PathParameterError(Vec>), + /// All paths which were attempted failed to send, with no channel state change taking place. + /// You can freely retry the payment in full (though you probably want to do so over different + /// paths than the ones selected). + AllFailedRetrySafe(Vec), + /// Some paths which were attempted failed to send, though possibly not all. At least some + /// paths have irrevocably committed to the HTLC and retrying the payment in full would result + /// in over-/re-payment. + /// + /// The results here are ordered the same as the paths in the route object which was passed to + /// send_payment, and any Errs which are not APIError::MonitorUpdateFailed can be safely + /// retried (though there is currently no API with which to do so). + /// + /// Any entries which contain Err(APIError::MonitorUpdateFailed) or Ok(()) MUST NOT be retried + /// as they will result in over-/re-payment. These HTLCs all either successfully sent (in the + /// case of Ok(())) or will send once channel_monitor_updated is called on the next-hop channel + /// with the latest update_id. + PartialFailure(Vec>), +} + macro_rules! handle_error { ($self: ident, $internal: expr, $their_node_id: expr) => { match $internal { @@ -1207,20 +1234,24 @@ impl ChannelMan /// payment_preimage tracking (which you should already be doing as they represent "proof of /// payment") and prevent double-sends yourself. /// - /// May generate a SendHTLCs message event on success, which should be relayed. + /// May generate SendHTLCs message(s) event on success, which should be relayed. + /// + /// Each path may have a different return value, and PaymentSendValue may return a Vec with + /// each entry matching the corresponding-index entry in the route paths, see + /// PaymentSendFailure for more info. /// - /// Raises APIError::RoutError when invalid route or forward parameter - /// (cltv_delta, fee, node public key) is specified. - /// Raises APIError::ChannelUnavailable if the next-hop channel is not available for updates - /// (including due to previous monitor update failure or new permanent monitor update failure). - /// Raised APIError::MonitorUpdateFailed if a new monitor update failure prevented sending the - /// relevant updates. + /// In general, a path may raise: + /// * APIError::RouteError when an invalid route or forwarding parameter (cltv_delta, fee, + /// node public key) is specified. + /// * APIError::ChannelUnavailable if the next-hop channel is not available for updates + /// (including due to previous monitor update failure or new permanent monitor update + /// failure). + /// * APIError::MonitorUpdateFailed if a new monitor update failure prevented sending the + /// relevant updates. /// - /// In case of APIError::RouteError/APIError::ChannelUnavailable, the payment send has failed - /// and you may wish to retry via a different route immediately. - /// In case of APIError::MonitorUpdateFailed, the commitment update has been irrevocably - /// committed on our end and we're just waiting for a monitor update to send it. Do NOT retry - /// the payment via a different route unless you intend to pay twice! + /// Note that depending on the type of the PaymentSendFailure the HTLC may have been + /// irrevocably committed to on our end. In such a case, do NOT retry the payment with a + /// different route unless you intend to pay twice! /// /// payment_secret is unrelated to payment_hash (or PaymentPreimage) and exists to authenticate /// the sender to the recipient and prevent payment-probing (deanonymization) attacks. For @@ -1230,87 +1261,142 @@ impl ChannelMan /// If a payment_secret *is* provided, we assume that the invoice had the payment_secret feature /// bit set (either as required or as available). If multiple paths are present in the Route, /// we assume the invoice had the basic_mpp feature set. - pub fn send_payment(&self, route: Route, payment_hash: PaymentHash, payment_secret: &Option) -> Result<(), APIError> { - if route.paths.len() < 1 || route.paths.len() > 1 { - return Err(APIError::RouteError{err: "We currently don't support MPP, and we need at least one path"}); + pub fn send_payment(&self, route: Route, payment_hash: PaymentHash, payment_secret: &Option) -> Result<(), PaymentSendFailure> { + if route.paths.len() < 1 { + return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "There must be at least one path to send over"})); } - if route.paths[0].len() < 1 || route.paths[0].len() > 20 { - return Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"}); + if route.paths.len() > 10 { + // This limit is completely arbitrary - there aren't any real fundamental path-count + // limits. After we support retrying individual paths we should likely bump this, but + // for now more than 10 paths likely carries too much one-path failure. + return Err(PaymentSendFailure::ParameterError(APIError::RouteError{err: "Sending over more than 10 paths is not currently supported"})); } + let mut total_value = 0; let our_node_id = self.get_our_node_id(); - for (idx, hop) in route.paths[0].iter().enumerate() { - if idx != route.paths[0].len() - 1 && hop.pubkey == our_node_id { - return Err(APIError::RouteError{err: "Path went through us but wasn't a simple rebalance loop to us"}); + let mut path_errs = Vec::with_capacity(route.paths.len()); + 'path_check: for path in route.paths.iter() { + if path.len() < 1 || path.len() > 20 { + path_errs.push(Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"})); + continue 'path_check; + } + for (idx, hop) in path.iter().enumerate() { + if idx != path.len() - 1 && hop.pubkey == our_node_id { + path_errs.push(Err(APIError::RouteError{err: "Path went through us but wasn't a simple rebalance loop to us"})); + continue 'path_check; + } } + total_value += path.last().unwrap().fee_msat; + path_errs.push(Ok(())); + } + if path_errs.iter().any(|e| e.is_err()) { + return Err(PaymentSendFailure::PathParameterError(path_errs)); } - - let (session_priv, prng_seed) = self.keys_manager.get_onion_rand(); let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1; + let mut results = Vec::new(); + 'path_loop: for path in route.paths.iter() { + macro_rules! check_res_push { + ($res: expr) => { match $res { + Ok(r) => r, + Err(e) => { + results.push(Err(e)); + continue 'path_loop; + }, + } + } + } - let onion_keys = secp_call!(onion_utils::construct_onion_keys(&self.secp_ctx, &route.paths[0], &session_priv), - APIError::RouteError{err: "Pubkey along hop was maliciously selected"}); - let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], payment_secret, cur_height)?; - if onion_utils::route_size_insane(&onion_payloads) { - return Err(APIError::RouteError{err: "Route size too large considering onion data"}); - } - let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, &payment_hash); + log_trace!(self, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id); + let (session_priv, prng_seed) = self.keys_manager.get_onion_rand(); - let _ = self.total_consistency_lock.read().unwrap(); + let onion_keys = check_res_push!(onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv) + .map_err(|_| APIError::RouteError{err: "Pubkey along hop was maliciously selected"})); + let (onion_payloads, htlc_msat, htlc_cltv) = check_res_push!(onion_utils::build_onion_payloads(&path, total_value, payment_secret, cur_height)); + if onion_utils::route_size_insane(&onion_payloads) { + check_res_push!(Err(APIError::RouteError{err: "Route size too large considering onion data"})); + } + let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, &payment_hash); - let err: Result<(), _> = loop { - let mut channel_lock = self.channel_state.lock().unwrap(); - let id = match channel_lock.short_to_id.get(&route.paths[0].first().unwrap().short_channel_id) { - None => return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!"}), - Some(id) => id.clone(), - }; + let _ = self.total_consistency_lock.read().unwrap(); - let channel_state = &mut *channel_lock; - if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) { - match { - if chan.get().get_their_node_id() != route.paths[0].first().unwrap().pubkey { - return Err(APIError::RouteError{err: "Node ID mismatch on first hop!"}); - } - if !chan.get().is_live() { - return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!"}); - } - break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute { - path: route.paths[0].clone(), - session_priv: session_priv.clone(), - first_hop_htlc_msat: htlc_msat, - }, onion_packet), channel_state, chan) - } { - Some((update_add, commitment_signed, monitor_update)) => { - if let Err(e) = self.monitor.update_monitor(chan.get().get_funding_txo().unwrap(), monitor_update) { - maybe_break_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, true); - // Note that MonitorUpdateFailed here indicates (per function docs) - // that we will resent the commitment update once we unfree monitor - // updating, so we have to take special care that we don't return - // something else in case we will resend later! - return Err(APIError::MonitorUpdateFailed); + let err: Result<(), _> = loop { + let mut channel_lock = self.channel_state.lock().unwrap(); + let id = match channel_lock.short_to_id.get(&path.first().unwrap().short_channel_id) { + None => check_res_push!(Err(APIError::ChannelUnavailable{err: "No channel available with first hop!"})), + Some(id) => id.clone(), + }; + + let channel_state = &mut *channel_lock; + if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) { + match { + if chan.get().get_their_node_id() != path.first().unwrap().pubkey { + check_res_push!(Err(APIError::RouteError{err: "Node ID mismatch on first hop!"})); } + if !chan.get().is_live() { + check_res_push!(Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!"})); + } + break_chan_entry!(self, chan.get_mut().send_htlc_and_commit(htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute { + path: path.clone(), + session_priv: session_priv.clone(), + first_hop_htlc_msat: htlc_msat, + }, onion_packet), channel_state, chan) + } { + Some((update_add, commitment_signed, monitor_update)) => { + if let Err(e) = self.monitor.update_monitor(chan.get().get_funding_txo().unwrap(), monitor_update) { + maybe_break_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, true); + // Note that MonitorUpdateFailed here indicates (per function docs) + // that we will resend the commitment update once monitor updating + // is restored. Therefore, we must return an error indicating that + // it is unsafe to retry the payment wholesale, which we do in the + // next check for MonitorUpdateFailed, below. + check_res_push!(Err(APIError::MonitorUpdateFailed)); + } - channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { - node_id: route.paths[0].first().unwrap().pubkey, - updates: msgs::CommitmentUpdate { - update_add_htlcs: vec![update_add], - update_fulfill_htlcs: Vec::new(), - update_fail_htlcs: Vec::new(), - update_fail_malformed_htlcs: Vec::new(), - update_fee: None, - commitment_signed, - }, - }); - }, - None => {}, - } - } else { unreachable!(); } - return Ok(()); - }; + channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { + node_id: path.first().unwrap().pubkey, + updates: msgs::CommitmentUpdate { + update_add_htlcs: vec![update_add], + update_fulfill_htlcs: Vec::new(), + update_fail_htlcs: Vec::new(), + update_fail_malformed_htlcs: Vec::new(), + update_fee: None, + commitment_signed, + }, + }); + }, + None => {}, + } + } else { unreachable!(); } + results.push(Ok(())); + continue 'path_loop; + }; - match handle_error!(self, err, route.paths[0].first().unwrap().pubkey) { - Ok(_) => unreachable!(), - Err(e) => { Err(APIError::ChannelUnavailable { err: e.err }) } + match handle_error!(self, err, path.first().unwrap().pubkey) { + Ok(_) => unreachable!(), + Err(e) => { + check_res_push!(Err(APIError::ChannelUnavailable { err: e.err })); + }, + } + } + let mut has_ok = false; + let mut has_err = false; + for res in results.iter() { + if res.is_ok() { has_ok = true; } + if res.is_err() { has_err = true; } + if let &Err(APIError::MonitorUpdateFailed) = res { + // MonitorUpdateFailed is inherently unsafe to retry, so we call it a + // PartialFailure. + has_err = true; + has_ok = true; + break; + } + } + if has_err && has_ok { + Err(PaymentSendFailure::PartialFailure(results)) + } else if has_err { + Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect())) + } else { + Ok(()) } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 5a655f6cc..8a887e7fa 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -4,7 +4,7 @@ use chain::chaininterface; use chain::transaction::OutPoint; use chain::keysinterface::KeysInterface; -use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret}; +use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure}; use ln::channelmonitor::{ChannelMonitor, ManyChannelMonitor}; use ln::router::{Route, Router, RouterReadArgs}; use ln::features::InitFeatures; @@ -273,6 +273,28 @@ macro_rules! get_local_commitment_txn { } } +macro_rules! unwrap_send_err { + ($res: expr, $all_failed: expr, $type: pat, $check: expr) => { + match &$res { + &Err(PaymentSendFailure::AllFailedRetrySafe(ref fails)) if $all_failed => { + assert_eq!(fails.len(), 1); + match fails[0] { + $type => { $check }, + _ => panic!(), + } + }, + &Err(PaymentSendFailure::PartialFailure(ref fails)) if !$all_failed => { + assert_eq!(fails.len(), 1); + match fails[0] { + Err($type) => { $check }, + _ => panic!(), + } + }, + _ => panic!(), + } + } +} + pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_chan_value: u64, expected_user_chan_id: u64) -> ([u8; 32], Transaction, OutPoint) { let chan_id = *node.network_chan_count.borrow(); @@ -915,12 +937,8 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou } let (_, our_payment_hash) = get_payment_preimage_hash!(origin_node); - - let err = origin_node.node.send_payment(route, our_payment_hash, &None).err().unwrap(); - match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"), - _ => panic!("Unknown error variants"), - }; + unwrap_send_err!(origin_node.node.send_payment(route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept")); } pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64, expected_value: u64) { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 5ac00e6e7..05c9468cb 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -6,7 +6,7 @@ use chain::transaction::OutPoint; use chain::keysinterface::{ChannelKeys, KeysInterface, SpendableOutputDescriptor}; use chain::chaininterface::{ChainListener, ChainWatchInterfaceUtil, BlockNotifier}; use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC}; -use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, BREAKDOWN_TIMEOUT}; +use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure, BREAKDOWN_TIMEOUT}; use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ManyChannelMonitor, ANTI_REORG_DELAY}; use ln::channel::{Channel, ChannelError}; use ln::{chan_utils, onion_utils}; @@ -861,10 +861,8 @@ fn updates_shutdown_wait() { assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); let (_, payment_hash) = get_payment_preimage_hash!(nodes[0]); - if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route_1, payment_hash, &None) {} - else { panic!("New sends should fail!") }; - if let Err(APIError::ChannelUnavailable {..}) = nodes[1].node.send_payment(route_2, payment_hash, &None) {} - else { panic!("New sends should fail!") }; + unwrap_send_err!(nodes[0].node.send_payment(route_1, payment_hash, &None), true, APIError::ChannelUnavailable {..}, {}); + unwrap_send_err!(nodes[1].node.send_payment(route_2, payment_hash, &None), true, APIError::ChannelUnavailable {..}, {}); assert!(nodes[2].node.claim_funds(our_payment_preimage, &None, 100_000)); check_added_monitors!(nodes[2], 1); @@ -1320,9 +1318,8 @@ fn holding_cell_htlc_counting() { // another HTLC. let route = nodes[1].router.get_route(&nodes[2].node.get_our_node_id(), None, &Vec::new(), 100000, TEST_FINAL_CLTV).unwrap(); let (_, payment_hash_1) = get_payment_preimage_hash!(nodes[0]); - if let APIError::ChannelUnavailable { err } = nodes[1].node.send_payment(route, payment_hash_1, &None).unwrap_err() { - assert_eq!(err, "Cannot push more than their max accepted HTLCs"); - } else { panic!("Unexpected event"); } + unwrap_send_err!(nodes[1].node.send_payment(route, payment_hash_1, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot push more than their max accepted HTLCs")); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot push more than their max accepted HTLCs".to_string(), 1); @@ -1563,11 +1560,8 @@ fn do_channel_reserve_test(test_recv: bool) { { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_0 + 1); assert!(route.paths[0].iter().rev().skip(1).all(|h| h.fee_msat == feemsat)); - let err = nodes[0].node.send_payment(route, our_payment_hash, &None).err().unwrap(); - match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"), - _ => panic!("Unknown error variants"), - } + 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 over the max HTLC value in flight our peer will accept")); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over the max HTLC value in flight our peer will accept".to_string(), 1); } @@ -1601,11 +1595,8 @@ fn do_channel_reserve_test(test_recv: bool) { 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); - let err = nodes[0].node.send_payment(route.clone(), our_payment_hash, &None).err().unwrap(); - match err { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), - _ => panic!("Unknown error variants"), - } + unwrap_send_err!(nodes[0].node.send_payment(route.clone(), our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot send value that would put us over their 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 over their reserve value".to_string(), 1); } @@ -1629,10 +1620,8 @@ fn do_channel_reserve_test(test_recv: bool) { let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat; { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1); - match nodes[0].node.send_payment(route, our_payment_hash, &None).err().unwrap() { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), - _ => panic!("Unknown error variants"), - } + 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 over their 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 over their reserve value".to_string(), 2); } @@ -1652,7 +1641,7 @@ fn do_channel_reserve_test(test_recv: bool) { 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], &None, cur_height).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, @@ -1694,10 +1683,8 @@ fn do_channel_reserve_test(test_recv: bool) { // test with outbound holding cell amount > 0 { let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_22+1); - match nodes[0].node.send_payment(route, our_payment_hash, &None).err().unwrap() { - APIError::ChannelUnavailable{err} => assert_eq!(err, "Cannot send value that would put us over their reserve value"), - _ => panic!("Unknown error variants"), - } + 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 over their 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 over their reserve value".to_string(), 3); } @@ -3038,7 +3025,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() { }; let current_height = nodes[1].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; - let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], &None, current_height).unwrap(); + let (onion_payloads, _amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(&route.paths[0], 50_000, &None, current_height).unwrap(); let onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.paths[0], &session_priv).unwrap(); let onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash); @@ -5471,7 +5458,7 @@ fn test_onion_failure() { let session_priv = SecretKey::from_slice(&[3; 32]).unwrap(); let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap(); - let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], &None, cur_height).unwrap(); + let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 40000, &None, cur_height).unwrap(); let mut new_payloads = Vec::new(); for payload in onion_payloads.drain(..) { new_payloads.push(BogusOnionHopData::new(payload)); @@ -5487,7 +5474,7 @@ fn test_onion_failure() { let session_priv = SecretKey::from_slice(&[3; 32]).unwrap(); let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap(); - let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], &None, cur_height).unwrap(); + let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 40000, &None, cur_height).unwrap(); let mut new_payloads = Vec::new(); for payload in onion_payloads.drain(..) { new_payloads.push(BogusOnionHopData::new(payload)); @@ -5672,7 +5659,7 @@ fn test_onion_failure() { let height = 1; route.paths[0][1].cltv_expiry_delta += CLTV_FAR_FAR_AWAY + route.paths[0][0].cltv_expiry_delta + 1; let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap(); - let (onion_payloads, _, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], &None, height).unwrap(); + let (onion_payloads, _, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 40000, &None, height).unwrap(); let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash); msg.cltv_expiry = htlc_cltv; msg.onion_routing_packet = onion_packet; @@ -5764,13 +5751,8 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() { route.paths[0][0].fee_msat = 100; - let err = nodes[0].node.send_payment(route, our_payment_hash, &None); - - if let Err(APIError::ChannelUnavailable{err}) = err { - assert_eq!(err, "Cannot send less than their minimum HTLC value"); - } else { - assert!(false); - } + unwrap_send_err!(nodes[0].node.send_payment(route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot send less than their minimum HTLC 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 less than their minimum HTLC value".to_string(), 1); } @@ -5788,13 +5770,8 @@ fn test_update_add_htlc_bolt2_sender_zero_value_msat() { route.paths[0][0].fee_msat = 0; - let err = nodes[0].node.send_payment(route, our_payment_hash, &None); - - if let Err(APIError::ChannelUnavailable{err}) = err { - assert_eq!(err, "Cannot send 0-msat HTLC"); - } else { - assert!(false); - } + unwrap_send_err!(nodes[0].node.send_payment(route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot send 0-msat HTLC")); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send 0-msat HTLC".to_string(), 1); } @@ -5833,13 +5810,8 @@ fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() { let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000000, 500000001).unwrap(); let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]); - let err = nodes[0].node.send_payment(route, our_payment_hash, &None); - - if let Err(APIError::RouteError{err}) = err { - assert_eq!(err, "Channel CLTV overflowed?!"); - } else { - assert!(false); - } + unwrap_send_err!(nodes[0].node.send_payment(route, our_payment_hash, &None), true, APIError::RouteError { err }, + assert_eq!(err, "Channel CLTV overflowed?!")); } #[test] @@ -5879,13 +5851,9 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment() } let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], 100000, TEST_FINAL_CLTV).unwrap(); let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]); - let err = nodes[0].node.send_payment(route, our_payment_hash, &None); + unwrap_send_err!(nodes[0].node.send_payment(route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err }, + assert_eq!(err, "Cannot push more than their max accepted HTLCs")); - if let Err(APIError::ChannelUnavailable{err}) = err { - assert_eq!(err, "Cannot push more than their max accepted HTLCs"); - } else { - assert!(false); - } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot push more than their max accepted HTLCs".to_string(), 1); } @@ -5905,13 +5873,9 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() { let route = nodes[0].router.get_route(&nodes[1].node.get_our_node_id(), None, &[], max_in_flight+1, TEST_FINAL_CLTV).unwrap(); let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]); - let err = nodes[0].node.send_payment(route, our_payment_hash, &None); + 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 over the max HTLC value in flight our peer will accept")); - if let Err(APIError::ChannelUnavailable{err}) = err { - assert_eq!(err, "Cannot send value that would put us over the max HTLC value in flight our peer will accept"); - } else { - assert!(false); - } assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over the max HTLC value in flight our peer will accept".to_string(), 1); @@ -5993,7 +5957,7 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() { let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1; let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::signing_only(), &route.paths[0], &session_priv).unwrap(); - let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], &None, cur_height).unwrap(); + let (onion_payloads, _htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(&route.paths[0], 3999999, &None, cur_height).unwrap(); let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash); let mut msg = msgs::UpdateAddHTLC { diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index ef0d6d2e8..ff89782be 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -108,7 +108,7 @@ pub(super) fn construct_onion_keys(secp_ctx: &Secp256k1, payment_secret_option: &Option, starting_htlc_offset: u32) -> Result<(Vec, u64, u32), APIError> { +pub(super) fn build_onion_payloads(path: &Vec, total_msat: u64, payment_secret_option: &Option, starting_htlc_offset: u32) -> Result<(Vec, u64, u32), APIError> { let mut cur_value_msat = 0u64; let mut cur_cltv = starting_htlc_offset; let mut last_short_channel_id = 0; @@ -127,7 +127,7 @@ pub(super) fn build_onion_payloads(path: &Vec, payment_secret_option: payment_data: if let &Some(ref payment_secret) = payment_secret_option { Some(msgs::FinalOnionHopData { payment_secret: payment_secret.clone(), - total_msat: hop.fee_msat, + total_msat, }) } else { None }, } -- 2.39.5