From 06ed714e9a8610bc73e41e67bec6c29cf98dd18a Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 9 Jan 2025 00:26:08 -0300 Subject: [PATCH 1/3] sweepbatcher: check that spending tx has an output Previously the code handling spending tx crashed if it doesn't have an output. This is likely to occur only in tests. --- sweepbatcher/sweep_batch.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 2e815899e..4e6451d16 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -1872,7 +1872,10 @@ func (b *batch) monitorConfirmations(ctx context.Context) error { func getFeePortionForSweep(spendTx *wire.MsgTx, numSweeps int, totalSweptAmt btcutil.Amount) (btcutil.Amount, btcutil.Amount) { - totalFee := int64(totalSweptAmt) - spendTx.TxOut[0].Value + totalFee := int64(totalSweptAmt) + if len(spendTx.TxOut) > 0 { + totalFee -= spendTx.TxOut[0].Value + } feePortionPerSweep := totalFee / int64(numSweeps) roundingDiff := totalFee - (int64(numSweeps) * feePortionPerSweep) @@ -1900,7 +1903,11 @@ func (b *batch) handleSpend(ctx context.Context, spendTx *wire.MsgTx) error { notifyList = make([]sweep, 0, len(b.sweeps)) ) b.batchTxid = &txHash - b.batchPkScript = spendTx.TxOut[0].PkScript + if len(spendTx.TxOut) > 0 { + b.batchPkScript = spendTx.TxOut[0].PkScript + } else { + b.log.Warnf("transaction %v has no outputs", txHash) + } // As a previous version of the batch transaction may get confirmed, // which does not contain the latest sweeps, we need to detect the From 89fbf9a5cf39ba898631c33922d5dba5370006bc Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 9 Jan 2025 00:32:16 -0300 Subject: [PATCH 2/3] sweepbatcher: rm unused field batch.blockEpochChan --- sweepbatcher/sweep_batch.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 4e6451d16..302136c96 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -214,10 +214,6 @@ type batch struct { // currentHeight is the current block height. currentHeight int32 - // blockEpochChan is the channel over which block epoch notifications - // are received. - blockEpochChan chan int32 - // spendChan is the channel over which spend notifications are received. spendChan chan *chainntnfs.SpendDetail @@ -362,7 +358,6 @@ func NewBatch(cfg batchConfig, bk batchKit) *batch { id: -1, state: Open, sweeps: make(map[lntypes.Hash]sweep), - blockEpochChan: make(chan int32), spendChan: make(chan *chainntnfs.SpendDetail), confChan: make(chan *chainntnfs.TxConfirmation, 1), reorgChan: make(chan struct{}, 1), @@ -407,7 +402,6 @@ func NewBatchFromDB(cfg batchConfig, bk batchKit) (*batch, error) { state: bk.state, primarySweepID: bk.primaryID, sweeps: bk.sweeps, - blockEpochChan: make(chan int32), spendChan: make(chan *chainntnfs.SpendDetail), confChan: make(chan *chainntnfs.TxConfirmation, 1), reorgChan: make(chan struct{}, 1), From 6efd01001440bfafae3bd528ee154ad4dd0f86ef Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Thu, 9 Jan 2025 01:43:30 -0300 Subject: [PATCH 3/3] sweepbatcher: run batch with currentHeight set Prevent a crash with "a height hint greater than 0 must be provided" error when monitorSpend starts at the beginning of batch.Run. The timer timerChan is now initialized at the start, because it was previously initialized after the first block (the current tip) was read from blockChan and now the first block is read before the main for-select loop to fill the field currentHeight in advance. --- sweepbatcher/sweep_batch.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 302136c96..a1cb339de 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -620,6 +620,16 @@ func (b *batch) Run(ctx context.Context) error { return err } + // Set currentHeight here, because it may be needed in monitorSpend. + select { + case b.currentHeight = <-blockChan: + b.log.Debugf("initial height for the batch is %v", + b.currentHeight) + + case <-runCtx.Done(): + return runCtx.Err() + } + // If a primary sweep exists we immediately start monitoring for its // spend. if b.primarySweepID != lntypes.ZeroHash { @@ -636,9 +646,9 @@ func (b *batch) Run(ctx context.Context) error { skipBefore := clock.Now().Add(b.cfg.initialDelay) // initialDelayChan is a timer which fires upon initial delay end. - // If initialDelay is 0, it does not fire to prevent race with - // blockChan which also fires immediately with current tip. Such a race - // may result in double publishing if batchPublishDelay is also 0. + // If initialDelay is set to 0, it will not trigger to avoid setting up + // timerChan twice, which could lead to double publishing if + // batchPublishDelay is also 0. var initialDelayChan <-chan time.Time if b.cfg.initialDelay > 0 { initialDelayChan = clock.TickAfter(b.cfg.initialDelay) @@ -647,9 +657,10 @@ func (b *batch) Run(ctx context.Context) error { // We use a timer in order to not publish new transactions at the same // time as the block epoch notification. This is done to prevent // unnecessary transaction publishments when a spend is detected on that - // block. This timer starts after new block arrives or initialDelay + // block. This timer starts after new block arrives (including the + // current tip which we read from blockChan above) or when initialDelay // completes. - var timerChan <-chan time.Time + timerChan := clock.TickAfter(b.cfg.batchPublishDelay) b.log.Infof("started, primary %x, total sweeps %v", b.primarySweepID[0:6], len(b.sweeps))