sweepbatcher: consider change in presigning and batch tx

Presigning sweeps takes change outputs into account.
Each primary deposit id of a sweep group points to
an optional change output.
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/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

@@ -26,6 +26,7 @@ import (
 	"github.com/lightninglabs/loop/loopdb"
 	"github.com/lightninglabs/loop/swap"
 	sweeppkg "github.com/lightninglabs/loop/sweep"
+	"github.com/lightninglabs/loop/utils"
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/input"
@@ -1239,10 +1240,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 {
@@ -1255,6 +1260,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 {
@@ -1300,6 +1312,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 {
@@ -1317,15 +1334,78 @@ 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 <= 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.
+	dustLimit := utils.DustLimitForPkScript(batchPkScript)
+	if fee+btcutil.Amount(sumChange)+dustLimit > batchAmt {
+		return nil, 0, 0, 0, fmt.Errorf("batch amount %v is <= the "+
+			"sum of change outputs %v plus fee %v and dust "+
+			"limit %v", batchAmt, btcutil.Amount(sumChange),
+			fee, dustLimit)
+	}
+
+	// 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 {
+		dustLimit = utils.DustLimitForPkScript(txOut.PkScript)
+		if btcutil.Amount(txOut.Value) < 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
 }