-
Notifications
You must be signed in to change notification settings - Fork 385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide inbound HTLC preimages to the EcdsaChannelSigner
#2753
Provide inbound HTLC preimages to the EcdsaChannelSigner
#2753
Conversation
4592add
to
b732ebc
Compare
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 #2753 +/- ##
==========================================
+ Coverage 88.55% 89.08% +0.53%
==========================================
Files 114 115 +1
Lines 89424 92387 +2963
Branches 89424 92387 +2963
==========================================
+ Hits 79186 82307 +3121
+ Misses 7858 7681 -177
- Partials 2380 2399 +19 ☔ View full report in Codecov by Sentry. |
b732ebc
to
f81b824
Compare
@@ -479,7 +479,8 @@ struct CommitmentStats<'a> { | |||
htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction | |||
local_balance_msat: u64, // local balance before fees but considering dust limits | |||
remote_balance_msat: u64, // remote balance before fees but considering dust limits | |||
preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment | |||
outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outbound vs outgoing, pick one 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm, good point, not just that, but I realized outbound/inbound is confusing on its own, cause they're preimages which are for outbound HTLCs, ie the preimages themselves are incoming from the peer....stuck with *bound and included the _htlc everywhere.
fe026a4
to
6f45bad
Compare
Feel free to squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK mod squash
The changes in this PR will help a user keep track of the inbound_htlc_preimages
along with outbound_htlc_preimages
, for the resolved HTLC.
The VLS signer has a desire to see preimages for resolved forwarded HTLCs when they are first claimed by us, even if that claim was for the inbound edge (where claiming strictly increases our balance). Luckily, providing that information is rather trivial, which we do here. Fixes lightningdevkit#2356
6f45bad
to
262072d
Compare
Squashed without further changes. |
The VLS signer has a desire to see preimages for resolved forwarded HTLCs when they are first claimed by us, even if that claim was for the inbound edge (where claiming strictly increases our balance).
Luckily, providing that information is rather trivial, which we do here.
Fixes #2356
Not 100% sure this is actually required, but if VLS really wants it we should just do it. See linked issue.