Uh oh!
There was an error while loading. Please reload this page.
Pub make onion - #2583
Conversation
| reply_path: Option<BlindedPath> | ||
| ) -> Result<Vec<u8>, SendError> { | ||
| let (_, onion_routing_packet) = self.make_onion_message(path, message, reply_path)?; | ||
| Ok(onion_routing_packet.onion_routing_packet.hop_data) |
There was a problem hiding this comment.
Shouldn't we return the full OnionMessage? Presumably whatever is handling it needs the blinding_point as well to send.
| } | ||
| /// Construct an onion message with contents `message` to the destination of `path`. | ||
| pub fn construct_onion_message<T: CustomOnionMessageContents>( |
There was a problem hiding this comment.
It seems like this method really wants to be static - IIUC your use-case is that you want to do the onion message construction on an edge node but then pass it to the first hop via some out-of-band protocol. Using an OnionMessenger could work for that, but seems a bit awkward to expose a method to return the message on a struct that's all about passing messages to peers. If we want a method that generates messages in a free-standing way, it probably should be a utility method, rather than a struct method taking &self. OTOH, if you want to use the struct, you could probably just intercept on the other send, calling send_onion_message then immediately calling next_onion_message_for_peer on the messenger.
There was a problem hiding this comment.
i see what you mean, its much cleaner as a static method. And u can use it just like SimpleRefOnionMessenger::make_onion_message. Thanks for taking a look!
| pub blinding_point: PublicKey, | ||
| pub(crate) onion_routing_packet: onion_message::Packet, | ||
| /// The full onion packet including hop data, pubkey, and hmac | ||
| pub onion_routing_packet: onion_message::Packet, |
There was a problem hiding this comment.
This isn't (actually) public because Packet isn't pub (its only pub from a private module). You'll need to make the packet module pub.
There was a problem hiding this comment.
Packet was already exported from the mod.rs as pub(crate), so I just made it pub and added doc comments
| pub version: u8, | ||
| /// The PublicKey used during shared secret generation | ||
| pub public_key: PublicKey, | ||
| /// 1300 bytes payload for the next hop |
There was a problem hiding this comment.
1300 bytes is for payment onions, not onion messages. This is arbitrary length.
| pub struct Packet { | ||
| /// Bolt 04 version number | ||
| pub version: u8, | ||
| /// The PublicKey used during shared secret generation |
There was a problem hiding this comment.
A bit more info would be nice, something like just "a random secp256k1 point which is ECDH'd to build a shared secret used to decrypt the hop_data.
There was a problem hiding this comment.
got it, thanks for the feedback
TheBlueMatt
commented
Sep 20, 2023
This LGTM, can you squash the fixup commits down so that each commit stands on its own and no commit cleans up or fixes code introduced in a previous commit? |
70acdbc to
a109c72CompareCodecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@## main #2583 +/- ##
==========================================
- Coverage 90.63% 88.77% -1.86%
==========================================
Files 113 113 Lines 59057 84510 +25453 Branches 59057 84510 +25453 ==========================================
+ Hits 53524 75027 +21503 - Misses 5533 7247 +1714 - Partials 0 2236 +2236
☔ View full report in Codecov by Sentry. |
Evanfeenstra
commented
Sep 20, 2023
ok done! |
Uh oh!
There was an error while loading. Please reload this page.
e115aa5 to
30b74a6Compare| /// Used in decrypting the onion packet's payload. | ||
| pub blinding_point: PublicKey, | ||
| pub(crate) onion_routing_packet: onion_message::Packet, | ||
| /// The full onion packet including hop data, pubkey, and hmac |
There was a problem hiding this comment.
In the future, docs should end in periods (particularly public docs)
| reply_path: Option<BlindedPath> | ||
| ) -> Result<(), SendError> { | ||
| let (introduction_node_id, onion_msg) = Self::create_onion_message( | ||
| &self.entropy_source, |
There was a problem hiding this comment.
nit: You have whitespace hanging off the end of line here and a few other places. This should be highlighted by a local git show depending on your terminal settings.
Adds a public
construct_onion_messagemethod on OnionMessenger. At sphinx chat we are experimenting with client-side onion route creation, and would like to use a method like this