From 29348a94df3626d65a0eb6606706e1601eed9051 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 22 Jul 2025 11:23:58 +0200 Subject: [PATCH] sweepbatcher: consider min relay fee when constructing batch tx if constructUnsignedTx constructs a batch transaction that is below the minimum relay fee, an error is returned. --- loopout_test.go | 1 + sweepbatcher/presigned.go | 38 +++++-- sweepbatcher/presigned_test.go | 8 +- sweepbatcher/sweep_batch.go | 57 +++++++---- sweepbatcher/sweep_batch_test.go | 9 +- sweepbatcher/sweep_batcher.go | 14 ++- sweepbatcher/sweep_batcher_presigned_test.go | 101 +++++++++++++++++-- 7 files changed, 184 insertions(+), 44 deletions(-) diff --git a/loopout_test.go b/loopout_test.go index 1cf90a4b..254ccea8 100644 --- a/loopout_test.go +++ b/loopout_test.go @@ -280,6 +280,7 @@ func testCustomSweepConfTarget(t *testing.T) { // yields a much higher fee rate. ctx.Lnd.SetFeeEstimate(testReq.SweepConfTarget, 250) ctx.Lnd.SetFeeEstimate(DefaultSweepConfTarget, 10000) + ctx.Lnd.SetMinRelayFee(250) cfg := newSwapConfig( &lnd.LndServices, loopdb.NewStoreMock(t), server, nil, diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index d49506e2..9485815f 100644 --- a/sweepbatcher/presigned.go +++ b/sweepbatcher/presigned.go @@ -28,8 +28,14 @@ func (b *batch) ensurePresigned(ctx context.Context, newSweeps []*sweep, "adding to an empty batch") } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFee: %w", err) + } + return ensurePresigned( - ctx, newSweeps, b.cfg.presignedHelper, b.cfg.chainParams, + ctx, newSweeps, b.cfg.presignedHelper, minRelayFeeRate, + b.cfg.chainParams, ) } @@ -43,6 +49,7 @@ type presignedTxChecker interface { // inputs of this group only. func ensurePresigned(ctx context.Context, newSweeps []*sweep, presignedTxChecker presignedTxChecker, + minRelayFeeRate chainfee.SatPerKWeight, chainParams *chaincfg.Params) error { sweeps := make([]sweep, len(newSweeps)) @@ -74,7 +81,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep, const feeRate = chainfee.FeePerKwFloor tx, _, _, _, err := constructUnsignedTx( - sweeps, destAddr, currentHeight, feeRate, + sweeps, destAddr, currentHeight, feeRate, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to construct unsigned tx "+ @@ -218,6 +225,14 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error { b.Infof("nextBlockFeeRate is %v", nextBlockFeeRate) + // Find the minRelayFeeRate. + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFeeRate: %w", err) + } + + b.Infof("minRelayFeeRate is %v", minRelayFeeRate) + // We need to restore previously added groups. We can do it by reading // all the sweeps from DB (they must be ordered) and grouping by swap. groups, err := b.getSweepsGroups(ctx) @@ -258,7 +273,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error { err = presign( ctx, b.cfg.presignedHelper, destAddr, primarySweepID, - sweeps, nextBlockFeeRate, + sweeps, nextBlockFeeRate, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to presign a transaction "+ @@ -300,7 +315,8 @@ type presigner interface { // 10x of the current next block feerate. func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, primarySweepID wire.OutPoint, sweeps []sweep, - nextBlockFeeRate chainfee.SatPerKWeight) error { + nextBlockFeeRate chainfee.SatPerKWeight, + minRelayFeeRate chainfee.SatPerKWeight) error { if presigner == nil { return fmt.Errorf("presigner is not installed") @@ -354,7 +370,7 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, for fr := start; fr <= stop; fr = (fr * factorPPM) / 1_000_000 { // Construct an unsigned transaction for this fee rate. tx, _, feeForWeight, fee, err := constructUnsignedTx( - sweeps, destAddr, currentHeight, fr, + sweeps, destAddr, currentHeight, fr, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to construct unsigned tx "+ @@ -411,15 +427,15 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, } // Determine the current minimum relay fee based on our chain backend. - minRelayFee, err := b.wallet.MinRelayFee(ctx) + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) if err != nil { - return 0, fmt.Errorf("failed to get minRelayFee: %w", err), + return 0, fmt.Errorf("failed to get minRelayFeeRate: %w", err), false } // Cache current height and desired feerate of the batch. currentHeight := b.currentHeight - feeRate := max(b.rbfCache.FeeRate, minRelayFee) + feeRate := max(b.rbfCache.FeeRate, minRelayFeeRate) // Append this sweep to an array of sweeps. This is needed to keep the // order of sweeps stored, as iterating the sweeps map does not @@ -441,7 +457,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Construct unsigned batch transaction. tx, weight, _, fee, err := constructUnsignedTx( - sweeps, address, currentHeight, feeRate, + sweeps, address, currentHeight, feeRate, minRelayFeeRate, ) if err != nil { return 0, fmt.Errorf("failed to construct tx: %w", err), @@ -460,7 +476,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Get a pre-signed transaction. const loadOnly = false signedTx, err := b.cfg.presignedHelper.SignTx( - ctx, b.primarySweepID, tx, batchAmt, minRelayFee, feeRate, + ctx, b.primarySweepID, tx, batchAmt, minRelayFeeRate, feeRate, loadOnly, ) if err != nil { @@ -470,7 +486,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Run sanity checks to make sure presignedHelper.SignTx complied with // all the invariants. - err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFee) + err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFeeRate) if err != nil { return 0, fmt.Errorf("signed tx doesn't correspond the "+ "unsigned tx: %w", err), false diff --git a/sweepbatcher/presigned_test.go b/sweepbatcher/presigned_test.go index d4f59373..35fe8bf2 100644 --- a/sweepbatcher/presigned_test.go +++ b/sweepbatcher/presigned_test.go @@ -15,6 +15,10 @@ import ( "github.com/stretchr/testify/require" ) +const ( + minRelayFeeRate = chainfee.FeePerKwFloor +) + // TestOrderedSweeps checks that methods batch.getOrderedSweeps and // batch.getSweepsGroups works properly. func TestOrderedSweeps(t *testing.T) { @@ -561,7 +565,7 @@ func TestEnsurePresigned(t *testing.T) { } err := ensurePresigned( - ctx, tc.sweeps, c, + ctx, tc.sweeps, c, minRelayFeeRate, &chaincfg.RegressionNetParams, ) switch { @@ -1010,7 +1014,7 @@ func TestPresign(t *testing.T) { err := presign( ctx, tc.presigner, tc.destAddr, tc.primarySweepID, tc.sweeps, - tc.nextBlockFeeRate, + tc.nextBlockFeeRate, minRelayFeeRate, ) if tc.wantErr != "" { require.Error(t, err) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 98f576df..7b5e1542 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -1296,9 +1296,9 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte, // outputs. If the main output value is below dust limit this function will // return an error. func constructUnsignedTx(sweeps []sweep, address btcutil.Address, - currentHeight int32, feeRate chainfee.SatPerKWeight) ( - *wire.MsgTx, lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, - error) { + currentHeight int32, feeRate chainfee.SatPerKWeight, + minRelayFeeRate chainfee.SatPerKWeight) (*wire.MsgTx, + lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, error) { // Sanity check, there should be at least 1 sweep in this batch. if len(sweeps) == 0 { @@ -1400,7 +1400,14 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, } // Clamp the calculated fee to the max allowed fee amount for the batch. - fee := clampBatchFee(feeForWeight, batchAmt-btcutil.Amount(sumChange)) + fee, err := clampBatchFee( + feeForWeight, batchAmt-btcutil.Amount(sumChange), + minRelayFeeRate, weight, + ) + if err != nil { + return nil, 0, 0, 0, fmt.Errorf("failed to clamp batch "+ + "fee: %w", err) + } // Ensure that batch amount exceeds the sum of change outputs and the // fee, and that it is also greater than dust limit for the main @@ -1516,15 +1523,21 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, // known in advance to be non-cooperative (nonCoopHint) and not failed // to sign cooperatively in previous rounds (coopFailed). If any of them // fails, the sweep is excluded from all following rounds and another - // round is attempted. Otherwise the cycle completes and we sign the + // round is attempted. Otherwise, the cycle completes and we sign the // remaining sweeps non-cooperatively. var ( - tx *wire.MsgTx - weight lntypes.WeightUnit - feeForWeight btcutil.Amount - fee btcutil.Amount - coopInputs int + tx *wire.MsgTx + weight lntypes.WeightUnit + feeForWeight btcutil.Amount + fee btcutil.Amount + minRelayFeeRate chainfee.SatPerKWeight + coopInputs int ) + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return 0, fmt.Errorf("failed to get min relay fee: %w", err), + false + } for attempt := 1; ; attempt++ { b.Infof("Attempt %d of collecting cooperative signatures.", attempt) @@ -1533,6 +1546,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, var err error tx, weight, feeForWeight, fee, err = constructUnsignedTx( sweeps, address, b.currentHeight, b.rbfCache.FeeRate, + minRelayFeeRate, ) if err != nil { return 0, fmt.Errorf("failed to construct tx: %w", err), @@ -1584,7 +1598,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, // If there was any failure of cooperative signing, we need to // update weight estimates (since non-cooperative signing has // larger witness) and hence update the whole transaction and - // all the signatures. Otherwise we complete cooperative part. + // all the signatures. Otherwise, we complete cooperative part. if !newCoopFailures { break } @@ -1720,7 +1734,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, } // Publish the transaction. - err := b.wallet.PublishTransaction( + err = b.wallet.PublishTransaction( ctx, tx, b.cfg.txLabeler(b.id), ) if err != nil { @@ -2583,16 +2597,25 @@ func (b *batch) persistSweep(ctx context.Context, sweep sweep, // clampBatchFee takes the fee amount and total amount of the sweeps in the // batch and makes sure the fee is not too high. If the fee is too high, it is -// clamped to the maximum allowed fee. -func clampBatchFee(fee btcutil.Amount, - totalAmount btcutil.Amount) btcutil.Amount { +// clamped to the maximum allowed fee. If the clamped fee results in a fee rate +// below the minimum relay fee, an error is returned. +func clampBatchFee(fee btcutil.Amount, totalAmount btcutil.Amount, + minRelayFeeRate chainfee.SatPerKWeight, + weight lntypes.WeightUnit) (btcutil.Amount, error) { maxFeeAmount := btcutil.Amount(float64(totalAmount) * maxFeeToSwapAmtRatio) + clampedFee := fee if fee > maxFeeAmount { - return maxFeeAmount + clampedFee = maxFeeAmount } - return fee + clampedFeeRate := chainfee.NewSatPerKWeight(clampedFee, weight) + if clampedFeeRate < minRelayFeeRate { + return 0, fmt.Errorf("clamped fee rate %v is less than "+ + "minimum relay fee %v", clampedFeeRate, minRelayFeeRate) + } + + return clampedFee, nil } diff --git a/sweepbatcher/sweep_batch_test.go b/sweepbatcher/sweep_batch_test.go index 78527874..2b77f871 100644 --- a/sweepbatcher/sweep_batch_test.go +++ b/sweepbatcher/sweep_batch_test.go @@ -440,12 +440,13 @@ func TestConstructUnsignedTx(t *testing.T) { change: change1, }, }, - address: p2trAddress, - currentHeight: 800_000, - feeRate: 1, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1_000, + minRelayFeeRate: 50, wantErr: "batch amount 0.00100294 BTC is <= the sum " + "of change outputs 0.00100000 BTC plus fee " + - "0.00000001 BTC and dust limit 0.00000330 BTC", + "0.00000058 BTC and dust limit 0.00000330 BTC", }, { diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 2afeb327..f3b7265b 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -736,6 +736,10 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, if err != nil { return fmt.Errorf("failed to get nextBlockFeeRate: %w", err) } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFeeRate: %w", err) + } destPkscript, err := txscript.PayToAddrScript(destAddress) if err != nil { return fmt.Errorf("txscript.PayToAddrScript failed: %w", err) @@ -763,7 +767,7 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, return presign( ctx, b.presignedHelper, destAddress, primarySweepID, sweeps, - nextBlockFeeRate, + nextBlockFeeRate, minRelayFeeRate, ) } @@ -818,12 +822,18 @@ func (b *Batcher) AddSweep(ctx context.Context, sweepReq *SweepRequest) error { } } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get min relay fee: %w", err) + } + // If this is a presigned mode, make sure PresignSweepsGroup was called. // We skip the check for reorg-safely confirmed sweeps, because their // presigned transactions were already cleaned up from the store. if sweep.presigned && !fullyConfirmed { err := ensurePresigned( - ctx, sweeps, b.presignedHelper, b.chainParams, + ctx, sweeps, b.presignedHelper, minRelayFeeRate, + b.chainParams, ) if err != nil { return fmt.Errorf("inputs with primarySweep %v were "+ diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index fd28bc7d..d2907204 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -519,7 +519,7 @@ func testPresigned_min_relay_fee(t *testing.T, }() // Set high min_relay_fee. - lnd.SetMinRelayFee(400) + lnd.SetMinRelayFee(252) // Create the first sweep. swapHash1 := lntypes.Hash{1, 1, 1} @@ -554,16 +554,16 @@ func testPresigned_min_relay_fee(t *testing.T, // Wait for a transactions to be published. tx := <-lnd.TxPublishChannel gotFeeRate := presignedHelper.getTxFeerate(tx, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate) // Now decrease min_relay_fee and make sure fee rate doesn't decrease. // The only difference of tx2 is a higher lock_time. - lnd.SetMinRelayFee(300) + lnd.SetMinRelayFee(150) require.NoError(t, lnd.NotifyHeight(601)) tx2 := <-lnd.TxPublishChannel require.Equal(t, tx.TxOut[0].Value, tx2.TxOut[0].Value) gotFeeRate = presignedHelper.getTxFeerate(tx2, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate) require.Equal(t, uint32(601), tx2.LockTime) // Set a higher min_relay_fee, turn off the client and try presigned tx. @@ -1269,9 +1269,10 @@ func testPresigned_presigned_group_with_change(t *testing.T, require.NoError(t, lnd.NotifyHeight(601)) } -// testPresigned_presigned_group_with_dust_main_output tests passing multiple -// sweeps to the method PresignSweepsGroup. It tests that a dust change output of -// a primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. +// testPresigned_presigned_group_with_dust_main_output passes a dust main output +// and a change output to PresignSweepsGroup. It will fail because of the dust +// main output. Note that the min relay fee is set low enough to pass the +// clampBatchFee check, so the error is not related to the fee rate. func testPresigned_presigned_group_with_dust_main_output(t *testing.T, batcherStore testBatcherStore) { @@ -1285,9 +1286,12 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, customFeeRate := func(_ context.Context, _ lntypes.Hash, _ wire.OutPoint) (chainfee.SatPerKWeight, error) { - return chainfee.SatPerKWeight(10_000), nil + return chainfee.SatPerKWeight(100_000), nil } + // Set min relay fee low enough to pass the clampBatchFee check. + lnd.SetMinRelayFee(140) + presignedHelper := newMockPresignedHelper() batcher := NewBatcher( @@ -1345,6 +1349,81 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, require.ErrorContains(t, err, "were not presigned") } +// testPresigned_presigned_group_with_dust_below_relay_fee passes a tx with a +// dust main output and one change output to PresignSweepsGroup. The tx fee rate +// is below the min relay fee, hence clampBatchFee will return an error. +func testPresigned_presigned_group_with_dust_below_relay_fee(t *testing.T, + batcherStore testBatcherStore) { + + defer test.Guard(t)() + + lnd := test.NewMockLnd() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + customFeeRate := func(_ context.Context, _ lntypes.Hash, + _ wire.OutPoint) (chainfee.SatPerKWeight, error) { + + return chainfee.SatPerKWeight(100_000), nil + } + + presignedHelper := newMockPresignedHelper() + + batcher := NewBatcher( + lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, + testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, + batcherStore, presignedHelper, + WithCustomFeeRate(customFeeRate), + WithPresignedHelper(presignedHelper), + ) + + go func() { + err := batcher.Run(ctx) + checkBatcherError(t, err) + }() + + // Create a swap of two sweeps. + swapHash1 := lntypes.Hash{1, 1, 1} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + inputValue := int64(1_000_000) + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + } + dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + change := &wire.TxOut{ + Value: inputValue - dustLimit + 1, + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + + // An attempt to presign must fail. + err := batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, change, + ) + require.EqualError(t, err, "failed to construct unsigned tx for "+ + "feeRate 253 sat/kw: failed to clamp batch fee: clamped "+ + "fee rate 148 sat/kw is less than minimum relay fee 253 sat/kw") + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.ErrorContains(t, err, "were not presigned") +} + // testPresigned_presigned_group_with_dust_change tests passing multiple sweeps // to the method PresignSweepsGroup. It tests that a dust change output of a // primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. @@ -2102,6 +2181,12 @@ func TestPresigned(t *testing.T) { ) }) + t.Run("dust_main_output_below_min_relay_fee", func(t *testing.T) { + testPresigned_presigned_group_with_dust_below_relay_fee( + t, NewStoreMock(), + ) + }) + t.Run("dust_change", func(t *testing.T) { testPresigned_presigned_group_with_dust_change(t, NewStoreMock()) })