public static peel_onion method on OnionMessenger

[Evanfeenstra]

Thanks for merging #2583! This is a similar PR, but for the other end (receiving an onion message).

It introduces a new PeeledOnion enum and a new ReceiveError enum.

Of course, open to feedback about the naming, error handling, generics, or anything

[codecov-commenter]

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (fe6f166) 88.99%.
Report is 18 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2599      +/-   ##
==========================================
- Coverage   89.02%   88.99%   -0.04%     
==========================================
  Files         112      112              
  Lines       87168    87437     +269     
  Branches    87168    87437     +269     
==========================================
+ Hits        77605    77811     +206     
- Misses       7327     7387      +60     
- Partials     2236     2239       +3     

Files	Coverage Δ
lightning/src/onion_message/messenger.rs	82.04% <80.88%> (+0.64%)	⬆️

... and 21 files with indirect coverage changes

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

[TheBlueMatt]

I'm kinda curious why y'all don't want to just create a normal OnionMessager and pass it messages via handle_onion_message? There's nothing really specific about the LDK message handling interfaces that requires them to ride on the standard lightning P2P rails, in fact they're separated traits explicitly to support people not doing that.

[Evanfeenstra]

I'm kinda curious why y'all don't want to just create a normal OnionMessager and pass it messages via handle_onion_message? There's nothing really specific about the LDK message handling interfaces that requires them to ride on the standard lightning P2P rails, in fact they're separated traits explicitly to support people not doing that.

Then you would have a make a PeerManager as well, right? And connect a peer, then call read_event, so that peer_connected is called and the OnionMessenger pending_messages is not vacant?

Maybe that's a fine approach... I was just trying to make it as low-level as possible so it can run more efficiently in WASM, embedded, or whatever.

[TheBlueMatt]

No, absolutely not. The handle_onion_message function that you edit here is public and can be called directly.

[Evanfeenstra]

No, absolutely not. The handle_onion_message function that you edit here is public and can be called directly.

Hmm interesting... what if the caller is not the receiver, but just an intermediate hop? I get Dropping forwarded onion message to disconnected peer when calling handle_onion_message, as the pending_per_peer_msgs is Vacant. So next_onion_message_for_peer() is always None

Not sure the best way to unwrap one layer of an onion message in a free-standing way...

[TheBlueMatt]

If you want to forward you'd have to call peer_connected first (though I understood your use-case to not include forwarding? Just doing this at the endpoints). The interface PeerManager uses to talk to the OnionMessenger (and ChannelManager, etc) are all fully public. There's nothing it does that's magic, and anything it normally does can be done manually.

[Evanfeenstra]

If you want to forward you'd have to call peer_connected first (though I understood your use-case to not include forwarding? Just doing this at the endpoints). The interface PeerManager uses to talk to the OnionMessenger (and ChannelManager, etc) are all fully public. There's nothing it does that's magic, and anything it normally does can be done manually.

Ok I see what you mean now! Most things can be accomplished by running peer_connected manually.

Our use-case is clients messaging each other via multiple hops ("mixing" through a network of servers for privacy, so server operators cannot track who is chatting). The last routing hop might not always know which recipients it's routing onions to... those clients may connect in the future to fetch stored onions destined for them.

I realize this doesn't make sense in the context of Lightning payments... but currently I don't see a way to unwrap a forwarded onion without knowing the next peer id ahead of time

[TheBlueMatt]

Ah, okay, that makes sense. Will pick this back up after 0.0.117 ships, sadly this missed the cut-off.

[TheBlueMatt]

Basically LGTM, I think, needs rebase tho.

[Evanfeenstra]

rebased to main and squashed. Should be good to go now

1 more feedback item outstanding

just noticed something