From ad89b38d9e27f21a1a90cddc7707963feb6b778a Mon Sep 17 00:00:00 2001 From: MacLane Wilkison Date: Mon, 20 Jul 2026 10:07:38 -0400 Subject: [PATCH 1/4] Apply the safe minimum sweep-fee floor to all wallet transactions Extract the 25% buffer + minimum floor + Bridge-max bound into a shared applyWalletTxFeeFloor helper and a shared minWalletTxSatPerVByteFee const, then apply it to redemptions, moving funds, and moved funds sweeps in addition to deposit sweeps. These are all non-RBF wallet transactions that jam the wallet if they get stuck at the relay floor, so the same protection applies (per lrsaturnino review on #4172). EstimateRedemptionFee now takes the redemption tx max total fee so the floor can be bounded by it; the caller fetches it from GetRedemptionParameters. deposit sweep fee estimation is refactored onto the shared helper with no behavior change. Co-Authored-By: Claude Fable 5 --- pkg/tbtcpg/deposit_sweep.go | 66 ++++------------------- pkg/tbtcpg/deposit_sweep_fee_test.go | 4 +- pkg/tbtcpg/fee.go | 74 +++++++++++++++++++++++++ pkg/tbtcpg/fee_test.go | 81 ++++++++++++++++++++++++++++ pkg/tbtcpg/moved_funds_sweep.go | 8 +++ pkg/tbtcpg/moved_funds_sweep_test.go | 16 ++++-- pkg/tbtcpg/moving_funds.go | 8 +++ pkg/tbtcpg/moving_funds_test.go | 10 ++-- pkg/tbtcpg/redemptions.go | 37 +++++++++++-- pkg/tbtcpg/redemptions_test.go | 70 ++++++++++++++++++++---- 10 files changed, 294 insertions(+), 80 deletions(-) create mode 100644 pkg/tbtcpg/fee.go create mode 100644 pkg/tbtcpg/fee_test.go diff --git a/pkg/tbtcpg/deposit_sweep.go b/pkg/tbtcpg/deposit_sweep.go index b5c2346a25..2872af7bc9 100644 --- a/pkg/tbtcpg/deposit_sweep.go +++ b/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,24 +657,9 @@ 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 @@ -702,20 +667,9 @@ func estimateDepositsSweepFee( // 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. diff --git a/pkg/tbtcpg/deposit_sweep_fee_test.go b/pkg/tbtcpg/deposit_sweep_fee_test.go index eb79da7b4d..185da44781 100644 --- a/pkg/tbtcpg/deposit_sweep_fee_test.go +++ b/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", }, } diff --git a/pkg/tbtcpg/fee.go b/pkg/tbtcpg/fee.go new file mode 100644 index 0000000000..2c95e57a74 --- /dev/null +++ b/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 +} diff --git a/pkg/tbtcpg/fee_test.go b/pkg/tbtcpg/fee_test.go new file mode 100644 index 0000000000..6aec062e44 --- /dev/null +++ b/pkg/tbtcpg/fee_test.go @@ -0,0 +1,81 @@ +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, + }, + "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, + ) + } + }) + } +} diff --git a/pkg/tbtcpg/moved_funds_sweep.go b/pkg/tbtcpg/moved_funds_sweep.go index 628669ed2a..9657950627 100644 --- a/pkg/tbtcpg/moved_funds_sweep.go +++ b/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 } diff --git a/pkg/tbtcpg/moved_funds_sweep_test.go b/pkg/tbtcpg/moved_funds_sweep_test.go index e0b1963381..85b43d4b19 100644 --- a/pkg/tbtcpg/moved_funds_sweep_test.go +++ b/pkg/tbtcpg/moved_funds_sweep_test.go @@ -365,7 +365,9 @@ func TestMovedFundsSweepAction_ProposeMovedFundsSweep(t *testing.T) { expectedProposal: &tbtc.MovedFundsSweepProposal{ MovingFundsTxHash: movingFundsTxHash, MovingFundsTxOutputIndex: movingFundsTxOutputIndex, - SweepTxFee: big.NewInt(4450), + // raw 4450 (178 vByte * 25 sat/vByte), buffered to + // ceil(25*1.25)=32 sat/vByte * 178 = 5696, below the 6000 cap. + SweepTxFee: big.NewInt(5696), }, }, } @@ -431,14 +433,18 @@ func TestEstimateMovedFundsSweepFee(t *testing.T) { "estimated fee correct, one input": { sweepTxMaxTotalFee: 3000, hasMainUtxo: false, - expectedFee: 1760, - expectedError: nil, + // raw 1760 (110 vByte * 16 sat/vByte), buffered to + // ceil(16*1.25)=20 sat/vByte * 110 = 2200, below the cap. + expectedFee: 2200, + expectedError: nil, }, "estimated fee correct, two inputs": { sweepTxMaxTotalFee: 3000, hasMainUtxo: true, - expectedFee: 2848, - expectedError: nil, + // raw 2848 (178 vByte * 16 sat/vByte); buffered 20 sat/vByte * 178 + // = 3560 exceeds the 3000 cap, so it is bounded down to the cap. + expectedFee: 3000, + expectedError: nil, }, "estimated fee too high": { sweepTxMaxTotalFee: 2500, diff --git a/pkg/tbtcpg/moving_funds.go b/pkg/tbtcpg/moving_funds.go index 22a842abc8..351687ad37 100644 --- a/pkg/tbtcpg/moving_funds.go +++ b/pkg/tbtcpg/moving_funds.go @@ -656,5 +656,13 @@ func EstimateMovingFundsFee( return 0, ErrFeeTooHigh } + // Enforce the safe minimum fee rate and buffer so a non-RBF moving funds + // transaction is never broadcast below the floor where it could get stuck + // and jam the wallet. + totalFee, err = applyWalletTxFeeFloor(totalFee, transactionSize, txMaxTotalFee) + if err != nil { + return 0, err + } + return totalFee, nil } diff --git a/pkg/tbtcpg/moving_funds_test.go b/pkg/tbtcpg/moving_funds_test.go index 4f24ae8ef7..2b3bc36f7d 100644 --- a/pkg/tbtcpg/moving_funds_test.go +++ b/pkg/tbtcpg/moving_funds_test.go @@ -568,8 +568,10 @@ func TestMovingFundsAction_ProposeMovingFunds(t *testing.T) { "fee estimated": { fee: 0, // trigger fee estimation expectedProposal: &tbtc.MovingFundsProposal{ - TargetWallets: targetWallets, - MovingFundsTxFee: big.NewInt(4300), + TargetWallets: targetWallets, + // raw 4300 (172 vByte * 25 sat/vByte), buffered to + // ceil(25*1.25)=32 sat/vByte * 172 = 5504, below the 6000 cap. + MovingFundsTxFee: big.NewInt(5504), }, }, } @@ -655,7 +657,9 @@ func TestEstimateMovingFundsFee(t *testing.T) { }{ "estimated fee correct": { txMaxTotalFee: 6000, - expectedFee: 3248, + // raw 3248 (203 vByte * 16 sat/vByte), buffered to + // ceil(16*1.25)=20 sat/vByte * 203 = 4060, below the cap. + expectedFee: 4060, expectedError: nil, }, "estimated fee too high": { diff --git a/pkg/tbtcpg/redemptions.go b/pkg/tbtcpg/redemptions.go index 981e9a8eb7..978d12e826 100644 --- a/pkg/tbtcpg/redemptions.go +++ b/pkg/tbtcpg/redemptions.go @@ -210,16 +210,25 @@ func (rt *RedemptionTask) ProposeRedemption( taskLogger.Infof("preparing a redemption proposal") - // Estimate fee if it's missing. Do not check the estimated fee against - // the maximum total and per-request fees allowed by the Bridge. This - // is done during the on-chain validation of the proposal so there is no - // need to do it here. + // Estimate fee if it's missing. The per-request maximum fee is still + // checked during the on-chain validation of the proposal; here we bound + // the estimate by the maximum total fee only so the safe-minimum floor + // (see EstimateRedemptionFee) cannot produce a fee the Bridge would reject. if fee <= 0 { taskLogger.Infof("estimating redemption transaction fee") + _, _, _, txMaxTotalFee, _, _, _, err := rt.chain.GetRedemptionParameters() + if err != nil { + return nil, fmt.Errorf( + "cannot get redemption tx max total fee: [%w]", + err, + ) + } + estimatedFee, err := EstimateRedemptionFee( rt.btcChain, redeemersOutputScripts, + txMaxTotalFee, ) if err != nil { return nil, fmt.Errorf( @@ -462,10 +471,14 @@ redemptionRequestedLoop: } // EstimateRedemptionFee estimates fee for the redemption transaction that pays -// the provided redeemers output scripts. +// the provided redeemers output scripts. The estimated fee is floored at a safe +// minimum rate and bounded above by txMaxTotalFee (the Bridge maximum), so a +// non-RBF redemption is never broadcast below the floor where it could get stuck +// and jam the wallet. func EstimateRedemptionFee( btcChain bitcoin.Chain, redeemersOutputScripts []bitcoin.Script, + txMaxTotalFee uint64, ) (int64, error) { sizeEstimator := bitcoin.NewTransactionSizeEstimator(). // 1 P2WPKH main UTXO input. @@ -500,5 +513,19 @@ func EstimateRedemptionFee( return 0, fmt.Errorf("cannot estimate transaction fee: [%v]", err) } + // A raw estimate already above the Bridge maximum means the redemption is + // uneconomical to perform at the required fee; return an error rather than + // clamping to the maximum and broadcasting an underpriced transaction. + if uint64(totalFee) > txMaxTotalFee { + return 0, fmt.Errorf("estimated fee exceeds the maximum fee") + } + + // Enforce the safe minimum fee rate and buffer, bounded by the Bridge + // maximum. + totalFee, err = applyWalletTxFeeFloor(totalFee, transactionSize, txMaxTotalFee) + if err != nil { + return 0, err + } + return totalFee, nil } diff --git a/pkg/tbtcpg/redemptions_test.go b/pkg/tbtcpg/redemptions_test.go index 8f61e2f94b..aa56fdd74b 100644 --- a/pkg/tbtcpg/redemptions_test.go +++ b/pkg/tbtcpg/redemptions_test.go @@ -3,6 +3,7 @@ package tbtcpg_test import ( "encoding/hex" "math/big" + "strings" "testing" "github.com/go-test/deep" @@ -24,9 +25,6 @@ func TestEstimateRedemptionFee(t *testing.T) { return bytes } - btcChain := tbtcpg.NewLocalBitcoinChain() - btcChain.SetEstimateSatPerVByteFee(1, 16) - redeemersOutputScripts := []bitcoin.Script{ fromHex("76a9142cd680318747b720d67bf4246eb7403b476adb3488ac"), // P2PKH fromHex("0014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0"), // P2WPKH @@ -34,13 +32,61 @@ func TestEstimateRedemptionFee(t *testing.T) { fromHex("0020ef0b4d985752aa5ef6243e4c6f6bebc2a007e7d671ef27d4b1d0db8dcc93bc1c"), // P2WSH } - actualFee, err := tbtcpg.EstimateRedemptionFee(btcChain, redeemersOutputScripts) - if err != nil { - t.Fatal(err) + // The fixture above yields a 250 vByte redemption transaction. + const vsize = 250 + + tests := map[string]struct { + estimateSatPerVByte int64 + txMaxTotalFee uint64 + expectedFee int + expectErrorContains string + }{ + "estimate above the floor is buffered by 25%": { + estimateSatPerVByte: 16, + txMaxTotalFee: 100000, + expectedFee: 5000, // ceil(16*1.25)=20 sat/vByte * 250 vByte + }, + "low estimate is raised to the minimum floor": { + estimateSatPerVByte: 1, + txMaxTotalFee: 100000, + expectedFee: 1250, // max(5, ceil(1*1.25)=2)=5 sat/vByte * 250 vByte + }, + "minimum floor above the cap returns an error": { + estimateSatPerVByte: 1, + txMaxTotalFee: uint64(3 * vsize), // below the 5 sat/vByte floor + expectErrorContains: "minimum safe transaction fee", + }, } - expectedFee := 4000 // transactionVirtualSize * satPerVByteFee = 250 * 16 = 4000 - testutils.AssertIntsEqual(t, "fee", expectedFee, int(actualFee)) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + btcChain := tbtcpg.NewLocalBitcoinChain() + btcChain.SetEstimateSatPerVByteFee(1, tc.estimateSatPerVByte) + + actualFee, err := tbtcpg.EstimateRedemptionFee( + btcChain, + redeemersOutputScripts, + tc.txMaxTotalFee, + ) + + if tc.expectErrorContains != "" { + if err == nil { + t.Fatalf("expected an error, got fee [%d]", actualFee) + } + if !strings.Contains(err.Error(), tc.expectErrorContains) { + t.Fatalf( + "expected error containing [%s]; got [%v]", + tc.expectErrorContains, err, + ) + } + return + } + if err != nil { + t.Fatal(err) + } + testutils.AssertIntsEqual(t, "fee", tc.expectedFee, int(actualFee)) + }) + } } func TestRedemptionAction_FindPendingRedemptions(t *testing.T) { @@ -168,7 +214,9 @@ func TestRedemptionAction_ProposeRedemption(t *testing.T) { fee: 0, // trigger fee estimation expectedProposal: &tbtc.RedemptionProposal{ RedeemersOutputScripts: redeemersOutputScripts, - RedemptionTxFee: big.NewInt(4300), + // raw 4300 (172 vByte * 25 sat/vByte), buffered to + // ceil(25*1.25)=32 sat/vByte * 172 = 5504, below the cap. + RedemptionTxFee: big.NewInt(5504), }, }, } @@ -180,6 +228,10 @@ func TestRedemptionAction_ProposeRedemption(t *testing.T) { btcChain.SetEstimateSatPerVByteFee(1, 25) + // Fee estimation bounds the safe-minimum floor by the redemption + // tx max total fee; set a cap comfortably above the buffered fee. + tbtcChain.SetRedemptionParameters(0, 0, 0, 6000, 0, nil, 0) + for _, script := range redeemersOutputScripts { tbtcChain.SetPendingRedemptionRequest( walletPublicKeyHash, From f76f4ba6f4912cf56bdd047b51257d9e4001fecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Wed, 22 Jul 2026 12:46:29 +0000 Subject: [PATCH 2/4] docs: correct fee-floor per-request cap claims The redemption fee estimator bounds the floored/buffered fee by the total cap (txMaxTotalFee) only. The prior comment claimed this made the floor unable to produce a fee the Bridge would reject, which is inaccurate: the per-request TxMaxFee (snapshotted per request) is a separate cap enforced only by on-chain validation, and the floor can raise a request's fee share above it while the total stays within bounds. Correct the ProposeRedemption and EstimateRedemptionFee comments, document that applyWalletTxFeeFloor bounds only the total fee (and its caller precondition), and note why the final clamp is floor-safe. Comments only; no behavior change. --- pkg/tbtcpg/fee.go | 13 ++++++++++++- pkg/tbtcpg/redemptions.go | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/pkg/tbtcpg/fee.go b/pkg/tbtcpg/fee.go index 2c95e57a74..854ce96142 100644 --- a/pkg/tbtcpg/fee.go +++ b/pkg/tbtcpg/fee.go @@ -29,13 +29,21 @@ const minWalletTxSatPerVByteFee = 5 // - 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). +// - bounds the result by maxTotalFee (the Bridge maximum total fee 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. // +// 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 @@ -65,6 +73,9 @@ func applyWalletTxFeeFloor( 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) diff --git a/pkg/tbtcpg/redemptions.go b/pkg/tbtcpg/redemptions.go index 978d12e826..e54f1a7b1a 100644 --- a/pkg/tbtcpg/redemptions.go +++ b/pkg/tbtcpg/redemptions.go @@ -210,10 +210,14 @@ func (rt *RedemptionTask) ProposeRedemption( taskLogger.Infof("preparing a redemption proposal") - // Estimate fee if it's missing. The per-request maximum fee is still - // checked during the on-chain validation of the proposal; here we bound - // the estimate by the maximum total fee only so the safe-minimum floor - // (see EstimateRedemptionFee) cannot produce a fee the Bridge would reject. + // Estimate fee if it's missing. Here we bound the estimate by the maximum + // total fee only. The per-request maximum fee (TxMaxFee, snapshotted per + // request at request creation) is enforced solely by the on-chain + // validation of the proposal and is not checked here. Note that the + // safe-minimum floor (see EstimateRedemptionFee) raises the total fee and + // therefore each request's fee share, so it can push a share above that + // request's per-request maximum even while the total stays within + // txMaxTotalFee; such a proposal is rejected only by on-chain validation. if fee <= 0 { taskLogger.Infof("estimating redemption transaction fee") @@ -472,9 +476,10 @@ redemptionRequestedLoop: // EstimateRedemptionFee estimates fee for the redemption transaction that pays // the provided redeemers output scripts. The estimated fee is floored at a safe -// minimum rate and bounded above by txMaxTotalFee (the Bridge maximum), so a -// non-RBF redemption is never broadcast below the floor where it could get stuck -// and jam the wallet. +// minimum rate and bounded above by txMaxTotalFee (the Bridge total-fee +// maximum), so a non-RBF redemption is not proposed below the floor where it +// could get stuck and jam the wallet. Only the total fee is bounded here; the +// per-request maximum fee is enforced separately by on-chain validation. func EstimateRedemptionFee( btcChain bitcoin.Chain, redeemersOutputScripts []bitcoin.Script, From 6a7060800aa230b66058d2e22b06638efd72e3de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 23 Jul 2026 11:03:10 +0000 Subject: [PATCH 3/4] chore(tbtcpg): address fee-floor review feedback Restore the truncation invariant comment dropped when the deposit-sweep fee logic was extracted into applyWalletTxFeeFloor: the 25% buffer is applied to the truncated per-vByte rate, which is lossless only because EstimateFee returns an exact multiple of the vsize. Also correct the estimatedFee/txVsize unit description (satoshis and vBytes, not sat/vByte). Emit a distinct warning in ProposeRedemption when the floored per-request fee share would exceed the per-request maximum fee, so operators can tell this apart from a generic on-chain validation failure. Fee behavior is unchanged. Add coverage for previously untested branches: the redemption raw-estimate-above-cap error, and the minimum-floor clamp on the moving-funds and moved-funds-sweep paths. --- pkg/tbtcpg/fee.go | 12 +++++++-- pkg/tbtcpg/moved_funds_sweep_test.go | 39 ++++++++++++++++++---------- pkg/tbtcpg/moving_funds_test.go | 27 +++++++++++++------ pkg/tbtcpg/redemptions.go | 21 ++++++++++++++- pkg/tbtcpg/redemptions_test.go | 5 ++++ 5 files changed, 80 insertions(+), 24 deletions(-) diff --git a/pkg/tbtcpg/fee.go b/pkg/tbtcpg/fee.go index 854ce96142..29e7e8840e 100644 --- a/pkg/tbtcpg/fee.go +++ b/pkg/tbtcpg/fee.go @@ -34,8 +34,16 @@ const minWalletTxSatPerVByteFee = 5 // // 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. +// 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 diff --git a/pkg/tbtcpg/moved_funds_sweep_test.go b/pkg/tbtcpg/moved_funds_sweep_test.go index 85b43d4b19..e67a641914 100644 --- a/pkg/tbtcpg/moved_funds_sweep_test.go +++ b/pkg/tbtcpg/moved_funds_sweep_test.go @@ -425,39 +425,52 @@ func TestMovedFundsSweepAction_ProposeMovedFundsSweep(t *testing.T) { func TestEstimateMovedFundsSweepFee(t *testing.T) { var tests = map[string]struct { - sweepTxMaxTotalFee uint64 - hasMainUtxo bool - expectedFee uint64 - expectedError error + estimateSatPerVByte int64 + sweepTxMaxTotalFee uint64 + hasMainUtxo bool + expectedFee uint64 + expectedError error }{ "estimated fee correct, one input": { - sweepTxMaxTotalFee: 3000, - hasMainUtxo: false, + estimateSatPerVByte: 16, + sweepTxMaxTotalFee: 3000, + hasMainUtxo: false, // raw 1760 (110 vByte * 16 sat/vByte), buffered to // ceil(16*1.25)=20 sat/vByte * 110 = 2200, below the cap. expectedFee: 2200, expectedError: nil, }, "estimated fee correct, two inputs": { - sweepTxMaxTotalFee: 3000, - hasMainUtxo: true, + estimateSatPerVByte: 16, + sweepTxMaxTotalFee: 3000, + hasMainUtxo: true, // raw 2848 (178 vByte * 16 sat/vByte); buffered 20 sat/vByte * 178 // = 3560 exceeds the 3000 cap, so it is bounded down to the cap. expectedFee: 3000, expectedError: nil, }, + "low estimate is raised to the minimum floor": { + estimateSatPerVByte: 1, + sweepTxMaxTotalFee: 3000, + hasMainUtxo: false, + // raw 110 (110 vByte * 1 sat/vByte), buffered ceil(1*1.25)=2 is + // below the 5 sat/vByte floor, so clamped to 5 * 110 = 550. + expectedFee: 550, + expectedError: nil, + }, "estimated fee too high": { - sweepTxMaxTotalFee: 2500, - hasMainUtxo: true, - expectedFee: 0, - expectedError: tbtcpg.ErrSweepTxFeeTooHigh, + estimateSatPerVByte: 16, + sweepTxMaxTotalFee: 2500, + hasMainUtxo: true, + expectedFee: 0, + expectedError: tbtcpg.ErrSweepTxFeeTooHigh, }, } for testName, test := range tests { t.Run(testName, func(t *testing.T) { btcChain := tbtcpg.NewLocalBitcoinChain() - btcChain.SetEstimateSatPerVByteFee(1, 16) + btcChain.SetEstimateSatPerVByteFee(1, test.estimateSatPerVByte) actualFee, err := tbtcpg.EstimateMovedFundsSweepFee( btcChain, diff --git a/pkg/tbtcpg/moving_funds_test.go b/pkg/tbtcpg/moving_funds_test.go index 2b3bc36f7d..405f154193 100644 --- a/pkg/tbtcpg/moving_funds_test.go +++ b/pkg/tbtcpg/moving_funds_test.go @@ -651,28 +651,39 @@ func TestMovingFundsAction_ProposeMovingFunds(t *testing.T) { func TestEstimateMovingFundsFee(t *testing.T) { var tests = map[string]struct { - txMaxTotalFee uint64 - expectedFee uint64 - expectedError error + estimateSatPerVByte int64 + txMaxTotalFee uint64 + expectedFee uint64 + expectedError error }{ "estimated fee correct": { - txMaxTotalFee: 6000, + estimateSatPerVByte: 16, + txMaxTotalFee: 6000, // raw 3248 (203 vByte * 16 sat/vByte), buffered to // ceil(16*1.25)=20 sat/vByte * 203 = 4060, below the cap. expectedFee: 4060, expectedError: nil, }, + "low estimate is raised to the minimum floor": { + estimateSatPerVByte: 1, + txMaxTotalFee: 6000, + // raw 203 (203 vByte * 1 sat/vByte), buffered ceil(1*1.25)=2 is + // below the 5 sat/vByte floor, so clamped to 5 * 203 = 1015. + expectedFee: 1015, + expectedError: nil, + }, "estimated fee too high": { - txMaxTotalFee: 3000, - expectedFee: 0, - expectedError: tbtcpg.ErrFeeTooHigh, + estimateSatPerVByte: 16, + txMaxTotalFee: 3000, + expectedFee: 0, + expectedError: tbtcpg.ErrFeeTooHigh, }, } for testName, test := range tests { t.Run(testName, func(t *testing.T) { btcChain := tbtcpg.NewLocalBitcoinChain() - btcChain.SetEstimateSatPerVByteFee(1, 16) + btcChain.SetEstimateSatPerVByteFee(1, test.estimateSatPerVByte) targetWalletsCount := 4 diff --git a/pkg/tbtcpg/redemptions.go b/pkg/tbtcpg/redemptions.go index e54f1a7b1a..f09e263065 100644 --- a/pkg/tbtcpg/redemptions.go +++ b/pkg/tbtcpg/redemptions.go @@ -221,7 +221,7 @@ func (rt *RedemptionTask) ProposeRedemption( if fee <= 0 { taskLogger.Infof("estimating redemption transaction fee") - _, _, _, txMaxTotalFee, _, _, _, err := rt.chain.GetRedemptionParameters() + _, _, txMaxFee, txMaxTotalFee, _, _, _, err := rt.chain.GetRedemptionParameters() if err != nil { return nil, fmt.Errorf( "cannot get redemption tx max total fee: [%w]", @@ -242,6 +242,25 @@ func (rt *RedemptionTask) ProposeRedemption( } fee = estimatedFee + + // The safe-minimum floor raises the total fee and therefore each + // request's even fee share (~ totalFee / requestsCount). When that + // share exceeds the per-request maximum fee, on-chain validation + // rejects the whole proposal and the batch is aborted with no + // lower-fee retry. Emit a distinct warning so operators can tell this + // apart from a generic validation failure. txMaxFee is the current + // governance parameter; the actually enforced cap is the value + // snapshotted per request at creation, so this is a best-effort + // diagnostic rather than an exact predictor. + if share := fee / int64(len(redeemersOutputScripts)); uint64(share) > txMaxFee { + taskLogger.Warnf( + "floored redemption fee share [%d] exceeds the per-request "+ + "maximum fee [%d]; the proposal will likely be rejected "+ + "by on-chain validation", + share, + txMaxFee, + ) + } } taskLogger.Infof("redemption transaction fee: [%d]", fee) diff --git a/pkg/tbtcpg/redemptions_test.go b/pkg/tbtcpg/redemptions_test.go index aa56fdd74b..2fc03911f2 100644 --- a/pkg/tbtcpg/redemptions_test.go +++ b/pkg/tbtcpg/redemptions_test.go @@ -56,6 +56,11 @@ func TestEstimateRedemptionFee(t *testing.T) { txMaxTotalFee: uint64(3 * vsize), // below the 5 sat/vByte floor expectErrorContains: "minimum safe transaction fee", }, + "raw estimate above the cap returns an error": { + estimateSatPerVByte: 16, // raw 16*250 = 4000 + txMaxTotalFee: 3000, // below the raw estimate + expectErrorContains: "estimated fee exceeds the maximum fee", + }, } for name, tc := range tests { From 1128d434d7d1f1d592683ca3222e7dccb105439a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Fri, 24 Jul 2026 05:56:42 +0000 Subject: [PATCH 4/4] chore(tbtcpg): sharpen fee-floor error typing and per-request fee warning Address review findings on the wallet fee-floor work: - Add an exported ErrMaxFeeTooLow sentinel returned by applyWalletTxFeeFloor when the safe minimum floor exceeds the Bridge maximum, mirroring the existing ErrFeeTooHigh / ErrSweepTxFeeTooHigh raw-estimate sentinels so the floor-too-high condition is programmatically distinguishable. - Base the redemption per-request fee warning on the worst-case (last-request) share floor(total/count)+total%count, matching the on-chain fee distribution, instead of the even floor share that could miss a remainder-only rejection. - Cover the floor-exceeds-cap error path in the moving-funds and moved-funds-sweep estimators, add exact-cap boundary cases to the fee-floor helper test, and assert the per-request fee warning fires on the last-request share. --- pkg/tbtcpg/fee.go | 25 ++++-- pkg/tbtcpg/fee_test.go | 12 +++ pkg/tbtcpg/moved_funds_sweep_test.go | 10 +++ pkg/tbtcpg/moving_funds_test.go | 9 ++ pkg/tbtcpg/redemptions.go | 24 ++++-- pkg/tbtcpg/redemptions_test.go | 122 +++++++++++++++++++++++++++ 6 files changed, 187 insertions(+), 15 deletions(-) diff --git a/pkg/tbtcpg/fee.go b/pkg/tbtcpg/fee.go index 29e7e8840e..b1e2acc43e 100644 --- a/pkg/tbtcpg/fee.go +++ b/pkg/tbtcpg/fee.go @@ -1,6 +1,18 @@ package tbtcpg -import "fmt" +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 @@ -32,10 +44,10 @@ const minWalletTxSatPerVByteFee = 5 // - bounds the result by maxTotalFee (the Bridge maximum total fee 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 in satoshis and txVsize -// is the estimated transaction virtual size in vBytes. +// 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 @@ -69,7 +81,8 @@ func applyWalletTxFeeFloor( // 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]", + "%w: minimum fee [%d], maximum fee [%d]", + ErrMaxFeeTooLow, minWalletTxSatPerVByteFee*txVsize, maxTotalFee, ) diff --git a/pkg/tbtcpg/fee_test.go b/pkg/tbtcpg/fee_test.go index 6aec062e44..cdb522c53e 100644 --- a/pkg/tbtcpg/fee_test.go +++ b/pkg/tbtcpg/fee_test.go @@ -33,6 +33,18 @@ func TestApplyWalletTxFeeFloor(t *testing.T) { 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, diff --git a/pkg/tbtcpg/moved_funds_sweep_test.go b/pkg/tbtcpg/moved_funds_sweep_test.go index e67a641914..d010b6613f 100644 --- a/pkg/tbtcpg/moved_funds_sweep_test.go +++ b/pkg/tbtcpg/moved_funds_sweep_test.go @@ -465,6 +465,16 @@ func TestEstimateMovedFundsSweepFee(t *testing.T) { expectedFee: 0, expectedError: tbtcpg.ErrSweepTxFeeTooHigh, }, + "minimum floor exceeds the max total fee": { + estimateSatPerVByte: 1, + // raw 110 (110 vByte * 1 sat/vByte) is below the cap, so it passes + // the raw-estimate guard, but the 5 sat/vByte floor total (550) + // exceeds the cap, so a safe sweep cannot be built. + sweepTxMaxTotalFee: 400, + hasMainUtxo: false, + expectedFee: 0, + expectedError: tbtcpg.ErrMaxFeeTooLow, + }, } for testName, test := range tests { diff --git a/pkg/tbtcpg/moving_funds_test.go b/pkg/tbtcpg/moving_funds_test.go index 405f154193..c0b8a1913f 100644 --- a/pkg/tbtcpg/moving_funds_test.go +++ b/pkg/tbtcpg/moving_funds_test.go @@ -678,6 +678,15 @@ func TestEstimateMovingFundsFee(t *testing.T) { expectedFee: 0, expectedError: tbtcpg.ErrFeeTooHigh, }, + "minimum floor exceeds the max total fee": { + estimateSatPerVByte: 1, + // raw 203 (203 vByte * 1 sat/vByte) is below the cap, so it passes + // the raw-estimate guard, but the 5 sat/vByte floor total (1015) + // exceeds the cap, so a safe transaction cannot be built. + txMaxTotalFee: 500, + expectedFee: 0, + expectedError: tbtcpg.ErrMaxFeeTooLow, + }, } for testName, test := range tests { diff --git a/pkg/tbtcpg/redemptions.go b/pkg/tbtcpg/redemptions.go index f09e263065..6587d2dffe 100644 --- a/pkg/tbtcpg/redemptions.go +++ b/pkg/tbtcpg/redemptions.go @@ -244,20 +244,26 @@ func (rt *RedemptionTask) ProposeRedemption( fee = estimatedFee // The safe-minimum floor raises the total fee and therefore each - // request's even fee share (~ totalFee / requestsCount). When that - // share exceeds the per-request maximum fee, on-chain validation - // rejects the whole proposal and the batch is aborted with no - // lower-fee retry. Emit a distinct warning so operators can tell this - // apart from a generic validation failure. txMaxFee is the current - // governance parameter; the actually enforced cap is the value - // snapshotted per request at creation, so this is a best-effort + // request's fee share. When a share exceeds the per-request maximum + // fee, on-chain validation rejects the whole proposal and the batch is + // aborted with no lower-fee retry. Emit a distinct warning so operators + // can tell this apart from a generic validation failure. The largest + // share is the worst case to check: the on-chain fee distribution + // (see withRedemptionTotalFee) splits the total evenly and assigns the + // division remainder to the last request, so that request pays + // floor(total/count) + total%count. Checking only the even floor share + // would miss a rejection caused solely by the remainder. txMaxFee is + // the current governance parameter; the actually enforced cap is the + // value snapshotted per request at creation, so this is a best-effort // diagnostic rather than an exact predictor. - if share := fee / int64(len(redeemersOutputScripts)); uint64(share) > txMaxFee { + requestsCount := int64(len(redeemersOutputScripts)) + maxShare := fee/requestsCount + fee%requestsCount + if uint64(maxShare) > txMaxFee { taskLogger.Warnf( "floored redemption fee share [%d] exceeds the per-request "+ "maximum fee [%d]; the proposal will likely be rejected "+ "by on-chain validation", - share, + maxShare, txMaxFee, ) } diff --git a/pkg/tbtcpg/redemptions_test.go b/pkg/tbtcpg/redemptions_test.go index 2fc03911f2..2aef02fa73 100644 --- a/pkg/tbtcpg/redemptions_test.go +++ b/pkg/tbtcpg/redemptions_test.go @@ -2,6 +2,7 @@ package tbtcpg_test import ( "encoding/hex" + "fmt" "math/big" "strings" "testing" @@ -273,3 +274,124 @@ func TestRedemptionAction_ProposeRedemption(t *testing.T) { }) } } + +// warnCapturingLogger records Warnf messages so tests can assert on the +// per-request fee warning emitted during redemption fee estimation. All other +// log methods are inherited as no-ops from testutils.MockLogger. +type warnCapturingLogger struct { + testutils.MockLogger + warnings []string +} + +func (l *warnCapturingLogger) Warnf(format string, args ...interface{}) { + l.warnings = append(l.warnings, fmt.Sprintf(format, args...)) +} + +// TestRedemptionAction_ProposeRedemption_PerRequestFeeWarning verifies that the +// diagnostic warning about a floored fee share exceeding the per-request maximum +// fee (TxMaxFee) is emitted for the worst-case (largest) share. The on-chain fee +// distribution assigns the division remainder to the last request, so that +// request pays floor(total/count)+total%count. The warning must reflect that +// last-request share, not the smaller even share. +func TestRedemptionAction_ProposeRedemption_PerRequestFeeWarning(t *testing.T) { + fromHex := func(hexString string) []byte { + bytes, err := hex.DecodeString(hexString) + if err != nil { + t.Fatal(err) + } + return bytes + } + + var walletPublicKeyHash [20]byte + copy(walletPublicKeyHash[:], fromHex("")) + + // Three redeemer output scripts make the estimated fee (6496 at 25 sat/vByte + // buffered to 32) indivisible by the request count: the even share is + // 6496/3 = 2165 and the last request pays 2165 + 6496%3 = 2166. + redeemersOutputScripts := []bitcoin.Script{ + fromHex("00140000000000000000000000000000000000000001"), + fromHex("00140000000000000000000000000000000000000002"), + fromHex("00140000000000000000000000000000000000000003"), + } + + var tests = map[string]struct { + txMaxFee uint64 + expectWarning bool + }{ + "worst-case share within the per-request cap": { + txMaxFee: 3000, // 2166 <= 3000 + expectWarning: false, + }, + "even share at the cap but last-request share exceeds it": { + // The even share 2165 equals the cap (a floor-division check would + // not warn), but the last request pays 2166 and would be rejected. + txMaxFee: 2165, + expectWarning: true, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + tbtcChain := tbtcpg.NewLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + btcChain.SetEstimateSatPerVByteFee(1, 25) + + // txMaxFee at index 2; txMaxTotalFee at index 3, set comfortably + // above the estimated total (6496) so it does not bound the fee. + tbtcChain.SetRedemptionParameters(0, 0, test.txMaxFee, 8000, 0, nil, 0) + + for _, script := range redeemersOutputScripts { + tbtcChain.SetPendingRedemptionRequest( + walletPublicKeyHash, + &tbtc.RedemptionRequest{ + RedeemerOutputScript: script, + }, + ) + } + + expectedProposal := &tbtc.RedemptionProposal{ + RedeemersOutputScripts: redeemersOutputScripts, + RedemptionTxFee: big.NewInt(6496), + } + + err := tbtcChain.SetRedemptionProposalValidationResult( + walletPublicKeyHash, + expectedProposal, + true, + ) + if err != nil { + t.Fatal(err) + } + + task := tbtcpg.NewRedemptionTask(tbtcChain, btcChain) + + logger := &warnCapturingLogger{} + + _, err = task.ProposeRedemption( + logger, + walletPublicKeyHash, + redeemersOutputScripts, + 0, // trigger fee estimation + ) + if err != nil { + t.Fatal(err) + } + + warned := false + for _, w := range logger.warnings { + if strings.Contains(w, "exceeds the per-request maximum fee") { + warned = true + break + } + } + + if warned != test.expectWarning { + t.Errorf( + "per-request fee warning emitted = %v, want %v\nwarnings: %v", + warned, test.expectWarning, logger.warnings, + ) + } + }) + } +}