Limit payment path length based on payment_metadata, custom TLVs, etc.
#3026
Conversation
lightning/src/ln/outbound_payment.rs
Outdated
|
|
||
| let num_reserved_bytes = match &route_params.payment_params.payee { | ||
| Payee::Blinded { route_hints, .. } => { | ||
| let largest_path = route_hints |
There was a problem hiding this comment.
Note: currently I’m evaluating the max path length based on the largest blinded path, i.e. the blinded path that limits us the most. Therefore, if some blinded paths are larger than others, this could unnecessarily limit us in pathfinding. We could set the max path length on a per-blinded-path basis? In practice, ISTM that blinded paths within the same invoice are likely to be of similar sizes to each other.
valentinewallace
force-pushed
the
2024-04-limit-path-len
branch
from
3f0da18
to
52bb5d2
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3026 +/- ##
==========================================
+ Coverage 89.79% 89.83% +0.03%
==========================================
Files 116 117 +1
Lines 96466 96909 +443
Branches 96466 96909 +443
==========================================
+ Hits 86619 87055 +436
- Misses 7290 7301 +11
+ Partials 2557 2553 -4 ☔ View full report in Codecov by Sentry. |
valentinewallace
force-pushed
the
2024-04-limit-path-len
branch
from
52bb5d2
to
1af8a0c
Compare
|
I found some more edge cases around how |
valentinewallace
force-pushed
the
2024-04-limit-path-len
branch
2 times, most recently
from
8ef1770
to
c8fbb96
Compare
|
Rebased to (hopefully) fix CI. This should be good for review. |
valentinewallace
force-pushed
the
2024-04-limit-path-len
branch
3 times, most recently
from
7ee7c37
to
8de126d
Compare
|
Feedback should be addressed. Also this needs a rebase so let me know when I can do that. |
lightning/src/ln/onion_utils.rs
Outdated
| node_features: NodeFeatures::empty(), | ||
| short_channel_id: 42, | ||
| channel_features: ChannelFeatures::empty(), | ||
| fee_msat: route_params.final_value_msat, |
There was a problem hiding this comment.
This could be higher if we have to overpay when routing, so the object could serialize a bit longer.
There was a problem hiding this comment.
Added some buffer here. I think my buffer is a bit conservative so let me know if it needs tuning.
Will be useful when we reuse this enum to calculate the maximum path length, to avoid cloning the vecs.
Doable now that we take Vecs by reference in OutboundOnionPayload.
Will be used in the next commit when we make this configurable in PaymentParameters.
Will be useful when we want to calculate the total size of the payloads without actually allocating for them.
Will be used so the outbound_payment module can calculate the maximum path length with knowledge of any custom TLVs or payment metadata present.
Also adds some testing by augmenting existing tests.
So the router knows how long the maximum payment path can be.
valentinewallace
force-pushed
the
2024-04-limit-path-len
branch
from
8de126d
to
6255996
Compare
Currently the maximum payment path length is hardcoded in the router to 19. However, the presence of
payment_metadata, custom TLVs, and/or blinded paths may mean that the actual limit is much shorter. Account for these extra onion fields when calculating and setting the maximum path length for use in pathfinding.Closes #2201.