Merge pull request #952 from starius/presigned-fixes2

sweepbatcher: fixes for presigned mode

Author: starius

loopdb/sqlc/batch.sql.go

@@ -37,14 +37,6 @@ UPDATE sweep_batches SET
         last_rbf_sat_per_kw = $6
 WHERE id = $1;
 
--- name: ConfirmBatch :exec
-UPDATE
-        sweep_batches
-SET
-        confirmed = TRUE
-WHERE
-        id = $1;
-
 -- name: UpsertSweep :exec
 INSERT INTO sweeps (
         swap_hash,

loopdb/sqlc/querier.go

@@ -1147,7 +1147,7 @@ func (s *loopOutSwap) waitForHtlcSpendConfirmedV2(globalCtx context.Context,
 	quitChan := make(chan bool, 1)
 
 	defer func() {
-		quitChan <- true
+		close(quitChan)
 	}()
 
 	notifier := sweepbatcher.SpendNotifier{

loopdb/sqlc/queries/batch.sql

@@ -7,6 +7,7 @@ import (
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/txscript"
+	"github.com/btcsuite/btcd/wire"
 	"github.com/lightninglabs/loop/loopdb"
 	"github.com/lightninglabs/loop/swap"
 	"github.com/lightninglabs/loop/utils"
@@ -71,7 +72,8 @@ func newLoopOutSweepFeerateProvider(sweeper sweeper,
 
 // GetMinFeeRate returns minimum required feerate for a sweep by swap hash.
 func (p *loopOutSweepFeerateProvider) GetMinFeeRate(ctx context.Context,
-	swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) {
+	swapHash lntypes.Hash,
+	_ wire.OutPoint) (chainfee.SatPerKWeight, error) {
 
 	_, feeRate, err := p.GetConfTargetAndFeeRate(ctx, swapHash)
 

loopout.go

@@ -36,13 +36,7 @@ func (b *batch) ensurePresigned(ctx context.Context, newSweeps []*sweep,
 // presignedTxChecker has methods to check if the inputs are presigned.
 type presignedTxChecker interface {
 	destPkScripter
-
-	// SignTx signs an unsigned transaction or returns a pre-signed tx.
-	// It is only called with loadOnly=true by ensurePresigned.
-	SignTx(ctx context.Context, primarySweepID wire.OutPoint,
-		tx *wire.MsgTx, inputAmt btcutil.Amount,
-		minRelayFee, feeRate chainfee.SatPerKWeight,
-		loadOnly bool) (*wire.MsgTx, error)
+	presigner
 }
 
 // ensurePresigned checks that there is a presigned transaction spending the
@@ -253,7 +247,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {
 
 		// Cache the destination address.
 		destAddr, err := getPresignedSweepsDestAddr(
-			ctx, b.cfg.presignedHelper, b.primarySweepID,
+			ctx, b.cfg.presignedHelper, primarySweepID,
 			b.cfg.chainParams,
 		)
 		if err != nil {
@@ -289,11 +283,12 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {
 
 // presigner tries to presign a batch transaction.
 type presigner interface {
-	// Presign tries to presign a batch transaction. If the method returns
-	// nil, it is guaranteed that future calls to SignTx on this set of
-	// sweeps return valid signed transactions.
-	Presign(ctx context.Context, primarySweepID wire.OutPoint,
-		tx *wire.MsgTx, inputAmt btcutil.Amount) error
+	// SignTx signs an unsigned transaction or returns a pre-signed tx.
+	// It is only called with loadOnly=true by ensurePresigned.
+	SignTx(ctx context.Context, primarySweepID wire.OutPoint,
+		tx *wire.MsgTx, inputAmt btcutil.Amount,
+		minRelayFee, feeRate chainfee.SatPerKWeight,
+		loadOnly bool) (*wire.MsgTx, error)
 }
 
 // presign tries to presign batch sweep transactions of the sweeps. It signs
@@ -372,7 +367,14 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
 		}
 
 		// Try to presign this transaction.
-		err = presigner.Presign(ctx, primarySweepID, tx, batchAmt)
+		const (
+			loadOnly    = false
+			minRelayFee = chainfee.AbsoluteFeePerKwFloor
+		)
+		_, err = presigner.SignTx(
+			ctx, primarySweepID, tx, batchAmt, minRelayFee, fr,
+			loadOnly,
+		)
 		if err != nil {
 			return fmt.Errorf("failed to presign unsigned tx %v "+
 				"for feeRate %v: %w", tx.TxHash(), fr, err)
@@ -407,9 +409,16 @@ 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)
+	if err != nil {
+		return 0, fmt.Errorf("failed to get minRelayFee: %w", err),
+			false
+	}
+
 	// Cache current height and desired feerate of the batch.
 	currentHeight := b.currentHeight
-	feeRate := b.rbfCache.FeeRate
+	feeRate := max(b.rbfCache.FeeRate, minRelayFee)
 
 	// 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
@@ -447,13 +456,6 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 		batchAmt += sweep.value
 	}
 
-	// Determine the current minimum relay fee based on our chain backend.
-	minRelayFee, err := b.wallet.MinRelayFee(ctx)
-	if err != nil {
-		return 0, fmt.Errorf("failed to get minRelayFee: %w", err),
-			false
-	}
-
 	// Get a pre-signed transaction.
 	const loadOnly = false
 	signedTx, err := b.cfg.presignedHelper.SignTx(
@@ -508,6 +510,9 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
 	b.batchTxid = &txHash
 	b.batchPkScript = tx.TxOut[0].PkScript
 
+	// Update cached FeeRate not to broadcast a tx with lower feeRate.
+	b.rbfCache.FeeRate = max(b.rbfCache.FeeRate, signedFeeRate)
+
 	return fee, nil, true
 }
 
@@ -599,7 +604,7 @@ func CheckSignedTx(unsignedTx, signedTx *wire.MsgTx, inputAmt btcutil.Amount,
 	unsignedOut := unsignedTx.TxOut[0]
 	signedOut := signedTx.TxOut[0]
 	if !bytes.Equal(unsignedOut.PkScript, signedOut.PkScript) {
-		return fmt.Errorf("mismatch of output pkScript: %v, %v",
+		return fmt.Errorf("mismatch of output pkScript: %x, %x",
 			unsignedOut.PkScript, signedOut.PkScript)
 	}
 

loopout_feerate.go

@@ -612,24 +612,30 @@ type mockPresigner struct {
 	failAt int
 }
 
-// Presign memorizes the value of the output and fails if the number of
+// SignTx memorizes the value of the output and fails if the number of
 // calls previously made is failAt.
-func (p *mockPresigner) Presign(ctx context.Context,
-	primarySweepID wire.OutPoint, tx *wire.MsgTx,
-	inputAmt btcutil.Amount) error {
+func (p *mockPresigner) SignTx(ctx context.Context,
+	primarySweepID wire.OutPoint, tx *wire.MsgTx, inputAmt btcutil.Amount,
+	minRelayFee, feeRate chainfee.SatPerKWeight,
+	loadOnly bool) (*wire.MsgTx, error) {
+
+	if ctx.Err() != nil {
+		return nil, ctx.Err()
+	}
 
 	if !hasInput(tx, primarySweepID) {
-		return fmt.Errorf("primarySweepID %v not in tx", primarySweepID)
+		return nil, fmt.Errorf("primarySweepID %v not in tx",
+			primarySweepID)
 	}
 
 	if len(p.outputs)+1 == p.failAt {
-		return fmt.Errorf("test error in Presign")
+		return nil, fmt.Errorf("test error in SignTx")
 	}
 
 	p.outputs = append(p.outputs, btcutil.Amount(tx.TxOut[0].Value))
 	p.lockTimes = append(p.lockTimes, tx.LockTime)
 
-	return nil
+	return tx, nil
 }
 
 // TestPresign checks that function presign presigns correct set of transactions

sweepbatcher/presigned.go

@@ -16,9 +16,6 @@ import (
 // Querier is the interface that contains all the queries generated
 // by sqlc for sweep batcher.
 type Querier interface {
-	// ConfirmBatch confirms a batch by setting the state to confirmed.
-	ConfirmBatch(ctx context.Context, id int32) error
-
 	// GetBatchSweeps fetches all the sweeps that are part a batch.
 	GetBatchSweeps(ctx context.Context, batchID int32) (
 		[]sqlc.Sweep, error)
@@ -124,11 +121,6 @@ func (s *SQLStore) UpdateSweepBatch(ctx context.Context, batch *dbBatch) error {
 	return s.baseDb.UpdateBatch(ctx, batchToUpdateArgs(*batch))
 }
 
-// ConfirmBatch confirms a batch by setting the state to confirmed.
-func (s *SQLStore) ConfirmBatch(ctx context.Context, id int32) error {
-	return s.baseDb.ConfirmBatch(ctx, id)
-}
-
 // FetchBatchSweeps fetches all the sweeps that are part a batch.
 func (s *SQLStore) FetchBatchSweeps(ctx context.Context, id int32) (
 	[]*dbSweep, error) {
@@ -201,7 +193,7 @@ type dbBatch struct {
 	// ID is the unique identifier of the batch.
 	ID int32
 
-	// Confirmed is set when the batch is fully confirmed.
+	// Confirmed is set when the batch is reorg-safely confirmed.
 	Confirmed bool
 
 	// BatchTxid is the txid of the batch transaction.
@@ -236,7 +228,7 @@ type dbSweep struct {
 	// Amount is the amount of the sweep.
 	Amount btcutil.Amount
 
-	// Completed indicates whether this sweep is completed.
+	// Completed indicates whether this sweep is fully-confirmed.
 	Completed bool
 }
 

sweepbatcher/presigned_test.go

@@ -77,22 +77,6 @@ func (s *StoreMock) UpdateSweepBatch(ctx context.Context,
 	return nil
 }
 
-// ConfirmBatch confirms a batch.
-func (s *StoreMock) ConfirmBatch(ctx context.Context, id int32) error {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	batch, ok := s.batches[id]
-	if !ok {
-		return errors.New("batch not found")
-	}
-
-	batch.Confirmed = true
-	s.batches[batch.ID] = batch
-
-	return nil
-}
-
 // FetchBatchSweeps fetches all the sweeps that belong to a batch.
 func (s *StoreMock) FetchBatchSweeps(ctx context.Context,
 	id int32) ([]*dbSweep, error) {