From 85b4bd07792fe3081e78a472893883eca595476a 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 | 24 ++++++++++----------- sweepbatcher/sweep_batch.go | 9 +++++--- sweepbatcher/sweep_batcher_test.go | 14 +++++++++--- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go index 6696d6583..431eee5e3 100644 --- a/sweepbatcher/greedy_batch_selection.go +++ b/sweepbatcher/greedy_batch_selection.go @@ -190,8 +190,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 8752358e0..6a85185df 100644 --- 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 TestEstimateSweepFeeIncrement(t *testing.T) { }, wantNewBatchFeeDetails: feeDetails{ FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), }, }, @@ -99,7 +99,7 @@ func TestEstimateSweepFeeIncrement(t *testing.T) { }, wantNewBatchFeeDetails: feeDetails{ FeeRate: highFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), }, }, @@ -120,7 +120,7 @@ func TestEstimateSweepFeeIncrement(t *testing.T) { }, wantNewBatchFeeDetails: feeDetails{ FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), IsExternalAddr: true, }, @@ -142,7 +142,7 @@ func TestEstimateSweepFeeIncrement(t *testing.T) { }, wantNewBatchFeeDetails: feeDetails{ FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(409), + CoopWeight: lntypes.WeightUnit(408), NonCoopWeight: lntypes.WeightUnit(699), IsExternalAddr: true, }, @@ -163,7 +163,7 @@ func TestEstimateSweepFeeIncrement(t *testing.T) { }, wantNewBatchFeeDetails: feeDetails{ FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), NonCoopHint: true, }, @@ -218,7 +218,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ BatchId: 1, FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), }, }, @@ -242,7 +242,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ BatchId: 1, FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(676), + CoopWeight: lntypes.WeightUnit(674), NonCoopWeight: lntypes.WeightUnit(1256), }, }, @@ -266,7 +266,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ BatchId: 1, FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(676), + CoopWeight: lntypes.WeightUnit(674), NonCoopWeight: lntypes.WeightUnit(1103), }, }, @@ -287,7 +287,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ BatchId: 1, FeeRate: highFeeRate, - CoopWeight: lntypes.WeightUnit(445), + CoopWeight: lntypes.WeightUnit(444), NonCoopWeight: lntypes.WeightUnit(735), }, }, @@ -312,7 +312,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ BatchId: 1, FeeRate: lowFeeRate, - CoopWeight: lntypes.WeightUnit(676), + CoopWeight: lntypes.WeightUnit(674), NonCoopWeight: lntypes.WeightUnit(1256), NonCoopHint: true, }, @@ -336,7 +336,7 @@ func TestEstimateBatchWeight(t *testing.T) { wantBatchFeeDetails: feeDetails{ 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 index c1d505265..05fdd9a11 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" @@ -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 index 592f86cd1..7314e9798 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)