[Custom Transactions] Add TxBuilder::get_available_balances#4026
Open
tankyleo wants to merge 12 commits intolightningdevkit:mainfrom
Open
[Custom Transactions] Add TxBuilder::get_available_balances#4026tankyleo wants to merge 12 commits intolightningdevkit:mainfrom
TxBuilder::get_available_balances#4026tankyleo wants to merge 12 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Contributor
Author
|
@TheBlueMatt would appreciate a high-level design pass on this one thank you. I largely just move |
tankyleo
commented
Aug 25, 2025
lightning/src/sign/tx_builder.rs
Outdated
Comment on lines
128
to
141
| fn get_available_balances( | ||
| &self, is_outbound_from_holder: bool, channel_value_satoshis: u64, | ||
| value_to_holder_msat: u64, pending_htlcs: &[HTLCAmountDirection], feerate_per_kw: u32, | ||
| dust_exposure_limiting_feerate: Option<u32>, max_dust_htlc_exposure_msat: u64, | ||
| holder_channel_constraints: ChannelConstraints, | ||
| counterparty_channel_constraints: ChannelConstraints, channel_type: &ChannelTypeFeatures, | ||
| ) -> crate::ln::channel::AvailableBalances; | ||
| fn get_next_commitment_stats( | ||
| &self, local: bool, is_outbound_from_holder: bool, channel_value_satoshis: u64, | ||
| value_to_holder_msat: u64, next_commitment_htlcs: &[HTLCAmountDirection], | ||
| addl_nondust_htlc_count: usize, feerate_per_kw: u32, | ||
| dust_exposure_limiting_feerate: Option<u32>, broadcaster_dust_limit_satoshis: u64, | ||
| channel_type: &ChannelTypeFeatures, | ||
| ) -> NextCommitmentStats; |
Contributor
Author
There was a problem hiding this comment.
I'm thinking we'll want to merge these two into one :)
Collaborator
There was a problem hiding this comment.
I feel like we could, yea?
Contributor
Author
There was a problem hiding this comment.
Current blocker for me is that get_available_balances is a function of both local and remote commitments, but will dig further thank you :)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4026 +/- ##
==========================================
+ Coverage 86.01% 86.02% +0.01%
==========================================
Files 156 156
Lines 102857 102891 +34
Branches 102857 102891 +34
==========================================
+ Hits 88471 88513 +42
Misses 11877 11877
+ Partials 2509 2501 -8
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:
|
TheBlueMatt
reviewed
Aug 25, 2025
Collaborator
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
I'm confused, I was under the impression we were going to pause the custom tx builder work for a month or two after landing #3921?
|
👋 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. |
Contributor
Author
Agreed yes this doesn't have to land anytime soon - I still wanted to get this PR out of my head and written before I stop |
24 tasks
TheBlueMatt
reviewed
Dec 12, 2025
Collaborator
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
API looks good to me, but, indeed, next_commitment_stats and available_balances feel like two things that are just close enough that they might as well be merged, no?
lightning/src/sign/tx_builder.rs
Outdated
| holder_channel_constraints.dust_limit_satoshis, | ||
| channel_type, | ||
| ) | ||
| // TODO: should `get_available_balances` be fallible ? |
Collaborator
There was a problem hiding this comment.
Hmmm, could definitely go either way. I do like the idea of merging get_next_commitment_stats and get_available_balances, at which point we'd want the joint thing to be fallible, though.
This commit has no functional changes.
This commit moves the previous `TxBuilder::get_next_commitment_stats` method to a private function, and then calls this function in `TxBuilder::get_onchain_stats`. Similar to the previous `TxBuilder::get_next_commitment_stats` method, `TxBuilder::get_onchain_stats` fails if the party cannot afford the HTLCs outbound from said party, and the anchors if they are the funder. Aside from the API changes on `TxBuilder`, there should be no functional changes in this commit.
Move calls to `TxBuilder::commit_tx_fee_sat` in `new_for_inbound_channel` and `new_for_outbound_channel` to `TxBuilder::get_onchain_stats`, and set the parameters such that the exact same behavior is maintained. We also replace calls to `TxBuilder::commit_tx_fee_sat` in `get_pending_htlc_stats`, `next_local_commit_tx_fee_msat`, and `next_remote_commit_tx_fee_msat` with `chan_utils::commit_tx_fee_sat`. All three functions get deleted in an upcoming commit, so we accept this temporary use of the `chan_utils::commit_tx_fee_sat` function.
We make temporary use of the raw `tx_builder::saturating_sub_anchor_outputs` function in `get_available_balances_for_scope`. This ok because we move most of the `get_available_balances_for_scope` function to the `TxBuilder::get_onchain_stats` call in an upcoming commit. Again, no functional change is introduced in this commit.
In an upcoming commit, we move `get_available_balances_for_scope` behind `TxBuilder::get_onchain_stats`, and pass channel parameters relevant to balance calculations in `TxBuilder::get_onchain_stats` via `ChannelConstraints`. There are no functional changes in this commit.
This snippet is currently used in `tx_builder::get_next_commitent_stats`, and will be used in an upcoming commit in `get_available_balances_for_scope`. There are no functional changes in this commit, as the `extra_accepted_htlc_dust_exposure` member of `NextCommitmentStats` was not used.
We no longer make use of `get_pending_htlc_stats`, `get_dust_buffer_feerate`, `next_local_commit_tx_fee_msat`, and `next_remote_commit_tx_fee_msat` in the `channel` module, and instead make use of tooling from the `tx_builder` module. `HTLCStats::pending_outbound_htlcs` is now `pending_htlcs.iter().filter(|htlc| htlc.outbound).count()`, and remains the same value. `HTLCStats::pending_inbound_htlcs_value_msat` is now calculated in `get_available_balances_for_scope`, and remains the same value. `HTLCStats::pending_outbound_htlcs_value_msat` is now calculated in `get_available_balances_for_scope`, and remains the same value. To determine whether a HTLC is dust for the purpose of calculating total dust exposure, we now refer only to `ChannelContext::feerate_per_kw`, and ignore any upcoming fee updates stored in `pending_update_fee`. The same applies for dust exposure due to excess fees; we ignore any fee updates in `ChannelContext::pending_update_fee`, and only refer to `ChannelContext::feerate_per_kw`. For outbound feerate updates, this is ok because all such updates first get placed in the holding cell. We validate dust exposure again upon freeing the feerate update from the holding cell, and immediately generate the corresponding commitment. For inbound feerate updates, it is possible that the peer sends us a feerate update that is in excess of our dust exposure limiting feerate, at the same time that we send non-dust HTLCs that exhaust the max dust exposure at the new feerate. This leads to a channel force-close when the peer sends us their commitment signed including the HTLCs and the new feerate. The same set of HTLCs is accounted for when calculating dust exposure. `max_reserved_commit_tx_fee_msat` and `min_reserved_commit_tx_fee_msat` both account for *all* pending HTLCs, including `AwaitingRemoteRevokeToRemove` and `AwaitingRemovedRemoteRevoke` HTLCs; these values previously only accounted for `LocalAnnounced`, `Committed`, and `RemoteRemoved` HTLCs. In the case where `is_outbound_from_holder` is false, `max_reserved_commit_tx_fee_msat` now also accounts for HTLCs in the holding cell, whereas it previously would not. These values are also the result of the feerate getting multiplied by the fee spike buffer increase multiple, instead of the final commitment transaction fee getting multiplied by that multiple. This results in higher values, as we multiply before the rounding down to the nearest satoshi. Finally, these values also account for any non-dust HTLCs that transition to dust at the higher feerate, resulting in lower values.
This is a direct code move to `tx_builder::get_available_balances`.
We choose to multiply `FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE` by the feerate when checking the fee spike buffer in `can_accept_incoming_htlc` instead of multiplying the multiple by the commitment transaction fee. This allows us to delete `NextCommitmentStats::commit_tx_fee_sat`, and return balances including the commitment transaction fee in `TxBuilder::get_onchain_stats`. This unblocks a good amount of cleanup. Note that this means LDK now rejects HTLCs that previous versions of LDK would have accepted. We made the mirroring change in `get_available_balances_for_scope` a few commits earlier. We also now account for non-dust HTLCs turning to dust at the multiplied feerate, decreasing the overall weight of the transaction. We also remove other fields in `NextCommitmentStats` which can be easily calculated in `channel` only. `TxBuilder::get_onchain_stats` could also check the reserve requirements, given that it gets the reserves in `ChannelConstraints`. I leave this to follow-up work.
Note that `AvailableBalances` will always refer to the holder's balances, even when `local` is set to `false`, when calling `TxBuilder::get_onchain_stats`.
tankyleo
commented
Feb 5, 2026
| self.get_holder_channel_constraints(funding), | ||
| self.get_counterparty_channel_constraints(funding), | ||
| funding.get_channel_type(), | ||
| ).unwrap().available_balances |
Contributor
Author
There was a problem hiding this comment.
If this direction is good I'll bubble any errors here upstream
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #3921