Open valentinewallace opened 3 months ago
Discussed offline with Matt and want to get some conceptual feedback on this design to resolve the issue:
Event
containing the onion message. The user is then responsible for storing this onion message until the peer comes back online. Rationale:
Drawbacks:
@arik-so @TheBlueMatt looking for conceptual feedback or ACKs on this design before putting up any code. @jkczyz feel free to weigh in too since this touches code you've worked on recently, but no pressure.
It would be possible to work this into OnionMessenger
's existing OnionMessageRecipient
buffering by adding another variant for forwarding to whitelisted peers. Currently, we use OnionMessageRecipient::PendingConnection
to generate Event::ConnectionNeeded
. So the new variant could be used to generate the new event and the existing buffering would mitigate DoS. Then the user could just call handle_onion_message
with the contents of the event when the peer is back online.
Alternatively, you could have a larger buffer for these messages and generate an Event::ConnectionNeeded
so that the messages are automatically delivered when the peer is back online. I guess the drawback is that the messages would be dropped if the node restarts or the buffer fills up? Though it puts less burden on the user as they just need to process the Event::ConnectionNeeded
as normal or know to send an OS notification.
Regarding including the message in the event so the user can directly send the message with the OS notification: That does save the time waiting for the peer to connect. But in most cases won't the peer need to reconnect to respond to the message anyway? Is there any other benefits? Seems the main benefit of including the message in the event is persistence flexibility. So in a sense, we are pushing the mailbox implementation to the user, no?
Discussed offline, going with the event-based approach for now rather than storing OMs internally in LDK. We won't be using a whitelist internally in LDK either, but users should keep one and discard any OM interception events that aren't for relevant peers.
As a follow-up, we should indicate to users when an intercepted OM has been successfully forwarded and is safe to delete. Updated the issue description for this.
Per #2298, we want to support the "onion message mailbox" feature. I.e., we want to be able to store onion message forwards on behalf of an offline next-hop peer and forward them later when the peer come back online.
We'll need a way for the
OnionMessenger
to know which OM forwards to offline peers need to be stored for later, and a way to store and retrieve them when the offline peer comes back online. We need this feature so that per lightning/bolts#989, when the payer sends aheld_htlc_available
OM to the recipient, the recipient’s LSP will hold onto that OM for them until they come back online.PeerManager
to indicate that it has received a ping/pong after sending the OM, after which we can generate another event inOnionMessenger
indicating that the intercepted OM is safe to delete. This race should be relatively rare but may be more common on mobile phones where sockets get disconnected as soon as an app goes into the background.