sweep/tx_input_set: account for required outputs in input set

If inputs require outputs to be added at the same time, this will
change the weight and amount calculations, so we must account for that.

We wait to get the weight estimator for the sweep tx until needed,
such that we can easily choose whether to include a change output or not
in the estimate. This is needed for the case where the second level
transactions can pay for their own fee, so no change output is needed.

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
 		}
 	}

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())
+}

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:]

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.