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

[mswilkison]

Follows up on #4172 / #4179 (lrsaturnino review): the fee floor is enforced on the fee-generation side, but nothing enforces it on the follower/validation side. ValidateDepositSweepProposal delegates to the chain validator, and the on-chain WalletProposalValidator only bounds the sweep fee from above (plus fee > 0) — no minimum. So a misbehaving or unpatched coordination leader can propose a sweep at the ~1 sat/vByte relay floor and get patched followers to sign it — the #4171 jam scenario.

Stacked on #4179 → #4172 (based on branch feat/floor-all-wallet-tx-fees), so this PR's diff shows only its incremental change: the follower-side soft check in pkg/tbtc/deposit_sweep.go. Merge #4172 then #4179 first.

Change

ValidateDepositSweepProposal now recomputes the safe minimum sweep fee (same vsize estimate + floor as the generator) and logs a warning if the proposed fee 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 below threshold. This PR gives operators detection of an underpricing leader without a liveness risk.

Why not hard enforcement here?

Hard enforcement of a fee minimum belongs where all signers apply the same rule with no version skew — on-chain in WalletProposalValidator (tbtc-v2 repo), or behind a coordinated all-nodes keep-client upgrade. This PR is the safe keep-client-side increment; the on-chain change is the tracked follow-up.

Notes

• The floor constant and deposit-script size are mirrored into pkg/tbtc (with keep-in-sync comments) because pkg/tbtcpg imports pkg/tbtc, so this package cannot import the canonical values without a dependency cycle.
• The check is log-only and its arithmetic mirrors the (tested) generator estimator, so no bespoke test is added — a dedicated test would require standing up the full ValidateDepositSweepProposal mock harness purely to assert a log line.

Summary by CodeRabbit

• Improvements
  • Improved transaction fee estimation for deposits, redemptions, moved funds, and moving funds.
  • Added minimum safety floors and buffering to help prevent underpriced transactions.
  • Fee estimates now respect configured maximum limits and report an error when safe fees cannot fit within those limits.
  • Added validation warnings for unusually low proposed deposit sweep fees without rejecting valid proposals.
• Tests
  • Expanded coverage for fee floors, buffers, maximum limits, and error conditions.

[coderabbitai]

📝 Walkthrough

Walkthrough

Fee estimation now applies a buffered minimum wallet fee rate with maximum-fee bounds across deposit sweeps, moved funds, moving funds, and redemptions. Deposit sweep validation also logs underpriced proposals without rejecting them, with updated tests and fixtures.

Changes

Wallet transaction fee floor

Layer / File(s)	Summary
Fee floor calculation pkg/tbtcpg/fee.go, pkg/tbtcpg/fee_test.go	Adds the minimum wallet fee-rate constant and shared logic for buffering, floor enforcement, invalid-size errors, and maximum-fee caps.
Deposit sweep fee enforcement pkg/tbtcpg/deposit_sweep.go, pkg/tbtcpg/deposit_sweep_fee_test.go, pkg/tbtcpg/internal/test/testdata/*	Exports the deposit script-size constant, applies the shared fee floor to deposit sweep estimates, and updates coverage and expected proposal fees.
Moved and moving funds fee enforcement pkg/tbtcpg/moved_funds_sweep.go, pkg/tbtcpg/moved_funds_sweep_test.go, pkg/tbtcpg/moving_funds.go, pkg/tbtcpg/moving_funds_test.go	Applies the shared fee floor to moved-funds and moving-funds estimates and updates fee expectations.
Redemption fee bounds pkg/tbtcpg/redemptions.go, pkg/tbtcpg/redemptions_test.go	Passes the Bridge maximum fee into redemption estimation, rejects over-cap raw estimates, and applies the shared floor with expanded tests.
Follower sweep fee warning pkg/tbtc/deposit_sweep.go, pkg/tbtc/sweep_fee_sync_test.go	Adds log-only detection for underpriced proposed sweep fees and verifies mirrored fee constants.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProposalEstimator
  participant applyWalletTxFeeFloor
  participant BridgeParameters
  participant FeeProposal
  ProposalEstimator->>BridgeParameters: read maximum total fee
  ProposalEstimator->>applyWalletTxFeeFloor: raw fee, transaction vsize, maximum fee
  applyWalletTxFeeFloor->>FeeProposal: buffered, floored, capped fee

Loading

Possibly related PRs

• threshold-network/keep-core#4172: Both changes update deposit sweep fee-floor handling and low-fee sweep validation.

Suggested reviewers: lrsaturnino, piotr-roslaniec

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: a follower-side, log-only check for sweep fees below the safe floor.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch feat/follower-sweep-fee-soft-check

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)

pkg/tbtcpg/internal/test/testdata/propose_sweep_scenario_0.json

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

pkg/tbtcpg/moved_funds_sweep_test.go (1)

436-447: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add integration coverage for every fee-floor outcome.

The updated tests exercise buffering and capping, but neither estimator verifies the minimum-rate floor or the error returned when the minimum floor cannot fit within the maximum fee.

• pkg/tbtcpg/moved_funds_sweep_test.go#L436-L447: add low-estimate and floor-over-cap cases.
• pkg/tbtcpg/moving_funds_test.go#L660-L662: add the same minimum-floor and floor-over-cap coverage.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtcpg/moved_funds_sweep_test.go` around lines 436 - 447, Add integration
test cases in pkg/tbtcpg/moved_funds_sweep_test.go (lines 436-447) and
pkg/tbtcpg/moving_funds_test.go (lines 660-662) covering both a low estimate
raised to the minimum fee-rate floor and a maximum fee below that floor,
asserting the expected fee and returned error for each outcome. Use the existing
estimator test cases and symbols in each file, with no production-code changes.

pkg/tbtc/sweep_fee_sync_test.go (1)

21-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This does not actually detect a stale pkg/tbtc mirror.

The assertions compare canonical values only with test literals; changing pkg/tbtc/deposit_sweep.go to a different private value still passes. Add a behavior-level test for ValidateDepositSweepProposal using canonical tbtcpg values: no warning exactly at the threshold and a warning one satoshi below it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtc/sweep_fee_sync_test.go` around lines 21 - 45, Extend
TestSweepFeeConstantsMirrorTbtcpg with a behavior-level test for
ValidateDepositSweepProposal that uses canonical tbtcpg values, asserting no
warning at the exact minimum threshold and a warning when the value is one
satoshi below it. Ensure the test exercises the pkg/tbtc mirror rather than
comparing only against duplicated literals.

pkg/tbtcpg/redemptions_test.go (1)

44-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing coverage for the new "raw estimate exceeds cap" branch.

EstimateRedemptionFee (pkg/tbtcpg/redemptions.go, Lines 516-521) now errors early when the raw, unbuffered estimate already exceeds txMaxTotalFee ("estimated fee exceeds the maximum fee"). None of the three table cases exercise this branch — the existing error case instead hits the floor-vs-cap check inside applyWalletTxFeeFloor. Consider adding a case where estimateSatPerVByte * vsize > txMaxTotalFee.

Example additional test case

"raw estimate above the cap returns an error": {
    estimateSatPerVByte: 500,
    txMaxTotalFee:       1000, // below raw 500*250=125000
    expectErrorContains: "estimated fee exceeds the maximum fee",
},

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtcpg/redemptions_test.go` around lines 44 - 59, Add a table-driven case
in the redemption fee tests for the raw-estimate cap branch in
EstimateRedemptionFee: choose estimateSatPerVByte and txMaxTotalFee values where
estimateSatPerVByte multiplied by vsize exceeds the cap, and assert the error
contains "estimated fee exceeds the maximum fee". Keep the existing floor-vs-cap
error case unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/tbtcpg/fee.go`:
- Around line 62-68: Update the fee calculation around rate and totalFee so the
25% buffer is applied before integer division truncates fractional estimates,
preserving ceil(estimatedFee × 1.25). Then apply MinWalletTxSatPerVByteFee as
the minimum rate and retain the existing total fee calculation and cap behavior.

---

Nitpick comments:
In `@pkg/tbtc/sweep_fee_sync_test.go`:
- Around line 21-45: Extend TestSweepFeeConstantsMirrorTbtcpg with a
behavior-level test for ValidateDepositSweepProposal that uses canonical tbtcpg
values, asserting no warning at the exact minimum threshold and a warning when
the value is one satoshi below it. Ensure the test exercises the pkg/tbtc mirror
rather than comparing only against duplicated literals.

In `@pkg/tbtcpg/moved_funds_sweep_test.go`:
- Around line 436-447: Add integration test cases in
pkg/tbtcpg/moved_funds_sweep_test.go (lines 436-447) and
pkg/tbtcpg/moving_funds_test.go (lines 660-662) covering both a low estimate
raised to the minimum fee-rate floor and a maximum fee below that floor,
asserting the expected fee and returned error for each outcome. Use the existing
estimator test cases and symbols in each file, with no production-code changes.

In `@pkg/tbtcpg/redemptions_test.go`:
- Around line 44-59: Add a table-driven case in the redemption fee tests for the
raw-estimate cap branch in EstimateRedemptionFee: choose estimateSatPerVByte and
txMaxTotalFee values where estimateSatPerVByte multiplied by vsize exceeds the
cap, and assert the error contains "estimated fee exceeds the maximum fee". Keep
the existing floor-vs-cap error case unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f91cfed-17a6-4b1f-a1e5-5375293c8f4d

📥 Commits

Reviewing files that changed from the base of the PR and between f041178 and d6bba6f.

📒 Files selected for processing (13)

• pkg/tbtc/deposit_sweep.go
• pkg/tbtc/sweep_fee_sync_test.go
• pkg/tbtcpg/deposit_sweep.go
• pkg/tbtcpg/deposit_sweep_fee_test.go
• pkg/tbtcpg/fee.go
• pkg/tbtcpg/fee_test.go
• pkg/tbtcpg/internal/test/testdata/propose_sweep_scenario_0.json
• pkg/tbtcpg/moved_funds_sweep.go
• pkg/tbtcpg/moved_funds_sweep_test.go
• pkg/tbtcpg/moving_funds.go
• pkg/tbtcpg/moving_funds_test.go
• pkg/tbtcpg/redemptions.go
• pkg/tbtcpg/redemptions_test.go

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the 25% buffer for fractional fee rates.

Integer-dividing before buffering underprices non-integral estimates. For estimatedFee=999 and txVsize=200, this returns 1000, while ceil(999 * 1.25) is 1249. Buffer the total fee directly (or round the raw rate up before applying the buffer), then apply the floor and cap.

Proposed fix

-	rate := estimatedFee / txVsize
-	rate = (rate*5 + 3) / 4 // ceil(rate * 1.25)
-	if rate < MinWalletTxSatPerVByteFee {
-		rate = MinWalletTxSatPerVByteFee
-	}
-
-	totalFee := rate * txVsize
+	floorFee := MinWalletTxSatPerVByteFee * txVsize
+	bufferedFee := estimatedFee + (estimatedFee+3)/4 // ceil(estimatedFee * 1.25)
+	totalFee := bufferedFee
+	if totalFee < floorFee {
+		totalFee = floorFee
+	}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	rate := estimatedFee / txVsize
	rate = (rate*5 + 3) / 4 // ceil(rate * 1.25)
	if rate < MinWalletTxSatPerVByteFee {
	rate = MinWalletTxSatPerVByteFee
	}
	totalFee := rate * txVsize
	floorFee := MinWalletTxSatPerVByteFee * txVsize
	bufferedFee := estimatedFee + (estimatedFee+3)/4 // ceil(estimatedFee * 1.25)
	totalFee := bufferedFee
	if totalFee < floorFee {
	totalFee = floorFee
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtcpg/fee.go` around lines 62 - 68, Update the fee calculation around
rate and totalFee so the 25% buffer is applied before integer division truncates
fractional estimates, preserving ceil(estimatedFee × 1.25). Then apply
MinWalletTxSatPerVByteFee as the minimum rate and retain the existing total fee
calculation and cap behavior.

[piotr-roslaniec]

Superseded by #4194. This branch's head lived on a fork that drifted onto an old main and went CONFLICTING. #4194 is the same work (follower-side soft check + review follow-ups) rebased clean onto current main. Continuing there.