sweepbatcher: consider change in presigning and batch tx

Presigning sweeps takes change outputs into account.
Each sweep belonging to the same sweep group points
to the same change output, if existent.
sweepbatcher.presign scans all passed sweeps for
change outputs and passes them to constructUnsignedTx.
Optional change of a swap is encoded in its sweeps
as a pointer to the same change output. This change
is taken into account when constructing the unsigned
batch transaction when it comes to tx weight and
outputs.

Author: hieblmi

sweepbatcher/greedy_batch_selection.go

@@ -210,6 +210,13 @@ func estimateBatchWeight(batch *batch) (feeDetails, error) {
 			err)
 	}
 
+	// Add change output weights.
+	for _, s := range batch.sweeps {
+		if s.change != nil {
+			weight.AddOutput(s.change.PkScript)
+		}
+	}
+
 	// Add inputs.
 	for _, sweep := range batch.sweeps {
 		if sweep.nonCoopHint || sweep.coopFailed {

sweepbatcher/presigned.go

@@ -51,6 +51,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep,
 			outpoint:  s.outpoint,
 			value:     s.value,
 			presigned: s.presigned,
+			change:    s.change,
 		}
 	}
 
@@ -493,10 +494,12 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 	signedFeeRate := chainfee.NewSatPerKWeight(fee, realWeight)
 
 	numSweeps := len(tx.TxIn)
+	numChange := len(tx.TxOut) - 1
 	b.Infof("attempting to publish custom signed tx=%v, desiredFeerate=%v,"+
-		" signedFeeRate=%v, weight=%v, fee=%v, sweeps=%d, destAddr=%s",
+		" signedFeeRate=%v, weight=%v, fee=%v, sweeps=%d, "+
+		"changeOutputs=%d, destAddr=%s",
 		txHash, feeRate, signedFeeRate, realWeight, fee, numSweeps,
-		address)
+		numChange, address)
 	b.debugLogTx("serialized batch", tx)
 
 	// Publish the transaction.
@@ -593,23 +596,31 @@ func CheckSignedTx(unsignedTx, signedTx *wire.MsgTx, inputAmt btcutil.Amount,
 	}
 
 	// Compare outputs.
-	if len(unsignedTx.TxOut) != 1 {
-		return fmt.Errorf("unsigned tx has %d outputs, want 1",
-			len(unsignedTx.TxOut))
-	}
-	if len(signedTx.TxOut) != 1 {
-		return fmt.Errorf("the signed tx has %d outputs, want 1",
+	if len(unsignedTx.TxOut) != len(signedTx.TxOut) {
+		return fmt.Errorf("unsigned tx has %d outputs, signed tx has "+
+			"%d outputs, should be equal", len(unsignedTx.TxOut),
 			len(signedTx.TxOut))
 	}
-	unsignedOut := unsignedTx.TxOut[0]
-	signedOut := signedTx.TxOut[0]
-	if !bytes.Equal(unsignedOut.PkScript, signedOut.PkScript) {
-		return fmt.Errorf("mismatch of output pkScript: %x, %x",
-			unsignedOut.PkScript, signedOut.PkScript)
+	for i, o := range unsignedTx.TxOut {
+		if !bytes.Equal(o.PkScript, signedTx.TxOut[i].PkScript) {
+			return fmt.Errorf("mismatch of output pkScript: %x, %x",
+				o.PkScript, signedTx.TxOut[i].PkScript)
+		}
+		if i != 0 && o.Value != signedTx.TxOut[i].Value {
+			return fmt.Errorf("mismatch of output value: %d, %d",
+				o.Value, signedTx.TxOut[i].Value)
+		}
+	}
+
+	// Calculate the total value of all outputs to help determine the
+	// transaction fee.
+	totalOutputValue := btcutil.Amount(0)
+	for _, o := range signedTx.TxOut {
+		totalOutputValue += btcutil.Amount(o.Value)
 	}
 
 	// Find the feerate of signedTx.
-	fee := inputAmt - btcutil.Amount(signedOut.Value)
+	fee := inputAmt - totalOutputValue
 	weight := lntypes.WeightUnit(
 		blockchain.GetTransactionWeight(btcutil.NewTx(signedTx)),
 	)

sweepbatcher/presigned_test.go

@@ -1460,7 +1460,8 @@ func TestCheckSignedTx(t *testing.T) {
 			},
 			inputAmt:    3_000_000,
 			minRelayFee: 253,
-			wantErr:     "unsigned tx has 2 outputs, want 1",
+			wantErr: "unsigned tx has 2 outputs, signed tx " +
+				"has 1 outputs, should be equal",
 		},
 
 		{
@@ -1517,7 +1518,153 @@ func TestCheckSignedTx(t *testing.T) {
 			},
 			inputAmt:    3_000_000,
 			minRelayFee: 253,
-			wantErr:     "the signed tx has 2 outputs, want 1",
+			wantErr: "unsigned tx has 1 outputs, signed tx " +
+				"has 2 outputs, should be equal",
+		},
+
+		{
+			name: "pkscript mismatch",
+			unsignedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    2999374,
+						PkScript: batchPkScript,
+					},
+				},
+				LockTime: 800_000,
+			},
+			signedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+						Witness: wire.TxWitness{
+							[]byte("test"),
+						},
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    2999374,
+						PkScript: []byte{0xaf, 0xfe}, // Just to make it different.
+					},
+				},
+				LockTime: 799_999,
+			},
+			inputAmt:    3_000_000,
+			minRelayFee: 253,
+			wantErr:     "mismatch of output pkScript",
+		},
+
+		{
+			name: "value mismatch, first output",
+			unsignedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    2999374,
+						PkScript: batchPkScript,
+					},
+				},
+				LockTime: 800_000,
+			},
+			signedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+						Witness: wire.TxWitness{
+							[]byte("test"),
+						},
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    1_337_000, // Just to make it different.
+						PkScript: batchPkScript,
+					},
+				},
+				LockTime: 799_999,
+			},
+			inputAmt:    3_000_000,
+			minRelayFee: 253,
+			wantErr:     "",
+		},
+
+		{
+			name: "value mismatch, change output",
+			unsignedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+					},
+					{
+						PreviousOutPoint: op1,
+						Sequence:         2,
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    2999374,
+						PkScript: batchPkScript,
+					},
+					{
+						Value:    1_337_000,
+						PkScript: batchPkScript,
+					},
+				},
+				LockTime: 800_000,
+			},
+			signedTx: &wire.MsgTx{
+				Version: 2,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op2,
+						Sequence:         2,
+						Witness: wire.TxWitness{
+							[]byte("test"),
+						},
+					},
+					{
+						PreviousOutPoint: op1,
+						Sequence:         2,
+						Witness: wire.TxWitness{
+							[]byte("test"),
+						},
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    2_493_300,
+						PkScript: batchPkScript,
+					},
+					{
+						Value:    1_338, // Just to make it different.
+						PkScript: batchPkScript,
+					},
+				},
+				LockTime: 799_999,
+			},
+			inputAmt:    3_000_000,
+			minRelayFee: 253,
+			wantErr:     "mismatch of output value",
 		},
 
 		{

sweepbatcher/sweep_batch.go

@@ -6,6 +6,8 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"github.com/btcsuite/btcd/mempool"
+	"github.com/lightningnetwork/lnd/lnwallet"
 	"math"
 	"strings"
 	"sync"
@@ -125,6 +127,9 @@ type sweep struct {
 
 	// presigned is set, if the sweep should be handled in presigned mode.
 	presigned bool
+
+	// change is the optional change output of the sweep.
+	change *wire.TxOut
 }
 
 // batchState is the state of the batch.
@@ -1236,10 +1241,14 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte,
 }
 
 // constructUnsignedTx creates unsigned tx from the sweeps, paying to the addr.
-// It also returns absolute fee (from weight and clamped).
+// It also returns absolute fee (from weight and clamped). The main output is
+// the first output of the transaction, followed by an optional list of change
+// outputs. If the main output value is below dust limit this function will
+// return an error.
 func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
-	currentHeight int32, feeRate chainfee.SatPerKWeight) (*wire.MsgTx,
-	lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, error) {
+	currentHeight int32, feeRate chainfee.SatPerKWeight) (
+	*wire.MsgTx, lntypes.WeightUnit, btcutil.Amount, btcutil.Amount,
+	error) {
 
 	// Sanity check, there should be at least 1 sweep in this batch.
 	if len(sweeps) == 0 {
@@ -1252,6 +1261,13 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 		LockTime: uint32(currentHeight),
 	}
 
+	var changeOutputs []*wire.TxOut
+	for _, sweep := range sweeps {
+		if sweep.change != nil {
+			changeOutputs = append(changeOutputs, sweep.change)
+		}
+	}
+
 	// Add transaction inputs and estimate its weight.
 	var weightEstimate input.TxWeightEstimator
 	for _, sweep := range sweeps {
@@ -1297,6 +1313,11 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 			"failed: %w", err)
 	}
 
+	// Add the optional change outputs to weight estimates.
+	for _, o := range changeOutputs {
+		weightEstimate.AddOutput(o.PkScript)
+	}
+
 	// Keep track of the total amount this batch is sweeping back.
 	batchAmt := btcutil.Amount(0)
 	for _, sweep := range sweeps {
@@ -1314,15 +1335,80 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 		feeForWeight++
 	}
 
+	// Add the batch transaction output, which excludes the fees paid to
+	// miners. Reduce the amount by the sum of change outputs, if any.
+	var sumChange int64
+	for _, change := range changeOutputs {
+		sumChange += change.Value
+	}
+
+	// Ensure that the batch amount is greater than the sum of change.
+	if batchAmt <= btcutil.Amount(sumChange) {
+		return nil, 0, 0, 0, fmt.Errorf("batch amount %v is less than "+
+			"the sum of change outputs %v", batchAmt,
+			btcutil.Amount(sumChange))
+	}
+
 	// Clamp the calculated fee to the max allowed fee amount for the batch.
-	fee := clampBatchFee(feeForWeight, batchAmt)
+	fee := clampBatchFee(feeForWeight, batchAmt-btcutil.Amount(sumChange))
 
-	// Add the batch transaction output, which excludes the fees paid to
-	// miners.
+	// Ensure that batch amount exceeds the sum of change outputs and the
+	// fee, and that it is also greater than dust limit for the main
+	// output.
+	if fee+btcutil.Amount(sumChange)+
+		lnwallet.DustLimitForSize(len(batchPkScript)) > batchAmt {
+
+		return nil, 0, 0, 0, fmt.Errorf("batch amount %v is less than "+
+			"the sum of change outputs %v and fee %v",
+			batchAmt, btcutil.Amount(sumChange), fee)
+	}
+
+	// Add the main output first.
 	batchTx.AddTxOut(&wire.TxOut{
 		PkScript: batchPkScript,
-		Value:    int64(batchAmt - fee),
+		Value:    int64(batchAmt-fee) - sumChange,
 	})
+	// Then add change outputs.
+	for _, txOut := range changeOutputs {
+		batchTx.AddTxOut(&wire.TxOut{
+			PkScript: txOut.PkScript,
+			Value:    txOut.Value,
+		})
+	}
+
+	// Check that for each swap, inputs exceed the change outputs.
+	if len(changeOutputs) != 0 {
+		swap2Inputs := make(map[lntypes.Hash]btcutil.Amount)
+		swap2Change := make(map[lntypes.Hash]btcutil.Amount)
+		for _, sweep := range sweeps {
+			swap2Inputs[sweep.swapHash] += sweep.value
+			if sweep.change != nil {
+				swap2Change[sweep.swapHash] +=
+					btcutil.Amount(sweep.change.Value)
+			}
+		}
+
+		for swapHash, inputs := range swap2Inputs {
+			change := swap2Change[swapHash]
+			if inputs <= change {
+				return nil, 0, 0, 0, fmt.Errorf(""+
+					"inputs %v <= change %v for swap %x",
+					inputs, change, swapHash[:6])
+			}
+		}
+	}
+
+	// Ensure that each output is above dust limit.
+	for _, txOut := range batchTx.TxOut {
+		// copied from lnwallet.DustLimitForSize
+		txout := &wire.TxOut{PkScript: txOut.PkScript}
+		dustlimit := btcutil.Amount(mempool.GetDustThreshold(txout))
+		if txOut.Value < int64(dustlimit) {
+			return nil, 0, 0, 0, fmt.Errorf("output %v is below "+
+				"dust limit %v", btcutil.Amount(txOut.Value),
+				dustlimit)
+		}
+	}
 
 	return batchTx, weight, feeForWeight, fee, nil
 }