Apply the safe minimum fee floor to all wallet transactions

pkg/tbtcpg/deposit_sweep.go

@@ -21,26 +21,6 @@ import (
 // This will ensure that deposit sweep transaction fees are not underestimated.
 const depositScriptByteSize = 126
 
-// minSweepTxSatPerVByteFee is the minimum fee rate, in sat/vByte, applied to
-// deposit sweep transactions. A fee oracle can return an unusably low estimate
-// (down to the 1 sat/vByte relay floor enforced by the Electrum client) in an
-// uncongested mempool. Because a sweep consolidates significant wallet value
-// and is not RBF-enabled, it cannot be replaced once broadcast, so a floor-rate
-// sweep can get stuck in the mempool and jam the wallet: no new sweep can be
-// built while the previous one is unconfirmed. This minimum keeps the sweep fee
-// safely above the relay floor while remaining far below the Bridge's
-// per-deposit maximum fee. The value is intentionally conservative and could be
-// made configurable; see threshold-network/keep-core#4171.
-//
-// NOTE: this static floor and the 25% buffer applied below are a stopgap for
-// the current fire-and-forget, non-RBF sweep path: because a stuck sweep cannot
-// be fee-bumped, the fee must be right on the first broadcast. Once RBF /
-// fee-bumping lands (Part B, tracked in #4171) the safety net shifts to
-// monitor-and-bump, and this policy should be revisited rather than carried
-// forward unchanged: the defensive buffer can be dropped and the floor relaxed
-// toward the live estimate, keeping only a small relay-propagation minimum.
-const minSweepTxSatPerVByteFee = 5
-
 // DepositSweepLookBackBlocks is the look-back period in blocks used
 // when searching for submitted deposit-related events. It's equal to
 // 30 days assuming 12 seconds per block.
@@ -510,7 +490,7 @@ func (dst *DepositSweepTask) ProposeDepositsSweep(
 			// the deposits stay unswept. Log it distinctly at WARN so operators
 			// can tell this apart from a benign "no deposits to sweep" outcome;
 			// in particular, a safe-minimum-fee abort (see
-			// minSweepTxSatPerVByteFee) can indicate a misconfigured, too-low
+			// minWalletTxSatPerVByteFee) can indicate a misconfigured, too-low
 			// per-deposit maximum fee that will strand deposits until governance
 			// raises it.
 			taskLogger.Warnf("cannot estimate sweep transaction fee: [%v]", err)
@@ -580,9 +560,9 @@ func (dst *DepositSweepTask) ProposeDepositsSweep(
 //   - 1 P2WPKH output
 //
 // An error is returned if any estimated fee exceeds the maximum fee allowed by
-// the Bridge contract, or if the minimum safe sweep fee (see
-// minSweepTxSatPerVByteFee) required to avoid a stuck, unbumpable sweep would
-// itself exceed that Bridge maximum.
+// the Bridge contract, or if the minimum safe fee (see minWalletTxSatPerVByteFee)
+// required to avoid a stuck, unbumpable sweep would itself exceed that Bridge
+// maximum.
 func EstimateDepositsSweepFee(
 	chain Chain,
 	btcChain bitcoin.Chain,
@@ -677,45 +657,19 @@ func estimateDepositsSweepFee(
 		return 0, 0, fmt.Errorf("estimated fee exceeds the maximum fee")
 	}
 
-	// A sweep must never be broadcast below a safe minimum fee rate, or it may
-	// get stuck in the mempool and jam the wallet (see minSweepTxSatPerVByteFee).
-	// If even that minimum fee exceeds the Bridge maximum, a safe sweep cannot be
-	// constructed; return an error rather than silently broadcasting an
-	// underpriced transaction.
-	if uint64(minSweepTxSatPerVByteFee*transactionSize) > totalMaxFee {
-		return 0, 0, fmt.Errorf(
-			"minimum safe sweep fee [%d] exceeds the maximum fee [%d]",
-			minSweepTxSatPerVByteFee*transactionSize,
-			totalMaxFee,
-		)
-	}
-
-	// Add a 25% buffer over the oracle estimate so there is margin during the
-	// estimate-to-broadcast delay and the fee stays adaptive under congestion
-	// (see threshold-network/keep-core#4171), then enforce the minimum floor and
-	// bound the result by the Bridge maximum (which the floor cannot exceed, per
-	// the check above).
+	// Enforce the safe minimum fee rate and 25% buffer, bounded by the Bridge
+	// maximum, so a sweep is never broadcast below the floor where it could get
+	// stuck and jam the wallet. Errors if even the floor exceeds the maximum.
 	//
 	// Caveat: transactionSize assumes all deposit inputs are witness (P2WSH), per
 	// this function's doc comment. A sweep that includes legacy P2SH deposits has
 	// a larger on-wire vsize than estimated here, so the effective on-wire rate
 	// can land slightly below the floor for such (rare) sweeps. It still dominates
 	// the 1 sat/vByte relay floor this fix targets; a fully accurate floor would
 	// require deposit-type-aware sizing.
-	// rate is an exact integer here because EstimateFee returns totalFee as
-	// satPerVByteFee * transactionSize (an exact multiple of the size), so the
-	// buffer is applied without truncation loss. If that contract changes, apply
-	// the buffer to totalFee directly instead of to the truncated rate.
-	rate := totalFee / transactionSize
-	rate = (rate*5 + 3) / 4 // ceil(rate * 1.25)
-	if rate < minSweepTxSatPerVByteFee {
-		rate = minSweepTxSatPerVByteFee
-	}
-	totalFee = rate * transactionSize
-	if uint64(totalFee) > totalMaxFee {
-		// totalMaxFee is bounded by Bitcoin's total supply (~2.1e15 sat), far
-		// below math.MaxInt64, so this narrowing cast cannot overflow.
-		totalFee = int64(totalMaxFee)
+	totalFee, err = applyWalletTxFeeFloor(totalFee, transactionSize, totalMaxFee)
+	if err != nil {
+		return 0, 0, err
 	}
 
 	// Compute the actual sat/vbyte fee for informational purposes.

pkg/tbtcpg/deposit_sweep_fee_test.go

@@ -100,7 +100,7 @@ func TestEstimateDepositsSweepFee_MinimumFloorAndBuffer(t *testing.T) {
 			// substring pins this to the floor-exceeds-cap branch specifically,
 			// distinguishing it from the raw-fee-exceeds-cap error.
 			perDepositMaxFee:    uint64(3 * size1),
-			expectErrorContains: "minimum safe sweep fee",
+			expectErrorContains: "minimum safe transaction fee",
 		},
 		"multi-deposit minimum floor above the cap returns an error": {
 			depositsCount:       3,
@@ -111,7 +111,7 @@ func TestEstimateDepositsSweepFee_MinimumFloorAndBuffer(t *testing.T) {
 			// the cap, so the raw-fee check passes and the floor branch is the
 			// one exercised.
 			perDepositMaxFee:    uint64(size3),
-			expectErrorContains: "minimum safe sweep fee",
+			expectErrorContains: "minimum safe transaction fee",
 		},
 	}
 

pkg/tbtcpg/fee.go

@@ -0,0 +1,106 @@
+package tbtcpg
+
+import (
+	"errors"
+	"fmt"
+)
+
+// ErrMaxFeeTooLow indicates that the Bridge maximum total fee is too low to
+// build a wallet transaction at the safe minimum fee rate, so a non-underpriced
+// transaction cannot be constructed. It mirrors the raw-estimate-too-high
+// sentinels (ErrFeeTooHigh, ErrSweepTxFeeTooHigh): both signal an unserviceable
+// fee configuration, one bounding from above and one from below.
+var ErrMaxFeeTooLow = errors.New(
+	"minimum safe transaction fee exceeds the maximum fee",
+)
+
+// minWalletTxSatPerVByteFee is the minimum fee rate, in sat/vByte, applied to
+// wallet Bitcoin transactions (deposit sweeps, redemptions, moving funds, moved
+// funds sweeps). A fee oracle can return an unusably low estimate (down to the
+// 1 sat/vByte relay floor enforced by the Electrum client) in an uncongested
+// mempool. Because these transactions spend or consolidate significant wallet
+// value and are not RBF-enabled, they cannot be replaced once broadcast, so a
+// floor-rate transaction can get stuck in the mempool and jam the wallet: no
+// new wallet transaction can be built while the previous one is unconfirmed.
+// This minimum keeps the fee safely above the relay floor while remaining far
+// below the Bridge's maximum fee. The value is intentionally conservative and
+// could be made configurable; see threshold-network/keep-core#4171.
+//
+// NOTE: this static floor and the 25% buffer applied in applyWalletTxFeeFloor
+// are a stopgap for the current fire-and-forget, non-RBF wallet transaction
+// path: because a stuck transaction cannot be fee-bumped, the fee must be right
+// on the first broadcast. Once RBF / fee-bumping lands (Part B, tracked in
+// #4171) the safety net shifts to monitor-and-bump, and this policy should be
+// revisited rather than carried forward unchanged: the defensive buffer can be
+// dropped and the floor relaxed toward the live estimate, keeping only a small
+// relay-propagation minimum.
+const minWalletTxSatPerVByteFee = 5
+
+// applyWalletTxFeeFloor raises a raw oracle fee estimate to a safe value for a
+// non-RBF wallet transaction. It:
+//   - adds a 25% buffer over the oracle estimate so there is margin during the
+//     estimate-to-broadcast delay and the fee stays adaptive under congestion,
+//   - enforces a floor of minWalletTxSatPerVByteFee sat/vByte, and
+//   - bounds the result by maxTotalFee (the Bridge maximum total fee for the
+//     transaction).
+//
+// It returns ErrMaxFeeTooLow if the minimum floor alone would exceed
+// maxTotalFee - a safe transaction cannot be built, so the caller must not
+// broadcast an underpriced one. estimatedFee is the raw oracle fee in satoshis
+// and txVsize is the estimated transaction virtual size in vBytes.
+//
+// The 25% buffer is applied to the truncated per-vByte rate
+// (estimatedFee / txVsize). This is lossless only because EstimateFee returns
+// the fee as satPerVByteFee * txVsize (an exact multiple of the vsize), so the
+// integer division recovers the exact rate. If that contract ever changes so
+// estimatedFee is no longer an exact multiple of txVsize, apply the buffer to
+// estimatedFee directly instead of to the truncated rate; otherwise up to
+// txVsize-1 sat is silently dropped before buffering and the tx is underpriced.
+//
+// maxTotalFee bounds only the total transaction fee. Where the Bridge also
+// enforces a per-request cap (e.g. the redemption TxMaxFee), satisfying that
+// cap is the caller's or on-chain validation's responsibility; this helper is
+// unaware of it. Callers are expected to reject a raw estimate already above
+// maxTotalFee before calling (all current callers do); the result is in any
+// case clamped down to maxTotalFee.
+//
+// The buffer and floor are applied to the estimated vsize; a transaction whose
+// real on-wire vsize is larger than estimated (e.g. a deposit sweep containing
+// legacy P2SH inputs) can land slightly below the intended rate, but still far
+// above the relay floor this guards against.
+func applyWalletTxFeeFloor(
+	estimatedFee int64,
+	txVsize int64,
+	maxTotalFee uint64,
+) (int64, error) {
+	if txVsize <= 0 {
+		return 0, fmt.Errorf("invalid transaction virtual size [%d]", txVsize)
+	}
+
+	// If even the minimum floor exceeds the Bridge maximum, a safe transaction
+	// cannot be constructed; error rather than silently broadcast underpriced.
+	if uint64(minWalletTxSatPerVByteFee*txVsize) > maxTotalFee {
+		return 0, fmt.Errorf(
+			"%w: minimum fee [%d], maximum fee [%d]",
+			ErrMaxFeeTooLow,
+			minWalletTxSatPerVByteFee*txVsize,
+			maxTotalFee,
+		)
+	}
+
+	rate := estimatedFee / txVsize
+	rate = (rate*5 + 3) / 4 // ceil(rate * 1.25)
+	if rate < minWalletTxSatPerVByteFee {
+		rate = minWalletTxSatPerVByteFee
+	}
+
+	// Clamp down to the Bridge maximum total fee. This can never drop the fee
+	// below the floor: the floor-vs-cap guard above already guaranteed
+	// maxTotalFee is at least the minimum floor total.
+	totalFee := rate * txVsize
+	if uint64(totalFee) > maxTotalFee {
+		totalFee = int64(maxTotalFee)
+	}
+
+	return totalFee, nil
+}

pkg/tbtcpg/fee_test.go

@@ -0,0 +1,93 @@
+package tbtcpg
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestApplyWalletTxFeeFloor(t *testing.T) {
+	const vsize = 200
+
+	tests := map[string]struct {
+		estimatedFee        int64
+		txVsize             int64
+		maxTotalFee         uint64
+		expectedFee         int64
+		expectErrorContains string
+	}{
+		"estimate above the floor is buffered by 25%": {
+			estimatedFee: 4000, // rate 20 sat/vByte
+			txVsize:      vsize,
+			maxTotalFee:  100000,
+			expectedFee:  5000, // ceil(20*1.25)=25 sat/vByte * 200
+		},
+		"low estimate is raised to the minimum floor": {
+			estimatedFee: vsize, // rate 1 sat/vByte
+			txVsize:      vsize,
+			maxTotalFee:  100000,
+			expectedFee:  1000, // max(5, ceil(1*1.25)=2)=5 sat/vByte * 200
+		},
+		"buffered fee above the cap is bounded to the cap": {
+			estimatedFee: 4000, // rate 20 -> buffered 25 sat/vByte * 200 = 5000
+			txVsize:      vsize,
+			maxTotalFee:  4500, // below the buffered 5000
+			expectedFee:  4500,
+		},
+		"buffered fee exactly at the cap is not clamped": {
+			estimatedFee: 4000, // rate 20 -> buffered 25 sat/vByte * 200 = 5000
+			txVsize:      vsize,
+			maxTotalFee:  5000, // exactly the buffered total
+			expectedFee:  5000,
+		},
+		"minimum floor exactly at the cap is allowed": {
+			estimatedFee: 100, // rate 0 -> floored to 5 sat/vByte
+			txVsize:      vsize,
+			maxTotalFee:  1000, // exactly the 5 sat/vByte floor total (5*200)
+			expectedFee:  1000,
+		},
+		"minimum floor above the cap returns an error": {
+			estimatedFee:        100,
+			txVsize:             vsize,
+			maxTotalFee:         800, // below the 5 sat/vByte floor (1000)
+			expectErrorContains: "minimum safe transaction fee",
+		},
+		"non-positive virtual size returns an error": {
+			estimatedFee:        1000,
+			txVsize:             0,
+			maxTotalFee:         100000,
+			expectErrorContains: "invalid transaction virtual size",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			fee, err := applyWalletTxFeeFloor(
+				tc.estimatedFee,
+				tc.txVsize,
+				tc.maxTotalFee,
+			)
+
+			if tc.expectErrorContains != "" {
+				if err == nil {
+					t.Fatalf("expected an error, got fee [%d]", fee)
+				}
+				if !strings.Contains(err.Error(), tc.expectErrorContains) {
+					t.Fatalf(
+						"expected error containing [%s]; got [%v]",
+						tc.expectErrorContains, err,
+					)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: [%v]", err)
+			}
+			if fee != tc.expectedFee {
+				t.Errorf(
+					"unexpected fee\nexpected: [%d]\nactual:   [%d]",
+					tc.expectedFee, fee,
+				)
+			}
+		})
+	}
+}

pkg/tbtcpg/moved_funds_sweep.go

@@ -411,5 +411,13 @@ func EstimateMovedFundsSweepFee(
 		return 0, ErrSweepTxFeeTooHigh
 	}
 
+	// Enforce the safe minimum fee rate and buffer so a non-RBF moved funds
+	// sweep transaction is never broadcast below the floor where it could get
+	// stuck and jam the wallet.
+	totalFee, err = applyWalletTxFeeFloor(totalFee, transactionSize, sweepTxMaxTotalFee)
+	if err != nil {
+		return 0, err
+	}
+
 	return totalFee, nil
 }