From db5e0d1d279149f0ca322452f74c2995bde9dd03 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 29 Jun 2024 23:55:32 -0300 Subject: [PATCH] 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 d1ebf384..09bdee53 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 8fa9c08e..a12f0aea 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 0d071ec1..dd9e5da7 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 592f86cd..7314e979 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)