From a5871d6f3ee7ceb59519332848032480e6cd2af2 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sat, 11 Oct 2025 14:44:31 -0300 Subject: [PATCH] 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 91a6f2d1..4415d3fc 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 c31aaf90..245e1d38 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()) })