From 1290ebe5359582a21a5750d26b7766572693cffd Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 27 Jun 2024 12:48:46 -0300 Subject: [PATCH 01/12] sweepbatcher: fix copy-paste in sweep creation --- sweepbatcher/sweep_batcher.go | 44 +++++++++++++---------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 1fc6d00e9..6642f6124 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -799,29 +799,8 @@ func (b *Batcher) writeToErrChan(ctx context.Context, err error) error { func (b *Batcher) convertSweep(ctx context.Context, dbSweep *dbSweep) ( *sweep, error) { - s, err := b.sweepStore.FetchSweep(ctx, dbSweep.SwapHash) - if err != nil { - return nil, fmt.Errorf("failed to fetch sweep data for %x: %w", - dbSweep.SwapHash[:6], err) - } - - return &sweep{ - swapHash: dbSweep.SwapHash, - outpoint: dbSweep.Outpoint, - value: dbSweep.Amount, - confTarget: s.ConfTarget, - timeout: s.Timeout, - initiationHeight: s.InitiationHeight, - htlc: s.HTLC, - preimage: s.Preimage, - swapInvoicePaymentAddr: s.SwapInvoicePaymentAddr, - htlcKeys: s.HTLCKeys, - htlcSuccessEstimator: s.HTLCSuccessEstimator, - protocolVersion: s.ProtocolVersion, - isExternalAddr: s.IsExternalAddr, - destAddr: s.DestAddr, - minFeeRate: s.MinFeeRate, - }, nil + return b.loadSweep(ctx, dbSweep.SwapHash, dbSweep.Outpoint, + dbSweep.Amount) } // LoopOutFetcher is used to load LoopOut swaps from the database. @@ -897,16 +876,25 @@ func NewSweepFetcherFromSwapStore(swapStore LoopOutFetcher, func (b *Batcher) fetchSweep(ctx context.Context, sweepReq SweepRequest) (*sweep, error) { - s, err := b.sweepStore.FetchSweep(ctx, sweepReq.SwapHash) + return b.loadSweep(ctx, sweepReq.SwapHash, sweepReq.Outpoint, + sweepReq.Value) +} + +// loadSweep loads inputs of sweep from the database and from FeeRateProvider +// if needed and returns an assembled sweep object. +func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, + outpoint wire.OutPoint, value btcutil.Amount) (*sweep, error) { + + s, err := b.sweepStore.FetchSweep(ctx, swapHash) if err != nil { return nil, fmt.Errorf("failed to fetch sweep data for %x: %w", - sweepReq.SwapHash[:6], err) + swapHash[:6], err) } return &sweep{ - swapHash: sweepReq.SwapHash, - outpoint: sweepReq.Outpoint, - value: sweepReq.Value, + swapHash: swapHash, + outpoint: outpoint, + value: value, confTarget: s.ConfTarget, timeout: s.Timeout, initiationHeight: s.InitiationHeight, From 1f1a27447cde622df9f47ae71112ebc2065ee468 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 27 Jun 2024 13:04:06 -0300 Subject: [PATCH 02/12] sweepbatcher: fix copy-paste with batchConfig --- sweepbatcher/sweep_batcher.go | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 6642f6124..288a2469c 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -506,11 +506,7 @@ func (b *Batcher) handleSweep(ctx context.Context, sweep *sweep, // spinUpBatch spins up a new batch and returns it. func (b *Batcher) spinUpBatch(ctx context.Context) (*batch, error) { - cfg := batchConfig{ - maxTimeoutDistance: defaultMaxTimeoutDistance, - noBumping: b.noBumping, - customMuSig2Signer: b.customMuSig2Signer, - } + cfg := b.newBatchConfig(defaultMaxTimeoutDistance) switch b.chainParams { case &chaincfg.MainNetParams: @@ -625,11 +621,7 @@ func (b *Batcher) spinUpBatchFromDB(ctx context.Context, batch *batch) error { quit: b.quit, } - cfg := batchConfig{ - maxTimeoutDistance: batch.cfg.maxTimeoutDistance, - noBumping: b.noBumping, - customMuSig2Signer: b.customMuSig2Signer, - } + cfg := b.newBatchConfig(batch.cfg.maxTimeoutDistance) newBatch, err := NewBatchFromDB(cfg, batchKit) if err != nil { @@ -689,11 +681,7 @@ func (b *Batcher) FetchUnconfirmedBatches(ctx context.Context) ([]*batch, } batch.rbfCache = rbfCache - bchCfg := batchConfig{ - maxTimeoutDistance: bch.MaxTimeoutDistance, - noBumping: b.noBumping, - customMuSig2Signer: b.customMuSig2Signer, - } + bchCfg := b.newBatchConfig(bch.MaxTimeoutDistance) batch.cfg = &bchCfg batches = append(batches, &batch) @@ -909,3 +897,12 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, minFeeRate: s.MinFeeRate, }, nil } + +// newBatchConfig creates new batch config. +func (b *Batcher) newBatchConfig(maxTimeoutDistance int32) batchConfig { + return batchConfig{ + maxTimeoutDistance: maxTimeoutDistance, + noBumping: b.noBumping, + customMuSig2Signer: b.customMuSig2Signer, + } +} From ccd1b312ca8bbd07e9c2a6b9c8de5afb9d64de8d Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 27 Jun 2024 13:19:37 -0300 Subject: [PATCH 03/12] sweepbatcher: fix copy-paste with batch kit --- sweepbatcher/sweep_batcher.go | 55 ++++++++++++++++------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 288a2469c..578a427df 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -516,17 +516,7 @@ func (b *Batcher) spinUpBatch(ctx context.Context) (*batch, error) { cfg.batchPublishDelay = defaultPublishDelay } - batchKit := batchKit{ - returnChan: b.sweepReqs, - wallet: b.wallet, - chainNotifier: b.chainNotifier, - signerClient: b.signerClient, - musig2SignSweep: b.musig2ServerSign, - verifySchnorrSig: b.VerifySchnorrSig, - purger: b.AddSweep, - store: b.store, - quit: b.quit, - } + batchKit := b.newBatchKit() batch := NewBatch(cfg, batchKit) @@ -601,25 +591,15 @@ func (b *Batcher) spinUpBatchFromDB(ctx context.Context, batch *batch) error { logger := batchPrefixLogger(fmt.Sprintf("%d", batch.id)) - batchKit := batchKit{ - id: batch.id, - batchTxid: batch.batchTxid, - batchPkScript: batch.batchPkScript, - state: batch.state, - primaryID: primarySweep.SwapHash, - sweeps: sweeps, - rbfCache: rbfCache, - returnChan: b.sweepReqs, - wallet: b.wallet, - chainNotifier: b.chainNotifier, - signerClient: b.signerClient, - musig2SignSweep: b.musig2ServerSign, - verifySchnorrSig: b.VerifySchnorrSig, - purger: b.AddSweep, - store: b.store, - log: logger, - quit: b.quit, - } + batchKit := b.newBatchKit() + batchKit.id = batch.id + batchKit.batchTxid = batch.batchTxid + batchKit.batchPkScript = batch.batchPkScript + batchKit.state = batch.state + batchKit.primaryID = primarySweep.SwapHash + batchKit.sweeps = sweeps + batchKit.rbfCache = rbfCache + batchKit.log = logger cfg := b.newBatchConfig(batch.cfg.maxTimeoutDistance) @@ -906,3 +886,18 @@ func (b *Batcher) newBatchConfig(maxTimeoutDistance int32) batchConfig { customMuSig2Signer: b.customMuSig2Signer, } } + +// newBatchKit creates new batch kit. +func (b *Batcher) newBatchKit() batchKit { + return batchKit{ + returnChan: b.sweepReqs, + wallet: b.wallet, + chainNotifier: b.chainNotifier, + signerClient: b.signerClient, + musig2SignSweep: b.musig2ServerSign, + verifySchnorrSig: b.VerifySchnorrSig, + purger: b.AddSweep, + store: b.store, + quit: b.quit, + } +} From e8141578acde61236701800117e70b32135e7880 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 27 Jun 2024 14:10:24 -0300 Subject: [PATCH 04/12] 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 c82461918..47ab5731b 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 578a427df..f67b828f8 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 82c82584b..592f86cd1 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) { From 019d3c613f6ea423b56c5cb448e54e5eb12bf20f Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Fri, 28 Jun 2024 21:34:12 -0300 Subject: [PATCH 05/12] sweep: factor out function AddOutputEstimate It adds output to transaction weight estimator depending on address type. --- sweep/sweeper.go | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 8d89ef43a..0af0ed8c4 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -207,6 +207,30 @@ func (s *Sweeper) GetSweepFeeDetails(ctx context.Context, // Calculate weight for this tx. var weightEstimate input.TxWeightEstimator + + // Add output. + if err := AddOutputEstimate(&weightEstimate, destAddr); err != nil { + return 0, 0, 0, fmt.Errorf("failed to add output weight "+ + "estimate: %w", err) + } + + // Add input. + err = addInputEstimate(&weightEstimate) + if err != nil { + return 0, 0, 0, fmt.Errorf("failed to add input weight "+ + "estimate: %w", err) + } + + // Find weight. + weight := weightEstimate.Weight() + + return feeRate.FeeForWeight(weight), feeRate, weight, nil +} + +// AddOutputEstimate adds output to weight estimator. +func AddOutputEstimate(weightEstimate *input.TxWeightEstimator, + destAddr btcutil.Address) error { + switch destAddr.(type) { case *btcutil.AddressWitnessScriptHash: weightEstimate.AddP2WSHOutput() @@ -224,16 +248,8 @@ func (s *Sweeper) GetSweepFeeDetails(ctx context.Context, weightEstimate.AddP2TROutput() default: - return 0, 0, 0, fmt.Errorf("estimate fee: unknown address "+ - "type %T", destAddr) - } - - err = addInputEstimate(&weightEstimate) - if err != nil { - return 0, 0, 0, err + return fmt.Errorf("unknown address type %T", destAddr) } - weight := weightEstimate.Weight() - - return feeRate.FeeForWeight(weight), feeRate, weight, nil + return nil } From 44d9147175481adcd370b748150c5aa84a73d2e2 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Fri, 28 Jun 2024 21:34:55 -0300 Subject: [PATCH 06/12] sweepbatcher: add greedy batch selection algorithm If custom fee rates are used, the algorithm is tried. It selects a batch for the sweep using the greedy algorithm, which minimizes costs, and adds the sweep to the batch. If it fails, old algorithm is used, trying to add to any batch, or starting a new batch. --- sweepbatcher/greedy_batch_selection.go | 347 ++++++++++ sweepbatcher/greedy_batch_selection_test.go | 698 ++++++++++++++++++++ sweepbatcher/sweep_batch.go | 6 + sweepbatcher/sweep_batcher.go | 31 +- 4 files changed, 1079 insertions(+), 3 deletions(-) create mode 100644 sweepbatcher/greedy_batch_selection.go create mode 100644 sweepbatcher/greedy_batch_selection_test.go diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go new file mode 100644 index 000000000..d1ebf3841 --- /dev/null +++ b/sweepbatcher/greedy_batch_selection.go @@ -0,0 +1,347 @@ +package sweepbatcher + +import ( + "context" + "errors" + "fmt" + "sort" + + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/txscript" + sweeppkg "github.com/lightninglabs/loop/sweep" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" +) + +// greedyAddSweep selects a batch for the sweep using the greedy algorithm, +// which minimizes costs, and adds the sweep to the batch. To accomplish this, +// it first collects fee details about the sweep being added, about a potential +// new batch composed of this sweep only, and about all existing batches. Then +// it passes the data to selectBatches() function, which emulates adding the +// sweep to each batch and creating new batch for the sweep, and calculates the +// costs of each alternative. Based on the estimates of selectBatches(), this +// method adds the sweep to the batch that results in the least overall fee +// increase, or creates new batch for it. If the sweep is not accepted by an +// existing batch (may happen because of too distant timeouts), next batch is +// tried in the list returned by selectBatches(). If adding fails or new batch +// creation fails, this method returns an error. If this method fails for any +// reason, the caller falls back to the simple algorithm (method handleSweep). +func (b *Batcher) greedyAddSweep(ctx context.Context, sweep *sweep) error { + if b.customFeeRate == nil { + return errors.New("greedy batch selection algorithm requires " + + "setting custom fee rate provider") + } + + // Collect weight and fee rate info about the sweep and new batch. + sweepFeeDetails, newBatchFeeDetails, err := estimateSweepFeeIncrement( + sweep, + ) + if err != nil { + return fmt.Errorf("failed to estimate tx weight for "+ + "sweep %x: %w", sweep.swapHash[:6], err) + } + + // Collect weight and fee rate info about existing batches. + batches := make([]feeDetails, 0, len(b.batches)) + for _, existingBatch := range b.batches { + batchFeeDetails, err := estimateBatchWeight(existingBatch) + if err != nil { + return fmt.Errorf("failed to estimate tx weight for "+ + "batch %d: %w", existingBatch.id, err) + } + batches = append(batches, batchFeeDetails) + } + + // Run the algorithm. Get batchId of possible batches, sorted from best + // to worst. + batchesIds, err := selectBatches( + batches, sweepFeeDetails, newBatchFeeDetails, + ) + if err != nil { + return fmt.Errorf("batch selection algorithm failed for sweep "+ + "%x: %w", sweep.swapHash[:6], err) + } + + // Try batches, starting with the best. + for _, batchId := range batchesIds { + // If the best option is to start new batch, do it. + if batchId == newBatchSignal { + return b.spinUpNewBatch(ctx, sweep) + } + + // Locate the batch to add the sweep to. + bestBatch, has := b.batches[batchId] + if !has { + return fmt.Errorf("batch selection algorithm returned "+ + "batch id %d which doesn't exist, for sweep %x", + batchId, sweep.swapHash[:6]) + } + + // Add the sweep to the batch. + accepted, err := bestBatch.addSweep(ctx, sweep) + if err != nil { + return fmt.Errorf("batch selection algorithm returned "+ + "batch id %d for sweep %x, but adding failed: "+ + "%w", batchId, sweep.swapHash[:6], err) + } + if accepted { + return nil + } + + log.Debugf("Batch selection algorithm returned batch id %d for"+ + " sweep %x, but acceptance failed.", batchId, + sweep.swapHash[:6]) + } + + return fmt.Errorf("no batch accepted sweep %x", sweep.swapHash[:6]) +} + +// estimateSweepFeeIncrement returns fee details for adding the sweep to a batch +// and for creating new batch with this sweep only. +func estimateSweepFeeIncrement(s *sweep) (feeDetails, feeDetails, error) { + // Create a fake batch with this sweep. + batch := &batch{ + rbfCache: rbfCache{ + FeeRate: s.minFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + s.swapHash: *s, + }, + } + + // Estimate new batch. + fd1, err := estimateBatchWeight(batch) + if err != nil { + return feeDetails{}, feeDetails{}, err + } + + // Add the same sweep again to measure weight increments. + swapHash2 := s.swapHash + swapHash2[0]++ + batch.sweeps[swapHash2] = *s + + // Estimate weight of a batch with two sweeps. + fd2, err := estimateBatchWeight(batch) + if err != nil { + return feeDetails{}, feeDetails{}, err + } + + // Create feeDetails for sweep. + sweepFeeDetails := feeDetails{ + FeeRate: s.minFeeRate, + NonCoopHint: s.nonCoopHint, + IsExternalAddr: s.isExternalAddr, + + // Calculate sweep weight as a difference. + CoopWeight: fd2.CoopWeight - fd1.CoopWeight, + NonCoopWeight: fd2.NonCoopWeight - fd1.NonCoopWeight, + } + + return sweepFeeDetails, fd1, nil +} + +// estimateBatchWeight estimates batch weight and returns its fee details. +func estimateBatchWeight(batch *batch) (feeDetails, error) { + // Make sure the batch is not empty. + if len(batch.sweeps) == 0 { + return feeDetails{}, errors.New("empty batch") + } + + // Make sure fee rate is valid. + if batch.rbfCache.FeeRate < chainfee.AbsoluteFeePerKwFloor { + return feeDetails{}, fmt.Errorf("feeRate is too low: %v", + batch.rbfCache.FeeRate) + } + + // Find if the batch has at least one non-cooperative sweep. + hasNonCoop := false + for _, sweep := range batch.sweeps { + if sweep.nonCoopHint { + hasNonCoop = true + } + } + + // Find some sweep of the batch. It is used if there is just one sweep. + var theSweep sweep + for _, sweep := range batch.sweeps { + theSweep = sweep + break + } + + // Find sweep destination address (type) for weight estimations. + var destAddr btcutil.Address + if theSweep.isExternalAddr { + if theSweep.destAddr == nil { + return feeDetails{}, errors.New("isExternalAddr=true," + + " but destAddr is nil") + } + destAddr = theSweep.destAddr + } else { + // Assume it is taproot by default. + destAddr = (*btcutil.AddressTaproot)(nil) + } + + // Make two estimators: for coop and non-coop cases. + var coopWeight, nonCoopWeight input.TxWeightEstimator + + // Add output weight to the estimator. + err := sweeppkg.AddOutputEstimate(&coopWeight, destAddr) + if err != nil { + return feeDetails{}, fmt.Errorf("sweep.AddOutputEstimate: %w", + err) + } + err = sweeppkg.AddOutputEstimate(&nonCoopWeight, destAddr) + if err != nil { + return feeDetails{}, fmt.Errorf("sweep.AddOutputEstimate: %w", + err) + } + + // Add inputs. + for _, sweep := range batch.sweeps { + // TODO: it should be txscript.SigHashDefault. + coopWeight.AddTaprootKeySpendInput(txscript.SigHashAll) + + err = sweep.htlcSuccessEstimator(&nonCoopWeight) + if err != nil { + return feeDetails{}, fmt.Errorf("htlcSuccessEstimator "+ + "failed: %w", err) + } + } + + return feeDetails{ + BatchId: batch.id, + FeeRate: batch.rbfCache.FeeRate, + CoopWeight: coopWeight.Weight(), + NonCoopWeight: nonCoopWeight.Weight(), + NonCoopHint: hasNonCoop, + IsExternalAddr: theSweep.isExternalAddr, + }, nil +} + +// newBatchSignal is the value that indicates a new batch. It is returned by +// selectBatches to encode new batch creation. +const newBatchSignal = -1 + +// feeDetails is either a batch or a sweep and it holds data important for +// selection of a batch to add the sweep to (or new batch creation). +type feeDetails struct { + BatchId int32 + FeeRate chainfee.SatPerKWeight + CoopWeight lntypes.WeightUnit + NonCoopWeight lntypes.WeightUnit + NonCoopHint bool + IsExternalAddr bool +} + +// fee returns fee of onchain transaction representing this instance. +func (e feeDetails) fee() btcutil.Amount { + var weight lntypes.WeightUnit + if e.NonCoopHint { + weight = e.NonCoopWeight + } else { + weight = e.CoopWeight + } + + return e.FeeRate.FeeForWeight(weight) +} + +// combine returns new feeDetails, combining properties. +func (e1 feeDetails) combine(e2 feeDetails) feeDetails { + // The fee rate is max of two fee rates. + feeRate := e1.FeeRate + if feeRate < e2.FeeRate { + feeRate = e2.FeeRate + } + + return feeDetails{ + FeeRate: feeRate, + CoopWeight: e1.CoopWeight + e2.CoopWeight, + NonCoopWeight: e1.NonCoopWeight + e2.NonCoopWeight, + NonCoopHint: e1.NonCoopHint || e2.NonCoopHint, + IsExternalAddr: e1.IsExternalAddr || e2.IsExternalAddr, + } +} + +// selectBatches returns the list of id of batches sorted from best to worst. +// Creation a new batch is encoded as newBatchSignal. For each batch its fee +// rate and two weights are provided: weight in case of cooperative spending and +// weight in case non-cooperative spending (using preimages instead of taproot +// key spend). Also, a hint is provided to signal if the batch has to use +// non-cooperative spending path. The same data is also provided to the sweep +// for which we are selecting a batch to add. In case of the sweep weights are +// weight deltas resulted from adding the sweep. Finally, the same data is +// provided for new batch having this sweep only. The algorithm compares costs +// of adding the sweep to each existing batch, and costs of new batch creation +// for this sweep and returns BatchId of the winning batch. If the best option +// is to create a new batch, return newBatchSignal. Each fee details has also +// IsExternalAddr flag. There is a rule that sweeps having flag IsExternalAddr +// must go in individual batches. Cooperative spending is only available if all +// the sweeps support cooperative spending path. +func selectBatches(batches []feeDetails, sweep, oneSweepBatch feeDetails) ( + []int32, error) { + + // If the sweep has IsExternalAddr flag, the sweep can't be added to + // a batch, so create new batch for it. + if sweep.IsExternalAddr { + return []int32{newBatchSignal}, nil + } + + // alternative holds batch ID and its cost. + type alternative struct { + batchId int32 + cost btcutil.Amount + } + + // Create the list of possible actions and their costs. + alternatives := make([]alternative, 0, len(batches)+1) + + // Track the best batch to add a sweep to. The default case is new batch + // creation with this sweep only in it. The cost is its full fee. + alternatives = append(alternatives, alternative{ + batchId: newBatchSignal, + cost: oneSweepBatch.fee(), + }) + + // Try to add the sweep to every batch, calculate the costs and + // find the batch adding to which results in minimum costs. + for _, batch := range batches { + // If the batch has IsExternalAddr flag, the sweep can't be + // added to it, so skip the batch. + if batch.IsExternalAddr { + continue + } + + // Add the sweep to the batch virtually. + combinedBatch := batch.combine(sweep) + + // The cost is the fee increase. + cost := combinedBatch.fee() - batch.fee() + + // The cost must be positive, because we added a sweep. + if cost <= 0 { + return nil, fmt.Errorf("got non-positive cost of "+ + "adding sweep to batch %d: %d", batch.BatchId, + cost) + } + + // Track the best batch, according to the costs. + alternatives = append(alternatives, alternative{ + batchId: batch.BatchId, + cost: cost, + }) + } + + // Sort the alternatives by cost. The lower the cost, the better. + sort.Slice(alternatives, func(i, j int) bool { + return alternatives[i].cost < alternatives[j].cost + }) + + // Collect batches IDs. + batchesIds := make([]int32, len(alternatives)) + for i, alternative := range alternatives { + batchesIds[i] = alternative.batchId + } + + return batchesIds, nil +} diff --git a/sweepbatcher/greedy_batch_selection_test.go b/sweepbatcher/greedy_batch_selection_test.go new file mode 100644 index 000000000..8fa9c08e5 --- /dev/null +++ b/sweepbatcher/greedy_batch_selection_test.go @@ -0,0 +1,698 @@ +package sweepbatcher + +import ( + "testing" + + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" + "github.com/lightninglabs/loop/swap" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" +) + +// Useful constants for tests. +const ( + lowFeeRate = chainfee.FeePerKwFloor + highFeeRate = chainfee.SatPerKWeight(30000) + + coopInputWeight = lntypes.WeightUnit(231) + nonCoopInputWeight = lntypes.WeightUnit(521) + nonCoopPenalty = nonCoopInputWeight - coopInputWeight + coopNewBatchWeight = lntypes.WeightUnit(445) + nonCoopNewBatchWeight = coopNewBatchWeight + nonCoopPenalty + + // p2pkhDiscount is weight discount P2PKH output has over P2TR output. + p2pkhDiscount = lntypes.WeightUnit( + input.P2TROutputSize-input.P2PKHOutputSize, + ) * 4 + + coopTwoSweepBatchWeight = coopNewBatchWeight + coopInputWeight + nonCoopTwoSweepBatchWeight = coopTwoSweepBatchWeight + + 2*nonCoopPenalty + v2v3BatchWeight = nonCoopTwoSweepBatchWeight - 153 +) + +// testHtlcV2SuccessEstimator adds weight of non-cooperative input to estimator +// using HTLC v2. +func testHtlcV2SuccessEstimator(estimator *input.TxWeightEstimator) error { + swapHash := lntypes.Hash{1, 1, 1} + htlc, err := swap.NewHtlcV2( + 111, htlcKeys.SenderScriptKey, htlcKeys.ReceiverScriptKey, + swapHash, &chaincfg.RegressionNetParams, + ) + if err != nil { + return err + } + return htlc.AddSuccessToEstimator(estimator) +} + +// testHtlcV3SuccessEstimator adds weight of non-cooperative input to estimator +// using HTLC v3. +func testHtlcV3SuccessEstimator(estimator *input.TxWeightEstimator) error { + swapHash := lntypes.Hash{1, 1, 1} + htlc, err := swap.NewHtlcV3( + input.MuSig2Version100RC2, 111, + htlcKeys.SenderInternalPubKey, htlcKeys.ReceiverInternalPubKey, + htlcKeys.SenderScriptKey, htlcKeys.ReceiverScriptKey, swapHash, + &chaincfg.RegressionNetParams, + ) + if err != nil { + return err + } + return htlc.AddSuccessToEstimator(estimator) +} + +// TestEstimateSweepFeeIncrement tests that weight and fee estimations work +// correctly for a sweep and one sweep batch. +func TestEstimateSweepFeeIncrement(t *testing.T) { + // Useful variables reused in test cases. + se3 := testHtlcV3SuccessEstimator + trAddr := (*btcutil.AddressTaproot)(nil) + p2pkhAddr := (*btcutil.AddressPubKeyHash)(nil) + + cases := []struct { + name string + sweep *sweep + wantSweepFeeDetails feeDetails + wantNewBatchFeeDetails feeDetails + }{ + { + name: "regular", + sweep: &sweep{ + minFeeRate: lowFeeRate, + htlcSuccessEstimator: se3, + }, + wantSweepFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + wantNewBatchFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + + { + name: "high fee rate", + sweep: &sweep{ + minFeeRate: highFeeRate, + htlcSuccessEstimator: se3, + }, + wantSweepFeeDetails: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + wantNewBatchFeeDetails: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + + { + name: "isExternalAddr taproot", + sweep: &sweep{ + minFeeRate: lowFeeRate, + htlcSuccessEstimator: se3, + isExternalAddr: true, + destAddr: trAddr, + }, + wantSweepFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + IsExternalAddr: true, + }, + wantNewBatchFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + IsExternalAddr: true, + }, + }, + + { + name: "isExternalAddr P2PKH", + sweep: &sweep{ + minFeeRate: lowFeeRate, + htlcSuccessEstimator: se3, + isExternalAddr: true, + destAddr: p2pkhAddr, + }, + wantSweepFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + IsExternalAddr: true, + }, + wantNewBatchFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight - + p2pkhDiscount, + NonCoopWeight: nonCoopNewBatchWeight - + p2pkhDiscount, + IsExternalAddr: true, + }, + }, + + { + name: "non-coop", + sweep: &sweep{ + minFeeRate: lowFeeRate, + htlcSuccessEstimator: se3, + nonCoopHint: true, + }, + wantSweepFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + NonCoopHint: true, + }, + wantNewBatchFeeDetails: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + gotSweepFeeDetails, gotNewBatchFeeDetails, err := + estimateSweepFeeIncrement(tc.sweep) + require.NoError(t, err) + require.Equal( + t, tc.wantSweepFeeDetails, gotSweepFeeDetails, + ) + require.Equal( + t, tc.wantNewBatchFeeDetails, + gotNewBatchFeeDetails, + ) + }) + } +} + +// TestEstimateBatchWeight tests that weight and fee estimations work correctly +// for batches. +func TestEstimateBatchWeight(t *testing.T) { + // Useful variables reused in test cases. + swapHash1 := lntypes.Hash{1, 1, 1} + swapHash2 := lntypes.Hash{2, 2, 2} + se2 := testHtlcV2SuccessEstimator + se3 := testHtlcV3SuccessEstimator + trAddr := (*btcutil.AddressTaproot)(nil) + + cases := []struct { + name string + batch *batch + wantBatchFeeDetails feeDetails + }{ + { + name: "one sweep regular batch", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se3, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + + { + name: "two sweeps regular batch", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se3, + }, + swapHash2: { + htlcSuccessEstimator: se3, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopTwoSweepBatchWeight, + NonCoopWeight: nonCoopTwoSweepBatchWeight, + }, + }, + + { + name: "v2 and v3 sweeps", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se2, + }, + swapHash2: { + htlcSuccessEstimator: se3, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopTwoSweepBatchWeight, + NonCoopWeight: v2v3BatchWeight, + }, + }, + + { + name: "high fee rate", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: highFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se3, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + + { + name: "non-coop", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se3, + }, + swapHash2: { + htlcSuccessEstimator: se3, + nonCoopHint: true, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopTwoSweepBatchWeight, + NonCoopWeight: nonCoopTwoSweepBatchWeight, + NonCoopHint: true, + }, + }, + + { + name: "isExternalAddr", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[lntypes.Hash]sweep{ + swapHash1: { + htlcSuccessEstimator: se3, + isExternalAddr: true, + destAddr: trAddr, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + IsExternalAddr: true, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + gotBatchFeeDetails, err := estimateBatchWeight(tc.batch) + require.NoError(t, err) + require.Equal( + t, tc.wantBatchFeeDetails, gotBatchFeeDetails, + ) + }) + } +} + +// TestSelectBatches tests greedy batch selection algorithm. +func TestSelectBatches(t *testing.T) { + cases := []struct { + name string + batches []feeDetails + sweep, oneSweepBatch feeDetails + wantBestBatchesIds []int32 + }{ + { + name: "no existing batches", + batches: []feeDetails{}, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{newBatchSignal}, + }, + + { + name: "low fee sweep, low fee existing batch", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{1, newBatchSignal}, + }, + + { + name: "low fee sweep, high fee existing batch", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{newBatchSignal, 1}, + }, + + { + name: "low fee sweep, low + high fee existing batches", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{1, newBatchSignal, 2}, + }, + + { + name: "high fee sweep, low + high fee existing batches", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{2, newBatchSignal, 1}, + }, + + { + name: "high fee noncoop sweep", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + NonCoopHint: true, + }, + oneSweepBatch: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + wantBestBatchesIds: []int32{newBatchSignal, 2, 1}, + }, + + { + name: "high fee noncoop sweep, high batch noncoop", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + }, + sweep: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + NonCoopHint: true, + }, + oneSweepBatch: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + wantBestBatchesIds: []int32{2, newBatchSignal, 1}, + }, + + { + name: "low fee noncoop sweep", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + NonCoopHint: true, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + wantBestBatchesIds: []int32{newBatchSignal, 1, 2}, + }, + + { + name: "low fee noncoop sweep, low batch noncoop", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + NonCoopHint: true, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + NonCoopHint: true, + }, + wantBestBatchesIds: []int32{1, newBatchSignal, 2}, + }, + + { + name: "external address sweep", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + IsExternalAddr: true, + }, + oneSweepBatch: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + IsExternalAddr: true, + }, + wantBestBatchesIds: []int32{newBatchSignal}, + }, + + { + name: "external address batch", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: highFeeRate - 1, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + { + BatchId: 2, + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + IsExternalAddr: true, + }, + }, + sweep: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopInputWeight, + NonCoopWeight: nonCoopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: highFeeRate, + CoopWeight: coopNewBatchWeight, + NonCoopWeight: nonCoopNewBatchWeight, + }, + wantBestBatchesIds: []int32{1, newBatchSignal}, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + gotBestBatchesIds, err := selectBatches( + tc.batches, tc.sweep, tc.oneSweepBatch, + ) + require.NoError(t, err) + require.Equal( + t, tc.wantBestBatchesIds, gotBestBatchesIds, + ) + }) + } +} diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 47ab5731b..0d071ec1c 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -101,6 +101,11 @@ type sweep struct { // 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 + + // nonCoopHint is set, if the sweep can not be spent cooperatively and + // has to be spent using preimage. This is only used in fee estimations + // when selecting a batch for the sweep to minimize fees. + nonCoopHint bool } // batchState is the state of the batch. @@ -838,6 +843,7 @@ func (b *batch) publishBatchCoop(ctx context.Context) (btcutil.Amount, PreviousOutPoint: sweep.outpoint, }) + // TODO: it should be txscript.SigHashDefault. weightEstimate.AddTaprootKeySpendInput(txscript.SigHashAll) } diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index f67b828f8..bfbeb2ce9 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -121,6 +121,11 @@ type SweepInfo struct { // DestAddr is the destination address of the sweep. DestAddr btcutil.Address + + // NonCoopHint is set, if the sweep can not be spent cooperatively and + // has to be spent using preimage. This is only used in fee estimations + // when selecting a batch for the sweep to minimize fees. + NonCoopHint bool } // SweepFetcher is used to get details of a sweep. @@ -465,6 +470,19 @@ func (b *Batcher) handleSweep(ctx context.Context, sweep *sweep, } } + // If custom fee rate provider is used, run the greedy algorithm of + // batch selection to minimize costs. + if b.customFeeRate != nil { + err := b.greedyAddSweep(ctx, sweep) + if err == nil { + // The greedy algorithm succeeded. + return nil + } + + log.Warnf("Greedy batch selection algorithm failed for sweep "+ + "%x, falling back to old approach.", sweep.swapHash[:6]) + } + // If one of the batches accepts the sweep, we provide it to that batch. for _, batch := range b.batches { accepted, err := batch.addSweep(ctx, sweep) @@ -481,13 +499,19 @@ func (b *Batcher) handleSweep(ctx context.Context, sweep *sweep, // If no batch is capable of accepting the sweep, we spin up a fresh // batch and hand the sweep over to it. - batch, err := b.spinUpBatch(ctx) + return b.spinUpNewBatch(ctx, sweep) +} + +// spinUpNewBatch creates new batch, starts it and adds the sweep to it. +func (b *Batcher) spinUpNewBatch(ctx context.Context, sweep *sweep) error { + // Spin up a fresh batch. + newBatch, err := b.spinUpBatch(ctx) if err != nil { return err } // Add the sweep to the fresh batch. - accepted, err := batch.addSweep(ctx, sweep) + accepted, err := newBatch.addSweep(ctx, sweep) if err != nil { return err } @@ -496,7 +520,7 @@ func (b *Batcher) handleSweep(ctx context.Context, sweep *sweep, // we should return the error. if !accepted { return fmt.Errorf("sweep %x was not accepted by new batch %d", - sweep.swapHash[:6], batch.id) + sweep.swapHash[:6], newBatch.id) } return nil @@ -889,6 +913,7 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, isExternalAddr: s.IsExternalAddr, destAddr: s.DestAddr, minFeeRate: minFeeRate, + nonCoopHint: s.NonCoopHint, }, nil } From db5e0d1d279149f0ca322452f74c2995bde9dd03 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 29 Jun 2024 23:55:32 -0300 Subject: [PATCH 07/12] sweepbatcher: fix inaccuracies in fee estimations Use SigHashDefault instead of SigHashAll in weight estimations. Actual code uses SigHashDefault, not SigHashAll. SigHashAll is 1wu larger. Use the actual destination address in weight estimator, not taproot always. In the test the type of address is P2WPKH, not taproot. Updated testSweepFetcher to calculate fee, fee rate and weight accurately and to compare the data observed in the published transaction with the estimates. --- sweepbatcher/greedy_batch_selection.go | 3 +-- sweepbatcher/greedy_batch_selection_test.go | 4 ++-- sweepbatcher/sweep_batch.go | 14 ++++++++++---- sweepbatcher/sweep_batcher_test.go | 14 +++++++++++--- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go index d1ebf3841..09bdee53c 100644 --- a/sweepbatcher/greedy_batch_selection.go +++ b/sweepbatcher/greedy_batch_selection.go @@ -199,8 +199,7 @@ func estimateBatchWeight(batch *batch) (feeDetails, error) { // Add inputs. for _, sweep := range batch.sweeps { - // TODO: it should be txscript.SigHashDefault. - coopWeight.AddTaprootKeySpendInput(txscript.SigHashAll) + coopWeight.AddTaprootKeySpendInput(txscript.SigHashDefault) err = sweep.htlcSuccessEstimator(&nonCoopWeight) if err != nil { diff --git a/sweepbatcher/greedy_batch_selection_test.go b/sweepbatcher/greedy_batch_selection_test.go index 8fa9c08e5..a12f0aeac 100644 --- a/sweepbatcher/greedy_batch_selection_test.go +++ b/sweepbatcher/greedy_batch_selection_test.go @@ -17,10 +17,10 @@ const ( lowFeeRate = chainfee.FeePerKwFloor highFeeRate = chainfee.SatPerKWeight(30000) - coopInputWeight = lntypes.WeightUnit(231) + coopInputWeight = lntypes.WeightUnit(230) nonCoopInputWeight = lntypes.WeightUnit(521) nonCoopPenalty = nonCoopInputWeight - coopInputWeight - coopNewBatchWeight = lntypes.WeightUnit(445) + coopNewBatchWeight = lntypes.WeightUnit(444) nonCoopNewBatchWeight = coopNewBatchWeight + nonCoopPenalty // p2pkhDiscount is weight discount P2PKH output has over P2TR output. diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 0d071ec1c..dd9e5da7b 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -22,6 +22,7 @@ import ( "github.com/lightninglabs/loop/labels" "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" + sweeppkg "github.com/lightninglabs/loop/sweep" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" @@ -737,7 +738,10 @@ func (b *batch) publishBatch(ctx context.Context) (btcutil.Amount, error) { return fee, err } - weightEstimate.AddP2TROutput() + err = sweeppkg.AddOutputEstimate(&weightEstimate, address) + if err != nil { + return fee, err + } weight := weightEstimate.Weight() feeForWeight := b.rbfCache.FeeRate.FeeForWeight(weight) @@ -843,8 +847,7 @@ func (b *batch) publishBatchCoop(ctx context.Context) (btcutil.Amount, PreviousOutPoint: sweep.outpoint, }) - // TODO: it should be txscript.SigHashDefault. - weightEstimate.AddTaprootKeySpendInput(txscript.SigHashAll) + weightEstimate.AddTaprootKeySpendInput(txscript.SigHashDefault) } var address btcutil.Address @@ -870,7 +873,10 @@ func (b *batch) publishBatchCoop(ctx context.Context) (btcutil.Amount, return fee, err, false } - weightEstimate.AddP2TROutput() + err = sweeppkg.AddOutputEstimate(&weightEstimate, address) + if err != nil { + return fee, err, false + } weight := weightEstimate.Weight() feeForWeight := b.rbfCache.FeeRate.FeeForWeight(weight) diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index 592f86cd1..7314e9798 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -1937,7 +1938,7 @@ func testSweepFetcher(t *testing.T, store testStore, // Provide min fee rate for the sweep. feeRate := chainfee.SatPerKWeight(30000) amt := btcutil.Amount(1_000_000) - weight := lntypes.WeightUnit(445) // Weight for 1-to-1 tx. + weight := lntypes.WeightUnit(396) // Weight for 1-to-1 tx. expectedFee := feeRate.FeeForWeight(weight) swap := &loopdb.LoopOutContract{ @@ -2000,8 +2001,9 @@ func testSweepFetcher(t *testing.T, store testStore, } batcher := NewBatcher(lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, - testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, - batcherStore, sweepFetcher, WithCustomFeeRate(customFeeRate)) + nil, testVerifySchnorrSig, lnd.ChainParams, + batcherStore, sweepFetcher, WithCustomFeeRate(customFeeRate), + WithCustomSignMuSig2(testSignMuSig2func)) var wg sync.WaitGroup wg.Add(1) @@ -2052,6 +2054,12 @@ func testSweepFetcher(t *testing.T, store testStore, out := btcutil.Amount(tx.TxOut[0].Value) gotFee := amt - out require.Equal(t, expectedFee, gotFee, "fees don't match") + gotWeight := lntypes.WeightUnit( + blockchain.GetTransactionWeight(btcutil.NewTx(tx)), + ) + require.Equal(t, weight, gotWeight, "weights don't match") + gotFeeRate := chainfee.NewSatPerKWeight(gotFee, gotWeight) + require.Equal(t, feeRate, gotFeeRate, "fee rates don't match") // Make sure we have stored the batch. batches, err := batcherStore.FetchUnconfirmedSweepBatches(ctx) From f5e97c035d6f92d49493bfa31fc33be2908a3225 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sun, 30 Jun 2024 22:59:08 -0300 Subject: [PATCH 08/12] sweepbatcher: log batcher failures This is useful to debug. It used to fail silently. --- sweepbatcher/sweep_batcher.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index bfbeb2ce9..7e66207d5 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -375,18 +375,22 @@ func (b *Batcher) Run(ctx context.Context) error { case sweepReq := <-b.sweepReqs: sweep, err := b.fetchSweep(runCtx, sweepReq) if err != nil { + log.Warnf("fetchSweep failed: %v.", err) return err } err = b.handleSweep(runCtx, sweep, sweepReq.Notifier) if err != nil { + log.Warnf("handleSweep failed: %v.", err) return err } case err := <-b.errChan: + log.Warnf("Batcher received an error: %v.", err) return err case <-runCtx.Done(): + log.Infof("Stopping Batcher: run context cancelled.") return runCtx.Err() } } From 5dac7ca75bab202296d86bcf6df6de6898faf0f0 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Mon, 15 Jul 2024 11:22:48 -0300 Subject: [PATCH 09/12] sweep: fix godoc of GetSweepFeeDetails --- sweep/sweeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sweep/sweeper.go b/sweep/sweeper.go index 0af0ed8c4..e6b3df462 100644 --- a/sweep/sweeper.go +++ b/sweep/sweeper.go @@ -191,8 +191,8 @@ func (s *Sweeper) GetSweepFee(ctx context.Context, return fee, err } -// GetSweepFee calculates the required tx fee to spend to P2WKH. It takes a -// function that is expected to add the weight of the input to the weight +// GetSweepFeeDetails calculates the required tx fee to spend to P2WKH. It takes +// a function that is expected to add the weight of the input to the weight // estimator. It returns also the fee rate and transaction weight. func (s *Sweeper) GetSweepFeeDetails(ctx context.Context, addInputEstimate func(*input.TxWeightEstimator) error, From 75641c3573f8cea2c8a1f0f00dbcc2c6748bcbaa Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Mon, 15 Jul 2024 12:24:57 -0300 Subject: [PATCH 10/12] sweepbatcher: always fill sweep.minFeeRate Use customFeeRate if it is provided, otherwise use wallet's EstimateFeeRate. Added flag SkipNextBump to rbfCache to avoid extra bumping upon updating fee rate externally. Fix test TestSweepBatcherCloseDuringAdding, it didn't have confTarget and failed in wallet.EstimateFeeRate call. --- sweepbatcher/sweep_batch.go | 21 +++++++++++++++++++-- sweepbatcher/sweep_batcher.go | 16 +++++++++++++--- sweepbatcher/sweep_batcher_test.go | 5 +++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index dd9e5da7b..9a18aba10 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -156,6 +156,10 @@ type rbfCache struct { // FeeRate is the last used fee rate we used to publish a batch tx. FeeRate chainfee.SatPerKWeight + + // SkipNextBump instructs updateRbfRate to skip one fee bumping. + // It is set upon updating FeeRate externally. + SkipNextBump bool } // batch is a collection of sweeps that are published together. @@ -417,6 +421,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { if b.primarySweepID == sweep.swapHash { b.cfg.batchConfTarget = sweep.confTarget b.rbfCache.FeeRate = sweep.minFeeRate + b.rbfCache.SkipNextBump = true } return true, nil @@ -459,6 +464,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { b.primarySweepID = sweep.swapHash b.cfg.batchConfTarget = sweep.confTarget b.rbfCache.FeeRate = sweep.minFeeRate + b.rbfCache.SkipNextBump = true // We also need to start the spend monitor for this new primary // sweep. @@ -476,6 +482,7 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { // the batch is the basis for fee bumps. if b.rbfCache.FeeRate < sweep.minFeeRate { b.rbfCache.FeeRate = sweep.minFeeRate + b.rbfCache.SkipNextBump = true } return true, b.persistSweep(ctx, *sweep, false) @@ -1141,10 +1148,15 @@ func (b *batch) updateRbfRate(ctx context.Context) error { // If the feeRate is unset then we never published before, so we // retrieve the fee estimate from our wallet. if b.rbfCache.FeeRate == 0 { + // We set minFeeRate in each sweep, so fee rate is expected to + // be initiated here. + b.log.Warnf("rbfCache.FeeRate is 0, which must not happen.") + if b.cfg.batchConfTarget == 0 { b.log.Warnf("updateRbfRate called with zero " + "batchConfTarget") } + b.log.Infof("initializing rbf fee rate for conf target=%v", b.cfg.batchConfTarget) rate, err := b.wallet.EstimateFeeRate( @@ -1157,8 +1169,13 @@ func (b *batch) updateRbfRate(ctx context.Context) error { // Set the initial value for our fee rate. b.rbfCache.FeeRate = rate } else if !b.cfg.noBumping { - // Bump the fee rate by the configured step. - b.rbfCache.FeeRate += defaultFeeRateStep + if b.rbfCache.SkipNextBump { + // Skip fee bumping, unset the flag, to bump next time. + b.rbfCache.SkipNextBump = false + } else { + // Bump the fee rate by the configured step. + b.rbfCache.FeeRate += defaultFeeRateStep + } } b.rbfCache.LastHeight = b.currentHeight diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 7e66207d5..b12f391f2 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -885,9 +885,8 @@ 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. + // Find minimum fee rate for the sweep. Use customFeeRate if it is + // provided, otherwise use wallet's EstimateFeeRate. var minFeeRate chainfee.SatPerKWeight if b.customFeeRate != nil { minFeeRate, err = b.customFeeRate(ctx, swapHash) @@ -899,6 +898,17 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, return nil, fmt.Errorf("min fee rate too low (%v) for "+ "%x", minFeeRate, swapHash[:6]) } + } else { + if s.ConfTarget == 0 { + log.Warnf("Fee estimation was requested for zero "+ + "confTarget for sweep %x.", swapHash[:6]) + } + minFeeRate, err = b.wallet.EstimateFeeRate(ctx, s.ConfTarget) + if err != nil { + return nil, fmt.Errorf("failed to estimate fee rate "+ + "for %x, confTarget=%d: %w", swapHash[:6], + s.ConfTarget, err) + } } return &sweep{ diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index 7314e9798..96b77fc51 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -2110,8 +2110,9 @@ func testSweepBatcherCloseDuringAdding(t *testing.T, store testStore, Preimage: lntypes.Preimage{i}, }, - DestAddr: destAddr, - SwapInvoice: swapInvoice, + DestAddr: destAddr, + SwapInvoice: swapInvoice, + SweepConfTarget: 111, } err = store.CreateLoopOut(ctx, swapHash, swap) From 026cf0d47ab104e3cbed4a20bb52437cf31e1453 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Mon, 15 Jul 2024 12:26:43 -0300 Subject: [PATCH 11/12] sweepbatcher: always try greedy batch selection Now that sweep.minFeeRate is always set, greedy batch selection has all needed inputs to work even without customFeeRate provider. --- sweepbatcher/greedy_batch_selection.go | 5 ----- sweepbatcher/sweep_batcher.go | 19 ++++++++----------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go index 09bdee53c..15585af80 100644 --- a/sweepbatcher/greedy_batch_selection.go +++ b/sweepbatcher/greedy_batch_selection.go @@ -28,11 +28,6 @@ import ( // creation fails, this method returns an error. If this method fails for any // reason, the caller falls back to the simple algorithm (method handleSweep). func (b *Batcher) greedyAddSweep(ctx context.Context, sweep *sweep) error { - if b.customFeeRate == nil { - return errors.New("greedy batch selection algorithm requires " + - "setting custom fee rate provider") - } - // Collect weight and fee rate info about the sweep and new batch. sweepFeeDetails, newBatchFeeDetails, err := estimateSweepFeeIncrement( sweep, diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index b12f391f2..670f7e9ab 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -474,19 +474,16 @@ func (b *Batcher) handleSweep(ctx context.Context, sweep *sweep, } } - // If custom fee rate provider is used, run the greedy algorithm of - // batch selection to minimize costs. - if b.customFeeRate != nil { - err := b.greedyAddSweep(ctx, sweep) - if err == nil { - // The greedy algorithm succeeded. - return nil - } - - log.Warnf("Greedy batch selection algorithm failed for sweep "+ - "%x, falling back to old approach.", sweep.swapHash[:6]) + // Try to run the greedy algorithm of batch selection to minimize costs. + err = b.greedyAddSweep(ctx, sweep) + if err == nil { + // The greedy algorithm succeeded. + return nil } + log.Warnf("Greedy batch selection algorithm failed for sweep %x: %v. "+ + "Falling back to old approach.", sweep.swapHash[:6], err) + // If one of the batches accepts the sweep, we provide it to that batch. for _, batch := range b.batches { accepted, err := batch.addSweep(ctx, sweep) From 7a61d5e743cfd0e7018a2a840972d42268b7e11c Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Mon, 15 Jul 2024 14:44:25 -0300 Subject: [PATCH 12/12] loopout_test: enable logging in sweepbatcher --- loopout_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/loopout_test.go b/loopout_test.go index 03cd0e241..1297f90f1 100644 --- a/loopout_test.go +++ b/loopout_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "math" + "os" "testing" "time" @@ -11,6 +12,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btclog" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/sweep" @@ -255,6 +257,11 @@ func TestCustomSweepConfTarget(t *testing.T) { func testCustomSweepConfTarget(t *testing.T) { defer test.Guard(t)() + // Setup logger for sweepbatcher. + logger := btclog.NewBackend(os.Stdout).Logger("SWEEP") + logger.SetLevel(btclog.LevelTrace) + sweepbatcher.UseLogger(logger) + lnd := test.NewMockLnd() ctx := test.NewContext(t, lnd) server := newServerMock(lnd)