[bug]: `CloseChannel`: `no_wait=True` still waiting

[ZZiigguurraatt]

I try to use the CloseChannel no_wait option. If I set no_wait=True, it still waits for the transaction to be confirmed before stopping the gRPC streaming call. It's possible that's not the intent, but the documentation here is a bit confusing what sentence refers to no_wait=True and what refers to no_wait=False. Possibly we need another option that controls whether or not the call waits for the close to be confirmed?

lnd/lnrpc/lightning.proto

Lines 2159 to 2167 in 3707b1f

	// If true, then the rpc call will not block while it awaits a closing txid
	// to be broadcasted to the mempool. To obtain the closing tx one has to
	// listen to the stream for the particular updates. Moreover if a coop close
	// is specified and this flag is set to true the coop closing flow will be
	// initiated even if HTLCs are active on the channel. The channel will wait
	// until all HTLCs are resolved and then start the coop closing process. The
	// channel will be disabled in the meantime and will disallow any new HTLCs.
	bool no_wait = 8;

[Roasbeef]

This is about waiting for the channel to clear in the case of active HTLCs.

If that flag is false, and you have a pending HTLC on the channel you're trying to close, it'll error out.

If it's true, and you have active HTLCs, it'll wait for those to clear and then try once that's the case.

Setting it to true is better for UX, as you can just fire and forget, and lnd will coop close when it's able to.

[Roasbeef]

Possibly we need another option that controls whether or not the call waits for the close to be confirmed?

The call returns streaming RPCs that tell you when we broadcast initially, and then later once things are confirmed.

[ZZiigguurraatt]

Possibly we need another option that controls whether or not the call waits for the close to be confirmed?

The call returns streaming RPCs that tell you when we broadcast initially, and then later once things are confirmed.

Yeah, I'm wondering if there should be an option to hangup after broadcast, but before confirming. Or, maybe the client should just figure out how to do that themselves?

[ZZiigguurraatt]

If that flag is false, and you have a pending HTLC on the channel you're trying to close, it'll error out.

I think this is the main thing that is missing from the docs, that it will error out. Right now, I'm confused because if it is already going to block to wait for confirmation, why do I need an extra option to block for in flight HTLC?

If it's true, and you have active HTLCs, it'll wait for those to clear and then try once that's the case.

no_wait=True to me means wait=False, so why would it wait for HTLC to clear in that case?

Setting it to true is better for UX, as you can just fire and forget, and lnd will coop close when it's able to.

What is the default?

[Roasbeef]

Or, maybe the client should just figure out how to do that themselves?

As is, the client can do that themselves.

What is the default?

The default is the original behavior: if you have active HTLCs, and try to coop close, it'll error out.

I agree documentation can be improved here. Feel free to make a PR with a suggestion.

We can either rename the issue title, or close this as it's about waiting for HTLCs to clear, not waiting to broadcast or not.

[ZZiigguurraatt]

Was this feature necessary as part of co-op close or was it just added at the same time as a convenience/coincidence? Like without co-op close it could still be useful, right?

[Roasbeef]

Was this feature necessary as part of co-op close or was it just added at the same time as a convenience/coincidence? Like without co-op close it could still be useful, right?

This was added before the RBF coop close stuff. This was intended to make the API easier to use. As before the client had to retry in the background until a channel was clear to coop close. This could be difficult for a high volume backbone channel. The new API allows the client to submit the RPC call, then just wait until lnd finds an opportunity to truly shutdown the channel.

[ZZiigguurraatt]

The default is the original behavior: if you have active HTLCs, and try to coop close, it'll error out.

So, to me this sounds like no_wait=True which means wait=False would be the default behavior. However, this conflicts with your statement

If it's true, and you have active HTLCs, it'll wait for those to clear and then try once that's the case.

[ZZiigguurraatt]

Also, could

Moreover if a coop close is specified and this flag is set to true the coop closing flow will be initiated even if HTLCs are active on the channel. The channel will wait until all HTLCs are resolved and then start the coop closing process. The channel will be disabled in the meantime and will disallow any new HTLCs.

be simplified to just say

"if a coop close is specified, this option is ignored"

?

The reason I say this is because if no_wait=True means wait=False and if "his flag is set to true the coop closing flow will be initiated even if HTLCs are active on the channel. The channel will wait until all HTLCs are resolved and then start the coop closing process. The channel will be disabled in the meantime and will disallow any new HTLCs", it reads to me like the option is just ignored.

[ZZiigguurraatt]

I feel like some of these confusions were introduced in 59443fa

Like this also seems backwards to me:

lnd/cmd/commands/commands.go

Lines 1135 to 1137 in c1740c1

	// This makes sure that a coop close will also be executed if
	// active HTLCs are present on the channel.
	NoWait: true,

Does NoWait: true not mean Wait: false?

Also,

lnd/cmd/commands/commands.go

Lines 1057 to 1062 in c1740c1

	cli.BoolFlag{
	Name: "block",
	Usage: `block will wait for the channel to be closed,
	"meaning that it will wait for the channel close tx to
	get 1 confirmation.`,
	},

seems to be mostly unrelated to the no_wait option? If so, I think we need to better clarify this a bit.

@ziggie1984 , do you have any input on these confusions?

[ZZiigguurraatt]

I've done some testing and if I try to close a channel with an in flight HTLC without setting no_wait, I get the error

cannot coop close channel with active htlcs (number of active htlcs: 1), bypass this check and initiate the coop close by setting no_wait=true

so, this leads me to believe that the default is no_wait=false.

Also, it seems to me like

    // If true, then the rpc call will not block while it awaits a closing txid
    // to be broadcasted to the mempool.

should be changed to

    // If false, then the rpc call will error out if there are in flight HTLC and
    // the channel can't be cooperatively closed right now.
    // If true, then the rpc call will block while it awaits all in flight HTLC to
    // to be settled and a closing txid is to be broadcasted to the mempool.

?

But I'm really confused why this parameter is called no_wait because it seems wait is what it is really doing.

[ZZiigguurraatt]

Possibly

    // If true, then the rpc call will not block while it awaits a closing txid
    // to be broadcasted to the mempool.

refers to force closing a channel only and that is where my confusion lies?