Batch on-chain claims more aggressively per channel
When batch claiming was first added, it was only done so for claims which were not pinnable, i.e. those which can only be claimed by us.
This was the conservative choice - pinning of outputs claimed by a batch would leave the entire batch unable to confirm on-chain. However, if pinning is considered an attack that can be executed with a high probability of success, then there is no reason not to batch claims of pinnable outputs together, separate from unpinnable outputs.
Whether specific outputs are pinnable can change over time - those that are not pinnable will eventually become pinnable at the height at which our counterparty can spend them. Thus, outputs are treated as pinnable if they're within COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE of that height.
Aside from outputs being pinnable or not, locktimes are also a factor for batching claims. HTLC-Timeout claims have locktimes fixed by the counterparty's signature and thus can only be aggregated with other HTLCs of the same CLTV, which we have to check for.
The complexity required here is worth it - aggregation can save users a significant amount of fees in the case of a force-closure, and directly impacts the number of UTXOs needed as a reserve for anchors.
This change depends on https://github.com/lightningdevkit/rust-lightning/pull/3297.
Codecov Report
Attention: Patch coverage is 98.11617% with 12 lines in your changes missing coverage. Please review.
Project coverage is 90.46%. Comparing base (
726dd5c) to head (5355ab9). Report is 29 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3340 +/- ##
==========================================
+ Coverage 89.69% 90.46% +0.77%
==========================================
Files 130 130
Lines 107335 112294 +4959
Branches 107335 112294 +4959
==========================================
+ Hits 96273 101591 +5318
+ Misses 8660 8343 -317
+ Partials 2402 2360 -42
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Needs rebase now :tada:
Would like to get confirmation on one case --
In test_bump_penalty_txn_on_revoked_commitment, we have 1 revoked HTLC output where the counterparty can claim it at height 41 and another where the counterparty can claim it at 81. The current height is 25.
Currently we'll aggregate these outputs, but it seems like the former output is more urgent and may warrant more aggressive fee-bumping, so aggregating them might result in paying more fees due to the overall increased transaction size? Just want to make sure this is the intended behavior.
Would like to get confirmation on one case --
In
test_bump_penalty_txn_on_revoked_commitment, we have 1 revoked HTLC output where the counterparty can claim it at height 41 and another where the counterparty can claim it at 81. The current height is 25.Currently we'll aggregate these outputs, but it seems like the former output is more urgent and may warrant more aggressive fee-bumping, so aggregating them might result in paying more fees due to the overall increased transaction size? Just want to make sure this is the intended behavior.
I definitely think there is some trade-off to be made there that we can optimize further at the cost of some complexity, and such cases would indeed exist right now. Between always having separate transactions and always aggregating, aggregating aggressively seems to the better choice though - the total weight across all transactions will decrease and perhaps transactions will confirm quickly before additional fee bumping.
Squashed the fixups.
This seriously helped with previously broken unit tests for a PR I'm working on, thank you so much!