diff --git a/sweep/tx_input_set.go b/sweep/tx_input_set.go index 59a587fc..b05b5ab0 100644 --- a/sweep/tx_input_set.go +++ b/sweep/tx_input_set.go @@ -31,15 +31,22 @@ const ( ) type txInputSetState struct { - // weightEstimate is the (worst case) tx weight with the current set of - // inputs. - weightEstimate *weightEstimator + // feeRate is the fee rate to use for the sweep transaction. + feeRate chainfee.SatPerKWeight // inputTotal is the total value of all inputs. inputTotal btcutil.Amount - // outputValue is the value of the tx output. - outputValue btcutil.Amount + // requiredOutput is the sum of the outputs committed to by the inputs. + requiredOutput btcutil.Amount + + // changeOutput is the value of the change output. This will be what is + // left over after subtracting the requiredOutput and the tx fee from + // the inputTotal. + // + // NOTE: This might be below the dust limit, or even negative since it + // is the change remaining in csse we pay the fee for a change output. + changeOutput btcutil.Amount // inputs is the set of tx inputs. inputs []input.Input @@ -52,11 +59,42 @@ type txInputSetState struct { force bool } +// weightEstimate is the (worst case) tx weight with the current set of +// inputs. It takes a parameter whether to add a change output or not. +func (t *txInputSetState) weightEstimate(change bool) *weightEstimator { + weightEstimate := newWeightEstimator(t.feeRate) + for _, i := range t.inputs { + // Can ignore error, because it has already been checked when + // calculating the yields. + _ = weightEstimate.add(i) + + r := i.RequiredTxOut() + if r != nil { + weightEstimate.addOutput(r) + } + } + + // Add a change output to the weight estimate if requested. + if change { + weightEstimate.addP2WKHOutput() + } + + return weightEstimate +} + +// totalOutput is the total amount left for us after paying fees. +// +// NOTE: This might be dust. +func (t *txInputSetState) totalOutput() btcutil.Amount { + return t.requiredOutput + t.changeOutput +} + func (t *txInputSetState) clone() txInputSetState { s := txInputSetState{ - weightEstimate: t.weightEstimate.clone(), + feeRate: t.feeRate, inputTotal: t.inputTotal, - outputValue: t.outputValue, + changeOutput: t.changeOutput, + requiredOutput: t.requiredOutput, walletInputTotal: t.walletInputTotal, force: t.force, inputs: make([]input.Input, len(t.inputs)), @@ -97,7 +135,7 @@ func newTxInputSet(wallet Wallet, feePerKW, dustLimit := dustLimit(relayFee) state := txInputSetState{ - weightEstimate: newWeightEstimator(feePerKW), + feeRate: feePerKW, } b := txInputSet{ @@ -107,16 +145,36 @@ func newTxInputSet(wallet Wallet, feePerKW, txInputSetState: state, } - // Add the sweep tx output to the weight estimate. - b.weightEstimate.addP2WKHOutput() - return &b } -// dustLimitReached returns true if we've accumulated enough inputs to meet the -// dust limit. -func (t *txInputSet) dustLimitReached() bool { - return t.outputValue >= t.dustLimit +// enoughInput returns true if we've accumulated enough inputs to pay the fees +// and have at least one output that meets the dust limit. +func (t *txInputSet) enoughInput() bool { + // If we have a change output above dust, then we certainly have enough + // inputs to the transaction. + if t.changeOutput >= t.dustLimit { + return true + } + + // We did not have enough input for a change output. Check if we have + // enough input to pay the fees for a transaction with no change + // output. + fee := t.weightEstimate(false).fee() + if t.inputTotal < t.requiredOutput+fee { + return false + } + + // We could pay the fees, but we still need at least one output to be + // above the dust limit for the tx to be valid (we assume that these + // required outputs only get added if they are above dust) + for _, inp := range t.inputs { + if inp.RequiredTxOut() != nil { + return true + } + } + + return false } // add adds a new input to the set. It returns a bool indicating whether the @@ -131,28 +189,35 @@ func (t *txInputSet) addToState(inp input.Input, constraints addConstraints) *tx return nil } + // If the input comes with a required tx out that is below dust, we + // won't add it. + reqOut := inp.RequiredTxOut() + if reqOut != nil && btcutil.Amount(reqOut.Value) < t.dustLimit { + return nil + } + // Clone the current set state. s := t.clone() // Add the new input. s.inputs = append(s.inputs, inp) - // Can ignore error, because it has already been checked when - // calculating the yields. - _ = s.weightEstimate.add(inp) - // Add the value of the new input. value := btcutil.Amount(inp.SignDesc().Output.Value) s.inputTotal += value // Recalculate the tx fee. - fee := s.weightEstimate.fee() + fee := s.weightEstimate(true).fee() // Calculate the new output value. - s.outputValue = s.inputTotal - fee + if reqOut != nil { + s.requiredOutput += btcutil.Amount(reqOut.Value) + } + s.changeOutput = s.inputTotal - s.requiredOutput - fee - // Calculate the yield of this input from the change in tx output value. - inputYield := s.outputValue - t.outputValue + // Calculate the yield of this input from the change in total tx output + // value. + inputYield := s.totalOutput() - t.totalOutput() switch constraints { @@ -192,11 +257,11 @@ func (t *txInputSet) addToState(inp input.Input, constraints addConstraints) *tx // 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 !s.force && s.walletInputTotal >= s.outputValue { + if !s.force && s.walletInputTotal >= s.totalOutput() { log.Debugf("Rejecting wallet input of %v, because it "+ "would make a negative yielding transaction "+ "(%v)", - value, s.outputValue-s.walletInputTotal) + value, s.totalOutput()-s.walletInputTotal) return nil } @@ -250,8 +315,9 @@ func (t *txInputSet) addPositiveYieldInputs(sweepableInputs []txInput) { // 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() { + // If we've already have enough to pay the transaction fees and have at + // least one output materialize, no action is needed. + if t.enoughInput() { return nil } @@ -275,7 +341,7 @@ func (t *txInputSet) tryAddWalletInputsIfNeeded() error { } // Return if we've reached the minimum output amount. - if t.dustLimitReached() { + if t.enoughInput() { return nil } } diff --git a/sweep/tx_input_set_test.go b/sweep/tx_input_set_test.go index d9e98f73..2f72b367 100644 --- a/sweep/tx_input_set_test.go +++ b/sweep/tx_input_set_test.go @@ -3,9 +3,11 @@ package sweep import ( "testing" + "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet" + "github.com/stretchr/testify/require" ) // TestTxInputSet tests adding various sized inputs to the set. @@ -34,12 +36,15 @@ func TestTxInputSet(t *testing.T) { t.Fatal("expected add of positively yielding input to succeed") } + fee := set.weightEstimate(true).fee() + require.Equal(t, btcutil.Amount(439), fee) + // The tx output should now be 700-439 = 261 sats. The dust limit isn't // reached yet. - if set.outputValue != 261 { + if set.totalOutput() != 261 { t.Fatal("unexpected output value") } - if set.dustLimitReached() { + if set.enoughInput() { t.Fatal("expected dust limit not yet to be reached") } @@ -48,10 +53,10 @@ func TestTxInputSet(t *testing.T) { if !set.add(createP2WKHInput(1000), constraintsRegular) { t.Fatal("expected add of positively yielding input to succeed") } - if set.outputValue != 988 { + if set.totalOutput() != 988 { t.Fatal("unexpected output value") } - if !set.dustLimitReached() { + if !set.enoughInput() { t.Fatal("expected dust limit to be reached") } } @@ -73,7 +78,7 @@ func TestTxInputSetFromWallet(t *testing.T) { if !set.add(createP2WKHInput(700), constraintsRegular) { t.Fatal("expected add of positively yielding input to succeed") } - if set.dustLimitReached() { + if set.enoughInput() { t.Fatal("expected dust limit not yet to be reached") } @@ -92,7 +97,7 @@ func TestTxInputSetFromWallet(t *testing.T) { t.Fatal(err) } - if !set.dustLimitReached() { + if !set.enoughInput() { t.Fatal("expected dust limit to be reached") } } @@ -117,3 +122,129 @@ func (m *mockWallet) ListUnspentWitness(minconfirms, maxconfirms int32) ( }, }, nil } + +type reqInput struct { + input.Input + + txOut *wire.TxOut +} + +func (r *reqInput) RequiredTxOut() *wire.TxOut { + return r.txOut +} + +// TestTxInputSetRequiredOutput tests that the tx input set behaves as expected +// when we add inputs that have required tx outs. +func TestTxInputSetRequiredOutput(t *testing.T) { + const ( + feeRate = 1000 + relayFee = 300 + maxInputs = 10 + ) + set := newTxInputSet(nil, feeRate, relayFee, maxInputs) + if set.dustLimit != 537 { + t.Fatalf("incorrect dust limit") + } + + // Attempt to add an input with a required txout below the dust limit. + // This should fail since we cannot trim such outputs. + inp := &reqInput{ + Input: createP2WKHInput(500), + txOut: &wire.TxOut{ + Value: 500, + PkScript: make([]byte, 33), + }, + } + require.False(t, set.add(inp, constraintsRegular), + "expected adding dust required tx out to fail") + + // Create a 1000 sat input that also has a required TxOut of 1000 sat. + // The fee to sweep this input to a P2WKH output is 439 sats. + inp = &reqInput{ + Input: createP2WKHInput(1000), + txOut: &wire.TxOut{ + Value: 1000, + PkScript: make([]byte, 22), + }, + } + require.True(t, set.add(inp, constraintsRegular), "failed adding input") + + // The fee needed to pay for this input and output should be 439 sats. + fee := set.weightEstimate(false).fee() + require.Equal(t, btcutil.Amount(439), fee) + + // Since the tx set currently pays no fees, we expect the current + // change to actually be negative, since this is what it would cost us + // in fees to add a change output. + feeWithChange := set.weightEstimate(true).fee() + if set.changeOutput != -feeWithChange { + t.Fatalf("expected negative change of %v, had %v", + -feeWithChange, set.changeOutput) + } + + // This should also be reflected by not having enough input. + require.False(t, set.enoughInput()) + + // Get a weight estimate without change output, and add an additional + // input to it. + dummyInput := createP2WKHInput(1000) + weight := set.weightEstimate(false) + require.NoError(t, weight.add(dummyInput)) + + // Now we add a an input that is large enough to pay the fee for the + // transaction without a change output, but not large enough to afford + // adding a change output. + extraInput1 := weight.fee() + 100 + require.True(t, set.add(createP2WKHInput(extraInput1), constraintsRegular), + "expected add of positively yielding input to succeed") + + // The change should be negative, since we would have to add a change + // output, which we cannot yet afford. + if set.changeOutput >= 0 { + t.Fatal("expected change to be negaitve") + } + + // Even though we cannot afford a change output, the tx set is valid, + // since we can pay the fees without the change output. + require.True(t, set.enoughInput()) + + // Get another weight estimate, this time with a change output, and + // figure out how much we must add to afford a change output. + weight = set.weightEstimate(true) + require.NoError(t, weight.add(dummyInput)) + + // We add what is left to reach this value. + extraInput2 := weight.fee() - extraInput1 + 100 + + // Add this input, which should result in the change now being 100 sats. + require.True(t, set.add(createP2WKHInput(extraInput2), constraintsRegular)) + + // The change should be 100, since this is what is left after paying + // fees in case of a change output. + change := set.changeOutput + if change != 100 { + t.Fatalf("expected change be 100, was %v", change) + } + + // Even though the change output is dust, we have enough for fees, and + // we have an output, so it should be considered enough to craft a + // valid sweep transaction. + require.True(t, set.enoughInput()) + + // Finally we add an input that should push the change output above the + // dust limit. + weight = set.weightEstimate(true) + require.NoError(t, weight.add(dummyInput)) + + // We expect the change to everything that is left after paying the tx + // fee. + extraInput3 := weight.fee() - extraInput1 - extraInput2 + 1000 + require.True(t, set.add(createP2WKHInput(extraInput3), constraintsRegular)) + + change = set.changeOutput + if change != 1000 { + t.Fatalf("expected change to be %v, had %v", 1000, change) + + } + require.True(t, set.enoughInput()) +} diff --git a/sweep/txgenerator.go b/sweep/txgenerator.go index 3f456b29..784a7ebd 100644 --- a/sweep/txgenerator.go +++ b/sweep/txgenerator.go @@ -110,17 +110,19 @@ func generateInputPartitionings(sweepableInputs []txInput, // the dust limit, stop sweeping. Because of the sorting, // continuing with the remaining inputs will only lead to sets // with an even lower output value. - if !txInputs.dustLimitReached() { - log.Debugf("Set value %v below dust limit of %v", - txInputs.outputValue, txInputs.dustLimit) + if !txInputs.enoughInput() { + log.Debugf("Set value %v (r=%v, c=%v) below dust "+ + "limit of %v", txInputs.totalOutput(), + txInputs.requiredOutput, txInputs.changeOutput, + txInputs.dustLimit) return sets, nil } 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()) + txInputs.totalOutput()-txInputs.walletInputTotal, + txInputs.weightEstimate(true).weight()) sets = append(sets, txInputs.inputs) sweepableInputs = sweepableInputs[inputCount:] diff --git a/sweep/weight_estimator.go b/sweep/weight_estimator.go index 693c3f25..011094fe 100644 --- a/sweep/weight_estimator.go +++ b/sweep/weight_estimator.go @@ -26,22 +26,6 @@ func newWeightEstimator(feeRate chainfee.SatPerKWeight) *weightEstimator { } } -// clone returns a copy of this weight estimator. -func (w *weightEstimator) clone() *weightEstimator { - parents := make(map[chainhash.Hash]struct{}, len(w.parents)) - for hash := range w.parents { - parents[hash] = struct{}{} - } - - return &weightEstimator{ - estimator: w.estimator, - feeRate: w.feeRate, - parents: parents, - parentsFee: w.parentsFee, - parentsWeight: w.parentsWeight, - } -} - // add adds the weight of the given input to the weight estimate. func (w *weightEstimator) add(inp input.Input) error { // If there is a parent tx, add the parent's fee and weight.