From 75641c3573f8cea2c8a1f0f00dbcc2c6748bcbaa Mon Sep 17 00:00:00 2001
From: Boris Nagaev <bnagaev@gmail.com>
Date: Mon, 15 Jul 2024 12:24:57 -0300
Subject: [PATCH] sweepbatcher: always fill sweep.minFeeRate

Use customFeeRate if it is provided, otherwise use wallet's EstimateFeeRate.

Added flag SkipNextBump to rbfCache to avoid extra bumping upon updating
fee rate externally.

Fix test TestSweepBatcherCloseDuringAdding, it didn't have confTarget and
failed in wallet.EstimateFeeRate call.
---
 sweepbatcher/sweep_batch.go        | 21 +++++++++++++++++++--
 sweepbatcher/sweep_batcher.go      | 16 +++++++++++++---
 sweepbatcher/sweep_batcher_test.go |  5 +++--
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go
index dd9e5da..9a18aba 100644
--- a/sweepbatcher/sweep_batch.go
+++ b/sweepbatcher/sweep_batch.go
@@ -156,6 +156,10 @@ type rbfCache struct {
 
 	// FeeRate is the last used fee rate we used to publish a batch tx.
 	FeeRate chainfee.SatPerKWeight
+
+	// SkipNextBump instructs updateRbfRate to skip one fee bumping.
+	// It is set upon updating FeeRate externally.
+	SkipNextBump bool
 }
 
 // batch is a collection of sweeps that are published together.
@@ -417,6 +421,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) {
 		if b.primarySweepID == sweep.swapHash {
 			b.cfg.batchConfTarget = sweep.confTarget
 			b.rbfCache.FeeRate = sweep.minFeeRate
+			b.rbfCache.SkipNextBump = true
 		}
 
 		return true, nil
@@ -459,6 +464,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) {
 		b.primarySweepID = sweep.swapHash
 		b.cfg.batchConfTarget = sweep.confTarget
 		b.rbfCache.FeeRate = sweep.minFeeRate
+		b.rbfCache.SkipNextBump = true
 
 		// We also need to start the spend monitor for this new primary
 		// sweep.
@@ -476,6 +482,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) {
 	// the batch is the basis for fee bumps.
 	if b.rbfCache.FeeRate < sweep.minFeeRate {
 		b.rbfCache.FeeRate = sweep.minFeeRate
+		b.rbfCache.SkipNextBump = true
 	}
 
 	return true, b.persistSweep(ctx, *sweep, false)
@@ -1141,10 +1148,15 @@ func (b *batch) updateRbfRate(ctx context.Context) error {
 	// If the feeRate is unset then we never published before, so we
 	// retrieve the fee estimate from our wallet.
 	if b.rbfCache.FeeRate == 0 {
+		// We set minFeeRate in each sweep, so fee rate is expected to
+		// be initiated here.
+		b.log.Warnf("rbfCache.FeeRate is 0, which must not happen.")
+
 		if b.cfg.batchConfTarget == 0 {
 			b.log.Warnf("updateRbfRate called with zero " +
 				"batchConfTarget")
 		}
+
 		b.log.Infof("initializing rbf fee rate for conf target=%v",
 			b.cfg.batchConfTarget)
 		rate, err := b.wallet.EstimateFeeRate(
@@ -1157,8 +1169,13 @@ func (b *batch) updateRbfRate(ctx context.Context) error {
 		// Set the initial value for our fee rate.
 		b.rbfCache.FeeRate = rate
 	} else if !b.cfg.noBumping {
-		// Bump the fee rate by the configured step.
-		b.rbfCache.FeeRate += defaultFeeRateStep
+		if b.rbfCache.SkipNextBump {
+			// Skip fee bumping, unset the flag, to bump next time.
+			b.rbfCache.SkipNextBump = false
+		} else {
+			// Bump the fee rate by the configured step.
+			b.rbfCache.FeeRate += defaultFeeRateStep
+		}
 	}
 
 	b.rbfCache.LastHeight = b.currentHeight
diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go
index 7e66207..b12f391 100644
--- a/sweepbatcher/sweep_batcher.go
+++ b/sweepbatcher/sweep_batcher.go
@@ -885,9 +885,8 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash,
 			swapHash[:6], err)
 	}
 
-	// minFeeRate is 0 by default. If customFeeRate is not provided, then
-	// rbfCache.FeeRate is also 0 and method batch.updateRbfRate() updates
-	// it to current fee rate according to batchConfTarget.
+	// Find minimum fee rate for the sweep. Use customFeeRate if it is
+	// provided, otherwise use wallet's EstimateFeeRate.
 	var minFeeRate chainfee.SatPerKWeight
 	if b.customFeeRate != nil {
 		minFeeRate, err = b.customFeeRate(ctx, swapHash)
@@ -899,6 +898,17 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash,
 			return nil, fmt.Errorf("min fee rate too low (%v) for "+
 				"%x", minFeeRate, swapHash[:6])
 		}
+	} else {
+		if s.ConfTarget == 0 {
+			log.Warnf("Fee estimation was requested for zero "+
+				"confTarget for sweep %x.", swapHash[:6])
+		}
+		minFeeRate, err = b.wallet.EstimateFeeRate(ctx, s.ConfTarget)
+		if err != nil {
+			return nil, fmt.Errorf("failed to estimate fee rate "+
+				"for %x, confTarget=%d: %w", swapHash[:6],
+				s.ConfTarget, err)
+		}
 	}
 
 	return &sweep{
diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go
index 7314e97..96b77fc 100644
--- a/sweepbatcher/sweep_batcher_test.go
+++ b/sweepbatcher/sweep_batcher_test.go
@@ -2110,8 +2110,9 @@ func testSweepBatcherCloseDuringAdding(t *testing.T, store testStore,
 				Preimage: lntypes.Preimage{i},
 			},
 
-			DestAddr:    destAddr,
-			SwapInvoice: swapInvoice,
+			DestAddr:        destAddr,
+			SwapInvoice:     swapInvoice,
+			SweepConfTarget: 111,
 		}
 
 		err = store.CreateLoopOut(ctx, swapHash, swap)