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.
lightninglabs · Jun 30, 2024 · 34c2c1c · 34c2c1c
1 parent 08474b8
commit 34c2c1c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 20 deletions.
diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go
@@ -175,8 +175,7 @@ func estimateBatchWeight(batch *batch) (*entity, 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
@@ -17,7 +17,7 @@ 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(396)
@@ -81,7 +81,7 @@ func TestEstimateSweepWeight(t *testing.T) {
 			},
 			wantNewBatchEntity: entity{
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(445),
+				CoopWeight:    lntypes.WeightUnit(444),
 				NonCoopWeight: lntypes.WeightUnit(735),
 			},
 		},
@@ -99,7 +99,7 @@ func TestEstimateSweepWeight(t *testing.T) {
 			},
 			wantNewBatchEntity: entity{
 				FeeRate:       highFeeRate,
-				CoopWeight:    lntypes.WeightUnit(445),
+				CoopWeight:    lntypes.WeightUnit(444),
 				NonCoopWeight: lntypes.WeightUnit(735),
 			},
 		},
@@ -120,7 +120,7 @@ func TestEstimateSweepWeight(t *testing.T) {
 			},
 			wantNewBatchEntity: entity{
 				FeeRate:        lowFeeRate,
-				CoopWeight:     lntypes.WeightUnit(445),
+				CoopWeight:     lntypes.WeightUnit(444),
 				NonCoopWeight:  lntypes.WeightUnit(735),
 				IsExternalAddr: true,
 			},
@@ -142,7 +142,7 @@ func TestEstimateSweepWeight(t *testing.T) {
 			},
 			wantNewBatchEntity: entity{
 				FeeRate:        lowFeeRate,
-				CoopWeight:     lntypes.WeightUnit(409),
+				CoopWeight:     lntypes.WeightUnit(408),
 				NonCoopWeight:  lntypes.WeightUnit(699),
 				IsExternalAddr: true,
 			},
@@ -163,7 +163,7 @@ func TestEstimateSweepWeight(t *testing.T) {
 			},
 			wantNewBatchEntity: entity{
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(445),
+				CoopWeight:    lntypes.WeightUnit(444),
 				NonCoopWeight: lntypes.WeightUnit(735),
 				NonCoopHint:   true,
 			},
@@ -215,7 +215,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:       1,
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(445),
+				CoopWeight:    lntypes.WeightUnit(444),
 				NonCoopWeight: lntypes.WeightUnit(735),
 			},
 		},
@@ -239,7 +239,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:       1,
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(676),
+				CoopWeight:    lntypes.WeightUnit(674),
 				NonCoopWeight: lntypes.WeightUnit(1256),
 			},
 		},
@@ -263,7 +263,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:       1,
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(676),
+				CoopWeight:    lntypes.WeightUnit(674),
 				NonCoopWeight: lntypes.WeightUnit(1103),
 			},
 		},
@@ -284,7 +284,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:       1,
 				FeeRate:       highFeeRate,
-				CoopWeight:    lntypes.WeightUnit(445),
+				CoopWeight:    lntypes.WeightUnit(444),
 				NonCoopWeight: lntypes.WeightUnit(735),
 			},
 		},
@@ -309,7 +309,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:       1,
 				FeeRate:       lowFeeRate,
-				CoopWeight:    lntypes.WeightUnit(676),
+				CoopWeight:    lntypes.WeightUnit(674),
 				NonCoopWeight: lntypes.WeightUnit(1256),
 				NonCoopHint:   true,
 			},
@@ -333,7 +333,7 @@ func TestEstimateBatchWeight(t *testing.T) {
 			wantBatchEntity: entity{
 				BatchId:        1,
 				FeeRate:        lowFeeRate,
-				CoopWeight:     lntypes.WeightUnit(445),
+				CoopWeight:     lntypes.WeightUnit(444),
 				NonCoopWeight:  lntypes.WeightUnit(735),
 				IsExternalAddr: true,
 			},

diff --git 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"
@@ -843,8 +844,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 +870,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
@@ -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)