Improve docs on newly-public structs after #2700 #2762
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2762 +/- ##
==========================================
- Coverage 88.66% 88.61% -0.05%
==========================================
Files 115 115
Lines 90446 90749 +303
Branches 90446 90749 +303
==========================================
+ Hits 80190 80416 +226
- Misses 7856 7902 +46
- Partials 2400 2431 +31 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for improving the docs! The code is so much easier to read and understand now.
Completely agree with exposing fee-skimming acceptance to peel_payment_onion, allowing checking for skimmed fees for (expected) HTLCs.
I've got a few ideas for the docs that I think could make them even better.
| #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug | ||
| pub struct PendingHTLCInfo { | ||
| /// Further routing details based on whether the HTLC is being forwarded or received. | ||
| pub routing: PendingHTLCRouting, | ||
| /// Shared secret from the previous hop. | ||
| /// Used encrypt failure packets in the event that the HTLC needs to be failed backwards. | ||
| /// The onion shared secret we build with the sender used to decrypt the onion. |
There was a problem hiding this comment.
Though “sender” makes perfect sense in the context, we use “sender” usually for the original creator of the payment. Would “previous hop” be a better phrasing here?
| /// The onion shared secret we build with the sender used to decrypt the onion. | |
| /// The onion shared secret we build with the previous hop used to decrypt the onion. |
There was a problem hiding this comment.
No, the shared secret is something we calculate with the sender, even if they're five hops away.
There was a problem hiding this comment.
I've been trying to get my head around this concept, and after a bit of digging, I found some relevant info in Bolt #4.
It reads:
Intermediate hops store the shared secret from the forward path and reuse it to obfuscate any corresponding return packet during each hop. In addition, each node locally stores data regarding its own sending peer in the route, so it knows where to return forward any eventual return packets.
AFAIU, this talks about creating a secret with the own sending peer (adjacent peer) and not with The Sender.
So, can you help me point to a resource I can read up to understand why and how we create keys with The Sender and do so in a way that doesn't compromise privacy?
Thanks a lot!
There was a problem hiding this comment.
Its admittedly kinda both. The actual full shared secret is only shared between us and the sender, but its built to us iteratively - there's an OnionPacket::public_key which is updated at every hop and we do an ECDH with that to build the shared secret. The sender knows the pubkey that each hop will see (cause its just updated using each hop's node_id), but the ECDH only works if you have one side's private key, which only us and the sender do.
TheBlueMatt
force-pushed
the
2023-11-htlc-docs
branch
2 times, most recently
from
23c39ac to
0566483
Compare
|
Addressed feedback and rebased. |
Now that `PendingHTLCRouting` is public, its docs should be meaningful to developers not working directly on LDK, and thus needs substantially more information than it previously had. This adds much of that information.
Now that `PendingHTLCInfo` is public, its docs should be meaningful to developers not working directly on LDK, and thus needs substantially more information than it previously had. This adds much of that information.
LSP users who wish to use `peel_payment_onion` to understand if they'd accept an HTLC prior to receit should be able to check the skimmed fees just like they would for full payment receipt. Thus, we need to expose the fee-skimming acceptance bool to `peel_payment_onion`, which we do here, in addition to some doc cleanups.
TheBlueMatt
force-pushed
the
2023-11-htlc-docs
branch
from
0566483 to
63ee03f
Compare
|
Squashed with no further changes. |
Various doc fleshings, plus one small change.