Implement route blinding test vectors #3204

valentinewallace · 2024-07-24T19:08:12Z

See https://github.com/lightning/bolts/blob/3fffab3b889c9a6818130b56fb6dcfdf6906f90e/bolt04/route-blinding-test.json for test vectors.

This also adds support for next_blinding_override in blinded payment paths, and fixes a ser bug for blinded hop features.

Release note

codecov · 2024-07-24T19:21:34Z

Codecov Report

Attention: Patch coverage is 92.08633% with 22 lines in your changes missing coverage. Please review.

Project coverage is 90.21%. Comparing base (8fe3a56) to head (6f6115f).
Report is 44 commits behind head on main.

Files	Patch %	Lines
lightning/src/ln/blinded_payment_tests.rs	91.34%	7 Missing and 13 partials ⚠️
lightning/src/blinded_path/payment.rs	90.00%	1 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3204      +/-   ##
==========================================
+ Coverage   89.73%   90.21%   +0.48%     
==========================================
  Files         123      124       +1     
  Lines      102287   106656    +4369     
  Branches   102287   106656    +4369     
==========================================
+ Hits        91784    96223    +4439     
+ Misses       7807     7759      -48     
+ Partials     2696     2674      -22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

TheBlueMatt

Needs to go in a release soon for the features thing, right?

TheBlueMatt · 2024-08-07T16:03:39Z

lightning/src/blinded_path/payment.rs

@@ -61,6 +61,9 @@ pub struct ForwardTlvs {
 	///
 	/// [`BlindedHop::encrypted_payload`]: crate::blinded_path::BlindedHop::encrypted_payload
 	pub features: BlindedHopFeatures,
+	/// Set if this [`BlindedPath`] is concatenated to another, to indicate the
+	/// [`BlindedPath::blinding_point`] of the appended blinded path.
+	pub next_blinding_override: Option<PublicKey>,


Why didn't we support this to begin with?

IIRC I was under the impression that this field only applies to onion messages (it's required for OMs), not payments. And no one actually sets it for payments, so it never popped up in interop testing. :/

I imagine no one set it ever, because its only useful for some kind of weird frankenstein trampoline afaict? What's the intended usecase for concatenating blinded paths? I mean I suppose we should just do it, but I'm curious why its there.

I think Rusty pointed out here that it can help prevent the intro node from being aware that it's the intro node: lightning/bolts#1182 (comment)? I think this is clarified in lightning/bolts#1182 but still need to review that.

Edit: it is clarified quite a bit in that PR:

Ah, so basically its saying we should send onion messages as a blinded path from us to the intro node, prepended to the real blinded path? Do we do that?

Yes, using next_blinding_override in OMs has always been required to prepend the unblinded path to the intro node.

Oops, I think I misread your message. I'm not sure I follow Rusty's logic, actually. It seems like creating a blinded path from us to the intro node trades off the intro node knowing that they're the intro node for the node prior to the intro node knowing that the next hop is the intro node(?). Unless we set next_blinding_override for every hop in our prepended blinded path, which I suppose is possible.

Ah, I guess I'm still not really understanding why this is here, but its not much complexity to support, sooooo

valentinewallace · 2024-08-16T18:03:03Z

Rebased due to conflicts.

valentinewallace · 2024-08-16T18:07:42Z

lightning/src/ln/features.rs

@@ -98,6 +98,7 @@ impl_feature_write_without_length!(Bolt12InvoiceFeatures);
 impl_feature_write_without_length!(ChannelTypeFeatures);
 impl_feature_write_without_length!(InvoiceRequestFeatures);
 impl_feature_write_without_length!(OfferFeatures);
+impl_feature_write_without_length!(BlindedHopFeatures);


Had to add this line to the feature ser fix due to changes in commit 0c5922e92a3ec3fdf9226cceeb9057ad9aa9dc19.

TheBlueMatt · 2024-08-19T14:01:05Z

error: public documentation for `next_blinding_override` links to private item `BlindedPath`
   --> lightning/src/blinded_path/payment.rs:204:20
    |
204 |     /// Set if this [`BlindedPath`] is concatenated to another, to indicate the
    |                       ^^^^^^^^^^^ this item is private

valentinewallace · 2024-08-19T14:20:19Z

Had a rebase error, squashed change with the following diff:

diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs
index 10d5d8de0..ca937c57d 100644
--- a/lightning/src/blinded_path/payment.rs
+++ b/lightning/src/blinded_path/payment.rs
@@ -201,8 +201,8 @@ pub struct ForwardTlvs {
        ///
        /// [`BlindedHop::encrypted_payload`]: crate::blinded_path::BlindedHop::encrypted_payload
        pub features: BlindedHopFeatures,
-       /// Set if this [`BlindedPath`] is concatenated to another, to indicate the
-       /// [`BlindedPath::blinding_point`] of the appended blinded path.
+       /// Set if this [`BlindedPaymentPath`] is concatenated to another, to indicate the
+       /// [`BlindedPaymentPath::blinding_point`] of the appended blinded path.
        pub next_blinding_override: Option<PublicKey>,
 }

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 2054e4e8e..e08b45558 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -227,8 +227,8 @@ pub struct BlindedForward {
        /// the introduction node.
        pub failure: BlindedFailure,
        /// Overrides the next hop's [`msgs::UpdateAddHTLC::blinding_point`]. Set if this HTLC is being
-       /// forwarded within a [`BlindedPath`] that was concatenated to another blinded path that starts
-       /// at the next hop.
+       /// forwarded within a [`BlindedPaymentPath`] that was concatenated to another blinded path that
+       /// starts at the next hop.
        pub next_blinding_override: Option<PublicKey>,
 }

TheBlueMatt · 2024-08-19T16:42:00Z

 error[E0063]: missing field `next_blinding_override` in initializer of `ForwardTlvs`
   --> src/invoice_request_deser.rs:104:9
    |
104 |         tlvs: ForwardTlvs {
    |               ^^^^^^^^^^^ missing `next_blinding_override`

error[E0063]: missing field `next_blinding_override` in initializer of `ForwardTlvs`
  --> src/refund_deser.rs:82:9
   |
82 |         tlvs: ForwardTlvs {
   |               ^^^^^^^^^^^ missing `next_blinding_override

This allow us to forward blinded payments where the blinded path that we are forwarding within was concatenated to another blinded path that starts at the next hop. Also allows constructing blinded paths using this override.

We were writing a length redundantly...

No reason to take a reference to a Deref.

valentinewallace · 2024-08-19T16:51:34Z

diff --git a/fuzz/src/invoice_request_deser.rs b/fuzz/src/invoice_request_deser.rs
index a5db1c4be..3abb0974e 100644
--- a/fuzz/src/invoice_request_deser.rs
+++ b/fuzz/src/invoice_request_deser.rs
@@ -113,6 +113,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
                                htlc_minimum_msat: 100,
                        },
                        features: BlindedHopFeatures::empty(),
+                       next_blinding_override: None,
                },
                node_id: pubkey(43),
                htlc_maximum_msat: 1_000_000_000_000,
diff --git a/fuzz/src/refund_deser.rs b/fuzz/src/refund_deser.rs
index 58dc68eed..17f255081 100644
--- a/fuzz/src/refund_deser.rs
+++ b/fuzz/src/refund_deser.rs
@@ -91,6 +91,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
                                htlc_minimum_msat: 100,
                        },
                        features: BlindedHopFeatures::empty(),
+                       next_blinding_override: None,
                },
                node_id: pubkey(43),
                htlc_maximum_msat: 1_000_000_000_000,

valentinewallace mentioned this pull request Jul 24, 2024

BOLT12 Offers Tracking Issue #1970

Open

60 tasks

valentinewallace force-pushed the 2024-07-rb-test-vectors branch from 9c228eb to 1f7d3fa Compare July 24, 2024 19:27

TheBlueMatt added this to the 0.0.124 milestone Aug 7, 2024

TheBlueMatt reviewed Aug 7, 2024

View reviewed changes

TheBlueMatt previously approved these changes Aug 14, 2024

View reviewed changes

jkczyz self-requested a review August 16, 2024 15:47

jkczyz previously approved these changes Aug 16, 2024

View reviewed changes

valentinewallace dismissed stale reviews from jkczyz and TheBlueMatt via 76d0d8d August 16, 2024 18:02

valentinewallace force-pushed the 2024-07-rb-test-vectors branch from 1f7d3fa to 76d0d8d Compare August 16, 2024 18:02

valentinewallace commented Aug 16, 2024

View reviewed changes

jkczyz previously approved these changes Aug 16, 2024

View reviewed changes

valentinewallace dismissed jkczyz’s stale review via f6ef1aa August 19, 2024 14:19

valentinewallace force-pushed the 2024-07-rb-test-vectors branch from 76d0d8d to f6ef1aa Compare August 19, 2024 14:19

jkczyz previously approved these changes Aug 19, 2024

View reviewed changes

valentinewallace added 4 commits August 19, 2024 12:47

Support next_blinding_override in blinded payment paths.

f68b124

This allow us to forward blinded payments where the blinded path that we are forwarding within was concatenated to another blinded path that starts at the next hop. Also allows constructing blinded paths using this override.

Fix blinded hop feature serialization.

9dba36a

We were writing a length redundantly...

Implement route blinding spec test vectors.

8739b06

Avoid && in public API for onion peeling/decoding.

6f6115f

No reason to take a reference to a Deref.

valentinewallace dismissed jkczyz’s stale review via 6f6115f August 19, 2024 16:48

valentinewallace force-pushed the 2024-07-rb-test-vectors branch from f6ef1aa to 6f6115f Compare August 19, 2024 16:48

TheBlueMatt approved these changes Aug 19, 2024

View reviewed changes

jkczyz approved these changes Aug 19, 2024

View reviewed changes

TheBlueMatt merged commit fb4403f into lightningdevkit:main Aug 19, 2024
18 of 21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement route blinding test vectors #3204

Implement route blinding test vectors #3204

valentinewallace commented Jul 24, 2024 •

edited

Loading

codecov bot commented Jul 24, 2024 •

edited

Loading

TheBlueMatt left a comment

TheBlueMatt Aug 7, 2024

valentinewallace Aug 7, 2024

TheBlueMatt Aug 7, 2024

valentinewallace Aug 7, 2024 •

edited

Loading

TheBlueMatt Aug 8, 2024

valentinewallace Aug 9, 2024

valentinewallace Aug 12, 2024

TheBlueMatt Aug 14, 2024

valentinewallace commented Aug 16, 2024

valentinewallace Aug 16, 2024

TheBlueMatt commented Aug 19, 2024

valentinewallace commented Aug 19, 2024

TheBlueMatt commented Aug 19, 2024

valentinewallace commented Aug 19, 2024

Implement route blinding test vectors #3204

Implement route blinding test vectors #3204

Conversation

valentinewallace commented Jul 24, 2024 • edited Loading

codecov bot commented Jul 24, 2024 • edited Loading

Codecov Report

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace commented Aug 16, 2024

Choose a reason for hiding this comment

TheBlueMatt commented Aug 19, 2024

valentinewallace commented Aug 19, 2024

TheBlueMatt commented Aug 19, 2024

valentinewallace commented Aug 19, 2024

valentinewallace commented Jul 24, 2024 •

edited

Loading

codecov bot commented Jul 24, 2024 •

edited

Loading

valentinewallace Aug 7, 2024 •

edited

Loading