From 574707484f99f015ef85299b337e03e1d5267cec 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 e20e24f60..d8a2cac70 100644 --- 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 index cd4a14e93..fe963e590 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 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 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)