From 864d7f24cdd858b890c7165c9de2c1254a228402 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Tue, 15 Oct 2024 10:36:27 -0300 Subject: [PATCH] sweepbatcher: make sure feerate does not decrease Previous behaviour was to overwrite batch's feerate with minFeeRate of its primary sweep, which could be lower that previus batch's feerate or lower that feerate of some other sweep. Instead, batch's feerate only grows and never declines and is at least as high as the highest feerate of its sweeps. Added a test to verify this. --- sweepbatcher/sweep_batch.go | 10 +- sweepbatcher/sweep_batcher_test.go | 180 +++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index d0bcbaa3..2e815899 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -470,10 +470,18 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { // batch's confirmation target and fee rate. if b.primarySweepID == sweep.swapHash { b.cfg.batchConfTarget = sweep.confTarget - b.rbfCache.FeeRate = sweep.minFeeRate b.rbfCache.SkipNextBump = true } + // Update batch's fee rate to be greater than or equal to + // minFeeRate of the sweep. Make sure batch's fee rate does not + // decrease (otherwise it won't pass RBF rules and won't be + // broadcasted) and that it is not lower that minFeeRate of + // other sweeps (so it is applied). + if b.rbfCache.FeeRate < sweep.minFeeRate { + b.rbfCache.FeeRate = sweep.minFeeRate + } + return true, nil } diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index 11c56143..def11c9b 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -3698,6 +3698,180 @@ func testWithMixedBatchCoopFailedOnly(t *testing.T, store testStore, wantWeight, wantWitnessSizes) } +// testFeeRateGrows tests that fee rate of a batch does not decrease and is at +// least as high as the highest fee rate of sweeps. +func testFeeRateGrows(t *testing.T, store testStore, + batcherStore testBatcherStore) { + + defer test.Guard(t)() + + lnd := test.NewMockLnd() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + sweepStore, err := NewSweepFetcherFromSwapStore(store, lnd.ChainParams) + require.NoError(t, err) + + // Create a map to store fee rates. + swap2feeRate := map[lntypes.Hash]chainfee.SatPerKWeight{} + var swap2feeRateMu sync.Mutex + setFeeRate := func(swapHash lntypes.Hash, rate chainfee.SatPerKWeight) { + swap2feeRateMu.Lock() + defer swap2feeRateMu.Unlock() + + swap2feeRate[swapHash] = rate + } + + customFeeRate := func(ctx context.Context, + swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) { + + swap2feeRateMu.Lock() + defer swap2feeRateMu.Unlock() + + return swap2feeRate[swapHash], nil + } + + const ( + feeRateLow = chainfee.SatPerKWeight(10_000) + feeRateMedium = chainfee.SatPerKWeight(30_000) + feeRateHigh = chainfee.SatPerKWeight(50_000) + ) + + batcher := NewBatcher(lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, + testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, + batcherStore, sweepStore, WithCustomFeeRate(customFeeRate)) + + go func() { + err := batcher.Run(ctx) + checkBatcherError(t, err) + }() + + // Create the first sweep. + swapHash1 := lntypes.Hash{1, 1, 1} + setFeeRate(swapHash1, feeRateMedium) + sweepReq1 := SweepRequest{ + SwapHash: swapHash1, + Value: 1_000_000, + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + }, + Notifier: &dummyNotifier, + } + + swap1 := &loopdb.LoopOutContract{ + SwapContract: loopdb.SwapContract{ + CltvExpiry: 111, + AmountRequested: 1_000_000, + ProtocolVersion: loopdb.ProtocolVersionMuSig2, + HtlcKeys: htlcKeys, + + // Make preimage unique to pass SQL constraints. + Preimage: lntypes.Preimage{1}, + }, + + DestAddr: destAddr, + SwapInvoice: swapInvoice, + SweepConfTarget: 111, + } + + err = store.CreateLoopOut(ctx, swapHash1, swap1) + require.NoError(t, err) + store.AssertLoopOutStored() + + // Deliver sweep request to batcher. + require.NoError(t, batcher.AddSweep(&sweepReq1)) + + // Since a batch was created we check that it registered for its primary + // sweep's spend. + <-lnd.RegisterSpendChannel + + // Wait for tx to be published. + <-lnd.TxPublishChannel + + // Make sure the fee rate is feeRateMedium. + batch := getOnlyBatch(batcher) + require.Len(t, batch.sweeps, 1) + require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate) + + // Now decrease the fee of sweep1. + setFeeRate(swapHash1, feeRateLow) + require.NoError(t, batcher.AddSweep(&sweepReq1)) + + // Tick tock next block. + err = lnd.NotifyHeight(601) + require.NoError(t, err) + + // Wait for tx to be published. + <-lnd.TxPublishChannel + + // Make sure the fee rate is still feeRateMedium. + require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate) + + // Add sweep2, with feeRateMedium. + swapHash2 := lntypes.Hash{2, 2, 2} + setFeeRate(swapHash2, feeRateMedium) + sweepReq2 := SweepRequest{ + SwapHash: swapHash2, + Value: 1_000_000, + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{2, 2}, + Index: 1, + }, + Notifier: &dummyNotifier, + } + + swap2 := &loopdb.LoopOutContract{ + SwapContract: loopdb.SwapContract{ + CltvExpiry: 111, + AmountRequested: 1_000_000, + ProtocolVersion: loopdb.ProtocolVersionMuSig2, + HtlcKeys: htlcKeys, + + // Make preimage unique to pass SQL constraints. + Preimage: lntypes.Preimage{2}, + }, + + DestAddr: destAddr, + SwapInvoice: swapInvoice, + SweepConfTarget: 111, + } + + err = store.CreateLoopOut(ctx, swapHash2, swap2) + require.NoError(t, err) + store.AssertLoopOutStored() + + // Deliver sweep request to batcher. + require.NoError(t, batcher.AddSweep(&sweepReq2)) + + // Tick tock next block. + err = lnd.NotifyHeight(602) + require.NoError(t, err) + + // Wait for tx to be published. + <-lnd.TxPublishChannel + + // Make sure the fee rate is still feeRateMedium. + require.Len(t, batch.sweeps, 2) + require.Equal(t, feeRateMedium, batch.rbfCache.FeeRate) + + // Now update fee rate of second sweep (which is not primary) to + // feeRateHigh. Fee rate of sweep 1 is still feeRateLow. + setFeeRate(swapHash2, feeRateHigh) + require.NoError(t, batcher.AddSweep(&sweepReq1)) + require.NoError(t, batcher.AddSweep(&sweepReq2)) + + // Tick tock next block. + err = lnd.NotifyHeight(603) + require.NoError(t, err) + + // Wait for tx to be published. + <-lnd.TxPublishChannel + + // Make sure the fee rate increased to feeRateHigh. + require.Equal(t, feeRateHigh, batch.rbfCache.FeeRate) +} + // TestSweepBatcherBatchCreation tests that sweep requests enter the expected // batch based on their timeout distance. func TestSweepBatcherBatchCreation(t *testing.T) { @@ -3839,6 +4013,12 @@ func TestWithMixedBatchCoopFailedOnly(t *testing.T) { runTests(t, testWithMixedBatchCoopFailedOnly) } +// TestFeeRateGrows tests that fee rate of a batch does not decrease and is at +// least as high as the highest fee rate of sweeps. +func TestFeeRateGrows(t *testing.T) { + runTests(t, testFeeRateGrows) +} + // testBatcherStore is BatcherStore used in tests. type testBatcherStore interface { BatcherStore