Serialize blinded Trampoline hops #3007

arik-so · 2024-04-19T15:20:14Z

Add BlindedForward and BlindedReceive variants to OutboundTrampolinePayload.

codecov-commenter · 2024-04-19T16:35:23Z

Codecov Report

Attention: Patch coverage is 97.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.89%. Comparing base (66fb520) to head (9d01751).
Report is 66 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/msgs.rs	97.50%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3007      +/-   ##
==========================================
+ Coverage   89.66%   90.89%   +1.23%     
==========================================
  Files         126      127       +1     
  Lines      102676   115370   +12694     
  Branches   102676   115370   +12694     
==========================================
+ Hits        92062   104864   +12802     
+ Misses       7894     7854      -40     
+ Partials     2720     2652      -68

Flag	Coverage Δ
	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

TheBlueMatt

Similarly, why is this draft? What's it waiting on?

TheBlueMatt · 2024-04-22T17:36:43Z

lightning/src/ln/msgs.rs

 			outgoing_node_id: PublicKey,
+		},
+		#[allow(unused)]
+		BlindedForward {


Can we get some docs, what does BlindedForward and BlindedReceive actually mean?

Any update here?

TheBlueMatt · 2024-06-17T13:59:16Z

CI is quite sad.

TheBlueMatt · 2024-06-24T22:36:38Z

Feel free to squash before any reviewers take a big look.

TheBlueMatt · 2024-06-29T17:23:52Z

lightning/src/ln/msgs.rs

+			/// List of blinded path options the last trampoline hop may choose to route through.
+			payment_paths: Vec<BlindedPath>,
+			/// If applicable, features of the BOLT12 invoice being paid.
+			invoice_features: Option<Bolt12InvoiceFeatures>


Are you sure this is supposed to be a Bolt12InvoiceFeatures and not a BlindedPathFeatures? I mean it makes sense just want to double-check.

by BlindedPathFeatures do you mean BlindedHopFeatures?

Uhhh...I guess?

yeah, pretty sure invoice features are correct here

TheBlueMatt · 2024-06-29T17:23:58Z

lightning/src/ln/msgs.rs

 			outgoing_node_id: PublicKey,
+		},
+		#[allow(unused)]
+		BlindedForward {


Any update here?

TheBlueMatt · 2024-10-07T14:12:08Z

lightning/src/ln/msgs.rs

+					p.inner_blinded_path().encode().into_iter().chain(p.payinfo.encode()).collect::<Vec<u8>>()
+				}).collect();


You're building multiple vecs here where I think we can get away with none. May need to support writing iterators over bytes, but that should be doable?

TheBlueMatt · 2024-10-11T15:31:30Z

lightning/src/ln/msgs.rs

+					(2, HighZeroBytesDroppedBigSize(*amt_to_forward), required),
+					(4, HighZeroBytesDroppedBigSize(*outgoing_cltv_value), required),
+					(21, invoice_features.as_ref().map(|m| WithoutLength(m)), option)
+				}, [&blinded_path_tlv]);


Can we expand this to support writing Iterator<u8>s?

you mean such as in BlindedReceive, which allows passing custom TLVs?

Yea, like that.

TheBlueMatt · 2024-10-17T00:05:00Z

lightning/src/ln/msgs.rs

+				}).collect();
+				let blinded_path_tlv = (22, blinded_path_value);
+				let custom_tlvs: Vec<&(u64, Vec<u8>)> = core::iter::once(blinded_path_tlv).collect();


Why are you collecting into a vec and then iterating it? Let's avoid the allocation.

TheBlueMatt · 2024-10-17T00:06:00Z

lightning/src/ln/msgs.rs

+				});
+			},
+			Self::BlindedReceive { sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, encrypted_tlvs, intro_node_blinding_point, keysend_preimage, custom_tlvs } => {
+				let keysend_tlv = keysend_preimage.map(|preimage| (5482373484, preimage.encode()));


Hmm, is this defined in the BlindedReceive proposed? And, if it is, can we swap it for a type in a sensible range so we can drop the collect + sort.

It is the identical type ID we currently use in the non-Trampoline payload, but no, Eclair does not have a defined value for this in their code at this time.

Let's pick one that isn't in the experimental range and pressure them to use that instead :)

TheBlueMatt · 2024-10-22T17:30:16Z

Looks like this needs rebase and CI was failing.

TheBlueMatt · 2024-10-23T16:34:44Z

lightning/src/ln/msgs.rs

+				let blinded_path_value: Vec<u8> = payment_paths.iter().flat_map(|p| {
+					p.inner_blinded_path().encode().into_iter().chain(p.payinfo.encode())
+				}).collect();


The current logic here, for each blinded path, allocates two vecs, and then at the end we allocate another vec. Instead, we should allocate one vec (with_capacity) and then write all the fields we want into it, saving two allocations per path.

TheBlueMatt · 2024-10-23T16:38:28Z

lightning/src/ln/msgs.rs

+				});
+			},
+			Self::BlindedReceive { sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, encrypted_tlvs, intro_node_blinding_point, keysend_preimage, custom_tlvs } => {
+				let keysend_tlv = keysend_preimage.map(|preimage| preimage.encode());


Please don't build a vec for 32 bytes. I'm kinda confused why we can't just write the keysend_preimage directly?

TheBlueMatt · 2024-10-23T17:18:46Z

lightning/src/ln/msgs.rs

+					current_payment_path.inner_blinded_path().write(&mut blinded_path_serialization)?;
+					current_payment_path.payinfo.write(&mut blinded_path_serialization)?;


If we're gonna handle the error anyway lets just write into a fixed-length buffer on stack?

TheBlueMatt

LGTM, feel free to squash.

TheBlueMatt · 2024-10-23T18:39:08Z

lightning/src/ln/msgs.rs

+						current_payment_path.inner_blinded_path().write(&mut blinded_path_slice)?;
+						current_payment_path.payinfo.write(&mut blinded_path_slice)?;
+					}
+					2048 - blinded_path_slice.len()


super nit

Suggested change

2048 - blinded_path_slice.len()

blinded_path_serialization.len - blinded_path_slice.len()

TheBlueMatt

Pretty trivial, if there's any issues we'll find it when we go to use this stuff. Landing.

TheBlueMatt reviewed Apr 22, 2024

View reviewed changes

arik-so mentioned this pull request Apr 22, 2024

Trampoline #2299

Open

32 tasks

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from 67ecd63 to 4b631d6 Compare June 17, 2024 08:44

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch 2 times, most recently from e3c86f7 to 4c785b5 Compare June 20, 2024 15:41

arik-so marked this pull request as ready for review June 24, 2024 17:27

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from 4c785b5 to 16d1ba1 Compare June 26, 2024 12:55

TheBlueMatt reviewed Jun 29, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from 53b7f63 to b9f002e Compare September 22, 2024 14:07

TheBlueMatt reviewed Oct 7, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from b9f002e to 9d01751 Compare October 10, 2024 16:50

TheBlueMatt reviewed Oct 11, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from 9d01751 to aa73fe2 Compare October 11, 2024 17:12

TheBlueMatt reviewed Oct 17, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch 4 times, most recently from 017b9a7 to d0363d5 Compare October 23, 2024 15:58

TheBlueMatt reviewed Oct 23, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from d0363d5 to 589ef85 Compare October 23, 2024 17:00

TheBlueMatt reviewed Oct 23, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from 589ef85 to 3df5131 Compare October 23, 2024 18:08

TheBlueMatt reviewed Oct 23, 2024

View reviewed changes

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch 2 times, most recently from bbf5d18 to e11a5ab Compare October 23, 2024 18:45

arik-so added 4 commits October 23, 2024 13:06

Serialize blinded path forwards in Trampoline onions.

477216e

Test blinded Trampoline payload serialization.

f6b3004

Add non-legacy blinded Trampoline payloads.

9c9a31c

Test non-legacy blinded Trampoline payloads.

d111981

arik-so force-pushed the arik/trampoline/2024-04-trampoline-blinded-hop-serialization branch from e11a5ab to d111981 Compare October 23, 2024 20:06

TheBlueMatt approved these changes Oct 23, 2024

View reviewed changes

TheBlueMatt merged commit 206ab82 into lightningdevkit:main Oct 23, 2024
17 of 18 checks passed

		p.inner_blinded_path().encode().into_iter().chain(p.payinfo.encode()).collect::<Vec<u8>>()
		}).collect();

		current_payment_path.inner_blinded_path().write(&mut blinded_path_serialization)?;
		current_payment_path.payinfo.write(&mut blinded_path_serialization)?;

	2048 - blinded_path_slice.len()
	blinded_path_serialization.len - blinded_path_slice.len()

Serialize blinded Trampoline hops #3007

Serialize blinded Trampoline hops #3007

Uh oh!

Conversation

arik-so commented Apr 19, 2024

Uh oh!

codecov-commenter commented Apr 19, 2024 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jun 17, 2024

Uh oh!

TheBlueMatt commented Jun 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Oct 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Apr 19, 2024 •

edited by codecov bot

Loading

TheBlueMatt Oct 23, 2024 •

edited

Loading