Skip to content

Commit 3a7f6cb

Browse files
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.
1 parent 329acbf commit 3a7f6cb

File tree

3 files changed

+202
-17
lines changed

3 files changed

+202
-17
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,14 @@ pub struct PhantomRouteHints {
14541454
pub real_node_pubkey: PublicKey,
14551455
}
14561456

1457+
/// Information to retrieve a Channel from ChannelManager's internal maps.
1458+
enum ChanMapRetrieval {
1459+
/// Retrieve the Channel via its short channel id.
1460+
Scid(u64),
1461+
/// Retrieve the Channel via its channel_id and the counterparty's node id.
1462+
IdPubkey((PublicKey, [u8; 32])),
1463+
}
1464+
14571465
macro_rules! handle_error {
14581466
($self: ident, $internal: expr, $counterparty_node_id: expr) => { {
14591467
// In testing, ensure there are no deadlocks where the lock is already held upon
@@ -2297,9 +2305,11 @@ where
22972305
}
22982306
}
22992307

2300-
fn construct_recv_pending_htlc_info(&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32],
2301-
payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, ReceiveError>
2302-
{
2308+
fn construct_recv_pending_htlc_info(
2309+
&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32], payment_hash: PaymentHash,
2310+
amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>,
2311+
chan_retrieval_info: ChanMapRetrieval
2312+
) -> Result<PendingHTLCInfo, ReceiveError> {
23032313
// final_incorrect_cltv_expiry
23042314
if hop_data.outgoing_cltv_value > cltv_expiry {
23052315
return Err(ReceiveError {
@@ -2326,11 +2336,36 @@ where
23262336
});
23272337
}
23282338
if hop_data.amt_to_forward > amt_msat {
2329-
return Err(ReceiveError {
2330-
err_code: 19,
2331-
err_data: amt_msat.to_be_bytes().to_vec(),
2332-
msg: "Upstream node sent less than we were supposed to receive in payment",
2333-
});
2339+
let allow_underpay = loop {
2340+
let (counterparty_node_id, channel_id) = match chan_retrieval_info {
2341+
ChanMapRetrieval::Scid(scid) => {
2342+
match self.short_to_chan_info.read().unwrap().get(&scid).cloned() {
2343+
Some((node_id, chan_id)) => (node_id, chan_id),
2344+
None => break false,
2345+
}
2346+
},
2347+
ChanMapRetrieval::IdPubkey((pubkey, chan_id)) => (pubkey, chan_id),
2348+
};
2349+
let per_peer_state = self.per_peer_state.read().unwrap();
2350+
let peer_state_mtx = match per_peer_state.get(&counterparty_node_id) {
2351+
None => break false,
2352+
Some(peer_state) => peer_state,
2353+
};
2354+
let mut peer_state_lock = peer_state_mtx.lock().unwrap();
2355+
let peer_state = &mut *peer_state_lock;
2356+
let chan = match peer_state.channel_by_id.get_mut(&channel_id) {
2357+
None => break false,
2358+
Some(chan) => chan,
2359+
};
2360+
break chan.config().accept_underpaying_htlcs
2361+
};
2362+
if !allow_underpay {
2363+
return Err(ReceiveError {
2364+
err_code: 19,
2365+
err_data: amt_msat.to_be_bytes().to_vec(),
2366+
msg: "Upstream node sent less than we were supposed to receive in payment",
2367+
});
2368+
}
23342369
}
23352370

23362371
let routing = match hop_data.format {
@@ -2394,7 +2429,9 @@ where
23942429
})
23952430
}
23962431

2397-
fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> PendingHTLCStatus {
2432+
fn decode_update_add_htlc_onion(
2433+
&self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey
2434+
) -> PendingHTLCStatus {
23982435
macro_rules! return_malformed_err {
23992436
($msg: expr, $err_code: expr) => {
24002437
{
@@ -2453,7 +2490,10 @@ where
24532490
let pending_forward_info = match next_hop {
24542491
onion_utils::Hop::Receive(next_hop_data) => {
24552492
// OUR PAYMENT!
2456-
match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None) {
2493+
match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash,
2494+
msg.amount_msat, msg.cltv_expiry, None,
2495+
ChanMapRetrieval::IdPubkey((*counterparty_node_id, msg.channel_id)))
2496+
{
24572497
Ok(info) => {
24582498
// Note that we could obviously respond immediately with an update_fulfill_htlc
24592499
// message, however that would leak that we are the recipient of this payment, so
@@ -3372,7 +3412,11 @@ where
33723412
};
33733413
match next_hop {
33743414
onion_utils::Hop::Receive(hop_data) => {
3375-
match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value, Some(phantom_shared_secret)) {
3415+
match self.construct_recv_pending_htlc_info(hop_data,
3416+
incoming_shared_secret, payment_hash, outgoing_amt_msat,
3417+
outgoing_cltv_value, Some(phantom_shared_secret),
3418+
ChanMapRetrieval::Scid(prev_short_channel_id))
3419+
{
33763420
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, vec![(info, prev_htlc_id)])),
33773421
Err(ReceiveError { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
33783422
}
@@ -5074,7 +5118,7 @@ where
50745118
//encrypted with the same key. It's not immediately obvious how to usefully exploit that,
50755119
//but we should prevent it anyway.
50765120

5077-
let pending_forward_info = self.decode_update_add_htlc_onion(msg);
5121+
let pending_forward_info = self.decode_update_add_htlc_onion(msg, counterparty_node_id);
50785122
let per_peer_state = self.per_peer_state.read().unwrap();
50795123
let peer_state_mutex = per_peer_state.get(counterparty_node_id)
50805124
.ok_or_else(|| {

lightning/src/ln/functional_test_utils.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,7 +2164,20 @@ pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route
21642164
(our_payment_preimage, our_payment_hash, our_payment_secret, payment_id)
21652165
}
21662166

2167-
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 {
2167+
pub fn do_claim_payment_along_route<'a, 'b, 'c>(
2168+
origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool,
2169+
our_payment_preimage: PaymentPreimage
2170+
) -> u64 {
2171+
let extra_fees = vec![0; expected_paths.len()];
2172+
do_claim_payment_along_route_with_extra_penultimate_hop_fees(origin_node, expected_paths,
2173+
&extra_fees[..], skip_last, our_payment_preimage)
2174+
}
2175+
2176+
pub fn do_claim_payment_along_route_with_extra_penultimate_hop_fees<'a, 'b, 'c>(
2177+
origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], expected_extra_fees:
2178+
&[u32], skip_last: bool, our_payment_preimage: PaymentPreimage
2179+
) -> u64 {
2180+
assert_eq!(expected_paths.len(), expected_extra_fees.len());
21682181
for path in expected_paths.iter() {
21692182
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
21702183
}
@@ -2214,7 +2227,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
22142227
}
22152228
}
22162229

2217-
for (expected_route, (path_msgs, next_hop)) in expected_paths.iter().zip(per_path_msgs.drain(..)) {
2230+
for (i, (expected_route, (path_msgs, next_hop))) in expected_paths.iter().zip(per_path_msgs.drain(..)).enumerate() {
22182231
let mut next_msgs = Some(path_msgs);
22192232
let mut expected_next_node = next_hop;
22202233

@@ -2229,10 +2242,10 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
22292242
}
22302243
}
22312244
macro_rules! mid_update_fulfill_dance {
2232-
($node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => {
2245+
($idx: expr, $node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => {
22332246
{
22342247
$node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
2235-
let fee = {
2248+
let mut fee = {
22362249
let per_peer_state = $node.node.per_peer_state.read().unwrap();
22372250
let peer_state = per_peer_state.get(&$prev_node.node.get_our_node_id())
22382251
.unwrap().lock().unwrap();
@@ -2243,6 +2256,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
22432256
channel.config().forwarding_fee_base_msat
22442257
}
22452258
};
2259+
if $idx == 1 { fee += expected_extra_fees[i]; }
22462260
expect_payment_forwarded!($node, $next_node, $prev_node, Some(fee as u64), false, false);
22472261
expected_total_fee_msat += fee as u64;
22482262
check_added_monitors!($node, 1);
@@ -2274,7 +2288,7 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
22742288
} else {
22752289
next_node = expected_route[expected_route.len() - 1 - idx - 1];
22762290
}
2277-
mid_update_fulfill_dance!(node, prev_node, next_node, update_next_msgs);
2291+
mid_update_fulfill_dance!(idx, node, prev_node, next_node, update_next_msgs);
22782292
} else {
22792293
assert!(!update_next_msgs);
22802294
assert!(node.node.get_and_clear_pending_msg_events().is_empty());

lightning/src/ln/payment_tests.rs

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,133 @@ fn do_test_intercepted_payment(test: InterceptTest) {
15461546
}
15471547
}
15481548

1549+
#[test]
1550+
fn accept_underpaying_htlcs_config() {
1551+
do_accept_underpaying_htlcs_config(1);
1552+
do_accept_underpaying_htlcs_config(2);
1553+
do_accept_underpaying_htlcs_config(3);
1554+
}
1555+
1556+
fn do_accept_underpaying_htlcs_config(num_mpp_parts: usize) {
1557+
let chanmon_cfgs = create_chanmon_cfgs(3);
1558+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
1559+
let mut intercept_forwards_config = test_default_channel_config();
1560+
intercept_forwards_config.accept_intercept_htlcs = true;
1561+
let mut underpay_config = test_default_channel_config();
1562+
underpay_config.channel_config.accept_underpaying_htlcs = true;
1563+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), Some(underpay_config)]);
1564+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
1565+
1566+
let mut chan_ids = Vec::new();
1567+
for _ in 0..num_mpp_parts {
1568+
let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000, 0);
1569+
let channel_id = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 2_000_000, 0).0.channel_id;
1570+
chan_ids.push(channel_id);
1571+
}
1572+
1573+
// Send the initial payment.
1574+
let amt_msat = 900_000;
1575+
let skimmed_fee_msat = 20;
1576+
let mut route_hints = Vec::new();
1577+
for _ in 0..num_mpp_parts {
1578+
route_hints.push(RouteHint(vec![RouteHintHop {
1579+
src_node_id: nodes[1].node.get_our_node_id(),
1580+
short_channel_id: nodes[1].node.get_intercept_scid(),
1581+
fees: RoutingFees {
1582+
base_msat: 1000,
1583+
proportional_millionths: 0,
1584+
},
1585+
cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
1586+
htlc_minimum_msat: None,
1587+
htlc_maximum_msat: Some(amt_msat / num_mpp_parts as u64 + 5),
1588+
}]));
1589+
}
1590+
let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
1591+
.with_route_hints(route_hints).unwrap()
1592+
.with_bolt11_features(nodes[2].node.invoice_features()).unwrap();
1593+
let route_params = RouteParameters {
1594+
payment_params,
1595+
final_value_msat: amt_msat,
1596+
};
1597+
let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap();
1598+
nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
1599+
PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
1600+
check_added_monitors!(nodes[0], num_mpp_parts); // one monitor per path
1601+
let mut events: Vec<SendEvent> = nodes[0].node.get_and_clear_pending_msg_events().into_iter().map(|e| SendEvent::from_event(e)).collect();
1602+
assert_eq!(events.len(), num_mpp_parts);
1603+
1604+
// Forward the intercepted payments.
1605+
for (idx, ev) in events.into_iter().enumerate() {
1606+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &ev.msgs[0]);
1607+
do_commitment_signed_dance(&nodes[1], &nodes[0], &ev.commitment_msg, false, true);
1608+
1609+
let events = nodes[1].node.get_and_clear_pending_events();
1610+
assert_eq!(events.len(), 1);
1611+
let (intercept_id, expected_outbound_amt_msat) = match events[0] {
1612+
crate::events::Event::HTLCIntercepted {
1613+
intercept_id, expected_outbound_amount_msat, payment_hash: pmt_hash, ..
1614+
} => {
1615+
assert_eq!(pmt_hash, payment_hash);
1616+
(intercept_id, expected_outbound_amount_msat)
1617+
},
1618+
_ => panic!()
1619+
};
1620+
nodes[1].node.forward_intercepted_htlc(intercept_id, &chan_ids[idx],
1621+
nodes[2].node.get_our_node_id(), expected_outbound_amt_msat - skimmed_fee_msat).unwrap();
1622+
expect_pending_htlcs_forwardable!(nodes[1]);
1623+
let payment_event = {
1624+
{
1625+
let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
1626+
assert_eq!(added_monitors.len(), 1);
1627+
added_monitors.clear();
1628+
}
1629+
let mut events = nodes[1].node.get_and_clear_pending_msg_events();
1630+
assert_eq!(events.len(), 1);
1631+
SendEvent::from_event(events.remove(0))
1632+
};
1633+
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]);
1634+
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event.commitment_msg, false, true);
1635+
if idx == num_mpp_parts - 1 {
1636+
expect_pending_htlcs_forwardable!(nodes[2]);
1637+
}
1638+
}
1639+
1640+
// Claim the payment and check that the skimmed fee is as expected.
1641+
let payment_preimage = nodes[2].node.get_payment_preimage(payment_hash, payment_secret).unwrap();
1642+
let events = nodes[2].node.get_and_clear_pending_events();
1643+
assert_eq!(events.len(), 1);
1644+
match events[0] {
1645+
crate::events::Event::PaymentClaimable {
1646+
ref payment_hash, ref purpose, amount_msat, counterparty_skimmed_fee_msat, receiver_node_id, ..
1647+
} => {
1648+
assert_eq!(payment_hash, payment_hash);
1649+
assert_eq!(amt_msat - skimmed_fee_msat * num_mpp_parts as u64, amount_msat);
1650+
assert_eq!(skimmed_fee_msat * num_mpp_parts as u64, counterparty_skimmed_fee_msat);
1651+
assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap());
1652+
match purpose {
1653+
crate::events::PaymentPurpose::InvoicePayment { payment_preimage: ev_payment_preimage,
1654+
payment_secret: ev_payment_secret, .. } =>
1655+
{
1656+
assert_eq!(payment_preimage, ev_payment_preimage.unwrap());
1657+
assert_eq!(payment_secret, *ev_payment_secret);
1658+
},
1659+
_ => panic!(),
1660+
}
1661+
},
1662+
_ => panic!("Unexpected event"),
1663+
}
1664+
let mut expected_paths_vecs = Vec::new();
1665+
let mut expected_paths = Vec::new();
1666+
for _ in 0..num_mpp_parts { expected_paths_vecs.push(vec!(&nodes[1], &nodes[2])); }
1667+
for i in 0..num_mpp_parts { expected_paths.push(&expected_paths_vecs[i][..]); }
1668+
let total_fee_msat = do_claim_payment_along_route_with_extra_penultimate_hop_fees(
1669+
&nodes[0], &expected_paths[..], &vec![skimmed_fee_msat as u32; num_mpp_parts][..], false,
1670+
payment_preimage);
1671+
// The sender doesn't know that the penultimate hop took an extra fee.
1672+
expect_payment_sent(&nodes[0], payment_preimage,
1673+
Some(Some(total_fee_msat - skimmed_fee_msat * num_mpp_parts as u64)), true);
1674+
}
1675+
15491676
#[derive(PartialEq)]
15501677
enum AutoRetry {
15511678
Success,

0 commit comments

Comments
 (0)