sweepbatcher: remove method Presign

Method Presign is not as reliable as SignTx, because it checks transaction by
txid and can miss for example if LockTime is different. SignTx can do everything
Presign was used for.

Author: starius

sweepbatcher/presigned.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
@@ -287,11 +281,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
@@ -370,7 +365,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)

sweepbatcher/presigned_test.go

@@ -553,24 +553,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/sweep_batch.go

@@ -481,7 +481,7 @@ func (b *batch) Errorf(format string, params ...interface{}) {
 // checkSweepToAdd checks if a sweep can be added or updated in the batch. The
 // caller must lock the event loop using scheduleNextCall. The function returns
 // if the sweep already exists in the batch. If presigned mode is enabled, the
-// result depends on the outcome of the method presignedHelper.Presign for a
+// result depends on the outcome of the method presignedHelper.SignTx for a
 // non-empty batch. For an empty batch, the input needs to pass
 // PresignSweepsGroup.
 func (b *batch) checkSweepToAdd(_ context.Context, sweep *sweep) (bool, error) {

sweepbatcher/sweep_batcher.go

@@ -158,18 +158,11 @@ type SignMuSig2 func(ctx context.Context, muSig2Version input.MuSig2Version,
 // fails (e.g. because one of the inputs is offline), an input can't be added to
 // a batch.
 type PresignedHelper 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. The implementation should
-	// first check if this transaction already exists in the store to skip
-	// cosigning if possible.
-	Presign(ctx context.Context, primarySweepID wire.OutPoint,
-		tx *wire.MsgTx, inputAmt btcutil.Amount) error
-
 	// DestPkScript returns destination pkScript used by the sweep batch
 	// with the primary outpoint specified. Returns an error, if such tx
 	// doesn't exist. If there are many such transactions, returns any of
 	// pkScript's; all of them should have the same destination pkScript.
+	// TODO: embed this data into SweepInfo.
 	DestPkScript(ctx context.Context,
 		primarySweepID wire.OutPoint) ([]byte, error)
 
@@ -905,7 +898,7 @@ func (b *Batcher) handleSweeps(ctx context.Context, sweeps []*sweep,
 
 // spinUpNewBatch creates new batch, starts it and adds the sweeps to it. If
 // presigned mode is enabled, the result also depends on outcome of
-// presignedHelper.Presign.
+// presignedHelper.SignTx.
 func (b *Batcher) spinUpNewBatch(ctx context.Context, sweeps []*sweep) error {
 	// Spin up a fresh batch.
 	newBatch, err := b.spinUpBatch(ctx)

sweepbatcher/sweep_batcher_presigned_test.go

@@ -96,42 +96,6 @@ func (h *mockPresignedHelper) getTxFeerate(tx *wire.MsgTx,
 	return chainfee.NewSatPerKWeight(fee, weight)
 }
 
-// Presign tries to presign the transaction. It succeeds if all the inputs
-// are online. In case of success it adds the transaction to presignedBatches.
-func (h *mockPresignedHelper) Presign(ctx context.Context,
-	primarySweepID wire.OutPoint, tx *wire.MsgTx,
-	inputAmt btcutil.Amount) error {
-
-	h.mu.Lock()
-	defer h.mu.Unlock()
-
-	// Check if such a transaction already exists. This is not only an
-	// optimization, but also enables re-adding multiple groups if sweeps
-	// are offline.
-	wantTxHash := tx.TxHash()
-	for _, candidate := range h.presignedBatches[primarySweepID] {
-		if candidate.TxHash() == wantTxHash {
-			return nil
-		}
-	}
-
-	if !hasInput(tx, primarySweepID) {
-		return fmt.Errorf("primarySweepID %v not in tx", primarySweepID)
-	}
-
-	if offline := h.offlineInputs(tx); len(offline) != 0 {
-		return fmt.Errorf("some inputs of tx are offline: %v", offline)
-	}
-
-	tx = tx.Copy()
-	h.sign(tx)
-	h.presignedBatches[primarySweepID] = append(
-		h.presignedBatches[primarySweepID], tx,
-	)
-
-	return nil
-}
-
 // DestPkScript returns destination pkScript used in presigned tx sweeping
 // these inputs.
 func (h *mockPresignedHelper) DestPkScript(ctx context.Context,
@@ -164,6 +128,11 @@ func (h *mockPresignedHelper) SignTx(ctx context.Context,
 			feeRate, minRelayFee)
 	}
 
+	if !hasInput(tx, primarySweepID) {
+		return nil, fmt.Errorf("primarySweepID %v not in tx",
+			primarySweepID)
+	}
+
 	// If all the inputs are online and loadOnly is not set, sign this exact
 	// transaction.
 	if offline := h.offlineInputs(tx); len(offline) == 0 && !loadOnly {
@@ -205,7 +174,8 @@ func (h *mockPresignedHelper) SignTx(ctx context.Context,
 	}
 
 	if bestTx == nil {
-		return nil, fmt.Errorf("no such presigned tx found")
+		return nil, fmt.Errorf("some outpoint is offline and no " +
+			"suitable presigned tx found")
 	}
 
 	return bestTx.Copy(), nil
@@ -1025,7 +995,7 @@ func testPresigned_presigned_group(t *testing.T,
 
 	// An attempt to presign must fail.
 	err = batcher.PresignSweepsGroup(ctx, group1, sweepTimeout, destAddr)
-	require.ErrorContains(t, err, "some inputs of tx are offline")
+	require.ErrorContains(t, err, "some outpoint is offline")
 
 	// Enable both outpoints.
 	presignedHelper.SetOutpointOnline(op2, true)