lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.73k stars 2.1k forks source link

Redundant sweep of local balance after remote force close #6855

Closed rkfg closed 1 year ago

rkfg commented 2 years ago

Background

Maybe I don't understand something here but my lnd sends my balance from one p2wpkh address to another after the channel was force closed remotely. There are no anchors, no HTLCs in the commitment, just two outputs: one belongs to the remote party and is a p2wsh address, the other is mine and is a p2wpkh address. For some reason I can't understand lnd makes a redundant tx for my output and wastes fee and blockspace. I already fully control this address, why send from it again?

Your environment

alexbosworth commented 2 years ago

it moves the funds into the main wallet keys space, this will make it easier if you had to do a rescan from seed to detect the funds

rkfg commented 2 years ago

Maybe it should be configurable? I wasn't very happy when it automatically paid 5 sat/vB without much urgency. I understand that fees are hard to estimate when the tx can be sent at an unknown time and there are certain risks of fraud. But in this case there are none except if my node crashes right after this remote force close which is very unlikely. I'd like to pay the bare minimum if it's needed at all, maybe it's possible to use a wallet address in commitment in the first place? I know it can be specified during channel opening so why not do it by default? What are the downsides?

For example, there are two forcibly closed channels from yesterday with the same peer: https://amboss.space/edge/701766x458x3 and https://amboss.space/edge/731076x215x1 All peers appear to be online as well as BCash_Is_Trash (I can connect to all three of them). And yet if you look at the corresponding txs their p2wpkh outputs were not swept.

saubyk commented 2 years ago

@guggero is this issue addressed by https://github.com/lightningnetwork/lnd/pull/6868?

rkfg commented 2 years ago

I still don't understand why we can't use wallet addresses in the peer commitments for our side (i.e. when the peer force closes we get our part directly into the wallet). I looked it up but couldn't find anything conclusive except some "privacy issues". Can only guess but maybe it's about reusing the same address for on-chain tx and force close? I would gladly use some addresses twice (if they can't be marked internally as reserved for a future force close) and potentially save thousands of sats. Still, I'm only guessing about the reasons for this sweep as it's still not explained.

alexbosworth commented 2 years ago

all the outputs on the anchor channels are script outputs so it couldn't use normal addresses due to the design of the anchor channel protocol

the funds are still part of the same wallet, just a different keyspace in the wallet

in theory moving the funds out of the script output is optional and could be covered with a custom rescan in the case of recovery but that is not implemented in lnd

rkfg commented 2 years ago

No, I specifically highlighted in OP that the other party's output is a regular P2WPKH, not P2WSH. That's the whole point of this post, why transfer from one P2WPKH to another P2WPKH when both are managed exclusively by my node and have no special conditions to spend.

alexbosworth commented 2 years ago

sorry I meant for anchors that is the case, although it's a similar scenario, i mean going forward all channels will be anchors and the older format will be phased out but this dynamic will still remain the same across the formats

rkfg commented 2 years ago

For anchors yes but that wasn't what happened with the channels I linked. I realize the benefits of anchors but still they're not universally supported and I usually don't check beforehand if the other node supports them. So this additional spend looks redundant and I hope it can be avoided by smarter address selection so that it doesn't need an additional sweep.

alexbosworth commented 2 years ago

the enabling feature for that might be the addition of a recovery scan that can sweep these single key outputs from the HD seed, currently that is a feature available in an external tool but not yet present in lnd

at that point like you said maybe you would then want to turn on something to reuse keys in the commitment transaction to avoid the potential scenario of a too-large gap limit

guggero commented 2 years ago

@rkfg the history of this is:

@saubyk no, this is related to https://github.com/lightningnetwork/lnd/issues/4778.

rkfg commented 2 years ago

@guggero thank you, now it's clear.

Roasbeef commented 1 year ago

Closing as a duplicate of https://github.com/lightningnetwork/lnd/issues/4778