sweepbatcher: consider min relay fee when constructing batch tx

if constructUnsignedTx constructs a batch transaction that is
below the minimum relay fee, an error is returned.

loopout_test.go

@@ -280,6 +280,7 @@ func testCustomSweepConfTarget(t *testing.T) {
 	// yields a much higher fee rate.
 	ctx.Lnd.SetFeeEstimate(testReq.SweepConfTarget, 250)
 	ctx.Lnd.SetFeeEstimate(DefaultSweepConfTarget, 10000)
+	ctx.Lnd.SetMinRelayFee(250)
 
 	cfg := newSwapConfig(
 		&lnd.LndServices, loopdb.NewStoreMock(t), server, nil,

sweepbatcher/presigned.go

@@ -28,8 +28,14 @@ func (b *batch) ensurePresigned(ctx context.Context, newSweeps []*sweep,
 			"adding to an empty batch")
 	}
 
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get minRelayFee: %w", err)
+	}
+
 	return ensurePresigned(
-		ctx, newSweeps, b.cfg.presignedHelper, b.cfg.chainParams,
+		ctx, newSweeps, b.cfg.presignedHelper, minRelayFeeRate,
+		b.cfg.chainParams,
 	)
 }
 
@@ -43,6 +49,7 @@ type presignedTxChecker interface {
 // inputs of this group only.
 func ensurePresigned(ctx context.Context, newSweeps []*sweep,
 	presignedTxChecker presignedTxChecker,
+	minRelayFeeRate chainfee.SatPerKWeight,
 	chainParams *chaincfg.Params) error {
 
 	sweeps := make([]sweep, len(newSweeps))
@@ -74,7 +81,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep,
 	const feeRate = chainfee.FeePerKwFloor
 
 	tx, _, _, _, err := constructUnsignedTx(
-		sweeps, destAddr, currentHeight, feeRate,
+		sweeps, destAddr, currentHeight, feeRate, minRelayFeeRate,
 	)
 	if err != nil {
 		return fmt.Errorf("failed to construct unsigned tx "+
@@ -218,6 +225,14 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {
 
 	b.Infof("nextBlockFeeRate is %v", nextBlockFeeRate)
 
+	// Find the minRelayFeeRate.
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get minRelayFeeRate: %w", err)
+	}
+
+	b.Infof("minRelayFeeRate is %v", minRelayFeeRate)
+
 	// We need to restore previously added groups. We can do it by reading
 	// all the sweeps from DB (they must be ordered) and grouping by swap.
 	groups, err := b.getSweepsGroups(ctx)
@@ -258,7 +273,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {
 
 		err = presign(
 			ctx, b.cfg.presignedHelper, destAddr, primarySweepID,
-			sweeps, nextBlockFeeRate,
+			sweeps, nextBlockFeeRate, minRelayFeeRate,
 		)
 		if err != nil {
 			return fmt.Errorf("failed to presign a transaction "+
@@ -300,7 +315,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,
+	minRelayFeeRate chainfee.SatPerKWeight) error {
 
 	if presigner == nil {
 		return fmt.Errorf("presigner is not installed")
@@ -354,7 +370,7 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
 	for fr := start; fr <= stop; fr = (fr * factorPPM) / 1_000_000 {
 		// Construct an unsigned transaction for this fee rate.
 		tx, _, feeForWeight, fee, err := constructUnsignedTx(
-			sweeps, destAddr, currentHeight, fr,
+			sweeps, destAddr, currentHeight, fr, minRelayFeeRate,
 		)
 		if err != nil {
 			return fmt.Errorf("failed to construct unsigned tx "+
@@ -411,15 +427,15 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 	}
 
 	// Determine the current minimum relay fee based on our chain backend.
-	minRelayFee, err := b.wallet.MinRelayFee(ctx)
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
 	if err != nil {
-		return 0, fmt.Errorf("failed to get minRelayFee: %w", err),
+		return 0, fmt.Errorf("failed to get minRelayFeeRate: %w", err),
 			false
 	}
 
 	// Cache current height and desired feerate of the batch.
 	currentHeight := b.currentHeight
-	feeRate := max(b.rbfCache.FeeRate, minRelayFee)
+	feeRate := max(b.rbfCache.FeeRate, minRelayFeeRate)
 
 	// Append this sweep to an array of sweeps. This is needed to keep the
 	// order of sweeps stored, as iterating the sweeps map does not
@@ -441,7 +457,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 
 	// Construct unsigned batch transaction.
 	tx, weight, _, fee, err := constructUnsignedTx(
-		sweeps, address, currentHeight, feeRate,
+		sweeps, address, currentHeight, feeRate, minRelayFeeRate,
 	)
 	if err != nil {
 		return 0, fmt.Errorf("failed to construct tx: %w", err),
@@ -460,7 +476,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 	// Get a pre-signed transaction.
 	const loadOnly = false
 	signedTx, err := b.cfg.presignedHelper.SignTx(
-		ctx, b.primarySweepID, tx, batchAmt, minRelayFee, feeRate,
+		ctx, b.primarySweepID, tx, batchAmt, minRelayFeeRate, feeRate,
 		loadOnly,
 	)
 	if err != nil {
@@ -470,7 +486,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 
 	// Run sanity checks to make sure presignedHelper.SignTx complied with
 	// all the invariants.
-	err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFee)
+	err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFeeRate)
 	if err != nil {
 		return 0, fmt.Errorf("signed tx doesn't correspond the "+
 			"unsigned tx: %w", err), false

sweepbatcher/presigned_test.go

@@ -15,6 +15,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const (
+	minRelayFeeRate = chainfee.FeePerKwFloor
+)
+
 // TestOrderedSweeps checks that methods batch.getOrderedSweeps and
 // batch.getSweepsGroups works properly.
 func TestOrderedSweeps(t *testing.T) {
@@ -561,7 +565,7 @@ func TestEnsurePresigned(t *testing.T) {
 			}
 
 			err := ensurePresigned(
-				ctx, tc.sweeps, c,
+				ctx, tc.sweeps, c, minRelayFeeRate,
 				&chaincfg.RegressionNetParams,
 			)
 			switch {
@@ -1010,7 +1014,7 @@ func TestPresign(t *testing.T) {
 			err := presign(
 				ctx, tc.presigner, tc.destAddr,
 				tc.primarySweepID, tc.sweeps,
-				tc.nextBlockFeeRate,
+				tc.nextBlockFeeRate, minRelayFeeRate,
 			)
 			if tc.wantErr != "" {
 				require.Error(t, err)

sweepbatcher/sweep_batch.go

@@ -1296,9 +1296,9 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte,
 // 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,
+	minRelayFeeRate 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 {
@@ -1400,7 +1400,14 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
 	}
 
 	// Clamp the calculated fee to the max allowed fee amount for the batch.
-	fee := clampBatchFee(feeForWeight, batchAmt-btcutil.Amount(sumChange))
+	fee, err := clampBatchFee(
+		feeForWeight, batchAmt-btcutil.Amount(sumChange),
+		minRelayFeeRate, weight,
+	)
+	if err != nil {
+		return nil, 0, 0, 0, fmt.Errorf("failed to clamp batch "+
+			"fee: %w", err)
+	}
 
 	// 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
@@ -1516,15 +1523,21 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error,
 	// known in advance to be non-cooperative (nonCoopHint) and not failed
 	// to sign cooperatively in previous rounds (coopFailed). If any of them
 	// fails, the sweep is excluded from all following rounds and another
-	// round is attempted. Otherwise the cycle completes and we sign the
+	// round is attempted. Otherwise, the cycle completes and we sign the
 	// remaining sweeps non-cooperatively.
 	var (
-		tx           *wire.MsgTx
-		weight       lntypes.WeightUnit
-		feeForWeight btcutil.Amount
-		fee          btcutil.Amount
-		coopInputs   int
+		tx              *wire.MsgTx
+		weight          lntypes.WeightUnit
+		feeForWeight    btcutil.Amount
+		fee             btcutil.Amount
+		minRelayFeeRate chainfee.SatPerKWeight
+		coopInputs      int
 	)
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
+	if err != nil {
+		return 0, fmt.Errorf("failed to get min relay fee: %w", err),
+			false
+	}
 	for attempt := 1; ; attempt++ {
 		b.Infof("Attempt %d of collecting cooperative signatures.",
 			attempt)
@@ -1533,6 +1546,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error,
 		var err error
 		tx, weight, feeForWeight, fee, err = constructUnsignedTx(
 			sweeps, address, b.currentHeight, b.rbfCache.FeeRate,
+			minRelayFeeRate,
 		)
 		if err != nil {
 			return 0, fmt.Errorf("failed to construct tx: %w", err),
@@ -1584,7 +1598,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error,
 		// If there was any failure of cooperative signing, we need to
 		// update weight estimates (since non-cooperative signing has
 		// larger witness) and hence update the whole transaction and
-		// all the signatures. Otherwise we complete cooperative part.
+		// all the signatures. Otherwise, we complete cooperative part.
 		if !newCoopFailures {
 			break
 		}
@@ -1720,7 +1734,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error,
 	}
 
 	// Publish the transaction.
-	err := b.wallet.PublishTransaction(
+	err = b.wallet.PublishTransaction(
 		ctx, tx, b.cfg.txLabeler(b.id),
 	)
 	if err != nil {
@@ -2583,16 +2597,25 @@ func (b *batch) persistSweep(ctx context.Context, sweep sweep,
 
 // clampBatchFee takes the fee amount and total amount of the sweeps in the
 // batch and makes sure the fee is not too high. If the fee is too high, it is
-// clamped to the maximum allowed fee.
-func clampBatchFee(fee btcutil.Amount,
-	totalAmount btcutil.Amount) btcutil.Amount {
+// clamped to the maximum allowed fee. If the clamped fee results in a fee rate
+// below the minimum relay fee, an error is returned.
+func clampBatchFee(fee btcutil.Amount, totalAmount btcutil.Amount,
+	minRelayFeeRate chainfee.SatPerKWeight,
+	weight lntypes.WeightUnit) (btcutil.Amount, error) {
 
 	maxFeeAmount := btcutil.Amount(float64(totalAmount) *
 		maxFeeToSwapAmtRatio)
 
+	clampedFee := fee
 	if fee > maxFeeAmount {
-		return maxFeeAmount
+		clampedFee = maxFeeAmount
 	}
 
-	return fee
+	clampedFeeRate := chainfee.NewSatPerKWeight(clampedFee, weight)
+	if clampedFeeRate < minRelayFeeRate {
+		return 0, fmt.Errorf("clamped fee rate %v is less than "+
+			"minimum relay fee %v", clampedFeeRate, minRelayFeeRate)
+	}
+
+	return clampedFee, nil
 }

sweepbatcher/sweep_batch_test.go

@@ -440,12 +440,13 @@ func TestConstructUnsignedTx(t *testing.T) {
 					change:   change1,
 				},
 			},
-			address:       p2trAddress,
-			currentHeight: 800_000,
-			feeRate:       1,
+			address:         p2trAddress,
+			currentHeight:   800_000,
+			feeRate:         1_000,
+			minRelayFeeRate: 50,
 			wantErr: "batch amount 0.00100294 BTC is <= the sum " +
 				"of change outputs 0.00100000 BTC plus fee " +
-				"0.00000001 BTC and dust limit 0.00000330 BTC",
+				"0.00000058 BTC and dust limit 0.00000330 BTC",
 		},
 
 		{

sweepbatcher/sweep_batcher.go

@@ -736,6 +736,10 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input,
 	if err != nil {
 		return fmt.Errorf("failed to get nextBlockFeeRate: %w", err)
 	}
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get minRelayFeeRate: %w", err)
+	}
 	destPkscript, err := txscript.PayToAddrScript(destAddress)
 	if err != nil {
 		return fmt.Errorf("txscript.PayToAddrScript failed: %w", err)
@@ -763,7 +767,7 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input,
 
 	return presign(
 		ctx, b.presignedHelper, destAddress, primarySweepID, sweeps,
-		nextBlockFeeRate,
+		nextBlockFeeRate, minRelayFeeRate,
 	)
 }
 
@@ -818,12 +822,18 @@ func (b *Batcher) AddSweep(ctx context.Context, sweepReq *SweepRequest) error {
 		}
 	}
 
+	minRelayFeeRate, err := b.wallet.MinRelayFee(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get min relay fee: %w", err)
+	}
+
 	// If this is a presigned mode, make sure PresignSweepsGroup was called.
 	// We skip the check for reorg-safely confirmed sweeps, because their
 	// presigned transactions were already cleaned up from the store.
 	if sweep.presigned && !fullyConfirmed {
 		err := ensurePresigned(
-			ctx, sweeps, b.presignedHelper, b.chainParams,
+			ctx, sweeps, b.presignedHelper, minRelayFeeRate,
+			b.chainParams,
 		)
 		if err != nil {
 			return fmt.Errorf("inputs with primarySweep %v were "+

sweepbatcher/sweep_batcher_presigned_test.go

@@ -519,7 +519,7 @@ func testPresigned_min_relay_fee(t *testing.T,
 	}()
 
 	// Set high min_relay_fee.
-	lnd.SetMinRelayFee(400)
+	lnd.SetMinRelayFee(252)
 
 	// Create the first sweep.
 	swapHash1 := lntypes.Hash{1, 1, 1}
@@ -554,16 +554,16 @@ func testPresigned_min_relay_fee(t *testing.T,
 	// Wait for a transactions to be published.
 	tx := <-lnd.TxPublishChannel
 	gotFeeRate := presignedHelper.getTxFeerate(tx, inputAmt)
-	require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate)
+	require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate)
 
 	// Now decrease min_relay_fee and make sure fee rate doesn't decrease.
 	// The only difference of tx2 is a higher lock_time.
-	lnd.SetMinRelayFee(300)
+	lnd.SetMinRelayFee(150)
 	require.NoError(t, lnd.NotifyHeight(601))
 	tx2 := <-lnd.TxPublishChannel
 	require.Equal(t, tx.TxOut[0].Value, tx2.TxOut[0].Value)
 	gotFeeRate = presignedHelper.getTxFeerate(tx2, inputAmt)
-	require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate)
+	require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate)
 	require.Equal(t, uint32(601), tx2.LockTime)
 
 	// Set a higher min_relay_fee, turn off the client and try presigned tx.
@@ -1269,9 +1269,10 @@ func testPresigned_presigned_group_with_change(t *testing.T,
 	require.NoError(t, lnd.NotifyHeight(601))
 }
 
-// testPresigned_presigned_group_with_dust_main_output tests passing multiple
-// sweeps to the method PresignSweepsGroup. It tests that a dust change output of
-// a primary deposit sweep is rejected by PresignSweepsGroup and AddSweep.
+// testPresigned_presigned_group_with_dust_main_output passes a dust main output
+// and a change output to PresignSweepsGroup. It will fail because of the dust
+// main output. Note that the min relay fee is set low enough to pass the
+// clampBatchFee check, so the error is not related to the fee rate.
 func testPresigned_presigned_group_with_dust_main_output(t *testing.T,
 	batcherStore testBatcherStore) {
 
@@ -1285,9 +1286,12 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T,
 	customFeeRate := func(_ context.Context, _ lntypes.Hash,
 		_ wire.OutPoint) (chainfee.SatPerKWeight, error) {
 
-		return chainfee.SatPerKWeight(10_000), nil
+		return chainfee.SatPerKWeight(100_000), nil
 	}
 
+	// Set min relay fee low enough to pass the clampBatchFee check.
+	lnd.SetMinRelayFee(140)
+
 	presignedHelper := newMockPresignedHelper()
 
 	batcher := NewBatcher(
@@ -1345,6 +1349,81 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T,
 	require.ErrorContains(t, err, "were not presigned")
 }
 
+// testPresigned_presigned_group_with_dust_below_relay_fee passes a tx with a
+// dust main output and one change output to PresignSweepsGroup. The tx fee rate
+// is below the min relay fee, hence clampBatchFee will return an error.
+func testPresigned_presigned_group_with_dust_below_relay_fee(t *testing.T,
+	batcherStore testBatcherStore) {
+
+	defer test.Guard(t)()
+
+	lnd := test.NewMockLnd()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	customFeeRate := func(_ context.Context, _ lntypes.Hash,
+		_ wire.OutPoint) (chainfee.SatPerKWeight, error) {
+
+		return chainfee.SatPerKWeight(100_000), nil
+	}
+
+	presignedHelper := newMockPresignedHelper()
+
+	batcher := NewBatcher(
+		lnd.WalletKit, lnd.ChainNotifier, lnd.Signer,
+		testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams,
+		batcherStore, presignedHelper,
+		WithCustomFeeRate(customFeeRate),
+		WithPresignedHelper(presignedHelper),
+	)
+
+	go func() {
+		err := batcher.Run(ctx)
+		checkBatcherError(t, err)
+	}()
+
+	// Create a swap of two sweeps.
+	swapHash1 := lntypes.Hash{1, 1, 1}
+	op1 := wire.OutPoint{
+		Hash:  chainhash.Hash{1, 1},
+		Index: 1,
+	}
+	inputValue := int64(1_000_000)
+	group1 := []Input{
+		{
+			Outpoint: op1,
+			Value:    1_000_000,
+		},
+	}
+	dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize))
+	change := &wire.TxOut{
+		Value:    inputValue - dustLimit + 1,
+		PkScript: []byte{0xaf, 0xfe},
+	}
+
+	presignedHelper.setChangeForPrimaryDeposit(op1, change)
+
+	// Enable only one of the sweeps.
+	presignedHelper.SetOutpointOnline(op1, true)
+
+	// An attempt to presign must fail.
+	err := batcher.PresignSweepsGroup(
+		ctx, group1, sweepTimeout, destAddr, change,
+	)
+	require.EqualError(t, err, "failed to construct unsigned tx for "+
+		"feeRate 253 sat/kw: failed to clamp batch fee: clamped "+
+		"fee rate 148 sat/kw is less than minimum relay fee 253 sat/kw")
+
+	// Add the sweep, triggering the publishing attempt.
+	err = batcher.AddSweep(ctx, &SweepRequest{
+		SwapHash: swapHash1,
+		Inputs:   group1,
+		Notifier: &dummyNotifier,
+	})
+	require.ErrorContains(t, err, "were not presigned")
+}
+
 // testPresigned_presigned_group_with_dust_change tests passing multiple sweeps
 // to the method PresignSweepsGroup. It tests that a dust change output of a
 // primary deposit sweep is rejected by PresignSweepsGroup and AddSweep.
@@ -2102,6 +2181,12 @@ func TestPresigned(t *testing.T) {
 		)
 	})
 
+	t.Run("dust_main_output_below_min_relay_fee", func(t *testing.T) {
+		testPresigned_presigned_group_with_dust_below_relay_fee(
+			t, NewStoreMock(),
+		)
+	})
+
 	t.Run("dust_change", func(t *testing.T) {
 		testPresigned_presigned_group_with_dust_change(t, NewStoreMock())
 	})