diff --git a/sweep/backend_mock_test.go b/sweep/backend_mock_test.go index 88132192..644ba59c 100644 --- a/sweep/backend_mock_test.go +++ b/sweep/backend_mock_test.go @@ -25,6 +25,8 @@ type mockBackend struct { unconfirmedSpendInputs map[wire.OutPoint]struct{} publishChan chan wire.MsgTx + + walletUtxos []*lnwallet.Utxo } func newMockBackend(t *testing.T, notifier *MockNotifier) *mockBackend { @@ -84,6 +86,16 @@ func (b *mockBackend) PublishTransaction(tx *wire.MsgTx) error { return err } +func (b *mockBackend) ListUnspentWitness(minconfirms, maxconfirms int32) ( + []*lnwallet.Utxo, error) { + + return b.walletUtxos, nil +} + +func (b *mockBackend) WithCoinSelectLock(f func() error) error { + return f() +} + func (b *mockBackend) deleteUnconfirmed(txHash chainhash.Hash) { b.lock.Lock() defer b.lock.Unlock() diff --git a/sweep/interface.go b/sweep/interface.go index fa35288e..948d8ddf 100644 --- a/sweep/interface.go +++ b/sweep/interface.go @@ -2,6 +2,7 @@ package sweep import ( "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/lnwallet" ) // Wallet contains all wallet related functionality required by sweeper. @@ -9,4 +10,18 @@ type Wallet interface { // PublishTransaction performs cursory validation (dust checks, etc) and // broadcasts the passed transaction to the Bitcoin network. PublishTransaction(tx *wire.MsgTx) error + + // ListUnspentWitness returns all unspent outputs which are version 0 + // witness programs. The 'minconfirms' and 'maxconfirms' parameters + // indicate the minimum and maximum number of confirmations an output + // needs in order to be returned by this method. + ListUnspentWitness(minconfirms, maxconfirms int32) ([]*lnwallet.Utxo, + error) + + // WithCoinSelectLock will execute the passed function closure in a + // synchronized manner preventing any coin selection operations from + // proceeding while the closure if executing. This can be seen as the + // ability to execute a function closure under an exclusive coin + // selection lock. + WithCoinSelectLock(f func() error) error } diff --git a/sweep/sweeper.go b/sweep/sweeper.go index a62447f1..9114cf8a 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -632,21 +632,27 @@ func (s *UtxoSweeper) collector(blockEpochs <-chan *chainntnfs.BlockEpoch) { func (s *UtxoSweeper) sweepCluster(cluster inputCluster, currentHeight int32) error { - // Examine pending inputs and try to construct lists of inputs. - inputLists, err := s.getInputLists(cluster, currentHeight) - if err != nil { - return fmt.Errorf("unable to examine pending inputs: %v", err) - } - - // Sweep selected inputs. - for _, inputs := range inputLists { - err := s.sweep(inputs, cluster.sweepFeeRate, currentHeight) + // Execute the sweep within a coin select lock. Otherwise the coins that + // we are going to spend may be selected for other transactions like + // funding of a channel. + return s.cfg.Wallet.WithCoinSelectLock(func() error { + // Examine pending inputs and try to construct + // lists of inputs. + inputLists, err := s.getInputLists(cluster, currentHeight) if err != nil { - return fmt.Errorf("unable to sweep inputs: %v", err) + return fmt.Errorf("unable to examine pending inputs: %v", err) } - } - return nil + // Sweep selected inputs. + for _, inputs := range inputLists { + err := s.sweep(inputs, cluster.sweepFeeRate, currentHeight) + if err != nil { + return fmt.Errorf("unable to sweep inputs: %v", err) + } + } + + return nil + }) } // bucketForFeeReate determines the proper bucket for a fee rate. This is done @@ -718,6 +724,10 @@ func (s *UtxoSweeper) scheduleSweep(currentHeight int32) error { startTimer := false for _, cluster := range s.clusterBySweepFeeRate() { // Examine pending inputs and try to construct lists of inputs. + // We don't need to obtain the coin selection lock, because we + // just need an indication as to whether we can sweep. More + // inputs may be added until we publish the transaction and + // coins that we select now may be used in other transactions. inputLists, err := s.getInputLists(cluster, currentHeight) if err != nil { return fmt.Errorf("get input lists: %v", err) @@ -823,6 +833,7 @@ func (s *UtxoSweeper) getInputLists(cluster inputCluster, allSets, err = generateInputPartitionings( append(retryInputs, newInputs...), s.relayFeeRate, cluster.sweepFeeRate, s.cfg.MaxInputsPerTx, + s.cfg.Wallet, ) if err != nil { return nil, fmt.Errorf("input partitionings: %v", err) @@ -832,7 +843,7 @@ func (s *UtxoSweeper) getInputLists(cluster inputCluster, // Create sets for just the new inputs. newSets, err := generateInputPartitionings( newInputs, s.relayFeeRate, cluster.sweepFeeRate, - s.cfg.MaxInputsPerTx, + s.cfg.MaxInputsPerTx, s.cfg.Wallet, ) if err != nil { return nil, fmt.Errorf("input partitionings: %v", err) @@ -908,7 +919,9 @@ func (s *UtxoSweeper) sweep(inputs inputSet, feeRate chainfee.SatPerKWeight, if !ok { // It can be that the input has been removed because it // exceed the maximum number of attempts in a previous - // input set. + // input set. It could also be that this input is an + // additional wallet input that was attached. In that + // case there also isn't a pending input to update. continue } diff --git a/sweep/sweeper_test.go b/sweep/sweeper_test.go index 9e5aa5f4..b63129c9 100644 --- a/sweep/sweeper_test.go +++ b/sweep/sweeper_test.go @@ -99,6 +99,12 @@ func createSweeperTestContext(t *testing.T) *sweeperTestContext { store := NewMockSweeperStore() backend := newMockBackend(t, notifier) + backend.walletUtxos = []*lnwallet.Utxo{ + { + Value: btcutil.Amount(10000), + AddressType: lnwallet.WitnessPubKey, + }, + } estimator := newMockFeeEstimator(10000, chainfee.FeePerKwFloor) @@ -407,7 +413,10 @@ func TestDust(t *testing.T) { } // No sweep transaction is expected now. The sweeper should recognize - // that the sweep output will not be relayed and not generate the tx. + // that the sweep output will not be relayed and not generate the tx. It + // isn't possible to attach a wallet utxo either, because the added + // weight would create a negatively yielding transaction at this fee + // rate. // Sweep another input that brings the tx output above the dust limit. largeInput := createTestInput(100000, input.CommitmentTimeLock) @@ -433,6 +442,50 @@ func TestDust(t *testing.T) { ctx.finish(1) } +// TestWalletUtxo asserts that inputs that are not big enough to raise above the +// dust limit are accompanied by a wallet utxo to make them sweepable. +func TestWalletUtxo(t *testing.T) { + ctx := createSweeperTestContext(t) + + // Sweeping a single output produces a tx of 439 weight units. At the + // fee floor, the sweep tx will pay 439*253/1000 = 111 sat in fees. + // + // Create an input so that the output after paying fees is still + // positive (183 sat), but less than the dust limit (537 sat) for the + // sweep tx output script (P2WPKH). + // + // What we now expect is that the sweeper will attach a utxo from the + // wallet. This increases the tx weight to 712 units with a fee of 180 + // sats. The tx yield becomes then 294-180 = 114 sats. + dustInput := createTestInput(294, input.WitnessKeyHash) + + _, err := ctx.sweeper.SweepInput( + &dustInput, + Params{Fee: FeePreference{FeeRate: chainfee.FeePerKwFloor}}, + ) + if err != nil { + t.Fatal(err) + } + + ctx.tick() + + sweepTx := ctx.receiveTx() + if len(sweepTx.TxIn) != 2 { + t.Fatalf("Expected tx to sweep 2 inputs, but contains %v "+ + "inputs instead", len(sweepTx.TxIn)) + } + + // Calculate expected output value based on wallet utxo of 10000 sats. + expectedOutputValue := int64(294 + 10000 - 180) + if sweepTx.TxOut[0].Value != expectedOutputValue { + t.Fatalf("Expected output value of %v, but got %v", + expectedOutputValue, sweepTx.TxOut[0].Value) + } + + ctx.backend.mine() + ctx.finish(1) +} + // TestNegativeInput asserts that no inputs with a negative yield are swept. // Negative yield means that the value minus the added fee is negative. func TestNegativeInput(t *testing.T) { diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 97eb94f5..a00ac6ca 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -1,9 +1,15 @@ package sweep import ( + "fmt" + "math" + + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/btcsuite/btcwallet/wallet/txrules" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" ) @@ -32,11 +38,18 @@ type txInputSet struct { // maxInputs is the maximum number of inputs that will be accepted in // the set. maxInputs int + + // walletInputTotal is the total value of inputs coming from the wallet. + walletInputTotal btcutil.Amount + + // wallet contains wallet functionality required by the input set to + // retrieve utxos. + wallet Wallet } // newTxInputSet constructs a new, empty input set. -func newTxInputSet(feePerKW, relayFee chainfee.SatPerKWeight, - maxInputs int) *txInputSet { +func newTxInputSet(wallet Wallet, feePerKW, + relayFee chainfee.SatPerKWeight, maxInputs int) *txInputSet { dustLimit := txrules.GetDustThreshold( input.P2WPKHSize, @@ -47,6 +60,7 @@ func newTxInputSet(feePerKW, relayFee chainfee.SatPerKWeight, feePerKW: feePerKW, dustLimit: dustLimit, maxInputs: maxInputs, + wallet: wallet, } // Add the sweep tx output to the weight estimate. @@ -64,9 +78,10 @@ func (t *txInputSet) dustLimitReached() bool { // add adds a new input to the set. It returns a bool indicating whether the // input was added to the set. An input is rejected if it decreases the tx // output value after paying fees. -func (t *txInputSet) add(input input.Input) bool { - // Stop if max inputs is reached. - if len(t.inputs) == t.maxInputs { +func (t *txInputSet) add(input input.Input, fromWallet bool) bool { + // Stop if max inputs is reached. Do not count additional wallet inputs, + // because we don't know in advance how many we may need. + if !fromWallet && len(t.inputs) >= t.maxInputs { return false } @@ -100,6 +115,39 @@ func (t *txInputSet) add(input input.Input) bool { return false } + // If this input comes from the wallet, verify that we still gain + // something with this transaction. + if fromWallet { + // Calculate the total value that we spend in this tx from the + // wallet if we'd add this wallet input. + newWalletTotal := t.walletInputTotal + value + + // In any case, we don't want to lose money by sweeping. If we + // don't get more out of the tx then we put in ourselves, do not + // add this wallet input. + // + // We should only add wallet inputs to get the tx output value + // above the dust limit, otherwise we'd only burn into fees. + // This is guarded by tryAddWalletInputsIfNeeded. + // + // TODO(joostjager): Possibly require a max ratio between the + // value of the wallet input and what we get out of this + // transaction. To prevent attaching and locking a big utxo for + // very little benefit. + if newWalletTotal >= newOutputValue { + log.Debugf("Rejecting wallet input of %v, because it "+ + "would make a negative yielding transaction "+ + "(%v)", + value, newOutputValue-newWalletTotal) + + return false + } + + // We've decided to add the wallet input. Increment the total + // wallet funds that go into this tx. + t.walletInputTotal = newWalletTotal + } + // Update running values. t.inputTotal = newInputTotal t.outputValue = newOutputValue @@ -123,10 +171,78 @@ func (t *txInputSet) addPositiveYieldInputs(sweepableInputs []txInput) { // succeed because it wouldn't increase the output value, // return. Assuming inputs are sorted by yield, any further // inputs wouldn't increase the output value either. - if !t.add(input) { + if !t.add(input, false) { return } } // We managed to add all inputs to the set. } + +// tryAddWalletInputsIfNeeded retrieves utxos from the wallet and tries adding as +// many as required to bring the tx output value above the given minimum. +func (t *txInputSet) tryAddWalletInputsIfNeeded() error { + // If we've already reached the dust limit, no action is needed. + if t.dustLimitReached() { + return nil + } + + // Retrieve wallet utxos. Only consider confirmed utxos to prevent + // problems around RBF rules for unconfirmed inputs. + utxos, err := t.wallet.ListUnspentWitness(1, math.MaxInt32) + if err != nil { + return err + } + + for _, utxo := range utxos { + input, err := createWalletTxInput(utxo) + if err != nil { + return err + } + + // If the wallet input isn't positively-yielding at this fee + // rate, skip it. + if !t.add(input, true) { + continue + } + + // Return if we've reached the minimum output amount. + if t.dustLimitReached() { + return nil + } + } + + // We were not able to reach the minimum output amount. + return nil +} + +// createWalletTxInput converts a wallet utxo into an object that can be added +// to the other inputs to sweep. +func createWalletTxInput(utxo *lnwallet.Utxo) (input.Input, error) { + var witnessType input.WitnessType + switch utxo.AddressType { + case lnwallet.WitnessPubKey: + witnessType = input.WitnessKeyHash + case lnwallet.NestedWitnessPubKey: + witnessType = input.NestedWitnessKeyHash + default: + return nil, fmt.Errorf("unknown address type %v", + utxo.AddressType) + } + + signDesc := &input.SignDescriptor{ + Output: &wire.TxOut{ + PkScript: utxo.PkScript, + Value: int64(utxo.Value), + }, + HashType: txscript.SigHashAll, + } + + // A height hint doesn't need to be set, because we don't monitor these + // inputs for spend. + heightHint := uint32(0) + + return input.NewBaseInput( + &utxo.OutPoint, witnessType, signDesc, heightHint, + ), nil +} diff --git a/sweep/tx_input_set_test.go b/sweep/tx_input_set_test.go index 557b5cb4..2fa50188 100644 --- a/sweep/tx_input_set_test.go +++ b/sweep/tx_input_set_test.go @@ -5,6 +5,7 @@ import ( "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet" ) // TestTxInputSet tests adding various sized inputs to the set. @@ -14,7 +15,7 @@ func TestTxInputSet(t *testing.T) { relayFee = 300 maxInputs = 10 ) - set := newTxInputSet(feeRate, relayFee, maxInputs) + set := newTxInputSet(nil, feeRate, relayFee, maxInputs) if set.dustLimit != 537 { t.Fatalf("incorrect dust limit") @@ -23,13 +24,13 @@ func TestTxInputSet(t *testing.T) { // Create a 300 sat input. The fee to sweep this input to a P2WKH output // is 439 sats. That means that this input yields -139 sats and we // expect it not to be added. - if set.add(createP2WKHInput(300)) { + if set.add(createP2WKHInput(300), false) { t.Fatal("expected add of negatively yielding input to fail") } // A 700 sat input should be accepted into the set, because it yields // positively. - if !set.add(createP2WKHInput(700)) { + if !set.add(createP2WKHInput(700), false) { t.Fatal("expected add of positively yielding input to succeed") } @@ -44,7 +45,7 @@ func TestTxInputSet(t *testing.T) { // Add a 1000 sat input. This increases the tx fee to 712 sats. The tx // output should now be 1000+700 - 712 = 988 sats. - if !set.add(createP2WKHInput(1000)) { + if !set.add(createP2WKHInput(1000), false) { t.Fatal("expected add of positively yielding input to succeed") } if set.outputValue != 988 { @@ -55,8 +56,54 @@ func TestTxInputSet(t *testing.T) { } } +// TestTxInputSetFromWallet tests adding a wallet input to a TxInputSet to reach +// the dust limit. +func TestTxInputSetFromWallet(t *testing.T) { + const ( + feeRate = 500 + relayFee = 300 + maxInputs = 10 + ) + + wallet := &mockWallet{} + set := newTxInputSet(wallet, feeRate, relayFee, maxInputs) + + // Add a 700 sat input to the set. It yields positively, but doesn't + // reach the output dust limit. + if !set.add(createP2WKHInput(700), false) { + t.Fatal("expected add of positively yielding input to succeed") + } + if set.dustLimitReached() { + t.Fatal("expected dust limit not yet to be reached") + } + + err := set.tryAddWalletInputsIfNeeded() + if err != nil { + t.Fatal(err) + } + + if !set.dustLimitReached() { + t.Fatal("expected dust limit to be reached") + } +} + // createP2WKHInput returns a P2WKH test input with the specified amount. func createP2WKHInput(amt btcutil.Amount) input.Input { input := createTestInput(int64(amt), input.WitnessKeyHash) return &input } + +type mockWallet struct { + Wallet +} + +func (m *mockWallet) ListUnspentWitness(minconfirms, maxconfirms int32) ( + []*lnwallet.Utxo, error) { + + return []*lnwallet.Utxo{ + { + AddressType: lnwallet.WitnessPubKey, + Value: 10000, + }, + }, nil +} diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index 4ef549e8..47c8d3c6 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -38,7 +38,7 @@ type inputSet []input.Input // dust limit are returned. func generateInputPartitionings(sweepableInputs []txInput, relayFeePerKW, feePerKW chainfee.SatPerKWeight, - maxInputsPerTx int) ([]inputSet, error) { + maxInputsPerTx int, wallet Wallet) ([]inputSet, error) { // Sort input by yield. We will start constructing input sets starting // with the highest yield inputs. This is to prevent the construction @@ -78,7 +78,7 @@ func generateInputPartitionings(sweepableInputs []txInput, // condition that the tx will be published with the specified // fee rate. txInputs := newTxInputSet( - feePerKW, relayFeePerKW, maxInputsPerTx, + wallet, feePerKW, relayFeePerKW, maxInputsPerTx, ) // From the set of sweepable inputs, keep adding inputs to the @@ -92,18 +92,27 @@ func generateInputPartitionings(sweepableInputs []txInput, return sets, nil } + // Check the current output value and add wallet utxos if + // needed to push the output value to the lower limit. + if err := txInputs.tryAddWalletInputsIfNeeded(); err != nil { + return nil, err + } + // If the output value of this block of inputs does not reach // the dust limit, stop sweeping. Because of the sorting, // continuing with the remaining inputs will only lead to sets - // with a even lower output value. + // with an even lower output value. if !txInputs.dustLimitReached() { log.Debugf("Set value %v below dust limit of %v", txInputs.outputValue, txInputs.dustLimit) return sets, nil } - log.Infof("Candidate sweep set of size=%v, has yield=%v", - inputCount, txInputs.outputValue) + log.Infof("Candidate sweep set of size=%v (+%v wallet inputs), "+ + "has yield=%v, weight=%v", + inputCount, len(txInputs.inputs)-inputCount, + txInputs.outputValue-txInputs.walletInputTotal, + txInputs.weightEstimate.Weight()) sets = append(sets, txInputs.inputs) sweepableInputs = sweepableInputs[inputCount:] @@ -119,12 +128,12 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte, inputs, txWeight := getWeightEstimate(inputs) - log.Infof("Creating sweep transaction for %v inputs (%s) "+ - "using %v sat/kw", len(inputs), inputTypeSummary(inputs), - int64(feePerKw)) - txFee := feePerKw.FeeForWeight(txWeight) + log.Infof("Creating sweep transaction for %v inputs (%s) "+ + "using %v sat/kw, tx_fee=%v", len(inputs), + inputTypeSummary(inputs), int64(feePerKw), txFee) + // Sum up the total value contained in the inputs. var totalSum btcutil.Amount for _, o := range inputs {