Fix possible dust HTLC sweep tx when feerate remains unchanged during a bump#3832
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3832 +/- ##
==========================================
- Coverage 89.91% 89.90% -0.01%
==========================================
Files 160 160
Lines 129307 129581 +274
Branches 129307 129581 +274
==========================================
+ Hits 116270 116505 +235
- Misses 10349 10383 +34
- Partials 2688 2693 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wpaulino
left a comment
There was a problem hiding this comment.
Thanks! Do you mind updating the commit message to follow these guidelines?
When bumping an HTLC sweep transaction, if the feerate remains unchanged, `feerate_bump` doesn't check whether the output amount is below the dust limit. However, this situation can occur if the transaction's inputs are modified. See also: lightningdevkit#3831
1a19a4a to
ab132ab
Compare
|
Hi, @wpaulino. |
|
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
1 similar comment
|
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
TheBlueMatt
left a comment
There was a problem hiding this comment.
Hmm, yea, I guess this makes sense. I mean if we think we don't have enough value in an HTLC output for it to be worth claiming at the feerate we need I'm not entirely sure its worth bothering, but I guess better to build the valid transaction that we think won't get confirmed than not.
The patch itself is trivial so gonna go ahead and land.
In my case, the HTLC output might contain RGB assets, so it's crucial to get it back. I've also observed that HTLCs claimed by the remote party aren't removed from the transaction until node restart. This causes HTLC sweep transactions to be rejected due to spent inputs, which consequently results in repeated failed feerate bumps. I'm working to identify the issue. |
Then this patch definitely doesn't suffice. You need to bring in additional fees to ensure you pay for the "invisible" value - probably want to move to anchor channels where you can that more easily.
Hmm, weird. Definitely let us know how it goes. |
Fix #3831