Refactor BlindedPath construction utils
#3248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3248 +/- ##
==========================================
- Coverage 89.82% 89.79% -0.03%
==========================================
Files 125 125
Lines 102867 102882 +15
Branches 102867 102882 +15
==========================================
- Hits 92398 92388 -10
- Misses 7753 7777 +24
- Partials 2716 2717 +1 ☔ View full report in Codecov by Sentry. |
|
Needs a rebase. |
When constructing a BlindedPath, utils::construct_blinded_hops uses two iterators. However, this makes it difficult to pad blinded hops to equal sizes without allocating a vector or cloning data. Refactor the construct_keys_callback utility function so that is can be used with an Iterator with different Item types. This allows using a single Iterator of tuples while still supporting its use only with pubkeys.
Instead of accepting iterators yielding PublicKey by reference in utils::construct_keys_callback, take iterators yielding values since these implement Copy and need to be copied anyway. This will help avoid a situation when padding where both a reference and mutable reference are needed.
When constructing a BlindedPath, an Option<Destination> parameter to construct_keys_callback is never passed. Make a separate utility function that doesn't take this parameter. This allows for using the build_keys_helper macro with a Iterator yielding items other than PublicKey.
When constructing a blinded path, two iterators are used: one for the pubkeys and another for Writeable TLVs. The first iterator is used in the build_keys_helper utility function while the second is used inside of a callback. Update this utility to work on any type that can be borrowed as a PublicKey. This allows for using a single iterator of tuples, which is necessary for padding the hops without additional allocations and clones.
Instead of using separate iterators for pubkeys and TLVs, use an iterator that yields a tuple of them. This allows for holding a mutable reference to the TLVs such that they can be padded. With two iterators, the pubkey iterator would have a reference to the ForwardNode slice when constructing a payment path. However, this would prevent holding mutable references in the TLVs iterator.
jkczyz
force-pushed
the
2024-08-blinded-path-utils-refactor
branch
from
667ffa6 to
76c9fda
Compare
TheBlueMatt
approved these changes
Aug 26, 2024
valentinewallace
approved these changes
Aug 26, 2024
dunxen
approved these changes
Aug 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When constructing a
BlindedPath, theconstruct_blinded_hopsutility function requires two iterators: one for public keys and another for TLVs. The first is used by theconstruct_keys_callbackfunction while the second is used in a callback passed to that function.There are few disadvantages to this:
Option<Destination>parameter is not relevant as it is only used for constructing the full onion.This PR refactors the utility functions such that only a single iterator is used. This is useful as an iterator over mutable TLVs can be used to add padding, in particular for payment paths when adding dummy hops. See #3177 (comment).