Add follower-side soft check for below-floor sweep fees

pkg/tbtc/deposit_sweep.go

@@ -49,6 +49,20 @@ const (
 	// the transaction is known on the Bitcoin chain. This delay is needed
 	// as spreading the transaction over the Bitcoin network takes time.
 	depositSweepBroadcastCheckDelay = 1 * time.Minute
+	// MinSweepTxSatPerVByteFee mirrors tbtcpg.MinWalletTxSatPerVByteFee, the safe
+	// minimum sweep fee rate. It is duplicated here because pkg/tbtcpg imports
+	// pkg/tbtc, so this package cannot import the canonical constant without a
+	// dependency cycle; keep the two in sync. It is exported so the external
+	// tbtc_test package can compare it against the canonical tbtcpg value
+	// (guarded by TestSweepFeeConstantsMirrorTbtcpg). It backs a follower-side
+	// soft (log-only) check that the leader's proposed sweep fee is not below
+	// the floor (see threshold-network/keep-core#4171).
+	MinSweepTxSatPerVByteFee = 5
+	// DepositScriptByteSize mirrors tbtcpg.DepositScriptByteSize, the worst-case
+	// deposit script size used to estimate the sweep transaction virtual size.
+	// Exported alongside MinSweepTxSatPerVByteFee for the same cross-package
+	// drift guard.
+	DepositScriptByteSize = 126
 )
 
 // DepositSweepProposal represents a deposit sweep proposal issued by a
@@ -461,6 +475,53 @@ func ValidateDepositSweepProposal(
 		"deposit sweep proposal is valid",
 	)
 
+	// Follower-side soft check on the proposed fee. The on-chain
+	// WalletProposalValidator only bounds the sweep fee from above, not below,
+	// so a misbehaving or unpatched leader can propose a fee at the ~1 sat/vByte
+	// relay floor that this node would otherwise sign - the same underpricing
+	// that jams the wallet (see threshold-network/keep-core#4171). We recompute
+	// the safe minimum and warn if the proposal is below it.
+	//
+	// This is intentionally log-only, not a rejection: rejecting a below-floor
+	// proposal here would, during a mixed-version rollout, split signers (patched
+	// nodes reject, unpatched nodes sign) and could stall signing. Hard
+	// enforcement belongs on-chain in the WalletProposalValidator, or behind a
+	// coordinated all-nodes upgrade.
+	if sweepTxSize, sizeErr := bitcoin.NewTransactionSizeEstimator().
+		AddPublicKeyHashInputs(1, true).
+		AddScriptHashInputs(len(proposal.DepositsKeys), DepositScriptByteSize, true).
+		AddPublicKeyHashOutputs(1, true).
+		VirtualSize(); sizeErr != nil {
+		validateProposalLogger.Warnf(
+			"cannot estimate sweep tx size for the fee sanity check: [%v]",
+			sizeErr,
+		)
+	} else {
+		minSweepTxFee := big.NewInt(int64(MinSweepTxSatPerVByteFee) * sweepTxSize)
+
+		switch {
+		case proposal.SweepTxFee == nil:
+			validateProposalLogger.Warnf(
+				"proposal has no sweep tx fee set; expected at least the safe "+
+					"minimum [%d] ([%d] sat/vByte * [%d] vByte)",
+				minSweepTxFee,
+				MinSweepTxSatPerVByteFee,
+				sweepTxSize,
+			)
+		case proposal.SweepTxFee.Cmp(minSweepTxFee) < 0:
+			validateProposalLogger.Warnf(
+				"proposed sweep tx fee [%v] is below the safe minimum [%d] "+
+					"([%d] sat/vByte * [%d] vByte); the leader may be underpricing "+
+					"the sweep, which risks it getting stuck in the mempool and "+
+					"jamming the wallet",
+				proposal.SweepTxFee,
+				minSweepTxFee,
+				MinSweepTxSatPerVByteFee,
+				sweepTxSize,
+			)
+		}
+	}
+
 	deposits := make([]*Deposit, len(depositExtraInfo))
 	for i, dei := range depositExtraInfo {
 		deposits[i] = dei.Deposit

pkg/tbtc/sweep_fee_sync_test.go

@@ -0,0 +1,43 @@
+package tbtc_test
+
+import (
+	"testing"
+
+	"github.com/keep-network/keep-core/pkg/tbtc"
+	"github.com/keep-network/keep-core/pkg/tbtcpg"
+)
+
+// TestSweepFeeConstantsMirrorTbtcpg guards the sweep-fee constants that pkg/tbtc
+// duplicates from pkg/tbtcpg. The follower-side soft check
+// (threshold-network/keep-core#4171) recomputes the safe minimum sweep fee, but
+// pkg/tbtcpg imports pkg/tbtc, so pkg/tbtc cannot import the canonical constants
+// without a dependency cycle and hand-copies them instead.
+//
+// This test lives in the external tbtc_test package precisely because that
+// package can import both pkg/tbtc and pkg/tbtcpg without forming the cycle. It
+// compares the two actual constants directly - not against hand-copied literals
+// - so it fails whenever the pkg/tbtc mirror and the canonical tbtcpg value
+// drift apart, regardless of which side was changed. A literal-based guard
+// could be defeated by updating tbtcpg and the literal together while forgetting
+// the pkg/tbtc mirror; comparing the live values closes that gap.
+func TestSweepFeeConstantsMirrorTbtcpg(t *testing.T) {
+	if tbtc.MinSweepTxSatPerVByteFee != tbtcpg.MinWalletTxSatPerVByteFee {
+		t.Errorf(
+			"tbtc.MinSweepTxSatPerVByteFee [%d] has drifted from the canonical "+
+				"tbtcpg.MinWalletTxSatPerVByteFee [%d]; the follower soft check "+
+				"would warn at the wrong threshold",
+			tbtc.MinSweepTxSatPerVByteFee,
+			tbtcpg.MinWalletTxSatPerVByteFee,
+		)
+	}
+
+	if tbtc.DepositScriptByteSize != tbtcpg.DepositScriptByteSize {
+		t.Errorf(
+			"tbtc.DepositScriptByteSize [%d] has drifted from the canonical "+
+				"tbtcpg.DepositScriptByteSize [%d]; the follower soft check would "+
+				"estimate the sweep tx size incorrectly",
+			tbtc.DepositScriptByteSize,
+			tbtcpg.DepositScriptByteSize,
+		)
+	}
+}

pkg/tbtcpg/deposit_sweep.go

@@ -17,29 +17,10 @@ import (
 	"github.com/keep-network/keep-core/pkg/tbtc"
 )
 
-// Use the worst-case 126-byte deposit script with embedded extra data for estimation.
-// 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
+// DepositScriptByteSize is the worst-case 126-byte deposit script with embedded
+// extra data used for transaction size estimation. This ensures that deposit
+// sweep transaction fees are not underestimated.
+const DepositScriptByteSize = 126
 
 // DepositSweepLookBackBlocks is the look-back period in blocks used
 // when searching for submitted deposit-related events. It's equal to
@@ -510,7 +491,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)
@@ -581,7 +562,7 @@ func (dst *DepositSweepTask) ProposeDepositsSweep(
 //
 // 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
+// MinWalletTxSatPerVByteFee) required to avoid a stuck, unbumpable sweep would
 // itself exceed that Bridge maximum.
 func EstimateDepositsSweepFee(
 	chain Chain,
@@ -653,7 +634,7 @@ func estimateDepositsSweepFee(
 		// 1 P2WPKH main UTXO input.
 		AddPublicKeyHashInputs(1, true).
 		// depositsCount P2WSH deposit inputs.
-		AddScriptHashInputs(depositsCount, depositScriptByteSize, true).
+		AddScriptHashInputs(depositsCount, DepositScriptByteSize, true).
 		// 1 P2WPKH output.
 		AddPublicKeyHashOutputs(1, true).
 		VirtualSize()
@@ -677,45 +658,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

@@ -12,7 +12,7 @@ import (
 // with the given number of deposit inputs, mirroring the sizing that
 // EstimateDepositsSweepFee performs internally: 1 P2WPKH main-UTXO input,
 // depositsCount P2WSH deposit inputs, and 1 P2WPKH output. 126 ==
-// depositScriptByteSize.
+// DepositScriptByteSize.
 func sweepVirtualSize(t *testing.T, depositsCount int) int64 {
 	t.Helper()
 	size, err := bitcoin.NewTransactionSizeEstimator().
@@ -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,74 @@
+package tbtcpg
+
+import "fmt"
+
+// 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 for the transaction).
+//
+// It returns an error 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 and txVsize is the
+// estimated transaction virtual size, both in the usual sat / vByte units.
+//
+// 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(
+			"minimum safe transaction fee [%d] exceeds the maximum fee [%d]",
+			MinWalletTxSatPerVByteFee*txVsize,
+			maxTotalFee,
+		)
+	}
+
+	rate := estimatedFee / txVsize
+	rate = (rate*5 + 3) / 4 // ceil(rate * 1.25)
+	if rate < MinWalletTxSatPerVByteFee {
+		rate = MinWalletTxSatPerVByteFee
+	}
+
+	totalFee := rate * txVsize
+	if uint64(totalFee) > maxTotalFee {
+		totalFee = int64(maxTotalFee)
+	}
+
+	return totalFee, nil
+}