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.AddP2TROutput()
+		}
+	}
+
 	// 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,
 		}
 	}
 
@@ -257,7 +258,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {
 
 		err = presign(
 			ctx, b.cfg.presignedHelper, destAddr, primarySweepID,
-			sweeps, nextBlockFeeRate,
+			sweeps, nextBlockFeeRate, b.cfg.chainParams,
 		)
 		if err != nil {
 			return fmt.Errorf("failed to presign a transaction "+
@@ -299,7 +300,8 @@ type presigner interface {
 // 10x of the current next block feerate.
 func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
 	primarySweepID wire.OutPoint, sweeps []sweep,
-	nextBlockFeeRate chainfee.SatPerKWeight) error {
+	nextBlockFeeRate chainfee.SatPerKWeight,
+	chainParams *chaincfg.Params) error {
 
 	if presigner == nil {
 		return fmt.Errorf("presigner is not installed")
@@ -493,10 +495,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 +597,30 @@ 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)
+		}
+	}
+
+	// The first output is always the batch output.
+	batchOutput := signedTx.TxOut[0]
+
+	// Calculate the total value of change outputs to help determine the
+	// transaction fee.
+	totalChangeValue := btcutil.Amount(0)
+	for i := 1; i < len(signedTx.TxOut); i++ {
+		totalChangeValue += btcutil.Amount(signedTx.TxOut[i].Value)
 	}
 
 	// Find the feerate of signedTx.
-	fee := inputAmt - btcutil.Amount(signedOut.Value)
+	fee := inputAmt - btcutil.Amount(batchOutput.Value) - totalChangeValue
 	weight := lntypes.WeightUnit(
 		blockchain.GetTransactionWeight(btcutil.NewTx(signedTx)),
 	)

sweepbatcher/presigned_test.go

@@ -1011,6 +1011,7 @@ func TestPresign(t *testing.T) {
 				ctx, tc.presigner, tc.destAddr,
 				tc.primarySweepID, tc.sweeps,
 				tc.nextBlockFeeRate,
+				&chaincfg.RegressionNetParams,
 			)
 			if tc.wantErr != "" {
 				require.Error(t, err)
@@ -1460,7 +1461,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 +1519,8 @@ 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",
 		},
 
 		{

sweepbatcher/sweep_batch.go

@@ -125,6 +125,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 +1239,13 @@ 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.
 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 +1258,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 +1310,11 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 			"failed: %w", err)
 	}
 
+	// Add the optional change outputs to weight estimates.
+	for _, _ = range changeOutputs {
+		weightEstimate.AddP2TROutput()
+	}
+
 	// Keep track of the total amount this batch is sweeping back.
 	batchAmt := btcutil.Amount(0)
 	for _, sweep := range sweeps {
@@ -1318,11 +1336,50 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 	fee := clampBatchFee(feeForWeight, batchAmt)
 
 	// Add the batch transaction output, which excludes the fees paid to
-	// miners.
-	batchTx.AddTxOut(&wire.TxOut{
-		PkScript: batchPkScript,
-		Value:    int64(batchAmt - fee),
-	})
+	// miners. Reduce the amount by the sum of change outputs, if any.
+	if len(changeOutputs) == 0 {
+		batchTx.AddTxOut(&wire.TxOut{
+			PkScript: batchPkScript,
+			Value:    int64(batchAmt - fee),
+		})
+	} else {
+		// Reduce the batch output by the sum of change outputs.
+		var sumChange int64
+		for _, change := range changeOutputs {
+			sumChange += change.Value
+		}
+		// Add the main output first.
+		batchTx.AddTxOut(&wire.TxOut{
+			PkScript: batchPkScript,
+			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.
+	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])
+		}
+	}
 
 	return batchTx, weight, feeForWeight, fee, nil
 }
@@ -1396,7 +1453,9 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error,
 			attempt)
 
 		// Construct unsigned batch transaction.
-		var err error
+		var (
+			err error
+		)
 		tx, weight, feeForWeight, fee, err = constructUnsignedTx(
 			sweeps, address, b.currentHeight, b.rbfCache.FeeRate,
 		)

sweepbatcher/sweep_batch_test.go

@@ -29,6 +29,10 @@ func TestConstructUnsignedTx(t *testing.T) {
 		Hash:  chainhash.Hash{2, 2, 2},
 		Index: 2,
 	}
+	op3 := wire.OutPoint{
+		Hash:  chainhash.Hash{3, 3, 3},
+		Index: 3,
+	}
 
 	batchPkScript, err := txscript.PayToAddrScript(destAddr)
 	require.NoError(t, err)
@@ -40,6 +44,15 @@ func TestConstructUnsignedTx(t *testing.T) {
 	p2trPkScript, err := txscript.PayToAddrScript(p2trAddress)
 	require.NoError(t, err)
 
+	change1 := &wire.TxOut{
+		Value:    100_000,
+		PkScript: p2trPkScript,
+	}
+	change2 := &wire.TxOut{
+		Value:    200_000,
+		PkScript: p2trPkScript,
+	}
+
 	serializedPubKey := []byte{
 		0x02, 0x19, 0x2d, 0x74, 0xd0, 0xcb, 0x94, 0x34, 0x4c, 0x95,
 		0x69, 0xc2, 0xe7, 0x79, 0x01, 0x57, 0x3d, 0x8d, 0x79, 0x03,
@@ -223,7 +236,7 @@ func TestConstructUnsignedTx(t *testing.T) {
 				},
 				TxOut: []*wire.TxOut{
 					{
-						Value:    2400000,
+						Value:    2_400_000,
 						PkScript: batchPkScript,
 					},
 				},
@@ -265,7 +278,7 @@ func TestConstructUnsignedTx(t *testing.T) {
 				},
 				TxOut: []*wire.TxOut{
 					{
-						Value:    2999211,
+						Value:    2_999_211,
 						PkScript: batchPkScript,
 					},
 				},
@@ -275,6 +288,97 @@ func TestConstructUnsignedTx(t *testing.T) {
 			wantFee:          789,
 		},
 
+		{
+			name: "all sweeps same change output",
+			sweeps: []sweep{
+				{
+					outpoint: op1,
+					value:    1_000_000,
+					change:   change1,
+				},
+			},
+			address:       p2trAddress,
+			currentHeight: 800_000,
+			feeRate:       1000,
+			wantTx: &wire.MsgTx{
+				Version:  2,
+				LockTime: 800_000,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op1,
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    899_384,
+						PkScript: p2trPkScript,
+					},
+					{
+						Value:    change1.Value,
+						PkScript: change1.PkScript,
+					},
+				},
+			},
+			wantWeight:       616,
+			wantFeeForWeight: 616,
+			wantFee:          616,
+		},
+
+		{
+			name: "all sweeps different change outputs",
+			sweeps: []sweep{
+				{
+					outpoint: op1,
+					value:    1_000_000,
+				},
+				{
+					outpoint: op2,
+					value:    2_000_000,
+					change:   change1,
+				},
+				{
+					outpoint: op3,
+					value:    3_000_000,
+					change:   change2,
+				},
+			},
+			address:       p2trAddress,
+			currentHeight: 800_000,
+			feeRate:       1000,
+			wantTx: &wire.MsgTx{
+				Version:  2,
+				LockTime: 800_000,
+				TxIn: []*wire.TxIn{
+					{
+						PreviousOutPoint: op1,
+					},
+					{
+						PreviousOutPoint: op2,
+					},
+					{
+						PreviousOutPoint: op3,
+					},
+				},
+				TxOut: []*wire.TxOut{
+					{
+						Value:    5_698_752,
+						PkScript: p2trPkScript,
+					},
+					{
+						Value:    change1.Value,
+						PkScript: change1.PkScript,
+					},
+					{
+						Value:    change2.Value,
+						PkScript: change2.PkScript,
+					},
+				},
+			},
+			wantWeight:       1248,
+			wantFeeForWeight: 1248,
+			wantFee:          1248,
+		},
+
 		{
 			name: "weight estimator fails",
 			sweeps: []sweep{