Add onion mailbox for async recipients #632

joostjager · 2025-09-10T12:01:11Z

Adds functionality for LSPs to hold onion messages for their clients until they come back online.

Depends on lightningdevkit/rust-lightning#4046

ldk-reviews-bot · 2025-09-10T12:01:14Z

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

joostjager · 2025-09-12T07:28:26Z

src/lib.rs

 	}
+
+	#[allow(missing_docs)]
+	pub fn om_mailbox_is_empty(&self) -> bool {


I wanted to expose this to the integration test, but not sure how to do it without introducing an extra cfg flag

Well, that's why they are integration / black box tests, they test the public API. Would be good to drop this method. If you want to test internal functionality, you'd need to write a unit test.

I don't think it is so binary, but I understand your pov. As far as I know, there isn't really another way to detect that the onion message is intercepted and waiting. Would you want to use a (blind) sleep instead?

Using sleep 3s now.

ldk-reviews-bot · 2025-09-13T00:00:09Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-09-15T00:00:55Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

This needs a rebase now.

joostjager · 2025-09-16T07:33:22Z

Rebased after merge of #621

joostjager · 2025-09-17T07:27:15Z

Updated to latest version of lightningdevkit/rust-lightning#4046

src/config.rs

tnull · 2025-09-17T10:29:02Z

src/event.rs

-			LdkEvent::OnionMessageIntercepted { .. } => {
-				debug_assert!(false, "We currently don't support onion message interception, so this event should never be emitted.");
+			LdkEvent::OnionMessageIntercepted { peer_node_id, message } => {
+				self.om_mailbox.onion_message_intercepted(peer_node_id, message);


Should we still log an error and do nothing here if !config.async_payment_services_enabled?

Added trace log

tnull · 2025-09-17T10:30:05Z

src/builder.rs

 		},
 	};

+	let om_mailbox = Arc::new(OnionMessageMailbox::new());


Can we make this an Option that will only be set if async_payment_services_enabled?

I am not a fan of this type of optionality as there resources involved with an empty onion mailbox are negligible. But made the change.

tnull · 2025-09-17T10:31:17Z

src/lib.rs

 	}
+
+	#[allow(missing_docs)]
+	pub fn om_mailbox_is_empty(&self) -> bool {


Well, that's why they are integration / black box tests, they test the public API. Would be good to drop this method. If you want to test internal functionality, you'd need to write a unit test.

src/om_mailbox.rs

tnull · 2025-09-17T10:39:44Z

src/om_mailbox.rs

+}
+
+impl OnionMessageMailbox {
+	const MAX_MESSAGES_PER_PEER: usize = 100;


Hmm, these 100/100 values seems relatively restrictive and still results in ~300 MB of OM data in memory, AFAIU.

Maybe it would be worth to at least go for a 30/300 ratio, so that we allow for more potential peers?

Any of those constant can be right or wrong, no strong opinion about it. Changed.

tnull · 2025-09-17T10:44:53Z

src/om_mailbox.rs

+			let peer_to_remove = map
+				.iter()
+				.max_by_key(|(_, queue)| queue.len())
+				.map(|(peer, _)| peer.clone())


I think PublicKey is Copy.

Indeed. Made it *peer

tnull · 2025-09-17T10:46:48Z

src/om_mailbox.rs

+	const MAX_PEERS: usize = 100;
+
+	pub fn new() -> Self {
+		Self { map: Mutex::new(HashMap::new()) }


Would it make sense to pre-alloc some capacity from the start to avoid constant reallocations?

As this is triggered by a network message, this seems to be a negligible optimization?

Added MAX_PEERS capacity.

src/om_mailbox.rs

tnull

This needs yet another rebase now

This introduces an in-memory mailbox to hold onion messages until the receiver comes online. This is required for async payment `held_htlc_available` messages. The mailbox is bounded by a maximum number of peers and a maximum number of messages per peer.

joostjager · 2025-09-18T09:21:53Z

Rebased

joostjager changed the base branch from main to develop September 10, 2025 12:01

joostjager force-pushed the onion-mailbox branch from 996d102 to f70cd08 Compare September 10, 2025 12:04

joostjager requested a review from tnull September 10, 2025 12:12

joostjager force-pushed the onion-mailbox branch 2 times, most recently from 8612c01 to d9545a8 Compare September 11, 2025 14:30

joostjager added this to Weekly Goals Sep 11, 2025

joostjager self-assigned this Sep 11, 2025

joostjager force-pushed the onion-mailbox branch 2 times, most recently from 9818d0c to 89d74d5 Compare September 12, 2025 07:27

joostjager commented Sep 12, 2025

View reviewed changes

valentinewallace mentioned this pull request Sep 12, 2025

Async Payments lightningdevkit/rust-lightning#2298

Open

45 tasks

tnull reviewed Sep 16, 2025

View reviewed changes

joostjager force-pushed the onion-mailbox branch 2 times, most recently from 8192f71 to a971ee4 Compare September 16, 2025 07:32

joostjager force-pushed the onion-mailbox branch 2 times, most recently from a654b4e to 2cac461 Compare September 16, 2025 09:45

joostjager mentioned this pull request Sep 16, 2025

Update static invoice store for invoice requests #635

Merged

joostjager force-pushed the onion-mailbox branch from 2cac461 to ce98969 Compare September 16, 2025 12:35

joostjager requested a review from tnull September 16, 2025 12:36

joostjager force-pushed the onion-mailbox branch from ce98969 to d874141 Compare September 17, 2025 07:26

tnull reviewed Sep 17, 2025

View reviewed changes

joostjager force-pushed the onion-mailbox branch 2 times, most recently from cfc9387 to 39fe521 Compare September 17, 2025 12:36

tnull reviewed Sep 18, 2025

View reviewed changes

joostjager force-pushed the onion-mailbox branch from 39fe521 to 6ac6ab6 Compare September 18, 2025 09:19

joostjager added 3 commits September 18, 2025 11:20

dev: point to val branch

ed81997

f: review comments

7a1ad85

joostjager force-pushed the onion-mailbox branch from 6ac6ab6 to 7a1ad85 Compare September 18, 2025 09:21

Add onion mailbox for async recipients #632

Are you sure you want to change the base?

Add onion mailbox for async recipients #632

Uh oh!

Conversation

joostjager commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

ldk-reviews-bot commented Sep 13, 2025

Uh oh!

ldk-reviews-bot commented Sep 15, 2025

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

joostjager commented Sep 16, 2025

Uh oh!

joostjager commented Sep 17, 2025

Uh oh!

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!

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!

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

joostjager commented Sep 18, 2025

Uh oh!

Uh oh!

joostjager commented Sep 10, 2025 •

edited

Loading

ldk-reviews-bot commented Sep 10, 2025 •

edited

Loading