Skip to content

Support keysend to blinded paths #2935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions lightning/src/ln/blinded_payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,3 +1183,84 @@ fn conditionally_round_fwd_amt() {
let expected_fee = pass_claimed_payment_along_route(args);
expect_payment_sent(&nodes[0], payment_preimage, Some(Some(expected_fee)), true, true);
}

#[test]
fn blinded_keysend() {
let mut mpp_keysend_config = test_default_channel_config();
mpp_keysend_config.accept_mpp_keysend = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkczyz @TheBlueMatt FYI --

I ended up keeping the payment secret in keysend blinded paths. I'm not sure why I thought it wasn't supposed to be reused at first, but I think it's fine to reuse it and provides value for keysend payments.

As a result, it was easy to adapt the existing PendingHTLCRouting::ReceiveKeysend variant for blinded keysends.
However, reusing this variant means that users that want to receive blinded keysends have to set accept_mpp_keysend in their config or else LDK will reject the payment.

I think that's okay, but wanted to call it out in case there are differing opinions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems ok to me. But why does it need to be set true in this test if blinded_mpp_keysend tests MPP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept_mpp_keysend means that users opt into potentially breaking compat with < 0.0.116, which can happen if PendingHTLCRouting::ReceiveKeysend::payment_data is set. Because the payment secret will always be present in keysends to blinded paths, ::payment_data will always be Some, so users need to opt into the potential compat breakage. Does that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess? ISTM we could instead actually just use the keysend_preimage - only the sender has that, so no need to worry about the secret anywhere. In general I kinda anticipated no one would support reading a payment secret in a blinded-path-receive HTLC, but certainly we could require it 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess? ISTM we could instead actually just use the keysend_preimage - only the sender has that, so no need to worry about the secret anywhere. In general I kinda anticipated no one would support reading a payment secret in a blinded-path-receive HTLC, but certainly we could require it 🤷

I'm confused, we currently require payment secret to be present in all blinded path receives (so we can do stateless inbound payment verification). So the user isn't providing the payment secret, we're providing it back to ourselves in the blinded path.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay, nevermind, I was confused where that field was coming from.

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, Some(mpp_keysend_config)]);
let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;

let amt_msat = 5000;
let (keysend_preimage, _, payment_secret) = get_payment_preimage_hash(&nodes[2], None, None);
let route_params = get_blinded_route_parameters(amt_msat, payment_secret, 1,
1_0000_0000,
nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(),
&[&chan_upd_1_2], &chanmon_cfgs[2].keys_manager);

let payment_hash = nodes[0].node.send_spontaneous_payment_with_retry(Some(keysend_preimage), RecipientOnionFields::spontaneous_empty(), PaymentId(keysend_preimage.0), route_params, Retry::Attempts(0)).unwrap();
check_added_monitors(&nodes[0], 1);

let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[2]]];
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);

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], amt_msat, payment_hash, Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
}

#[test]
fn blinded_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);
let chan_1_3 = create_announced_chan_between_nodes(&nodes, 1, 3);
let chan_2_3 = create_announced_chan_between_nodes(&nodes, 2, 3);

let amt_msat = 15_000_000;
let (keysend_preimage, _, payment_secret) = get_payment_preimage_hash(&nodes[3], None, None);
let route_params = {
let pay_params = PaymentParameters::blinded(
vec![
blinded_payment_path(payment_secret, 1, 1_0000_0000,
vec![nodes[1].node.get_our_node_id(), nodes[3].node.get_our_node_id()], &[&chan_1_3.0.contents],
&chanmon_cfgs[3].keys_manager
),
blinded_payment_path(payment_secret, 1, 1_0000_0000,
vec![nodes[2].node.get_our_node_id(), nodes[3].node.get_our_node_id()], &[&chan_2_3.0.contents],
&chanmon_cfgs[3].keys_manager
),
]
)
.with_bolt12_features(channelmanager::provided_bolt12_invoice_features(&UserConfig::default()))
.unwrap();
RouteParameters::from_payment_params_and_value(pay_params, amt_msat)
};

let payment_hash = nodes[0].node.send_spontaneous_payment_with_retry(Some(keysend_preimage), RecipientOnionFields::spontaneous_empty(), PaymentId(keysend_preimage.0), route_params, Retry::Attempts(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], amt_msat, payment_hash.clone(),
Some(payment_secret), ev.clone(), false, Some(keysend_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], amt_msat, payment_hash.clone(),
Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
}
9 changes: 8 additions & 1 deletion lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ pub enum PendingHTLCRouting {
/// For HTLCs received by LDK, these will ultimately bubble back up as
/// [`RecipientOnionFields::custom_tlvs`].
custom_tlvs: Vec<(u64, Vec<u8>)>,
/// Set if this HTLC is the final hop in a multi-hop blinded path.
requires_blinded_error: bool,
},
}

Expand All @@ -221,6 +223,7 @@ impl PendingHTLCRouting {
match self {
Self::Forward { blinded: Some(BlindedForward { failure, .. }), .. } => Some(*failure),
Self::Receive { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode),
Self::ReceiveKeysend { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode),
_ => None,
}
}
Expand Down Expand Up @@ -4523,7 +4526,10 @@ where
(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data },
Some(payment_data), phantom_shared_secret, onion_fields)
},
PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry, custom_tlvs } => {
PendingHTLCRouting::ReceiveKeysend {
payment_data, payment_preimage, payment_metadata,
incoming_cltv_expiry, custom_tlvs, requires_blinded_error: _
} => {
let onion_fields = RecipientOnionFields {
payment_secret: payment_data.as_ref().map(|data| data.payment_secret),
payment_metadata,
Expand Down Expand Up @@ -9766,6 +9772,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
},
(2, ReceiveKeysend) => {
(0, payment_preimage, required),
(1, requires_blinded_error, (default_value, false)),
(2, incoming_cltv_expiry, required),
(3, payment_metadata, option),
(4, payment_data, option), // Added in 0.0.116
Expand Down
16 changes: 10 additions & 6 deletions lightning/src/ln/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1688,6 +1688,7 @@ mod fuzzy_internal_msgs {
payment_secret: PaymentSecret,
payment_constraints: PaymentConstraints,
intro_node_blinding_point: Option<PublicKey>,
keysend_preimage: Option<PaymentPreimage>,
}
}

Expand Down Expand Up @@ -1716,6 +1717,7 @@ mod fuzzy_internal_msgs {
cltv_expiry_height: u32,
encrypted_tlvs: Vec<u8>,
intro_node_blinding_point: Option<PublicKey>, // Set if the introduction node of the blinded path is the final node
keysend_preimage: Option<PaymentPreimage>,
}
}

Expand Down Expand Up @@ -2503,14 +2505,15 @@ impl Writeable for OutboundOnionPayload {
},
Self::BlindedReceive {
sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, encrypted_tlvs,
intro_node_blinding_point,
intro_node_blinding_point, keysend_preimage,
} => {
_encode_varint_length_prefixed_tlv!(w, {
(2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required),
(4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required),
(10, *encrypted_tlvs, required_vec),
(12, intro_node_blinding_point, option),
(18, HighZeroBytesDroppedBigSize(*total_msat), required)
(18, HighZeroBytesDroppedBigSize(*total_msat), required),
(5482373484, keysend_preimage, option)
});
},
}
Expand Down Expand Up @@ -2560,9 +2563,7 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, &NS)> for InboundOnionPayload w
}

if let Some(blinding_point) = intro_node_blinding_point.or(update_add_blinding_point) {
if short_id.is_some() || payment_data.is_some() || payment_metadata.is_some() ||
keysend_preimage.is_some()
{
if short_id.is_some() || payment_data.is_some() || payment_metadata.is_some() {
return Err(DecodeError::InvalidValue)
}
let enc_tlvs = encrypted_tlvs_opt.ok_or(DecodeError::InvalidValue)?.0;
Expand All @@ -2575,7 +2576,9 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, &NS)> for InboundOnionPayload w
ChaChaPolyReadAdapter { readable: BlindedPaymentTlvs::Forward(ForwardTlvs {
short_channel_id, payment_relay, payment_constraints, features
})} => {
if amt.is_some() || cltv_value.is_some() || total_msat.is_some() {
if amt.is_some() || cltv_value.is_some() || total_msat.is_some() ||
keysend_preimage.is_some()
{
return Err(DecodeError::InvalidValue)
}
Ok(Self::BlindedForward {
Expand All @@ -2597,6 +2600,7 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, &NS)> for InboundOnionPayload w
payment_secret,
payment_constraints,
intro_node_blinding_point,
keysend_preimage,
})
},
}
Expand Down
8 changes: 5 additions & 3 deletions lightning/src/ln/onion_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub(super) fn create_recv_pending_htlc_info(
cltv_expiry_height, payment_metadata, false),
msgs::InboundOnionPayload::BlindedReceive {
sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, payment_secret,
intro_node_blinding_point, payment_constraints, ..
intro_node_blinding_point, payment_constraints, keysend_preimage, ..
} => {
check_blinded_payment_constraints(
sender_intended_htlc_amt_msat, cltv_expiry, &payment_constraints
Expand All @@ -152,8 +152,9 @@ pub(super) fn create_recv_pending_htlc_info(
}
})?;
let payment_data = msgs::FinalOnionHopData { payment_secret, total_msat };
(Some(payment_data), None, Vec::new(), sender_intended_htlc_amt_msat, cltv_expiry_height,
None, intro_node_blinding_point.is_none())
(Some(payment_data), keysend_preimage, Vec::new(),
sender_intended_htlc_amt_msat, cltv_expiry_height, None,
intro_node_blinding_point.is_none())
}
msgs::InboundOnionPayload::Forward { .. } => {
return Err(InboundHTLCErr {
Expand Down Expand Up @@ -232,6 +233,7 @@ pub(super) fn create_recv_pending_htlc_info(
payment_metadata,
incoming_cltv_expiry: onion_cltv_expiry,
custom_tlvs,
requires_blinded_error,
}
} else if let Some(data) = payment_data {
PendingHTLCRouting::Receive {
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/onion_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ pub(super) fn build_onion_payloads(
cltv_expiry_height: cur_cltv + excess_final_cltv_expiry_delta,
encrypted_tlvs: blinded_hop.encrypted_payload.clone(),
intro_node_blinding_point: blinding_point.take(),
keysend_preimage: *keysend_preimage,
});
} else {
res.push(msgs::OutboundOnionPayload::BlindedForward {
Expand Down