From 9a3914dee6e3634045a75a273433921e79cdb4f7 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 24 May 2023 11:53:05 -0400 Subject: [PATCH] Allow receiving less than the onion claims to pay Useful for penultimate hops in routes to take an extra fee, if for example they opened a JIT channel to the payee and want them to help bear the channel open cost. --- lightning/src/ln/channelmanager.rs | 23 ++-- lightning/src/ln/functional_test_utils.rs | 24 +++- lightning/src/ln/payment_tests.rs | 127 ++++++++++++++++++++++ 3 files changed, 161 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4444e6cc5..5ebaa531c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2526,9 +2526,10 @@ where } } - fn construct_recv_pending_htlc_info(&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32], - payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result - { + fn construct_recv_pending_htlc_info( + &self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32], payment_hash: PaymentHash, + amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool + ) -> Result { // final_incorrect_cltv_expiry if hop_data.outgoing_cltv_value > cltv_expiry { return Err(ReceiveError { @@ -2554,7 +2555,7 @@ where msg: "The final CLTV expiry is too soon to handle", }); } - if hop_data.amt_to_forward > amt_msat { + if !allow_underpay && hop_data.amt_to_forward > amt_msat { return Err(ReceiveError { err_code: 19, err_data: amt_msat.to_be_bytes().to_vec(), @@ -2841,7 +2842,7 @@ where fn construct_pending_htlc_status<'a>( &self, msg: &msgs::UpdateAddHTLC, shared_secret: [u8; 32], decoded_hop: onion_utils::Hop, - next_packet_pubkey_opt: Option> + allow_underpay: bool, next_packet_pubkey_opt: Option> ) -> PendingHTLCStatus { macro_rules! return_err { ($msg: expr, $err_code: expr, $data: expr) => { @@ -2859,7 +2860,9 @@ where match decoded_hop { onion_utils::Hop::Receive(next_hop_data) => { // OUR PAYMENT! - match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None) { + match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, + msg.amount_msat, msg.cltv_expiry, None, allow_underpay) + { Ok(info) => { // Note that we could obviously respond immediately with an update_fulfill_htlc // message, however that would leak that we are the recipient of this payment, so @@ -3675,7 +3678,10 @@ where }; match next_hop { onion_utils::Hop::Receive(hop_data) => { - match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value, Some(phantom_shared_secret)) { + match self.construct_recv_pending_htlc_info(hop_data, + incoming_shared_secret, payment_hash, outgoing_amt_msat, + outgoing_cltv_value, Some(phantom_shared_secret), false) + { Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, vec![(info, prev_htlc_id)])), Err(ReceiveError { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret)) } @@ -5424,7 +5430,8 @@ where let pending_forward_info = match decoded_hop_res { Ok((next_hop, shared_secret, next_packet_pk_opt)) => - self.construct_pending_htlc_status(msg, shared_secret, next_hop, next_packet_pk_opt), + self.construct_pending_htlc_status(msg, shared_secret, next_hop, + chan.get().context.config().accept_underpaying_htlcs, next_packet_pk_opt), Err(e) => PendingHTLCStatus::Fail(e) }; let create_pending_htlc_status = |chan: &Channel<::Signer>, pending_forward_info: PendingHTLCStatus, error_code: u16| { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 9f1ffb28b..dc5f2b41f 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2186,7 +2186,20 @@ pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route (our_payment_preimage, our_payment_hash, our_payment_secret, payment_id) } -pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) -> u64 { +pub fn do_claim_payment_along_route<'a, 'b, 'c>( + origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, + our_payment_preimage: PaymentPreimage +) -> u64 { + let extra_fees = vec![0; expected_paths.len()]; + do_claim_payment_along_route_with_extra_penultimate_hop_fees(origin_node, expected_paths, + &extra_fees[..], skip_last, our_payment_preimage) +} + +pub fn do_claim_payment_along_route_with_extra_penultimate_hop_fees<'a, 'b, 'c>( + origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], expected_extra_fees: + &[u32], skip_last: bool, our_payment_preimage: PaymentPreimage +) -> u64 { + assert_eq!(expected_paths.len(), expected_extra_fees.len()); for path in expected_paths.iter() { assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id()); } @@ -2236,7 +2249,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, } } - for (expected_route, (path_msgs, next_hop)) in expected_paths.iter().zip(per_path_msgs.drain(..)) { + for (i, (expected_route, (path_msgs, next_hop))) in expected_paths.iter().zip(per_path_msgs.drain(..)).enumerate() { let mut next_msgs = Some(path_msgs); let mut expected_next_node = next_hop; @@ -2251,10 +2264,10 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, } } macro_rules! mid_update_fulfill_dance { - ($node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => { + ($idx: expr, $node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => { { $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0); - let fee = { + let mut fee = { let per_peer_state = $node.node.per_peer_state.read().unwrap(); let peer_state = per_peer_state.get(&$prev_node.node.get_our_node_id()) .unwrap().lock().unwrap(); @@ -2265,6 +2278,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, channel.context.config().forwarding_fee_base_msat } }; + if $idx == 1 { fee += expected_extra_fees[i]; } expect_payment_forwarded!($node, $next_node, $prev_node, Some(fee as u64), false, false); expected_total_fee_msat += fee as u64; check_added_monitors!($node, 1); @@ -2296,7 +2310,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, } else { next_node = expected_route[expected_route.len() - 1 - idx - 1]; } - mid_update_fulfill_dance!(node, prev_node, next_node, update_next_msgs); + mid_update_fulfill_dance!(idx, node, prev_node, next_node, update_next_msgs); } else { assert!(!update_next_msgs); assert!(node.node.get_and_clear_pending_msg_events().is_empty()); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 90c7ad762..fa607f680 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -1736,6 +1736,133 @@ fn do_test_intercepted_payment(test: InterceptTest) { } } +#[test] +fn accept_underpaying_htlcs_config() { + do_accept_underpaying_htlcs_config(1); + do_accept_underpaying_htlcs_config(2); + do_accept_underpaying_htlcs_config(3); +} + +fn do_accept_underpaying_htlcs_config(num_mpp_parts: usize) { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let mut intercept_forwards_config = test_default_channel_config(); + intercept_forwards_config.accept_intercept_htlcs = true; + let mut underpay_config = test_default_channel_config(); + underpay_config.channel_config.accept_underpaying_htlcs = true; + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), Some(underpay_config)]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let mut chan_ids = Vec::new(); + for _ in 0..num_mpp_parts { + let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000, 0); + let channel_id = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 2_000_000, 0).0.channel_id; + chan_ids.push(channel_id); + } + + // Send the initial payment. + let amt_msat = 900_000; + let skimmed_fee_msat = 20; + let mut route_hints = Vec::new(); + for _ in 0..num_mpp_parts { + route_hints.push(RouteHint(vec![RouteHintHop { + src_node_id: nodes[1].node.get_our_node_id(), + short_channel_id: nodes[1].node.get_intercept_scid(), + fees: RoutingFees { + base_msat: 1000, + proportional_millionths: 0, + }, + cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA, + htlc_minimum_msat: None, + htlc_maximum_msat: Some(amt_msat / num_mpp_parts as u64 + 5), + }])); + } + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_route_hints(route_hints).unwrap() + .with_bolt11_features(nodes[2].node.invoice_features()).unwrap(); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + }; + let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap(); + nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret), + PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap(); + check_added_monitors!(nodes[0], num_mpp_parts); // one monitor per path + let mut events: Vec = nodes[0].node.get_and_clear_pending_msg_events().into_iter().map(|e| SendEvent::from_event(e)).collect(); + assert_eq!(events.len(), num_mpp_parts); + + // Forward the intercepted payments. + for (idx, ev) in events.into_iter().enumerate() { + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &ev.msgs[0]); + do_commitment_signed_dance(&nodes[1], &nodes[0], &ev.commitment_msg, false, true); + + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + let (intercept_id, expected_outbound_amt_msat) = match events[0] { + crate::events::Event::HTLCIntercepted { + intercept_id, expected_outbound_amount_msat, payment_hash: pmt_hash, .. + } => { + assert_eq!(pmt_hash, payment_hash); + (intercept_id, expected_outbound_amount_msat) + }, + _ => panic!() + }; + nodes[1].node.forward_intercepted_htlc(intercept_id, &chan_ids[idx], + nodes[2].node.get_our_node_id(), expected_outbound_amt_msat - skimmed_fee_msat).unwrap(); + expect_pending_htlcs_forwardable!(nodes[1]); + let payment_event = { + { + let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), 1); + added_monitors.clear(); + } + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event.commitment_msg, false, true); + if idx == num_mpp_parts - 1 { + expect_pending_htlcs_forwardable!(nodes[2]); + } + } + + // Claim the payment and check that the skimmed fee is as expected. + let payment_preimage = nodes[2].node.get_payment_preimage(payment_hash, payment_secret).unwrap(); + let events = nodes[2].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + crate::events::Event::PaymentClaimable { + ref payment_hash, ref purpose, amount_msat, counterparty_skimmed_fee_msat, receiver_node_id, .. + } => { + assert_eq!(payment_hash, payment_hash); + assert_eq!(amt_msat - skimmed_fee_msat * num_mpp_parts as u64, amount_msat); + assert_eq!(skimmed_fee_msat * num_mpp_parts as u64, counterparty_skimmed_fee_msat); + assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap()); + match purpose { + crate::events::PaymentPurpose::InvoicePayment { payment_preimage: ev_payment_preimage, + payment_secret: ev_payment_secret, .. } => + { + assert_eq!(payment_preimage, ev_payment_preimage.unwrap()); + assert_eq!(payment_secret, *ev_payment_secret); + }, + _ => panic!(), + } + }, + _ => panic!("Unexpected event"), + } + let mut expected_paths_vecs = Vec::new(); + let mut expected_paths = Vec::new(); + for _ in 0..num_mpp_parts { expected_paths_vecs.push(vec!(&nodes[1], &nodes[2])); } + for i in 0..num_mpp_parts { expected_paths.push(&expected_paths_vecs[i][..]); } + let total_fee_msat = do_claim_payment_along_route_with_extra_penultimate_hop_fees( + &nodes[0], &expected_paths[..], &vec![skimmed_fee_msat as u32; num_mpp_parts][..], false, + payment_preimage); + // The sender doesn't know that the penultimate hop took an extra fee. + expect_payment_sent(&nodes[0], payment_preimage, + Some(Some(total_fee_msat - skimmed_fee_msat * num_mpp_parts as u64)), true); +} + #[derive(PartialEq)] enum AutoRetry { Success, -- 2.39.5