Drop ChannelHandshakeLimits::max_funding_satoshis#4318
Drop ChannelHandshakeLimits::max_funding_satoshis#4318tnull wants to merge 1 commit intolightningdevkit:mainfrom
ChannelHandshakeLimits::max_funding_satoshis#4318Conversation
|
I've assigned @valentinewallace as a reviewer! |
Previously, LDK would by default limit channels pre-Wumbo sizes and leave it to the user to bump `ChannelHandshakeLimits::max_funding_satoshis`. This has mostly historical reasons that aimed to allow limiting risk when Lightning and LDK were not as matured as today. By now, we do however expect ~all users to eventually want to bump this limit, and having them pick an arbitrary value (or pick a default ourselves) is kinda odd. Users that still want to limit risks have ample other means to do so, for example manually rejecting inbound channels via the manual-acceptence flow (via `Event::OpenChannelRequest`) or soon even limiting risk on a per-HTLC basis via general purpose HTLC interception. Furthermore, it turns out that our current implementation is wrong, as we do always announce `Wumbo`/`option_supports_large_channels` support via the `IN` feature in `ChannelManager` defaults, irrespective of what limit is configured. This has us announcing support for Wumbo channels to only then reject inbound requests in case a counterparty dares to actually try to open one. To address this, we here simply propose to drop the `max_funding_satoshis` field and corresponding checks entirely, and do what we've announced to the network for a long time: enable Wumbo by default.
6b6a405 to
cd49d13
Compare
valentinewallace
left a comment
There was a problem hiding this comment.
Probably won't lead to issues in practice. Would be curious to outline the risks a bit more explicitly in the PR description, such as allowing a random node to drain outbound liquidity or something like that. Or maybe it could point towards a change such as enabling manual channel acceptance by default.
Based on #3912 discussion, needs @TheBlueMatt's conceptual review
|
👋 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. |
|
Yea, given we want to move to just-anchors anyway I do kinda wonder if we shouldn't move to manual channel acceptance by default (or only?). Then dropping config limits is really obvious. |
Fixes #3912.
Previously, LDK would by default limit channels pre-Wumbo sizes and leave it to the user to bump
ChannelHandshakeLimits::max_funding_satoshis. This has mostly historical reasons that aimed to allow limiting risk when Lightning and LDK were not as matured as today. By now, we do however expect ~all users to eventually want to bump this limit, and having them pick an arbitrary value (or pick a default ourselves) is kinda odd. Users that still want to limit risks have ample other means to do so, for example manually rejecting inbound channels via the manual-acceptence flow (viaEvent::OpenChannelRequest) or soon even limiting risk on a per-HTLC basis via general purpose HTLC interception.Furthermore, it turns out that our current implementation is wrong, as we do always announce
Wumbo/option_supports_large_channelssupport via theINfeature inChannelManagerdefaults, irrespective of what limit is configured. This has us announcing support for Wumbo channels to only then reject inbound requests in case a counterparty dares to actually try to open one.To address this, we here simply propose to drop the
max_funding_satoshisfield and corresponding checks entirely, and do what we've announced to the network for a long time: enable Wumbo by default.