Skip to content
Open
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
6 changes: 6 additions & 0 deletions lightning-dns-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ mod test {
use lightning::onion_message::messenger::{
AOnionMessenger, Destination, MessageRouter, OnionMessagePath, OnionMessenger,
};
use lightning::routing::router::DEFAULT_PAYMENT_DUMMY_HOPS;
use lightning::sign::{KeysManager, NodeSigner, ReceiveAuthKey, Recipient};
use lightning::types::features::InitFeatures;
use lightning::types::payment::PaymentHash;
Expand Down Expand Up @@ -419,6 +420,11 @@ mod test {
let updates = get_htlc_update_msgs(&nodes[0], &payee_id);
nodes[1].node.handle_update_add_htlc(payer_id, &updates.update_add_htlcs[0]);
do_commitment_signed_dance(&nodes[1], &nodes[0], &updates.commitment_signed, false, false);

for _ in 0..DEFAULT_PAYMENT_DUMMY_HOPS {
nodes[1].node.process_pending_htlc_forwards();
}

expect_and_process_pending_htlcs(&nodes[1], false);

let claimable_events = nodes[1].node.get_and_clear_pending_events();
Expand Down
178 changes: 128 additions & 50 deletions lightning/src/blinded_path/payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use bitcoin::secp256k1::ecdh::SharedSecret;
use bitcoin::secp256k1::{self, PublicKey, Secp256k1, SecretKey};

use crate::blinded_path::message::MAX_DUMMY_HOPS_COUNT;
use crate::blinded_path::utils::{self, BlindedPathWithPadding};
use crate::blinded_path::{BlindedHop, BlindedPath, IntroductionNode, NodeIdLookUp};
use crate::crypto::streams::ChaChaDualPolyReadAdapter;
Expand All @@ -33,7 +34,6 @@ use crate::util::ser::{
Writeable, Writer,
};

use core::mem;
use core::ops::Deref;

#[allow(unused_imports)]
Expand Down Expand Up @@ -121,6 +121,32 @@ impl BlindedPaymentPath {
local_node_receive_key: ReceiveAuthKey, payee_tlvs: ReceiveTlvs, htlc_maximum_msat: u64,
min_final_cltv_expiry_delta: u16, entropy_source: ES, secp_ctx: &Secp256k1<T>,
) -> Result<Self, ()>
where
ES::Target: EntropySource,
{
BlindedPaymentPath::new_with_dummy_hops(
intermediate_nodes,
payee_node_id,
0,
local_node_receive_key,
payee_tlvs,
htlc_maximum_msat,
min_final_cltv_expiry_delta,
entropy_source,
secp_ctx,
)
}

/// Same as [`BlindedPaymentPath::new`], but allows specifying a number of dummy hops.
///
/// Note:
/// At most [`MAX_DUMMY_HOPS_COUNT`] dummy hops can be added to the blinded path.
pub fn new_with_dummy_hops<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
intermediate_nodes: &[PaymentForwardNode], payee_node_id: PublicKey,
dummy_hop_count: usize, local_node_receive_key: ReceiveAuthKey, payee_tlvs: ReceiveTlvs,
htlc_maximum_msat: u64, min_final_cltv_expiry_delta: u16, entropy_source: ES,
secp_ctx: &Secp256k1<T>,
) -> Result<Self, ()>
where
ES::Target: EntropySource,
{
Expand All @@ -145,6 +171,7 @@ impl BlindedPaymentPath {
secp_ctx,
intermediate_nodes,
payee_node_id,
dummy_hop_count,
payee_tlvs,
&blinding_secret,
local_node_receive_key,
Expand Down Expand Up @@ -191,28 +218,31 @@ impl BlindedPaymentPath {
NL::Target: NodeIdLookUp,
T: secp256k1::Signing + secp256k1::Verification,
{
match self.decrypt_intro_payload::<NS>(node_signer) {
Ok((
BlindedPaymentTlvs::Forward(ForwardTlvs { short_channel_id, .. }),
control_tlvs_ss,
)) => {
let next_node_id = match node_id_lookup.next_node_id(short_channel_id) {
Some(node_id) => node_id,
None => return Err(()),
};
let mut new_blinding_point = onion_utils::next_hop_pubkey(
secp_ctx,
self.inner_path.blinding_point,
control_tlvs_ss.as_ref(),
)
.map_err(|_| ())?;
mem::swap(&mut self.inner_path.blinding_point, &mut new_blinding_point);
self.inner_path.introduction_node = IntroductionNode::NodeId(next_node_id);
self.inner_path.blinded_hops.remove(0);
Ok(())
},
_ => Err(()),
}
let (next_node_id, control_tlvs_ss) =
match self.decrypt_intro_payload::<NS>(node_signer).map_err(|_| ())? {
(BlindedPaymentTlvs::Forward(ForwardTlvs { short_channel_id, .. }), ss) => {
let node_id = node_id_lookup.next_node_id(short_channel_id).ok_or(())?;
(node_id, ss)
},
(BlindedPaymentTlvs::Dummy, ss) => {
let node_id = node_signer.get_node_id(Recipient::Node)?;
(node_id, ss)
},
_ => return Err(()),
};

let new_blinding_point = onion_utils::next_hop_pubkey(
secp_ctx,
self.inner_path.blinding_point,
control_tlvs_ss.as_ref(),
)
.map_err(|_| ())?;

self.inner_path.blinding_point = new_blinding_point;
self.inner_path.introduction_node = IntroductionNode::NodeId(next_node_id);
self.inner_path.blinded_hops.remove(0);

Ok(())
}

pub(crate) fn decrypt_intro_payload<NS: Deref>(
Expand All @@ -234,9 +264,9 @@ impl BlindedPaymentPath {
.map_err(|_| ())?;

match (&readable, used_aad) {
(BlindedPaymentTlvs::Forward(_), false) | (BlindedPaymentTlvs::Receive(_), true) => {
Ok((readable, control_tlvs_ss))
},
(BlindedPaymentTlvs::Forward(_), false)
| (BlindedPaymentTlvs::Dummy, true)
| (BlindedPaymentTlvs::Receive(_), true) => Ok((readable, control_tlvs_ss)),
_ => Err(()),
}
}
Expand Down Expand Up @@ -346,6 +376,8 @@ pub struct ReceiveTlvs {
pub(crate) enum BlindedPaymentTlvs {
/// This blinded payment data is for a forwarding node.
Forward(ForwardTlvs),
/// This blinded payment data is dummy and is to be peeled by receiving node.
Dummy,
/// This blinded payment data is for the receiving node.
Receive(ReceiveTlvs),
}
Expand All @@ -363,6 +395,7 @@ pub(crate) enum BlindedTrampolineTlvs {
// Used to include forward and receive TLVs in the same iterator for encoding.
enum BlindedPaymentTlvsRef<'a> {
Forward(&'a ForwardTlvs),
Dummy,
Receive(&'a ReceiveTlvs),
}

Expand Down Expand Up @@ -532,6 +565,11 @@ impl<'a> Writeable for BlindedPaymentTlvsRef<'a> {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
match self {
Self::Forward(tlvs) => tlvs.write(w)?,
Self::Dummy => {
encode_tlv_stream!(w, {
(65539, (), required),
})
},
Self::Receive(tlvs) => tlvs.write(w)?,
}
Ok(())
Expand All @@ -548,32 +586,48 @@ impl Readable for BlindedPaymentTlvs {
(2, scid, option),
(8, next_blinding_override, option),
(10, payment_relay, option),
(12, payment_constraints, required),
(12, payment_constraints, option),
(14, features, (option, encoding: (BlindedHopFeatures, WithoutLength))),
(65536, payment_secret, option),
(65537, payment_context, option),
(65539, is_dummy, option)
});

if let Some(short_channel_id) = scid {
if payment_secret.is_some() {
return Err(DecodeError::InvalidValue);
}
Ok(BlindedPaymentTlvs::Forward(ForwardTlvs {
match (
scid,
next_blinding_override,
payment_relay,
payment_constraints,
features,
payment_secret,
payment_context,
is_dummy,
) {
(
Some(short_channel_id),
next_override,
Some(relay),
Some(constraints),
features,
None,
None,
None,
) => Ok(BlindedPaymentTlvs::Forward(ForwardTlvs {
short_channel_id,
payment_relay: payment_relay.ok_or(DecodeError::InvalidValue)?,
payment_constraints: payment_constraints.0.unwrap(),
next_blinding_override,
payment_relay: relay,
payment_constraints: constraints,
next_blinding_override: next_override,
features: features.unwrap_or_else(BlindedHopFeatures::empty),
}))
} else {
if payment_relay.is_some() || features.is_some() {
return Err(DecodeError::InvalidValue);
}
Ok(BlindedPaymentTlvs::Receive(ReceiveTlvs {
payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
payment_constraints: payment_constraints.0.unwrap(),
payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
}))
})),
(None, None, None, Some(constraints), None, Some(secret), Some(context), None) => {
Ok(BlindedPaymentTlvs::Receive(ReceiveTlvs {
payment_secret: secret,
payment_constraints: constraints,
payment_context: context,
}))
},
(None, None, None, None, None, None, None, Some(())) => Ok(BlindedPaymentTlvs::Dummy),
_ => return Err(DecodeError::InvalidValue),
}
}
}
Expand Down Expand Up @@ -620,22 +674,46 @@ pub(crate) const PAYMENT_PADDING_ROUND_OFF: usize = 30;
/// Construct blinded payment hops for the given `intermediate_nodes` and payee info.
pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
secp_ctx: &Secp256k1<T>, intermediate_nodes: &[PaymentForwardNode], payee_node_id: PublicKey,
payee_tlvs: ReceiveTlvs, session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey,
dummy_hop_count: usize, payee_tlvs: ReceiveTlvs, session_priv: &SecretKey,
local_node_receive_key: ReceiveAuthKey,
) -> Vec<BlindedHop> {
let dummy_count = core::cmp::min(dummy_hop_count, MAX_DUMMY_HOPS_COUNT);
let pks = intermediate_nodes
.iter()
.map(|node| (node.node_id, None))
.chain(core::iter::repeat((payee_node_id, Some(local_node_receive_key))).take(dummy_count))
.chain(core::iter::once((payee_node_id, Some(local_node_receive_key))));
let tlvs = intermediate_nodes
.iter()
.map(|node| BlindedPaymentTlvsRef::Forward(&node.tlvs))
.chain((0..dummy_count).map(|_| BlindedPaymentTlvsRef::Dummy))
Comment on lines +684 to +689
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can you use the same syntax for the two additions here? either repeat.take or (range).map, it reads funny to use both.

.chain(core::iter::once(BlindedPaymentTlvsRef::Receive(&payee_tlvs)));

let path = pks.zip(
tlvs.map(|tlv| BlindedPathWithPadding { tlvs: tlv, round_off: PAYMENT_PADDING_ROUND_OFF }),
);
let path: Vec<_> = pks
.zip(
tlvs.map(|tlv| BlindedPathWithPadding {
tlvs: tlv,
round_off: PAYMENT_PADDING_ROUND_OFF,
}),
)
.collect();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... is this collect only needed so we can perform the debug_assert below? Ideally, we could avoid that. Would it be possible to do the check in construct_blinded_hops while iterating?

Copy link
Member Author

Choose a reason for hiding this comment

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

I experimented with it a bit and came up with a solution that lets us move the check downstream (construct_blinded_hopsconstruct_keys_for_blinded_path) while iterating while avoiding clone or collect. reference

However, I noticed that construct_blinded_hops is shared between both compact (unpadded) and non-compact paths, which means we’d need to pass compactness information downstream for the invariant to hold.

I’m curious whether refactoring the parameter just for a debug assertion is a better design choice than keeping the small .collect() at the current level. Would love to hear your thoughts. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... I don't have a strong opinion. cc: @TheBlueMatt in case he cares about the extra allocation.

Maybe instead of moving the check we can chain an inspect to the iterator for performing the check. We'd need to drop the skip, which is fine, and would probably need to use peekable to get the first item without advancing the iterator. Then use a match within the inspect to not check BlindedPaymentTlvsRef::Receive.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget that Iterators are often Cloneable :).

$ git diff
diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs
index 2acd5d2098..042dceacc8 100644
--- a/lightning/src/blinded_path/payment.rs
+++ b/lightning/src/blinded_path/payment.rs
@@ -393,6 +393,7 @@ pub(crate) enum BlindedTrampolineTlvs {
 }

 // Used to include forward and receive TLVs in the same iterator for encoding.
+#[derive(Clone)]
 enum BlindedPaymentTlvsRef<'a> {
        Forward(&'a ForwardTlvs),
        Dummy,
@@ -689,22 +690,21 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
                .chain((0..dummy_count).map(|_| BlindedPaymentTlvsRef::Dummy))
                .chain(core::iter::once(BlindedPaymentTlvsRef::Receive(&payee_tlvs)));

-       let path: Vec<_> = pks
+       let path = pks
                .zip(
                        tlvs.map(|tlv| BlindedPathWithPadding {
                                tlvs: tlv,
                                round_off: PAYMENT_PADDING_ROUND_OFF,
                        }),
-               )
-               .collect();
+               );

        // Debug invariant: all non-final hops must have identical serialized size.
        #[cfg(debug_assertions)]
-       if let Some((_, first)) = path.first() {
-               if path.len() > 2 {
+       if let Some((_, first)) = path.clone().next() {
+               if path.clone().count() > 2 {
                        let expected = first.serialized_length();

-                       for (_, hop) in path.iter().skip(1).take(path.len() - 2) {
+                       for (_, hop) in path.clone().skip(1).take(path.clone().count() - 2) {
                                debug_assert!(
                                        hop.serialized_length() == expected,
                                        "All intermediate blinded hops must have identical serialized size"


// Debug invariant: all non-final hops must have identical serialized size.
#[cfg(debug_assertions)]
if let Some((_, first)) = path.first() {
if path.len() > 2 {
let expected = first.serialized_length();

for (_, hop) in path.iter().skip(1).take(path.len() - 2) {
debug_assert!(
hop.serialized_length() == expected,
"All intermediate blinded hops must have identical serialized size"
);
}
}
}

utils::construct_blinded_hops(secp_ctx, path, session_priv)
utils::construct_blinded_hops(secp_ctx, path.into_iter(), session_priv)
}

/// `None` if underflow occurs.
Expand Down
9 changes: 8 additions & 1 deletion lightning/src/ln/async_payments_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use crate::onion_message::messenger::{
use crate::onion_message::offers::OffersMessage;
use crate::onion_message::packet::ParsedOnionMessageContents;
use crate::prelude::*;
use crate::routing::router::{Payee, PaymentParameters};
use crate::routing::router::{Payee, PaymentParameters, DEFAULT_PAYMENT_DUMMY_HOPS};
use crate::sign::NodeSigner;
use crate::sync::Mutex;
use crate::types::features::Bolt12InvoiceFeatures;
Expand Down Expand Up @@ -1858,6 +1858,13 @@ fn expired_static_invoice_payment_path() {
blinded_path
.advance_path_by_one(&nodes[1].keys_manager, &nodes[1].node, &secp_ctx)
.unwrap();

for _ in 0..DEFAULT_PAYMENT_DUMMY_HOPS {
blinded_path
.advance_path_by_one(&nodes[2].keys_manager, &nodes[2].node, &secp_ctx)
.unwrap();
}

match blinded_path.decrypt_intro_payload(&nodes[2].keys_manager).unwrap().0 {
BlindedPaymentTlvs::Receive(tlvs) => tlvs.payment_constraints.max_cltv_expiry,
_ => panic!(),
Expand Down
Loading
Loading