From a5871d6f3ee7ceb59519332848032480e6cd2af2 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 11 Oct 2025 14:44:31 -0300 Subject: [PATCH 1/2] sweepbatcher: fix change fee accounting, add test Presigned sweeps that produce a change output misreported the on-chain fee. The fee portion was derived from the total swept amount minus only the first transaction output, so any change output was treated as additional fee. Update getFeePortionForSweep to subtract the value of every tx output so the fee portion reflects only the actual miner fee paid. Add regression coverage that sweeps a presigned input with change and asserts the spend and confirmation notifications report the corrected fee. --- sweepbatcher/sweep_batch.go | 4 +- sweepbatcher/sweep_batcher_presigned_test.go | 146 +++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 91a6f2d1e..4415d3fc4 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -2071,8 +2071,8 @@ func getFeePortionForSweep(spendTx *wire.MsgTx, numSweeps int, totalSweptAmt btcutil.Amount) (btcutil.Amount, btcutil.Amount) { totalFee := int64(totalSweptAmt) - if len(spendTx.TxOut) > 0 { - totalFee -= spendTx.TxOut[0].Value + for _, txOut := range spendTx.TxOut { + totalFee -= txOut.Value } feePortionPerSweep := totalFee / int64(numSweeps) roundingDiff := totalFee - (int64(numSweeps) * feePortionPerSweep) diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index c31aaf902..245e1d387 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -1,6 +1,7 @@ package sweepbatcher import ( + "bytes" "context" "fmt" "os" @@ -1268,6 +1269,147 @@ func testPresigned_presigned_group_with_change(t *testing.T, require.NoError(t, lnd.NotifyHeight(601)) } +// testPresigned_fee_portion_with_change ensures that the fee portion reported +// to clients accounts for change outputs in the presigned transaction. +func testPresigned_fee_portion_with_change(t *testing.T, + batcherStore testBatcherStore) { + + defer test.Guard(t)() + + lnd := test.NewMockLnd() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + customFeeRate := func(_ context.Context, _ lntypes.Hash, + _ wire.OutPoint) (chainfee.SatPerKWeight, error) { + + return chainfee.SatPerKWeight(10_000), nil + } + + presignedHelper := newMockPresignedHelper() + + batcher := NewBatcher( + lnd.WalletKit, lnd.ChainNotifier, lnd.Signer, + testMuSig2SignSweep, testVerifySchnorrSig, lnd.ChainParams, + batcherStore, presignedHelper, + WithCustomFeeRate(customFeeRate), + WithPresignedHelper(presignedHelper), + ) + + go func() { + err := batcher.Run(ctx) + checkBatcherError(t, err) + }() + + swapHash := lntypes.Hash{2, 2, 2} + op := wire.OutPoint{ + Hash: chainhash.Hash{2, 2}, + Index: 2, + } + group := []Input{ + { + Outpoint: op, + Value: 1_000_000, + }, + } + change := &wire.TxOut{ + Value: 250_000, + PkScript: []byte{0xca, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op, change) + presignedHelper.SetOutpointOnline(op, true) + + require.NoError(t, batcher.PresignSweepsGroup( + ctx, group, sweepTimeout, destAddr, change, + )) + + spendChan := make(chan *SpendDetail, 1) + confChan := make(chan *ConfDetail, 1) + notifier := &SpendNotifier{ + SpendChan: spendChan, + SpendErrChan: make(chan error, 1), + ConfChan: confChan, + ConfErrChan: make(chan error, 1), + QuitChan: make(chan bool, 1), + } + + require.NoError(t, batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash, + Inputs: group, + Notifier: notifier, + })) + + spendReg := <-lnd.RegisterSpendChannel + require.NotNil(t, spendReg) + require.NotNil(t, spendReg.Outpoint) + require.Equal(t, op, *spendReg.Outpoint) + + tx := <-lnd.TxPublishChannel + require.Len(t, tx.TxIn, 1) + require.Len(t, tx.TxOut, 2) + + var ( + outputSum int64 + foundChange bool + ) + for _, txOut := range tx.TxOut { + outputSum += txOut.Value + if txOut.Value != change.Value { + continue + } + + if !bytes.Equal(txOut.PkScript, change.PkScript) { + continue + } + + foundChange = true + } + + require.True(t, foundChange) + + totalInput := int64(group[0].Value) + require.LessOrEqual(t, outputSum, totalInput) + + expectedFee := btcutil.Amount(totalInput - outputSum) + require.Greater(t, expectedFee, btcutil.Amount(0)) + + txHash := tx.TxHash() + spendDetail := &chainntnfs.SpendDetail{ + SpentOutPoint: &op, + SpendingTx: tx, + SpenderTxHash: &txHash, + SpenderInputIndex: 0, + SpendingHeight: spendReg.HeightHint + 1, + } + lnd.SpendChannel <- spendDetail + + spend := <-spendChan + require.Equal(t, expectedFee, spend.OnChainFeePortion) + + confReg := <-lnd.RegisterConfChannel + require.True(t, bytes.Equal(tx.TxOut[0].PkScript, confReg.PkScript) || + bytes.Equal(tx.TxOut[1].PkScript, confReg.PkScript)) + + require.NoError( + t, lnd.NotifyHeight(spendReg.HeightHint+batchConfHeight+1), + ) + lnd.ConfChannel <- &chainntnfs.TxConfirmation{Tx: tx} + + require.Eventually(t, func() bool { + select { + case <-presignedHelper.cleanupCalled: + return true + default: + return false + } + }, test.Timeout, eventuallyCheckFrequency) + + conf := <-confChan + require.Equal(t, expectedFee, conf.OnChainFeePortion) +} + // testPresigned_presigned_group_with_identical_change_pkscript tests passing multiple sweeps to // the method PresignSweepsGroup. It tests that a change output of a primary // deposit sweep is properly added to the presigned transaction. @@ -2356,6 +2498,10 @@ func TestPresigned(t *testing.T) { testPresigned_presigned_group_with_change(t, NewStoreMock()) }) + t.Run("fee_portion_change", func(t *testing.T) { + testPresigned_fee_portion_with_change(t, NewStoreMock()) + }) + t.Run("identical change pkscript", func(t *testing.T) { testPresigned_presigned_group_with_identical_change_pkscript(t, NewStoreMock()) }) From 1f15c604acc780d9843bfb12743a4020a0e25524 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 11 Oct 2025 22:11:56 -0300 Subject: [PATCH 2/2] sweepbatcher: fix fee rate calculation (publish) We forgot to account for change outputs when checking the feerate of signed transaction. The bug resulted in fee rate overestimation in the log message. --- sweepbatcher/presigned.go | 5 ++++- sweepbatcher/sweep_batcher_presigned_test.go | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index 0e75240b7..e7eed04eb 100644 --- a/sweepbatcher/presigned.go +++ b/sweepbatcher/presigned.go @@ -506,7 +506,10 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Find actual fee rate of the signed transaction. It may differ from // the desired fee rate, because SignTx may return a presigned tx. - output := btcutil.Amount(tx.TxOut[0].Value) + var output btcutil.Amount + for _, txOut := range tx.TxOut { + output += btcutil.Amount(txOut.Value) + } fee = batchAmt - output signedFeeRate := chainfee.NewSatPerKWeight(fee, realWeight) diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index 245e1d387..606c5a887 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -1270,7 +1270,8 @@ func testPresigned_presigned_group_with_change(t *testing.T, } // testPresigned_fee_portion_with_change ensures that the fee portion reported -// to clients accounts for change outputs in the presigned transaction. +// to clients accounts for change outputs in the presigned transaction. It also +// is a regression test for feerate overestimation when tx is published. func testPresigned_fee_portion_with_change(t *testing.T, batcherStore testBatcherStore) { @@ -1350,6 +1351,14 @@ func testPresigned_fee_portion_with_change(t *testing.T, require.Len(t, tx.TxIn, 1) require.Len(t, tx.TxOut, 2) + // Mine a blocks to trigger republishing. + require.NoError(t, lnd.NotifyHeight(601)) + + // Make sure it is the same tx. + tx2 := <-lnd.TxPublishChannel + require.Len(t, tx2.TxOut, len(tx.TxOut)) + require.Equal(t, tx.TxOut[0].Value, tx2.TxOut[0].Value) + var ( outputSum int64 foundChange bool