[sweeper] reuse exclusive group UTXOs

[halseth]

trafficstars

This PR makes the sweeper re-use wallet UTXOs when it sweeps inputs from the same exclusive group.

The reason we can do this is that two sweep transactions having inputs from the same exclusive group won't ever be both valid. That means we can add the same wallet UTXOs to both transactions, and only one of them will be mined.

Motivation

The motivation for doing this was to fix a flake in the Neutrino integration tests (see below), but it also has other benefits:

• If we have multiple exclusive groups with many inputs in flight, we won't claim one UTXO per input, but instead one per exclusive group.
• In case of Neutrino, previously it wouldn't know that a confirmed TX spending an exclusive group would invalidate all other transactions from the same exclusive group. So the transaction would linger in the wallet unconfirmed until any of its valid inputs were spent. With this change, btcwallet is able to see that the wallet input has been spent by another transaction and remove it. Note that the issue remains for sweep transactions that have no wallet inputs added (probably doesn't happen at this point, since we only are using exclusive groups for anchors).

Description of test flake

The Neutrino itest flake that motivated this change was

 test_harness.go:91: Error outside of test: *errors.errorString unable to find Carol's force close tx in mempool: wanted 2, found 1 txs in mempool: [1098adba64ac54c391127b0e20d87cdf3f4cfa752c12c83138f11fd7126d80e7]
        /golang/src/github.com/lightningnetwork/lnd-worktree/lntest/itest/lnd_test.go:9574 (0x1a99705)
        	assertDLPExecuted: t.Fatalf("unable to find Carol's force close tx in mempool: %v",
        /golang/src/github.com/lightningnetwork/lnd-worktree/lntest/itest/lnd_channel_backup_test.go:1069 (0x1a46271)
        	testChanRestoreScenario: assertDLPExecuted(
        /golang/src/github.com/lightningnetwork/lnd-worktree/lntest/itest/lnd_channel_backup_test.go:374 (0x1abbf7b)
        	testChannelBackupRestore.func9: testChanRestoreScenario(h, net, &testCase, password)
        /usr/local/Cellar/go/1.15.5/libexec/src/testing/testing.go:1123 (0x111848f)
        	tRunner: fn(t)
        /usr/local/Cellar/go/1.15.5/libexec/src/runtime/asm_amd64.s:1374 (0x106ec01)
        	goexit: BYTE	$0x90	// NOP
=== CONT  TestLightningNetworkDaemon
    lnd_test.go:14395: Failure time: 2020-12-11 13:42:32.447
--- FAIL: TestLightningNetworkDaemon (182.60s)
    --- FAIL: TestLightningNetworkDaemon/54-of-70/neutrino/channel_backup_restore (180.99s)
        --- FAIL: TestLightningNetworkDaemon/54-of-70/neutrino/channel_backup_restore/restore_from_backup_file_anchors (130.95s)

This happened because the wallet only had one UTXO available for fee bumping when it tried to sweep both anchors (the anchor on the local and remote commit). If the non-existent anchor output would be attempted first, it would use this single wallet UTXO to craft the sweep. Next the anchor output actually in the mempool would be swept. But at this point the sweeper would have no wallet UTXOs available to bump the fee of the sweep, resulting in a failure to create the sweep tx:

SWPR: Set value -0.0003248 BTC (r=0 BTC, c=-0.0003248 BTC) below dust limit of 0.00000537 BTC 

For non-Neutrino backends (fullnodes) this is not a problem, since the backend will realize the first attempted sweep is spending a non-existent outpoint, and reject it. Since Neutrino doesn't have a mempool however, the wallet can't know that it spends an outpoint that doesn't exist, and it will be accepted, reserving the single wallet UTXO in the process.

Alternative solution attempted

Another alternative approach to solve this test failure was to add more available UTXO to the wallet before the test starts. However, I think this is the wrong approach for two reasons:

1. Every time a channel is force closed, we will try to spend our anchor for the 2 (sometimes 3) versions of the commitment transaction. Using Neutrino we could run into problems not having enough UTXOs.
2. Just giving the wallet more UTXOs creates a new test failure: Since the second (invalid) anchor sweep will be lingering in the wallet when the test ends, the wallet UTXO it spends will not contribute towards the wallet's confirmed balance anymore, and the balance check fails.

TODO

• [ ] Unit tests

Blocked by #4866 which causes some flakes

[Roasbeef]

This PR makes the sweeper re-use wallet UTXOs when it sweeps inputs from the same exclusive group.

Won't this cause only one of them to be rebroadcasted on startup for the underlying wallet itself?

[rubikputer]

I'm having some issues where LND tried to sweep outputs created by Pool that are spent, resulting in it making invalid transactions. If I reset the tx database then it still tries to sweep the outputs again. LND then keeps waiting for the tx to confirm that never will, and reports an incorrect total balance. I am using Neutrino, which seems to be the sticking point here.

I was referred to this PR on IRC, and it makes sense that the issue I'm having would be fixed by this if it's finished. I'm afraid if I use Pool again I'll run into this issue again so I'm holding off on experimenting more with that until this is fixed.

[yyforyongyu]

Turns out the neutrino flakes had already been found and fixed here😂 @guggero

[lightninglabs-deploy]

@halseth, remember to re-request review from reviewers when ready

[lightninglabs-deploy]

@halseth, remember to re-request review from reviewers when ready

[yyforyongyu]

Is this still relevant?

[lightninglabs-deploy]

@halseth, remember to re-request review from reviewers when ready

[yyforyongyu]

!lightninglabs-deploy mute 30d