Skip to content

Commit acc5951

Browse files
authored
Merge pull request #8545 from ziggie1984/dont-use-sweeper-unconfirmed-utxos
dont use sweeper unconfirmed utxos
2 parents 7fb2333 + 58e1288 commit acc5951

File tree

22 files changed

+741
-30
lines changed

22 files changed

+741
-30
lines changed

docs/release-notes/release-notes-0.18.0.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@
118118

119119
* [Fixed a bug in `btcd` that caused an incompatibility with
120120
`bitcoind v27.0`](https://github.com/lightningnetwork/lnd/pull/8573).
121+
122+
* [Fixed](https://github.com/lightningnetwork/lnd/pull/8609) a function call
123+
where arguments were swapped.
124+
125+
* [Fixed](https://github.com/lightningnetwork/lnd/pull/8545) utxo selection
126+
for the internal channel funding flow (Single and Batch Funding Flow). Now
127+
utxos which are unconfirmed and originated from the sweeper subsystem are not
128+
selected because they bear the risk of being replaced (BIP 125 RBF).
121129

122130
# New Features
123131
## Functional Enhancements

funding/manager.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,12 @@ type Config struct {
537537
// AliasManager is an implementation of the aliasHandler interface that
538538
// abstracts away the handling of many alias functions.
539539
AliasManager aliasHandler
540+
541+
// IsSweeperOutpoint queries the sweeper store for successfully
542+
// published sweeps. This is useful to decide for the internal wallet
543+
// backed funding flow to not use utxos still being swept by the sweeper
544+
// subsystem.
545+
IsSweeperOutpoint func(wire.OutPoint) bool
540546
}
541547

542548
// Manager acts as an orchestrator/bridge between the wallet's
@@ -4600,10 +4606,26 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
46004606
MinConfs: msg.MinConfs,
46014607
CommitType: commitType,
46024608
ChanFunder: msg.ChanFunder,
4603-
ZeroConf: zeroConf,
4604-
OptionScidAlias: scid,
4605-
ScidAliasFeature: scidFeatureVal,
4606-
Memo: msg.Memo,
4609+
// Unconfirmed Utxos which are marked by the sweeper subsystem
4610+
// are excluded from the coin selection because they are not
4611+
// final and can be RBFed by the sweeper subsystem.
4612+
AllowUtxoForFunding: func(u lnwallet.Utxo) bool {
4613+
// Utxos with at least 1 confirmation are safe to use
4614+
// for channel openings because they don't bare the risk
4615+
// of being replaced (BIP 125 RBF).
4616+
if u.Confirmations > 0 {
4617+
return true
4618+
}
4619+
4620+
// Query the sweeper storage to make sure we don't use
4621+
// an unconfirmed utxo still in use by the sweeper
4622+
// subsystem.
4623+
return !f.cfg.IsSweeperOutpoint(u.OutPoint)
4624+
},
4625+
ZeroConf: zeroConf,
4626+
OptionScidAlias: scid,
4627+
ScidAliasFeature: scidFeatureVal,
4628+
Memo: msg.Memo,
46074629
}
46084630

46094631
reservation, err := f.cfg.Wallet.InitChannelReservation(req)

funding/manager_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,11 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey,
556556
return nil, nil
557557
},
558558
AliasManager: aliasMgr,
559+
// For unit tests we default to false meaning that no funds
560+
// originated from the sweeper.
561+
IsSweeperOutpoint: func(wire.OutPoint) bool {
562+
return false
563+
},
559564
}
560565

561566
for _, op := range options {

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/btcsuite/btcd/btcutil/psbt v1.1.8
1111
github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0
1212
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
13-
github.com/btcsuite/btcwallet v0.16.10-0.20240410030101-6fe19a472a62
13+
github.com/btcsuite/btcwallet v0.16.10-0.20240404104514-b2f31f9045fb
1414
github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4
1515
github.com/btcsuite/btcwallet/wallet/txrules v1.2.1
1616
github.com/btcsuite/btcwallet/walletdb v1.4.2

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0/go.mod h1:7SFka0XMvUgj3hfZtyd
9292
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo=
9393
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
9494
github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
95-
github.com/btcsuite/btcwallet v0.16.10-0.20240410030101-6fe19a472a62 h1:MtcTVTcDbGdTJhfDc7LLikojyl0PYtSRNLwoRaLVbWI=
96-
github.com/btcsuite/btcwallet v0.16.10-0.20240410030101-6fe19a472a62/go.mod h1:2C3Q/MhYAKmk7F+Tey6LfKtKRTdQsrCf8AAAzzDPmH4=
95+
github.com/btcsuite/btcwallet v0.16.10-0.20240404104514-b2f31f9045fb h1:qoIOlBPRZWtfpcbQlNFf67Wz8ZlXo+mxQc9Pnbm/iqU=
96+
github.com/btcsuite/btcwallet v0.16.10-0.20240404104514-b2f31f9045fb/go.mod h1:2C3Q/MhYAKmk7F+Tey6LfKtKRTdQsrCf8AAAzzDPmH4=
9797
github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4 h1:poyHFf7+5+RdxNp5r2T6IBRD7RyraUsYARYbp/7t4D8=
9898
github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4/go.mod h1:GETGDQuyq+VFfH1S/+/7slLM/9aNa4l7P4ejX6dJfb0=
9999
github.com/btcsuite/btcwallet/wallet/txrules v1.2.1 h1:UZo7YRzdHbwhK7Rhv3PO9bXgTxiOH45edK5qdsdiatk=

itest/list_on_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,14 @@ var allTestCases = []*lntest.TestCase{
117117
Name: "batch channel funding",
118118
TestFunc: testBatchChanFunding,
119119
},
120+
{
121+
Name: "open channel with unstable utxos",
122+
TestFunc: testChannelFundingWithUnstableUtxos,
123+
},
124+
{
125+
Name: "open psbt channel with unstable utxos",
126+
TestFunc: testPsbtChanFundingWithUnstableUtxos,
127+
},
120128
{
121129
Name: "update channel policy",
122130
TestFunc: testUpdateChannelPolicy,

itest/lnd_channel_funding_utxo_selection_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ func runUtxoSelectionTestCase(ht *lntest.HarnessTest, alice,
330330
// When re-selecting a spent output for funding another channel we
331331
// expect the respective error message.
332332
if tc.reuseUtxo {
333-
expectedErrStr := fmt.Sprintf("outpoint already spent: %s:%d",
333+
expectedErrStr := fmt.Sprintf("outpoint already spent or "+
334+
"locked by another subsystem: %s:%d",
334335
selectedOutpoints[0].TxidStr,
335336
selectedOutpoints[0].OutputIndex)
336337
expectedErr := fmt.Errorf(expectedErrStr)

itest/lnd_funding_test.go

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/btcsuite/btcd/txscript"
1313
"github.com/btcsuite/btcd/wire"
1414
"github.com/lightningnetwork/lnd/chainreg"
15+
"github.com/lightningnetwork/lnd/fn"
1516
"github.com/lightningnetwork/lnd/funding"
1617
"github.com/lightningnetwork/lnd/input"
1718
"github.com/lightningnetwork/lnd/labels"
@@ -1250,3 +1251,211 @@ func deriveFundingShim(ht *lntest.HarnessTest, carol, dave *node.HarnessNode,
12501251

12511252
return fundingShim, chanPoint
12521253
}
1254+
1255+
// testChannelFundingWithUnstableUtxos tests channel openings with restricted
1256+
// utxo selection. Internal wallet utxos might be restricted due to another
1257+
// subsystems still using it therefore it would be unsecure to use them for
1258+
// channel openings. This test focuses on unconfirmed utxos which are still
1259+
// being used by the sweeper subsystem hence should only be used when confirmed.
1260+
func testChannelFundingWithUnstableUtxos(ht *lntest.HarnessTest) {
1261+
// Select funding amt below wumbo size because we later use fundMax to
1262+
// open a channel with the total balance.
1263+
fundingAmt := btcutil.Amount(3_000_000)
1264+
1265+
// We use STATIC_REMOTE_KEY channels because anchor sweeps would
1266+
// interfere and create additional utxos.
1267+
// Although its the current default we explicitly signal it.
1268+
cType := lnrpc.CommitmentType_STATIC_REMOTE_KEY
1269+
1270+
// First, we'll create two new nodes that we'll use to open channel
1271+
// between for this test.
1272+
carol := ht.NewNode("carol", nil)
1273+
// We'll attempt at max 2 pending channels, so Dave will need to accept
1274+
// two pending ones.
1275+
dave := ht.NewNode("dave", []string{
1276+
"--maxpendingchannels=2",
1277+
})
1278+
ht.EnsureConnected(carol, dave)
1279+
1280+
// Fund Carol's wallet with a confirmed utxo.
1281+
ht.FundCoins(fundingAmt, carol)
1282+
1283+
// Now spend the coins to create an unconfirmed transaction. This is
1284+
// necessary to test also the neutrino behaviour. For neutrino nodes
1285+
// only unconfirmed transactions originating from this node will be
1286+
// recognized as unconfirmed.
1287+
req := &lnrpc.NewAddressRequest{Type: AddrTypeTaprootPubkey}
1288+
resp := carol.RPC.NewAddress(req)
1289+
1290+
sendCoinsResp := carol.RPC.SendCoins(&lnrpc.SendCoinsRequest{
1291+
Addr: resp.Address,
1292+
SendAll: true,
1293+
SatPerVbyte: 1,
1294+
})
1295+
1296+
walletUtxo := ht.AssertNumUTXOsUnconfirmed(carol, 1)[0]
1297+
require.EqualValues(ht, sendCoinsResp.Txid, walletUtxo.Outpoint.TxidStr)
1298+
1299+
// We will attempt to open 2 channels at a time.
1300+
chanSize := btcutil.Amount(walletUtxo.AmountSat / 3)
1301+
1302+
// Open a channel to dave with an unconfirmed utxo. Although this utxo
1303+
// is unconfirmed it can be used to open a channel because it did not
1304+
// originated from the sweeper subsystem.
1305+
update := ht.OpenChannelAssertPending(carol, dave,
1306+
lntest.OpenChannelParams{
1307+
Amt: chanSize,
1308+
SpendUnconfirmed: true,
1309+
CommitmentType: cType,
1310+
})
1311+
chanPoint1 := lntest.ChanPointFromPendingUpdate(update)
1312+
1313+
// Verify that both nodes know about the channel.
1314+
ht.AssertNumPendingOpenChannels(carol, 1)
1315+
ht.AssertNumPendingOpenChannels(dave, 1)
1316+
1317+
// We open another channel on the fly, funds are unconfirmed but because
1318+
// the tx was not created by the sweeper we can use it and open another
1319+
// channel. This is a common use case when opening zeroconf channels,
1320+
// so unconfirmed utxos originated from prior channel opening are safe
1321+
// to use because channel opening should not be RBFed, at least not for
1322+
// now.
1323+
update = ht.OpenChannelAssertPending(carol, dave,
1324+
lntest.OpenChannelParams{
1325+
Amt: chanSize,
1326+
SpendUnconfirmed: true,
1327+
CommitmentType: cType,
1328+
})
1329+
1330+
chanPoint2 := lntest.ChanPointFromPendingUpdate(update)
1331+
1332+
ht.AssertNumPendingOpenChannels(carol, 2)
1333+
ht.AssertNumPendingOpenChannels(dave, 2)
1334+
1335+
// We expect the initial funding tx to confirm and also the two
1336+
// unconfirmed channel openings.
1337+
ht.MineBlocksAndAssertNumTxes(1, 3)
1338+
1339+
// Now we create an unconfirmed utxo which originated from the sweeper
1340+
// subsystem and hence is not safe to use for channel openings. We do
1341+
// that by dave force-closing the channel. Which let's carol sweep its
1342+
// to_remote output which is not encumbered by any relative locktime.
1343+
ht.CloseChannelAssertPending(dave, chanPoint2, true)
1344+
// Mine the force close commitment transaction.
1345+
ht.MineBlocksAndAssertNumTxes(1, 1)
1346+
1347+
// Mine one block to trigger the sweep transaction.
1348+
ht.MineEmptyBlocks(1)
1349+
1350+
// We need to wait for carol initiating the sweep of the to_remote
1351+
// output of chanPoint2.
1352+
utxos := ht.AssertNumUTXOsUnconfirmed(carol, 1)
1353+
1354+
// We filter for the unconfirmed utxo and try to open a channel with
1355+
// that utxo.
1356+
utxoOpt := fn.Find(func(u *lnrpc.Utxo) bool {
1357+
return u.Confirmations == 0
1358+
}, utxos)
1359+
fundingUtxo := utxoOpt.UnwrapOrFail(ht.T)
1360+
1361+
// Now try to open the channel with this utxo and expect an error.
1362+
expectedErr := fmt.Errorf("outpoint already spent or "+
1363+
"locked by another subsystem: %s:%d",
1364+
fundingUtxo.Outpoint.TxidStr,
1365+
fundingUtxo.Outpoint.OutputIndex)
1366+
1367+
ht.OpenChannelAssertErr(carol, dave,
1368+
lntest.OpenChannelParams{
1369+
FundMax: true,
1370+
SpendUnconfirmed: true,
1371+
Outpoints: []*lnrpc.OutPoint{
1372+
fundingUtxo.Outpoint,
1373+
},
1374+
}, expectedErr)
1375+
1376+
// The channel opening failed because the utxo was unconfirmed and
1377+
// originated from the sweeper subsystem. Now we confirm the
1378+
// to_remote sweep and expect the channel opening to work.
1379+
ht.MineBlocksAndAssertNumTxes(1, 1)
1380+
1381+
// Try opening the channel with the same utxo (now confirmed) again.
1382+
update = ht.OpenChannelAssertPending(carol, dave,
1383+
lntest.OpenChannelParams{
1384+
FundMax: true,
1385+
SpendUnconfirmed: true,
1386+
Outpoints: []*lnrpc.OutPoint{
1387+
fundingUtxo.Outpoint,
1388+
},
1389+
})
1390+
1391+
chanPoint3 := lntest.ChanPointFromPendingUpdate(update)
1392+
ht.AssertNumPendingOpenChannels(carol, 1)
1393+
ht.AssertNumPendingOpenChannels(dave, 1)
1394+
1395+
// We expect chanPoint3 to confirm.
1396+
ht.MineBlocksAndAssertNumTxes(1, 1)
1397+
1398+
// Force Close the channel and test the opening flow without preselected
1399+
// utxos.
1400+
// Before we tested the channel funding with a selected coin, now we
1401+
// want to make sure that our internal coin selection also adheres to
1402+
// the restictions of unstable utxos.
1403+
// We create the unconfirmed sweeper originating utxo just like before
1404+
// by force-closing a channel from dave's side.
1405+
ht.CloseChannelAssertPending(dave, chanPoint3, true)
1406+
ht.MineBlocksAndAssertNumTxes(1, 1)
1407+
1408+
// Mine one block to trigger the sweep transaction.
1409+
ht.MineEmptyBlocks(1)
1410+
1411+
// Wait for the to_remote sweep tx to show up in carol's wallet.
1412+
ht.AssertNumUTXOsUnconfirmed(carol, 1)
1413+
1414+
// Calculate the maximum amount our wallet has for the channel funding
1415+
// so that we will use all utxos.
1416+
carolBalance := carol.RPC.WalletBalance()
1417+
1418+
// Now calculate the fee for the channel opening transaction. We don't
1419+
// have to keep a channel reserve because we are using STATIC_REMOTE_KEY
1420+
// channels.
1421+
// NOTE: The TotalBalance includes the unconfirmed balance as well.
1422+
chanSize = btcutil.Amount(carolBalance.TotalBalance) -
1423+
fundingFee(2, false)
1424+
1425+
// We are trying to open a channel with the maximum amount and expect it
1426+
// to fail because one of the utxos cannot be used because it is
1427+
// unstable.
1428+
expectedErr = fmt.Errorf("not enough witness outputs to create " +
1429+
"funding transaction")
1430+
1431+
ht.OpenChannelAssertErr(carol, dave,
1432+
lntest.OpenChannelParams{
1433+
Amt: chanSize,
1434+
SpendUnconfirmed: true,
1435+
CommitmentType: cType,
1436+
}, expectedErr)
1437+
1438+
// Confirm the to_remote sweep utxo.
1439+
ht.MineBlocksAndAssertNumTxes(1, 1)
1440+
1441+
ht.AssertNumUTXOsConfirmed(carol, 2)
1442+
1443+
// Now after the sweep utxo is confirmed it is stable and can be used
1444+
// for channel openings again.
1445+
update = ht.OpenChannelAssertPending(carol, dave,
1446+
lntest.OpenChannelParams{
1447+
Amt: chanSize,
1448+
SpendUnconfirmed: true,
1449+
CommitmentType: cType,
1450+
})
1451+
chanPoint4 := lntest.ChanPointFromPendingUpdate(update)
1452+
1453+
// Verify that both nodes know about the channel.
1454+
ht.AssertNumPendingOpenChannels(carol, 1)
1455+
ht.AssertNumPendingOpenChannels(dave, 1)
1456+
1457+
ht.MineBlocksAndAssertNumTxes(1, 1)
1458+
1459+
ht.CloseChannel(carol, chanPoint1)
1460+
ht.CloseChannel(carol, chanPoint4)
1461+
}

0 commit comments

Comments
 (0)