[bug]: Unhandled commitment type in channel acceptor request with zero conf channels from CLN

TonyGiorgio commented 1 year ago

Issue and Steps to Reproduce

Sometime since CLN 12.0, CLN no longer opens zero conf channels to LND.

I have a CLN pinned to that commit and I have one that's on the v23.05rc2.

Both CLN's can open up zero conf channels just fine to LDK and CLN nodes. However, I'm trying to understand what might be the issue on LND's side. Odds are there's some issue on one side or another that I need to get to the bottom of, I have cross posted this to CLN here: https://github.com/ElementsProject/lightning/issues/6208

CLN 12.0

LND v0.16.1 (working):

2023-04-26 22:37:04.139 [INF] FNDG: Recv'd fundingRequest(amt=0.0011 BTC, push=0 mSAT, delay=6, pendingId=e180e9cf786ad5d73a432eaecdc4fd6d56db2733e0b64c14f115023f01b80bcb) from peer(0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a)
2023-04-26 22:37:04.139 [INF] CHFD: Performing funding tx coin selection using 0 sat/kw as fee rate
2023-04-26 22:37:04.193 [INF] FNDG: Requiring 0 confirmations for pendingChan(e180e9cf786ad5d73a432eaecdc4fd6d56db2733e0b64c14f115023f01b80bcb): amt=0.0011 BTC, push_amt=0 mSAT, committype=tweakless, upfrontShutdown=
2023-04-26 22:37:04.193 [INF] FNDG: Sending fundingResp for pending_id(e180e9cf786ad5d73a432eaecdc4fd6d56db2733e0b64c14f115023f01b80bcb)
2023-04-26 22:37:04.292 [INF] FNDG: completing pending_id(e180e9cf786ad5d73a432eaecdc4fd6d56db2733e0b64c14f115023f01b80bcb) with ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1)
2023-04-26 22:37:04.297 [INF] FNDG: sending FundingSigned for pending_id(e180e9cf786ad5d73a432eaecdc4fd6d56db2733e0b64c14f115023f01b80bcb) over ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1)
2023-04-26 22:37:04.299 [INF] CNCT: Creating new ChannelArbitrator for ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1)
2023-04-26 22:37:04.299 [INF] CNCT: ChannelArbitrator(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1): starting state=StateDefault, trigger=chainTrigger, triggerHeight=35125
2023-04-26 22:37:04.299 [INF] NTFN: New spend subscription: spend_id=1, outpoint=a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1, script=0 0632908820df2f2b9869dbeac80132a3bfbab9cc75eaa08fd5ae280ca345c0d2, height_hint=35125
2023-04-26 22:37:04.299 [INF] NTFN: Dispatching historical spend rescan for outpoint=a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1, script=0 0632908820df2f2b9869dbeac80132a3bfbab9cc75eaa08fd5ae280ca345c0d2, start=35125, end=35125
2023-04-26 22:37:04.301 [INF] CNCT: Close observer for ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1) active
2023-04-26 22:37:04.301 [INF] CHBU: Updating on-disk multi SCB backup: num_old_chans=0, num_new_chans=1
2023-04-26 22:37:04.301 [INF] NTFN: Historical spend dispatch finished for request outpoint=a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1, script=0 0632908820df2f2b9869dbeac80132a3bfbab9cc75eaa08fd5ae280ca345c0d2 (start=35125 end=35125) with details: <nil>
2023-04-26 22:37:04.305 [INF] CHBU: Updating backup file at /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/channel.backup
2023-04-26 22:37:04.310 [INF] CHBU: Swapping old multi backup file from /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/temp-dont-use.backup to /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/channel.backup
2023-04-26 22:37:04.311 [INF] CHBU: Updating on-disk multi SCB backup: num_old_chans=1, num_new_chans=1
2023-04-26 22:37:04.312 [INF] FNDG: Peer(0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a) is online, sending ChannelReady for ChannelID(bcb701adf48bec557ba86cda8d3118d6406234f5be600d91e8ff74c3975ae7a7)
2023-04-26 22:37:04.314 [WRN] CHBU: Replacing disk backup for ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1) w/ newer version
2023-04-26 22:37:04.316 [INF] CHBU: Updating backup file at /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/channel.backup
2023-04-26 22:37:04.319 [INF] CHBU: Swapping old multi backup file from /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/temp-dont-use.backup to /data/Dev/Lightning/lnd-linux-amd64-v0.16.1-beta/data/data/chain/bitcoin/signet/channel.backup
2023-04-26 22:37:04.425 [INF] PEER: Peer(0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a): New channel active ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1) with peer
2023-04-26 22:37:04.425 [INF] HSWC: ChannelLink(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1): starting
2023-04-26 22:37:04.427 [INF] HSWC: Trimming open circuits for chan_id=16000000:0:0, start_htlc_id=0
2023-04-26 22:37:04.427 [INF] HSWC: Adding live link chan_id=bcb701adf48bec557ba86cda8d3118d6406234f5be600d91e8ff74c3975ae7a7, short_chan_id=16000000:0:0
2023-04-26 22:37:04.427 [INF] CNCT: Attempting to update ContractSignals for ChannelPoint(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1)
2023-04-26 22:37:04.427 [INF] HSWC: ChannelLink(a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc:1): HTLC manager started, bandwidth=0 mSAT
2023-04-26 22:37:05.335 [INF] NTFN: New confirmation subscription: conf_id=1, txid=a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc, num_confs=6 height_hint=35125
2023-04-26 22:37:05.335 [INF] FNDG: Waiting for funding tx (a6e75a97c374ffe8910d60bef5346240d618318dda6ca87b55ec8bf4ad01b7bc) to reach 6 confirmations

CLN v23.05rc2

LND (not working):

2023-04-26 22:37:03.734 [WRN] CHAC: Unhandled commitment type in channel acceptor request: &{map[12:{} 46:{}]}
2023-04-26 22:37:03.735 [INF] FNDG: Recv'd fundingRequest(amt=0.0011 BTC, push=0 mSAT, delay=6, pendingId=9a6f95eef2f308acf1a8e28402c019b0be14ed76890d6b57dffdca73f0eafcd7) from peer(03315285ceb85cc1d90ee69d30da05953850b05f06cc140e66986cc210e75d90bf)
2023-04-26 22:37:03.735 [ERR] FNDG: channel type negotiation failed: requested channel type not supported
2023-04-26 22:37:03.735 [INF] FNDG: Cancelling funding reservation for node_key=03315285ceb85cc1d90ee69d30da05953850b05f06cc140e66986cc210e75d90bf, chan_id=9a6f95eef2f308acf1a8e28402c019b0be14ed76890d6b57dffdca73f0eafcd7
2023-04-26 22:37:03.735 [ERR] FNDG: unable to cancel reservation: no active reservations for peer(03315285ceb85cc1d90ee69d30da05953850b05f06cc140e66986cc210e75d90bf)

CLN logs:

2023-04-27T03:50:02.552Z DEBUG   lightningd: fundchannel_start: allocating uncommitted_channel
2023-04-27T03:50:02.553Z INFO    lightningd: Will open private channel with node 02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095
2023-04-27T03:50:02.555Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: pid 382458, msgfd 66
2023-04-27T03:50:02.555Z DEBUG   plugin-spenderp: mfc 34, dest 0: fundchannel_start 02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095.
2023-04-27T03:50:02.555Z DEBUG   hsmd: Client: Received message 30 from client
2023-04-27T03:50:02.555Z DEBUG   hsmd: Client: Received message 10 from client
2023-04-27T03:50:02.555Z DEBUG   hsmd: new_client: 595
2023-04-27T03:50:02.556Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-hsmd: Got WIRE_HSMD_GET_PER_COMMITMENT_POINT
2023-04-27T03:50:02.556Z DEBUG   hsmd: Client: Received message 18 from client
2023-04-27T03:50:02.556Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: funder_channel_start
2023-04-27T03:50:02.556Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: Setting their reserve to 1100sat
2023-04-27T03:50:02.556Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: peer_out WIRE_OPEN_CHANNEL
2023-04-27T03:50:02.556Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: billboard: Funding channel start: offered, now waiting for accept_channel
2023-04-27T03:50:02.672Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: peer_in WIRE_ERROR
2023-04-27T03:50:02.672Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: aborted opening negotiation: They sent error channel 6ca0f3f86dc554d48e22ed902860fb3baf8d209692c24e2f7a4a897d558bbbf1: funding failed due to internal error
2023-04-27T03:50:02.672Z DEBUG   02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095-openingd-chan#595: billboard perm: They sent error channel 6ca0f3f86dc554d48e22ed902860fb3baf8d209692c24e2f7a4a897d558bbbf1: funding failed due to internal error
2023-04-27T03:50:02.673Z DEBUG   plugin-spenderp: mfc 34, dest 0: failed! fundchannel_start 02a5a83747e3308dacd3769f014b5c504e0dfc73930b2278c63c8f1b1ce76cf095: {\"code\":-1,\"message\":\"They sent error channel 6ca0f3f86dc554d48e22ed902860fb3baf8d209692c24e2f7a4a897d558bbbf1: funding failed due to internal error\"}.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: parallel channel starts done.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: trying redo despite 'fundchannel_start' failure (They sent error channel 6ca0f3f86dc554d48e22ed902860fb3baf8d209692c24e2f7a4a897d558bbbf1: funding failed due to internal error); will cleanup for now.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: cleanup!
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: unreserveinputs task.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: Filtering destinations.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34, dest 0: failed.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: 1 destinations failed, failing.
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: cleanup!
2023-04-27T03:50:02.694Z DEBUG   plugin-spenderp: mfc 34: cleanup done, finishing command.

The thing that stands out is:

2023-04-26 22:37:03.734 [WRN] CHAC: Unhandled commitment type in channel acceptor request: &{map[12:{} 46:{}]}

When I look at the LND switch case that handles that, I do not see one that just handles those two channel type features: https://github.com/lightningnetwork/lnd/blob/fdc1ae9d57326cc82899e4c76d5069b14a764b96/chanacceptor/rpcacceptor.go#L274-L346

Being just StaticRemoteKeyRequired & ScidAliasRequired.

Question for LND:

I suspect CLN might have changed the channel types they were passing through. Hard to tell what they were, the logs only show what it does not like with the new version. Why can't it support StaticRemoteKeyRequired & ScidAliasRequired ?
Is there anything I can do to get around this? Or anything that CLN can do differently?

morehouse commented 1 year ago

Yes, CLN recently started sending this combination of StaticRemoteKeyRequired and ScidAliasRequired.

LND seems to expect the zero fee HTLC channel type with this combination, which CLN doesn't support yet.

Roasbeef commented 1 year ago

Yep, we only support zero fee anchors for zero conf channels. Zero fee anchors (as they're much safer) are the future!

2023-04-26 22:37:03.734 [WRN] CHAC: Unhandled commitment type in channel acceptor request: &{map[12:{} 46:{}]} 2023-04-26 22:37:03.735 [ERR] FNDG: channel type negotiation failed: requested channel type not supported

We should improve the logging here though.

TonyGiorgio commented 1 year ago

Yep, we only support zero fee anchors for zero conf channels. Zero fee anchors (as they're much safer) are the future!

2023-04-26 22:37:03.734 [WRN] CHAC: Unhandled commitment type in channel acceptor request: &{map[12:{} 46:{}]} 2023-04-26 22:37:03.735 [ERR] FNDG: channel type negotiation failed: requested channel type not supported

We should improve the logging here though.

I don't really get it though, because I could open up a zero conf channel with the previous version of CLN. Meaning that LND does support these zero conf channels without anchors. Seems like it's just a channel flagging concern, in which case I'll probably have to go back to CLN to tell them to stop sending over those flags?

And it was my understanding that LND won't open non-anchor channels but should be accepting them from others like the callouts with LDK specify in the original zero conf channel feature PR.

gkrizek commented 1 year ago

@Roasbeef got this comment:

the spec specifies that zero conf should require scid aliases too, however the CoreLN interpretation was it'd be ok to be optional, which was fixed in the latest release. However lnd seems not to like mandatory scid aliases causing connections to be dropped

I'm struggling to find where the issue is between the two. You were mentioning zero fee anchors but it sounds like this might be more related to flags? Can you help me understand that?

TonyGiorgio commented 1 year ago

@Roasbeef got this comment:

the spec specifies that zero conf should require scid aliases too, however the CoreLN interpretation was it'd be ok to be optional, which was fixed in the latest release. However lnd seems not to like mandatory scid aliases causing connections to be dropped

I'm struggling to find where the issue is between the two. You were mentioning zero fee anchors but it sounds like this might be more related to flags? Can you help me understand that?

Looks like that's in response to this: https://twitter.com/Snyke/status/1659544009037971456?s=20

I believe he is incorrect. LND will happy acknowledge channels with ScidAliasRequired, but only if they are at least signaling AnchorsZeroFeeHtlcTxRequired.

It sounds like it was a bug that CLN was was passing ScidAliasOptional before, and perhaps a bug that LND was allowing ScidAliasOptional channels to be opened to it without AnchorsZeroFeeHtlcTxRequired.

So in order for CLN to open zero conf channels again with LND, then there's a few options:

CLN reverts the change and passes ScidAliasOptional instead of ScidAliasRequired until compatibility is met.
CLN supports AnchorsZeroFeeHtlcTxRequired for zero conf channels
LND allows others to open (or allows the channel acceptor to bypass restrictions) when it's just a ScidAliasRequired and StaticRemoteKeyRequired channel without AnchorsZeroFeeHtlcTxRequired

Between @Roasbeef or @cdecker I'm not sure who will budge here, I don't see any way around this protocol incompatibility between the two implementations.

Roasbeef commented 1 year ago

So just to clarify the current state of things:

When creating channels, lnd will only accept zero conf channels that also have anchor zero fee tx:
- This is done as it's the absolute safest option. Given the current mempool environment, as soon as the other implementations roll out their version, we'll make this default and reject all other channels.
- We're already seeing enough pain out there where those with the legacy channel type are unable to confirm their closes as they were sent at 1 sat/byte, which is well below the current min relay fee amount. We don't plan on budging here, as it's the safest option the protocol offers.
When accepting channels, if the channel_type is set in the open channel message and feature bit present, we'll employ the same behavior above.
- IMO the channel_type is preferable as, then the responder knows exactly what they're getting into. This allows the channel acceptor to screen these channel types and reject them early. Some of the other implementers disagree with this, and don't send the type, instead relying on the remote party to respond with a min depth of zero.
- IIUC, to achieve cross compat, y'all have created a side signalling protocol to negotiate up front that the channel will be zero conf, then send that. IMO if channel_type was used universally as designed, this wouldn't be necessary in most cases.

Once again, lnd's behavior here has not changed.

Also zero conf and scid alias are distinct: you can open a static channel with scid alises np, but not also with zero conf (which also requires scid alias in order to be useable).

Returning to @TonyGiorgio's scenario in the OP: it looks like you're having lnd accept those channels from CLN. AFAICT, those channels wouldn't actually show up as zero conf from lnd's PoV if you do listchannels. As the responder, only if lnd sends min accept depth of zero will it "upgrade". This isn't done by default, and can only be done with a custom channel acceptor. AFAICT, you're doing this and responding with a depth of zero.

You correctly identified above that we don't explicitly support ScidAliasRequired + StaticRemoteKeyRequired, rationale is stated as above: to push people towards the most secure channel type. Support is nearly universal, I think within a few months, we'll be able to flip it to default as we desire.

With all that said, the error you're seeing is a bit further in the chain at: explicitNegotiateCommitmentType. We have a PR fixing a bug in that area, unrelated to this. I think we'd accept a PR to extend that to add the extra case for alias+static key. As mentioned above, I don't think accepting such channels is wise as you may never be able to close them, but I think y'all understand the risks.

gkrizek commented 1 year ago

Thanks for the run down @Roasbeef. This is helpful in understanding what's going on here. I hear you on anchor channels and I'm excited for them to be more widespread. Definitely not asking to regress that.

IIUC, to achieve cross compat, y'all have created a side signalling protocol to negotiate up front that the channel will be zero conf

We had previously had something like this but have since removed it.

This isn't done by default, and can only be done with a custom channel acceptor.

That is correct that we run a channel acceptor to facilitate this response of a zero depth.

I appreciate willingness to work with this solution until we're at a fully deployed state of zero fee anchor channels. To make sure I understand, would it be best to open a PR into #7177 or master?

morehouse commented 1 year ago

I appreciate willingness to work with this solution until we're at a fully deployed state of zero fee anchor channels. To make sure I understand, would it be best to open a PR into #7177 or master?

I would prefer a separate PR, as this issue seems unrelated to #7177.

gkrizek commented 1 year ago

@morehouse sounds good. I just opened a new PR for this into master.

gkrizek commented 1 year ago

@TonyGiorgio @Roasbeef @morehouse - It appears this compatibility issue is fixed in the latest CoreLN Version (https://github.com/ElementsProject/lightning/releases/tag/v23.05.1). See PR (https://github.com/ElementsProject/lightning/pull/6304).

We've tested that this does fix the issue and this version of CoreLN is able to open zero conf channels into LND once again. So that fixes our issue. The question now is; what do we do with #7740? I'm in favor of closing it as the superior solution was reached and this subpar solution is no longer necessary. Do we have any objections to that?

Thanks!

morehouse commented 1 year ago

I don't think https://github.com/ElementsProject/lightning/pull/6304 is a "superior solution"; it seems more like a hack to maintain CLN's previous (spec noncompliant) behavior when doing zero-conf between CLN and LND. I think the better solution is to make LND accept the specific channel type when it is explicitly negotiated (via channel_type), since LND seems to implicitly do SCID aliases anyway.

That said, CLN is working on zero fee anchors, so probably not a big deal if we just wait for that and don't fix things on the LND side. As all implementations start to support modern channel types, we may even want to reduce the channel types we support (to simplify code and protect users).

gkrizek commented 1 year ago

@morehouse Yeah I probably used words improperly there, haha. I was mainly meaning not supporting a less-ideal channel type in LND and instead wait for everyone to support the more preferred channel type.

saubyk commented 1 year ago

@gkrizek based on the comments above, I cleared the milestone and closed this issue. Feel free to reopen if you think otherwise.

lightningnetwork / lnd