Zero-fee commitments support#660
Conversation
|
👋 I see @tnull was un-assigned. |
| /// `option_anchor_zero_fee_commitments`. All the caveats and warnings in | ||
| /// [`AnchorChannelsConfig`] still apply. | ||
| /// [`AnchorChannelsConfig`]: Config::anchor_channels_config | ||
| pub enable_zero_fee_commitments: bool, |
There was a problem hiding this comment.
I don't think we'll wan to add a new flag here that's probably hard to understand for the user? Rather, shouldn't we enable this for the user based on our current 'trust model settings' here?
Also, from these docs it's very unclear what this setting even does, when the user would want to enable it, what drawbacks it has, etc
There was a problem hiding this comment.
FWIW, thinking about it again it seems that we should never set negotiate_anchor_zero_fee_commitments until we're positive our chain sources support submitpackage/TRUC, no? And once we are positive, we would always set it?
There was a problem hiding this comment.
Rather, shouldn't we enable this for the user based on our current 'trust model settings' here?
Don't quite follow here could you expand ? I think 0FC channels merit an explicit setting somewhere rather than derived from trust model settings.
Also, from these docs it's very unclear what this setting even does, when the user would want to enable it, what drawbacks it has, etc
Yes will expand
There was a problem hiding this comment.
Don't quite follow here could you expand ? I think 0FC channels merit an explicit setting somewhere rather than derived from trust model settings.
Why, what do they fundamentally change for the user compared to our three current modes (fully trusted/keep 0-reserve, still try to claim/keep X reserve, try to claim)? Keep in mind that communicating these three modes to the user is already very hard, they always have a very hard time understanding what this means. Now, how would we communicate any changed assumptions for 0FC here? If we already trust our counterparty already, wouldn't we always want to enable 0FC for the UX improvements?
There was a problem hiding this comment.
Why, what do they fundamentally change for the user compared to our three current modes (fully trusted/keep 0-reserve, still try to claim/keep X reserve, try to claim)?
Let me see I don't think they change anything ? Whether to enable or disable 0FC channels is orthogonal to these modes ie trusted_peers_no_reserve and per_channel_reserve_sats should have no influence on whether we enable 0FC channels (only that per_channel_reserve_sats should be set to some value). I suspect you don't agree :)
If we already trust our counterparty already, wouldn't we always want to enable 0FC for the UX improvements?
It seems to me trusting our counterparty -> keeping 0 reserve is orthogonal to whether the user wants to enable 0FC channels ? for example a user trusts their counterparty, but wants to wait for greater adoption of Core v29+ before using 0FC channels.
cb1cdf9 to
c874049
Compare
|
Marked as draft: I think we should wait for electrum and esplora submit package support before merging this PR. |
c874049 to
ef3ba7a
Compare
|
Successfully opened some 0FC channels, made payments, and force closed them with the esplora diff in this branch. https://mutinynet.com/tx/508a954d85f5b7daf224a2fdc54ea6de9a26c0f62f7d58284bf61c3cdfd346e6 |
ef3ba7a to
3ebd017
Compare
AnchorChannelsConfig::enable_zero_fee_commitments3ebd017 to
eda13d4
Compare
e136f33 to
d4a2a04
Compare
771f45b to
a7c7911
Compare
When we are preparing to open a channel to a peer, we should reserve onchain funds for an anchor channel when the peer's init features signals anchor channels as optional, as channel negotiation with such a peer can result in an anchor channel. Tests written with codex.
|
I pushed this tag: https://github.com/tankyleo/ldk-node/releases/tag/2026-07-01-0620-0fc-channels It points to commit 9b38158, the top commit of this branch right before the rebase onto the new main branch commit. |
71c0473 to
c8c2b88
Compare
|
Rebased on new main branch commit, and pushed a few more small fixes to the tests |
| Err(e) => self.log_broadcast_error(e, &[txid], &txs), | ||
| } | ||
| }, | ||
| 2.. => { |
There was a problem hiding this comment.
Codex:
High: Multi-transaction broadcasts now require package relay even when 0FC is disabled. At /home/tnull/workspace/ldk-node-pr-660/src/chain/mod.rs:521 the broadcaster drops transaction-type context, and all len >= 2 packages go through submitpackage in /home/tnull/workspace/ldk-node-pr-660/
src/chain/bitcoind.rs:630, /home/tnull/workspace/ldk-node-pr-660/src/chain/esplora.rs:463, and /home/tnull/workspace/ldk-node-pr-660/src/chain/electrum.rs:358. But package-support validation only runs when enable_zero_fee_commitments is true. Legacy anchor force-close bumps still emit
commitment+anchor transaction packages, so users with 0FC off and an Electrum/Esplora/bitcoind backend without package support can start successfully and then fail to broadcast the close-bump package. Either keep the old per-tx fallback for non-TRUC packages or require/validate package
support for all anchor channels.
There was a problem hiding this comment.
Either keep the old per-tx fallback for non-TRUC packages or require/validate package
support for all anchor channels.
Ah thank you good catch, hmm I need to decide which path is best
There was a problem hiding this comment.
I think Im going to go with "require support for all anchor channels"
There was a problem hiding this comment.
See below I now require support for submitpackage in all cases; this fits best with the current BroadcasterInterface API from rust-lightning.
There was a problem hiding this comment.
See below I now require support for
submitpackagein all cases; this fits best with the currentBroadcasterInterfaceAPI from rust-lightning.
Hmm, I'm not sure we'll want to do this in this version already. This would force all users to upgrade their backends, which in some cases might still not provide submitpackage support out-of-the box even in the newest version. I think we eventually should, but as noted above, IMO at least for v0.8 we should maintain backwards compat. with non-submitpackage supporting chain backends, as long as users don't want to use 0fc channels.
| } | ||
| } | ||
|
|
||
| fn supports_anchor_channel_type(init_features: &InitFeatures) -> bool { |
There was a problem hiding this comment.
Codex:
Medium: Reserve prechecks treat any peer advertising 0FC as if the next channel will be anchor-like, even when local 0FC negotiation is disabled. /home/tnull/workspace/ldk-node-pr-660/src/lib.rs:219 returns true for supports_anchor_zero_fee_commitments(), and that drives outbound reserve
checks at /home/tnull/workspace/ldk-node-pr-660/src/lib.rs:1360 and LSPS2 checks at /home/tnull/workspace/ldk-node-pr-660/src/liquidity/service/lsps2.rs:455. For a 0FC-only peer with enable_zero_fee_commitments = false, LDK won’t negotiate 0FC, so this can falsely reject opens or reserve
funds for a static channel. The predicate should include local config: legacy anchors supported by peer, or 0FC supported by peer and locally enabled.
There was a problem hiding this comment.
I opted to drop enable_zero_fee_commitments below since we now require support for submitpackage always.
Given the submitpackage requirement, we can turn on 0fc channels by default.
Therefore, if a peer signals either keyed anchor channels or 0fc channels, we should assert that we have enough reserves before beginning channel negotiation.
There was a problem hiding this comment.
A potential path forward that is less aggressive: keep enable_zero_fee_commitments, and turn 0fc channels off by default, but still require v29 RPC / submitpackage regardless of the enable_zero_fee_commitments flag.
There was a problem hiding this comment.
I opted to drop
enable_zero_fee_commitmentsbelow since we now require support forsubmitpackagealways.Given the
submitpackagerequirement, we can turn on 0fc channels by default.
Hmm, as mentioned above, we may want to require submitpackage at some point going forward, but not sure if we want to do that for v0.8 already, esp. given that most chain backends barely shipped submitpackage support, and there might be some left that didn't ship it yet. So it seems rushed to require our users to immediately upgrade to the latest versions, as we don't know if they have any other blockers.
c8c2b88 to
9ca78d5
Compare
We previously allowed users to disable anchor channels and drain their anchor reserve while still having anchor channels open or pending resolution. This was acceptable for keyed anchor channels, as the commitment transaction therein still contained some fees, and had some chance of getting mined into a block without any anchor bumps. In upcoming commits, we will add support for 0FC channels, and their commitment transactions have zero fees and depend entirely on the anchor reserve to reach miners and get confirmed in a block. It is thus dangerous to disable anchor channels and drain the reserve after 0FC channels have been opened. Therefore, we make `AnchorChannelsConfig` required, and prevent this case from ever happening.
The patch adds support for the `broadcast_package` method added in electrum protocol v1.6. Upcoming commits will require this patch to pass CI.
The mempool/electrs docker image used in those tests only supports submitpackage via the esplora interface, not the electrum interface.
We bump the Bitcoin Core version used in kotlin and python tests to support ephemeral dust. This is required for 0FC channels.
In upcoming commits we will read this knob to determine whether to negotiate 0FC channels. For now, we make a best-effort attempt to make sure the configured chain source supports 0FC channels if this knob is set. Do this roundtrip at the same time we make a roundtrip to retrieve the feerates to keep startup as fast as possible.
we may use this call to broadcast keyed anchor bumps
Implementations of `BroadcasterInterface` cannot assume any topological ordering on the transactions received, so here we order the received transactions before adding them to the broadcast queue. Any consumers of the queue can now assume all transactions received to be topologically sorted. Codex wrote the tests.
These will be useful when we add support for broadcasting packages in an upcoming commit.
We rely on the `BroadcasterInterface` contract whereby any multi-transaction vector must be a single child and its parents, and must be broadcasted together as a package using `submitpackage`. In a prior commit, we added the guarantee that any packages received from the broadcast queue are already topologically sorted, and hence can be passed directly to the `submit_package` Bitcoin Core RPC.
9ca78d5 to
e61e3bc
Compare
No description provided.