sweepbatcher: make sure feerate does not decrease

Previous behaviour was to overwrite batch's feerate with minFeeRate of
its primary sweep, which could be lower that previus batch's feerate or
lower that feerate of some other sweep.

Instead, batch's feerate only grows and never declines and is at least as high
as the highest feerate of its sweeps.

Added a test to verify this.

Author: starius

sweepbatcher/sweep_batch.go

@@ -470,10 +470,18 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) {
 		// batch's confirmation target and fee rate.
 		if b.primarySweepID == sweep.swapHash {
 			b.cfg.batchConfTarget = sweep.confTarget
-			b.rbfCache.FeeRate = sweep.minFeeRate
 			b.rbfCache.SkipNextBump = true
 		}
 
+		// Update batch's fee rate to be greater than or equal to
+		// minFeeRate of the sweep. Make sure batch's fee rate does not
+		// decrease (otherwise it won't pass RBF rules and won't be
+		// broadcasted) and that it is not lower that minFeeRate of
+		// other sweeps (so it is applied).
+		if b.rbfCache.FeeRate < sweep.minFeeRate {
+			b.rbfCache.FeeRate = sweep.minFeeRate
+		}
+
 		return true, nil
 	}
 

sweepbatcher/sweep_batcher_test.go

@@ -3698,6 +3698,180 @@ func testWithMixedBatchCoopFailedOnly(t *testing.T, store testStore,
 		wantWeight, wantWitnessSizes)
 }
 
+// testFeeRateGrows tests that fee rate of a batch does not decrease and is at
+// least as high as the highest fee rate of sweeps.
+func testFeeRateGrows(t *testing.T, store testStore,
+	batcherStore testBatcherStore) {
+
+	defer test.Guard(t)()
+
+	lnd := test.NewMockLnd()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	sweepStore, err := NewSweepFetcherFromSwapStore(store, lnd.ChainParams)
+	require.NoError(t, err)
+
+	// Create a map to store fee rates.
+	swap2feeRate := map[lntypes.Hash]chainfee.SatPerKWeight{}
+	var swap2feeRateMu sync.Mutex
+	setFeeRate := func(swapHash lntypes.Hash, rate chainfee.SatPerKWeight) {
+		swap2feeRateMu.Lock()
+		defer swap2feeRateMu.Unlock()
+
+		swap2feeRate[swapHash] = rate
+	}
+
+	customFeeRate := func(ctx context.Context,
+		swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) {
+
+		swap2feeRateMu.Lock()
+		defer swap2feeRateMu.Unlock()
+
+		return swap2feeRate[swapHash], nil
+	}
+
+	const (
+		feeRateLow    = chainfee.SatPerKWeight(10_000)
+		feeRateMedium = chainfee.SatPerKWeight(30_000)
+		feeRateHigh   = chainfee.SatPerKWeight(50_000)
+	)
+
+	batcher := NewBatcher(lnd.WalletKit, lnd.ChainNotifier, lnd.Signer,
+		testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams,
+		batcherStore, sweepStore, WithCustomFeeRate(customFeeRate))
+
+	go func() {
+		err := batcher.Run(ctx)
+		checkBatcherError(t, err)
+	}()
+
+	// Create the first sweep.
+	swapHash1 := lntypes.Hash{1, 1, 1}
+	setFeeRate(swapHash1, feeRateMedium)
+	sweepReq1 := SweepRequest{
+		SwapHash: swapHash1,
+		Value:    1_000_000,
+		Outpoint: wire.OutPoint{
+			Hash:  chainhash.Hash{1, 1},
+			Index: 1,
+		},
+		Notifier: &dummyNotifier,
+	}
+
+	swap1 := &loopdb.LoopOutContract{
+		SwapContract: loopdb.SwapContract{
+			CltvExpiry:      111,
+			AmountRequested: 1_000_000,
+			ProtocolVersion: loopdb.ProtocolVersionMuSig2,
+			HtlcKeys:        htlcKeys,
+
+			// Make preimage unique to pass SQL constraints.
+			Preimage: lntypes.Preimage{1},
+		},
+
+		DestAddr:        destAddr,
+		SwapInvoice:     swapInvoice,
+		SweepConfTarget: 111,
+	}
+
+	err = store.CreateLoopOut(ctx, swapHash1, swap1)
+	require.NoError(t, err)
+	store.AssertLoopOutStored()
+
+	// Deliver sweep request to batcher.
+	require.NoError(t, batcher.AddSweep(&sweepReq1))
+
+	// Since a batch was created we check that it registered for its primary
+	// sweep's spend.
+	<-lnd.RegisterSpendChannel
+
+	// Wait for tx to be published.
+	<-lnd.TxPublishChannel
+
+	// Make sure the fee rate is feeRateMedium.
+	batch := getOnlyBatch(batcher)
+	require.Len(t, batch.sweeps, 1)
+	require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate)
+
+	// Now decrease the fee of sweep1.
+	setFeeRate(swapHash1, feeRateLow)
+	require.NoError(t, batcher.AddSweep(&sweepReq1))
+
+	// Tick tock next block.
+	err = lnd.NotifyHeight(601)
+	require.NoError(t, err)
+
+	// Wait for tx to be published.
+	<-lnd.TxPublishChannel
+
+	// Make sure the fee rate is still feeRateMedium.
+	require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate)
+
+	// Add sweep2, with feeRateMedium.
+	swapHash2 := lntypes.Hash{2, 2, 2}
+	setFeeRate(swapHash2, feeRateMedium)
+	sweepReq2 := SweepRequest{
+		SwapHash: swapHash2,
+		Value:    1_000_000,
+		Outpoint: wire.OutPoint{
+			Hash:  chainhash.Hash{2, 2},
+			Index: 1,
+		},
+		Notifier: &dummyNotifier,
+	}
+
+	swap2 := &loopdb.LoopOutContract{
+		SwapContract: loopdb.SwapContract{
+			CltvExpiry:      111,
+			AmountRequested: 1_000_000,
+			ProtocolVersion: loopdb.ProtocolVersionMuSig2,
+			HtlcKeys:        htlcKeys,
+
+			// Make preimage unique to pass SQL constraints.
+			Preimage: lntypes.Preimage{2},
+		},
+
+		DestAddr:        destAddr,
+		SwapInvoice:     swapInvoice,
+		SweepConfTarget: 111,
+	}
+
+	err = store.CreateLoopOut(ctx, swapHash2, swap2)
+	require.NoError(t, err)
+	store.AssertLoopOutStored()
+
+	// Deliver sweep request to batcher.
+	require.NoError(t, batcher.AddSweep(&sweepReq2))
+
+	// Tick tock next block.
+	err = lnd.NotifyHeight(602)
+	require.NoError(t, err)
+
+	// Wait for tx to be published.
+	<-lnd.TxPublishChannel
+
+	// Make sure the fee rate is still feeRateMedium.
+	require.Len(t, batch.sweeps, 2)
+	require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate)
+
+	// Now update fee rate of second sweep (which is not primary) to
+	// feeRateHigh. Fee rate of sweep 1 is still feeRateLow.
+	setFeeRate(swapHash2, feeRateHigh)
+	require.NoError(t, batcher.AddSweep(&sweepReq1))
+	require.NoError(t, batcher.AddSweep(&sweepReq2))
+
+	// Tick tock next block.
+	err = lnd.NotifyHeight(603)
+	require.NoError(t, err)
+
+	// Wait for tx to be published.
+	<-lnd.TxPublishChannel
+
+	// Make sure the fee rate increased to feeRateHigh.
+	require.Equal(t, feeRateHigh, batch.rbfCache.FeeRate)
+}
+
 // TestSweepBatcherBatchCreation tests that sweep requests enter the expected
 // batch based on their timeout distance.
 func TestSweepBatcherBatchCreation(t *testing.T) {
@@ -3839,6 +4013,12 @@ func TestWithMixedBatchCoopFailedOnly(t *testing.T) {
 	runTests(t, testWithMixedBatchCoopFailedOnly)
 }
 
+// TestFeeRateGrows tests that fee rate of a batch does not decrease and is at
+// least as high as the highest fee rate of sweeps.
+func TestFeeRateGrows(t *testing.T) {
+	runTests(t, testFeeRateGrows)
+}
+
 // testBatcherStore is BatcherStore used in tests.
 type testBatcherStore interface {
 	BatcherStore