From e8141578acde61236701800117e70b32135e7880 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 27 Jun 2024 14:10:24 -0300 Subject: [PATCH] sweepbatcher: MinFeeRate -> WithCustomFeeRate The reason is because we want to know in advance if fee rates come from an external source or are determined by sweepbatcher internally. Also remove WithNoBumping option. It is now a part of WithCustomFeeRate: if WithCustomFeeRate is passed, automatic fee bumping by sweepbatcher is disabled. --- sweepbatcher/sweep_batch.go | 4 +- sweepbatcher/sweep_batcher.go | 60 ++++++++++++++++++------------ sweepbatcher/sweep_batcher_test.go | 28 ++++++++++---- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index c824619..47ab573 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -133,9 +133,7 @@ type batchConfig struct { batchPublishDelay time.Duration // noBumping instructs sweepbatcher not to fee bump itself and rely on - // external source of fee rates (MinFeeRate). To change the fee rate, - // the caller has to update it in the source of SweepInfo (interface - // SweepFetcher) and re-add the sweep by calling AddSweep. + // external source of fee rates (FeeRateProvider). noBumping bool // customMuSig2Signer is a custom signer. If it is set, it is used to diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 578a427..f67b828 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -121,10 +121,6 @@ type SweepInfo struct { // DestAddr is the destination address of the sweep. DestAddr btcutil.Address - - // MinFeeRate is minimum fee rate that must be used by a batch of - // the sweep. If it is specified, confTarget is ignored. - MinFeeRate chainfee.SatPerKWeight } // SweepFetcher is used to get details of a sweep. @@ -151,6 +147,11 @@ type SignMuSig2 func(ctx context.Context, muSig2Version input.MuSig2Version, // signature. type VerifySchnorrSig func(pubKey *btcec.PublicKey, hash, sig []byte) error +// FeeRateProvider is a function that returns min fee rate of a batch sweeping +// the UTXO of the swap. +type FeeRateProvider func(ctx context.Context, + swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) + // SweepRequest is a request to sweep a specific outpoint. type SweepRequest struct { // SwapHash is the hash of the swap that is being swept. @@ -247,11 +248,10 @@ type Batcher struct { // exit. wg sync.WaitGroup - // noBumping instructs sweepbatcher not to fee bump itself and rely on - // external source of fee rates (MinFeeRate). To change the fee rate, - // the caller has to update it in the source of SweepInfo (interface - // SweepFetcher) and re-add the sweep by calling AddSweep. - noBumping bool + // customFeeRate provides custom min fee rate per swap. The batch uses + // max of the fee rates of its swaps. In this mode confTarget is + // ignored and fee bumping by sweepbatcher is disabled. + customFeeRate FeeRateProvider // customMuSig2Signer is a custom signer. If it is set, it is used to // create musig2 signatures instead of musig2SignSweep and signerClient. @@ -262,11 +262,10 @@ type Batcher struct { // BatcherConfig holds batcher configuration. type BatcherConfig struct { - // noBumping instructs sweepbatcher not to fee bump itself and rely on - // external source of fee rates (MinFeeRate). To change the fee rate, - // the caller has to update it in the source of SweepInfo (interface - // SweepFetcher) and re-add the sweep by calling AddSweep. - noBumping bool + // customFeeRate provides custom min fee rate per swap. The batch uses + // max of the fee rates of its swaps. In this mode confTarget is + // ignored and fee bumping by sweepbatcher is disabled. + customFeeRate FeeRateProvider // customMuSig2Signer is a custom signer. If it is set, it is used to // create musig2 signatures instead of musig2SignSweep and signerClient. @@ -278,13 +277,12 @@ type BatcherConfig struct { // BatcherOption configures batcher behaviour. type BatcherOption func(*BatcherConfig) -// WithNoBumping instructs sweepbatcher not to fee bump itself and -// rely on external source of fee rates (MinFeeRate). To change the -// fee rate, the caller has to update it in the source of SweepInfo -// (interface SweepFetcher) and re-add the sweep by calling AddSweep. -func WithNoBumping() BatcherOption { +// WithCustomFeeRate instructs sweepbatcher not to fee bump itself and rely on +// external source of fee rates (FeeRateProvider). To apply a fee rate change, +// the caller should re-add the sweep by calling AddSweep. +func WithCustomFeeRate(customFeeRate FeeRateProvider) BatcherOption { return func(cfg *BatcherConfig) { - cfg.noBumping = true + cfg.customFeeRate = customFeeRate } } @@ -331,7 +329,7 @@ func NewBatcher(wallet lndclient.WalletKitClient, chainParams: chainparams, store: store, sweepStore: sweepStore, - noBumping: cfg.noBumping, + customFeeRate: cfg.customFeeRate, customMuSig2Signer: cfg.customMuSig2Signer, } } @@ -859,6 +857,22 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, swapHash[:6], err) } + // minFeeRate is 0 by default. If customFeeRate is not provided, then + // rbfCache.FeeRate is also 0 and method batch.updateRbfRate() updates + // it to current fee rate according to batchConfTarget. + var minFeeRate chainfee.SatPerKWeight + if b.customFeeRate != nil { + minFeeRate, err = b.customFeeRate(ctx, swapHash) + if err != nil { + return nil, fmt.Errorf("failed to fetch min fee rate "+ + "for %x: %w", swapHash[:6], err) + } + if minFeeRate < chainfee.AbsoluteFeePerKwFloor { + return nil, fmt.Errorf("min fee rate too low (%v) for "+ + "%x", minFeeRate, swapHash[:6]) + } + } + return &sweep{ swapHash: swapHash, outpoint: outpoint, @@ -874,7 +888,7 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, protocolVersion: s.ProtocolVersion, isExternalAddr: s.IsExternalAddr, destAddr: s.DestAddr, - minFeeRate: s.MinFeeRate, + minFeeRate: minFeeRate, }, nil } @@ -882,7 +896,7 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, func (b *Batcher) newBatchConfig(maxTimeoutDistance int32) batchConfig { return batchConfig{ maxTimeoutDistance: maxTimeoutDistance, - noBumping: b.noBumping, + noBumping: b.customFeeRate != nil, customMuSig2Signer: b.customMuSig2Signer, } } diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index 82c8258..592f86c 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -308,7 +308,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, } // testFeeBumping tests that sweep is RBFed with slightly higher fee rate after -// each block unless WithNoBumping is passed. +// each block unless WithCustomFeeRate is passed. func testFeeBumping(t *testing.T, store testStore, batcherStore testBatcherStore, noFeeBumping bool) { @@ -324,7 +324,14 @@ func testFeeBumping(t *testing.T, store testStore, // Disable fee bumping, if requested. var opts []BatcherOption if noFeeBumping { - opts = append(opts, WithNoBumping()) + customFeeRate := func(ctx context.Context, + swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) { + + // Always provide the same value, no bumping. + return test.DefaultMockFee, nil + } + + opts = append(opts, WithCustomFeeRate(customFeeRate)) } batcher := NewBatcher(lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, @@ -1931,8 +1938,7 @@ func testSweepFetcher(t *testing.T, store testStore, feeRate := chainfee.SatPerKWeight(30000) amt := btcutil.Amount(1_000_000) weight := lntypes.WeightUnit(445) // Weight for 1-to-1 tx. - bumpedFee := feeRate + 100 - expectedFee := bumpedFee.FeeForWeight(weight) + expectedFee := feeRate.FeeForWeight(weight) swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ @@ -1955,7 +1961,6 @@ func testSweepFetcher(t *testing.T, store testStore, ConfTarget: 123, Timeout: 111, SwapInvoicePaymentAddr: *swapPaymentAddr, - MinFeeRate: feeRate, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HTLCKeys: htlcKeys, HTLC: *htlc, @@ -1987,9 +1992,16 @@ func testSweepFetcher(t *testing.T, store testStore, require.NoError(t, err) store.AssertLoopOutStored() + customFeeRate := func(ctx context.Context, + swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) { + + // Always provide the same value, no bumping. + return feeRate, nil + } + batcher := NewBatcher(lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, - batcherStore, sweepFetcher) + batcherStore, sweepFetcher, WithCustomFeeRate(customFeeRate)) var wg sync.WaitGroup wg.Add(1) @@ -2238,7 +2250,7 @@ func TestSweepBatcherBatchCreation(t *testing.T) { } // TestFeeBumping tests that sweep is RBFed with slightly higher fee rate after -// each block unless WithNoBumping is passed. +// each block unless WithCustomFeeRate is passed. func TestFeeBumping(t *testing.T) { t.Run("regular", func(t *testing.T) { runTests(t, func(t *testing.T, store testStore, @@ -2248,7 +2260,7 @@ func TestFeeBumping(t *testing.T) { }) }) - t.Run("WithNoBumping", func(t *testing.T) { + t.Run("fixed fee rate", func(t *testing.T) { runTests(t, func(t *testing.T, store testStore, batcherStore testBatcherStore) {