Async send prefactor #4044
Async send prefactor #4044
Conversation
This method was rustfmt'd in the previous commit, here we clean up that formatting.
This method will be edited in upcoming commits, and the codebase policy is to at least consider removing rustfmt::skips when touching a method.
This method will be edited in upcoming commits, and the codebase policy is to at least consider removing rustfmt::skips when touching a method.
|
I've assigned @joostjager as a reviewer! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4044 +/- ##
==========================================
+ Coverage 88.73% 88.77% +0.04%
==========================================
Files 176 176
Lines 129042 129310 +268
Branches 129042 129310 +268
==========================================
+ Hits 114501 114794 +293
+ Misses 11939 11910 -29
- Partials 2602 2606 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
In upcoming commits, we will be adding several more conversions from PendingAddHTLCInfo into HTLCPreviousHopData. This conversion gets repeated all over the ChannelManager already, so lay some groundwork by DRYing it up.
valentinewallace
force-pushed
the
2025-09-async-send-prefactor
branch
from
ecab2b9 to
ef8bec7
Compare
Without this DRYing, we would be repeating the same code to instantiate the PendingAddHTLCInfo several more times in this method, in upcoming commits.
valentinewallace
force-pushed
the
2025-09-async-send-prefactor
branch
2 times, most recently
from
a1bbef7 to
dfbf3ae
Compare
The LDK codebase in general is comfortable panicking if the entropy source provided to it is dysfunctional. Up until now we made an exception for blinded path creation, where we would handle an error that could occur on mul_tweak that could only occur if the session_priv provided was not actually random. In comparable cases in onion_utils, we would panic instead. In upcoming commits, we will be including blinded paths in outbound revoke_and_ack messages as part of implementing async payments, where it is difficult to handle failing back an HTLC if blinded path creation fails. Thus we now have an incentive to make the blinded path creation methods infallible, so do so here.
valentinewallace
force-pushed
the
2025-09-async-send-prefactor
branch
from
dfbf3ae to
408001c
Compare
| }); | ||
|
|
||
| if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing { | ||
| let htlc_source = HTLCSource::PreviousHopData(payment.htlc_previous_hop_data()); |
There was a problem hiding this comment.
Phantom secret was set to None always, where as now it's taken from forward_info. No problem? Also at the last change in this file.
| }); | ||
|
|
||
| let mut prev_hop = payment.htlc_previous_hop_data(); | ||
| prev_hop.phantom_shared_secret = phantom_ss; |
There was a problem hiding this comment.
Perhaps add a comment why the phantom secret as set in htlc_previous_hop_data() isn't good.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
| $secp_ctx, | ||
| &Scalar::from_be_bytes(hop_pk_blinding_factor).unwrap(), | ||
| )? | ||
| pk.mul_tweak($secp_ctx, &Scalar::from_be_bytes(hop_pk_blinding_factor).unwrap()) |
There was a problem hiding this comment.
Do you know what the original reason was for making an exception here and not panicking for blinded paths specifically? Is there any way in which input causing this could be provided from the outside?
Some cleanups in preparation for implementing sending payments as an often-offline sender to an often-offline recipient.
cc #2298