From: Alec Chen Date: Wed, 5 Apr 2023 22:45:41 +0000 (-0500) Subject: Support receiving MPP keysend X-Git-Tag: v0.0.116-alpha1~13^2~1 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=8dde17773ace41d2d0bac2493abf2b01db67bd84;p=rust-lightning Support receiving MPP keysend This commit refactors a significant portion of the receive validation in `ChannelManager::process_pending_htlc_forwards` now that we repurpose previous MPP validation logic to accomodate keysends. This also removes a previous restriction on claiming, as well as tests sending and receiving MPP keysends. --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f3ab0ac5a..702ae86fb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3505,7 +3505,7 @@ where panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive"); } }; - let mut claimable_htlc = ClaimableHTLC { + let claimable_htlc = ClaimableHTLC { prev_hop: HTLCPreviousHopData { short_channel_id: prev_short_channel_id, outpoint: prev_funding_outpoint, @@ -3555,13 +3555,11 @@ where } macro_rules! check_total_value { - ($payment_data: expr, $payment_preimage: expr) => {{ + ($purpose: expr) => {{ let mut payment_claimable_generated = false; - let purpose = || { - events::PaymentPurpose::InvoicePayment { - payment_preimage: $payment_preimage, - payment_secret: $payment_data.payment_secret, - } + let is_keysend = match $purpose { + events::PaymentPurpose::SpontaneousPayment(_) => true, + events::PaymentPurpose::InvoicePayment { .. } => false, }; let mut claimable_payments = self.claimable_payments.lock().unwrap(); if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) { @@ -3573,9 +3571,18 @@ where .or_insert_with(|| { committed_to_claimable = true; ClaimablePayment { - purpose: purpose(), htlcs: Vec::new(), onion_fields: None, + purpose: $purpose.clone(), htlcs: Vec::new(), onion_fields: None, } }); + if $purpose != claimable_payment.purpose { + let log_keysend = |keysend| if keysend { "keysend" } else { "non-keysend" }; + log_trace!(self.logger, "Failing new {} HTLC with payment_hash {} as we already had an existing {} HTLC with the same payment hash", log_keysend(is_keysend), log_bytes!(payment_hash.0), log_keysend(!is_keysend)); + fail_htlc!(claimable_htlc, payment_hash); + } + if !self.default_configuration.accept_mpp_keysend && is_keysend && !claimable_payment.htlcs.is_empty() { + log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash and our config states we don't accept MPP keysend", log_bytes!(payment_hash.0)); + fail_htlc!(claimable_htlc, payment_hash); + } if let Some(earlier_fields) = &mut claimable_payment.onion_fields { if earlier_fields.check_merge(&mut onion_fields).is_err() { fail_htlc!(claimable_htlc, payment_hash); @@ -3584,38 +3591,27 @@ where claimable_payment.onion_fields = Some(onion_fields); } let ref mut htlcs = &mut claimable_payment.htlcs; - if htlcs.len() == 1 { - if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { - log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0)); - fail_htlc!(claimable_htlc, payment_hash); - } - } let mut total_value = claimable_htlc.sender_intended_value; let mut earliest_expiry = claimable_htlc.cltv_expiry; for htlc in htlcs.iter() { total_value += htlc.sender_intended_value; earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry); - match &htlc.onion_payload { - OnionPayload::Invoice { .. } => { - if htlc.total_msat != $payment_data.total_msat { - log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})", - log_bytes!(payment_hash.0), $payment_data.total_msat, htlc.total_msat); - total_value = msgs::MAX_VALUE_MSAT; - } - if total_value >= msgs::MAX_VALUE_MSAT { break; } - }, - _ => unreachable!(), + if htlc.total_msat != claimable_htlc.total_msat { + log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})", + log_bytes!(payment_hash.0), claimable_htlc.total_msat, htlc.total_msat); + total_value = msgs::MAX_VALUE_MSAT; } + if total_value >= msgs::MAX_VALUE_MSAT { break; } } // The condition determining whether an MPP is complete must // match exactly the condition used in `timer_tick_occurred` if total_value >= msgs::MAX_VALUE_MSAT { fail_htlc!(claimable_htlc, payment_hash); - } else if total_value - claimable_htlc.sender_intended_value >= $payment_data.total_msat { + } else if total_value - claimable_htlc.sender_intended_value >= claimable_htlc.total_msat { log_trace!(self.logger, "Failing HTLC with payment_hash {} as payment is already claimable", log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); - } else if total_value >= $payment_data.total_msat { + } else if total_value >= claimable_htlc.total_msat { #[allow(unused_assignments)] { committed_to_claimable = true; } @@ -3626,7 +3622,7 @@ where new_events.push_back((events::Event::PaymentClaimable { receiver_node_id: Some(receiver_node_id), payment_hash, - purpose: purpose(), + purpose: $purpose, amount_msat, via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), @@ -3674,49 +3670,23 @@ where fail_htlc!(claimable_htlc, payment_hash); } } - check_total_value!(payment_data, payment_preimage); + let purpose = events::PaymentPurpose::InvoicePayment { + payment_preimage: payment_preimage.clone(), + payment_secret: payment_data.payment_secret, + }; + check_total_value!(purpose); }, OnionPayload::Spontaneous(preimage) => { - let mut claimable_payments = self.claimable_payments.lock().unwrap(); - if claimable_payments.pending_claiming_payments.contains_key(&payment_hash) { - fail_htlc!(claimable_htlc, payment_hash); - } - match claimable_payments.claimable_payments.entry(payment_hash) { - hash_map::Entry::Vacant(e) => { - let amount_msat = claimable_htlc.value; - claimable_htlc.total_value_received = Some(amount_msat); - let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER); - let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); - e.insert(ClaimablePayment { - purpose: purpose.clone(), - onion_fields: Some(onion_fields.clone()), - htlcs: vec![claimable_htlc], - }); - let prev_channel_id = prev_funding_outpoint.to_channel_id(); - new_events.push_back((events::Event::PaymentClaimable { - receiver_node_id: Some(receiver_node_id), - payment_hash, - amount_msat, - purpose, - via_channel_id: Some(prev_channel_id), - via_user_channel_id: Some(prev_user_channel_id), - claim_deadline, - onion_fields: Some(onion_fields), - }, None)); - }, - hash_map::Entry::Occupied(_) => { - log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} for a duplicative payment hash", log_bytes!(payment_hash.0)); - fail_htlc!(claimable_htlc, payment_hash); - } - } + let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); + check_total_value!(purpose); } } }, hash_map::Entry::Occupied(inbound_payment) => { - if payment_data.is_none() { + if let OnionPayload::Spontaneous(_) = claimable_htlc.onion_payload { log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", log_bytes!(payment_hash.0)); fail_htlc!(claimable_htlc, payment_hash); - }; + } let payment_data = payment_data.unwrap(); if inbound_payment.get().payment_secret != payment_data.payment_secret { log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our expected payment secret.", log_bytes!(payment_hash.0)); @@ -3726,7 +3696,11 @@ where log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap()); fail_htlc!(claimable_htlc, payment_hash); } else { - let payment_claimable_generated = check_total_value!(payment_data, inbound_payment.get().payment_preimage); + let purpose = events::PaymentPurpose::InvoicePayment { + payment_preimage: inbound_payment.get().payment_preimage, + payment_secret: payment_data.payment_secret, + }; + let payment_claimable_generated = check_total_value!(purpose); if payment_claimable_generated { inbound_payment.remove_entry(); } @@ -4265,18 +4239,6 @@ where break; } expected_amt_msat = htlc.total_value_received; - - if let OnionPayload::Spontaneous(_) = &htlc.onion_payload { - // We don't currently support MPP for spontaneous payments, so just check - // that there's one payment here and move on. - if sources.len() != 1 { - log_error!(self.logger, "Somehow ended up with an MPP spontaneous payment - this should not be reachable!"); - debug_assert!(false); - valid_mpp = false; - break; - } - } - claimable_amt_msat += htlc.value; } mem::drop(per_peer_state); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index fa8bdcc58..c325adcbe 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2115,7 +2115,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p }, PaymentPurpose::SpontaneousPayment(payment_preimage) => { assert_eq!(expected_preimage.unwrap(), *payment_preimage); - assert!(our_payment_secret.is_none()); + assert_eq!(our_payment_secret, onion_fields.as_ref().unwrap().payment_secret); }, } assert_eq!(*amount_msat, recv_value); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 9e044d1c9..a051ce054 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -17,13 +17,13 @@ use crate::sign::EntropySource; use crate::chain::transaction::OutPoint; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason}; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields}; +use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; use crate::ln::features::InvoiceFeatures; -use crate::ln::msgs; +use crate::ln::{msgs, PaymentSecret, PaymentPreimage}; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::outbound_payment::Retry; use crate::routing::gossip::{EffectiveCapacity, RoutingFees}; -use crate::routing::router::{get_route, Path, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters}; +use crate::routing::router::{get_route, Path, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters, find_route}; use crate::routing::scoring::ChannelUsage; use crate::util::test_utils; use crate::util::errors::APIError; @@ -236,6 +236,177 @@ fn mpp_receive_timeout() { do_mpp_receive_timeout(false); } +#[test] +fn test_mpp_keysend() { + let mut mpp_keysend_config = test_default_channel_config(); + mpp_keysend_config.accept_mpp_keysend = true; + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, Some(mpp_keysend_config)]); + let nodes = create_network(4, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1); + create_announced_chan_between_nodes(&nodes, 0, 2); + create_announced_chan_between_nodes(&nodes, 1, 3); + create_announced_chan_between_nodes(&nodes, 2, 3); + let network_graph = nodes[0].network_graph.clone(); + + let payer_pubkey = nodes[0].node.get_our_node_id(); + let payee_pubkey = nodes[3].node.get_our_node_id(); + let recv_value = 15_000_000; + let route_params = RouteParameters { + payment_params: PaymentParameters::for_keysend(payee_pubkey, 40, true), + final_value_msat: recv_value, + }; + let scorer = test_utils::TestScorer::new(); + let random_seed_bytes = chanmon_cfgs[0].keys_manager.get_secure_random_bytes(); + let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger, + &scorer, &(), &random_seed_bytes).unwrap(); + + let payment_preimage = PaymentPreimage([42; 32]); + let payment_secret = PaymentSecret(payment_preimage.0); + let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), + RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_preimage.0)).unwrap(); + check_added_monitors!(nodes[0], 2); + + let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]]; + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + + let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); + pass_along_path(&nodes[0], expected_route[0], recv_value, payment_hash.clone(), + Some(payment_secret), ev.clone(), false, Some(payment_preimage)); + + let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); + pass_along_path(&nodes[0], expected_route[1], recv_value, payment_hash.clone(), + Some(payment_secret), ev.clone(), true, Some(payment_preimage)); + claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage); +} + +#[test] +fn test_reject_mpp_keysend_htlc() { + // This test enforces that we reject MPP keysend HTLCs if our config states we don't support + // MPP keysend. When receiving a payment, if we don't support MPP keysend we'll reject the + // payment if it's keysend and has a payment secret, never reaching our payment validation + // logic. To check that we enforce rejecting MPP keysends in our payment logic, here we send + // keysend payments without payment secrets, then modify them by adding payment secrets in the + // final node in between receiving the HTLCs and actually processing them. + let mut reject_mpp_keysend_cfg = test_default_channel_config(); + reject_mpp_keysend_cfg.accept_mpp_keysend = false; + + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, Some(reject_mpp_keysend_cfg)]); + let nodes = create_network(4, &node_cfgs, &node_chanmgrs); + let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id; + let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 2).0.contents.short_channel_id; + let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3).0.contents.short_channel_id; + let (update_a, _, chan_4_channel_id, _) = create_announced_chan_between_nodes(&nodes, 2, 3); + let chan_4_id = update_a.contents.short_channel_id; + let amount = 40_000; + let (mut route, payment_hash, payment_preimage, _) = get_route_and_payment_hash!(nodes[0], nodes[3], amount); + + // Pay along nodes[1] + route.paths[0].hops[0].pubkey = nodes[1].node.get_our_node_id(); + route.paths[0].hops[0].short_channel_id = chan_1_id; + route.paths[0].hops[1].short_channel_id = chan_3_id; + + let payment_id_0 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes()); + nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), RecipientOnionFields::spontaneous_empty(), payment_id_0).unwrap(); + check_added_monitors!(nodes[0], 1); + + let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + let update_add_0 = update_0.update_add_htlcs[0].clone(); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_0); + commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true); + expect_pending_htlcs_forwardable!(nodes[1]); + + check_added_monitors!(&nodes[1], 1); + let update_1 = get_htlc_update_msgs!(nodes[1], nodes[3].node.get_our_node_id()); + let update_add_1 = update_1.update_add_htlcs[0].clone(); + nodes[3].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_1); + commitment_signed_dance!(nodes[3], nodes[1], update_1.commitment_signed, false, true); + + assert!(nodes[3].node.get_and_clear_pending_msg_events().is_empty()); + for (_, pending_forwards) in nodes[3].node.forward_htlcs.lock().unwrap().iter_mut() { + for f in pending_forwards.iter_mut() { + match f { + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => { + match forward_info.routing { + PendingHTLCRouting::ReceiveKeysend { ref mut payment_data, .. } => { + *payment_data = Some(msgs::FinalOnionHopData { + payment_secret: PaymentSecret([42; 32]), + total_msat: amount * 2, + }); + }, + _ => panic!("Expected PendingHTLCRouting::ReceiveKeysend"), + } + }, + _ => {}, + } + } + } + expect_pending_htlcs_forwardable!(nodes[3]); + + // Pay along nodes[2] + route.paths[0].hops[0].pubkey = nodes[2].node.get_our_node_id(); + route.paths[0].hops[0].short_channel_id = chan_2_id; + route.paths[0].hops[1].short_channel_id = chan_4_id; + + let payment_id_1 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes()); + nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage), RecipientOnionFields::spontaneous_empty(), payment_id_1).unwrap(); + check_added_monitors!(nodes[0], 1); + + let update_2 = get_htlc_update_msgs!(nodes[0], nodes[2].node.get_our_node_id()); + let update_add_2 = update_2.update_add_htlcs[0].clone(); + nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add_2); + commitment_signed_dance!(nodes[2], nodes[0], &update_2.commitment_signed, false, true); + expect_pending_htlcs_forwardable!(nodes[2]); + + check_added_monitors!(&nodes[2], 1); + let update_3 = get_htlc_update_msgs!(nodes[2], nodes[3].node.get_our_node_id()); + let update_add_3 = update_3.update_add_htlcs[0].clone(); + nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &update_add_3); + commitment_signed_dance!(nodes[3], nodes[2], update_3.commitment_signed, false, true); + + assert!(nodes[3].node.get_and_clear_pending_msg_events().is_empty()); + for (_, pending_forwards) in nodes[3].node.forward_htlcs.lock().unwrap().iter_mut() { + for f in pending_forwards.iter_mut() { + match f { + &mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) => { + match forward_info.routing { + PendingHTLCRouting::ReceiveKeysend { ref mut payment_data, .. } => { + *payment_data = Some(msgs::FinalOnionHopData { + payment_secret: PaymentSecret([42; 32]), + total_msat: amount * 2, + }); + }, + _ => panic!("Expected PendingHTLCRouting::ReceiveKeysend"), + } + }, + _ => {}, + } + } + } + expect_pending_htlcs_forwardable!(nodes[3]); + check_added_monitors!(nodes[3], 1); + + // Fail back along nodes[2] + let update_fail_0 = get_htlc_update_msgs!(&nodes[3], &nodes[2].node.get_our_node_id()); + nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &update_fail_0.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[2], nodes[3], update_fail_0.commitment_signed, false); + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_4_channel_id }]); + check_added_monitors!(nodes[2], 1); + + let update_fail_1 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &update_fail_1.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[2], update_fail_1.commitment_signed, false); + + expect_payment_failed_conditions(&nodes[0], payment_hash, true, PaymentFailedConditions::new()); + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], vec![HTLCDestination::FailedPayment { payment_hash }]); +} + + #[test] fn no_pending_leak_on_initial_send_failure() { // In an earlier version of our payment tracking, we'd have a retry entry even when the initial