diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index 9485815f..0e75240b 100644 --- a/sweepbatcher/presigned.go +++ b/sweepbatcher/presigned.go @@ -78,7 +78,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep, const currentHeight = 0 // Check if we can sign with minimum fee rate. - const feeRate = chainfee.FeePerKwFloor + feeRate := minRelayFeeRate tx, _, _, _, err := constructUnsignedTx( sweeps, destAddr, currentHeight, feeRate, minRelayFeeRate, @@ -330,6 +330,10 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, return fmt.Errorf("nextBlockFeeRate is not set") } + if minRelayFeeRate == 0 { + return fmt.Errorf("minRelayFeeRate is not set") + } + // Keep track of the total amount this batch is sweeping back. batchAmt := btcutil.Amount(0) for _, sweep := range sweeps { @@ -345,9 +349,9 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, return fmt.Errorf("timeout is invalid: %d", timeout) } - // Go from the floor (1.01 sat/vbyte) to 2k sat/vbyte with step of 1.2x. + // Go from minRelayFeeRate to 2k sat/vbyte with step of 1.2x. + var start = minRelayFeeRate const ( - start = chainfee.FeePerKwFloor stop = chainfee.AbsoluteFeePerKwFloor * 2_000 factorPPM = 1_200_000 timeoutThreshold = 50 @@ -384,12 +388,9 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, } // Try to presign this transaction. - const ( - loadOnly = false - minRelayFee = chainfee.AbsoluteFeePerKwFloor - ) + const loadOnly = false _, err = presigner.SignTx( - ctx, primarySweepID, tx, batchAmt, minRelayFee, fr, + ctx, primarySweepID, tx, batchAmt, minRelayFeeRate, fr, loadOnly, ) if err != nil { diff --git a/sweepbatcher/presigned_test.go b/sweepbatcher/presigned_test.go index 35fe8bf2..06220e69 100644 --- a/sweepbatcher/presigned_test.go +++ b/sweepbatcher/presigned_test.go @@ -15,10 +15,6 @@ import ( "github.com/stretchr/testify/require" ) -const ( - minRelayFeeRate = chainfee.FeePerKwFloor -) - // TestOrderedSweeps checks that methods batch.getOrderedSweeps and // batch.getSweepsGroups works properly. func TestOrderedSweeps(t *testing.T) { @@ -490,6 +486,7 @@ func TestEnsurePresigned(t *testing.T) { primarySweepID wire.OutPoint sweeps []*sweep destPkScript []byte + minRelayFeeRate chainfee.SatPerKWeight wantInputAmt btcutil.Amount destPkScriptErr error signedTxErr error @@ -504,8 +501,24 @@ func TestEnsurePresigned(t *testing.T) { timeout: 1000, }, }, - destPkScript: batchPkScript, - wantInputAmt: 1_000_000, + destPkScript: batchPkScript, + minRelayFeeRate: chainfee.FeePerKwFloor, + wantInputAmt: 1_000_000, + }, + + { + name: "one input, higher minRelayFeeRate", + primarySweepID: op1, + sweeps: []*sweep{ + { + outpoint: op1, + value: 1_000_000, + timeout: 1000, + }, + }, + destPkScript: batchPkScript, + minRelayFeeRate: 1000, + wantInputAmt: 1_000_000, }, { @@ -523,8 +536,9 @@ func TestEnsurePresigned(t *testing.T) { timeout: 1000, }, }, - destPkScript: batchPkScript, - wantInputAmt: 3_000_000, + destPkScript: batchPkScript, + minRelayFeeRate: chainfee.FeePerKwFloor, + wantInputAmt: 3_000_000, }, { @@ -537,6 +551,7 @@ func TestEnsurePresigned(t *testing.T) { timeout: 1000, }, }, + minRelayFeeRate: chainfee.FeePerKwFloor, destPkScriptErr: fmt.Errorf("test DestPkScript error"), }, @@ -550,8 +565,9 @@ func TestEnsurePresigned(t *testing.T) { timeout: 1000, }, }, - destPkScript: batchPkScript, - signedTxErr: fmt.Errorf("test SignTx error"), + destPkScript: batchPkScript, + minRelayFeeRate: chainfee.FeePerKwFloor, + signedTxErr: fmt.Errorf("test SignTx error"), }, } @@ -565,7 +581,7 @@ func TestEnsurePresigned(t *testing.T) { } err := ensurePresigned( - ctx, tc.sweeps, c, minRelayFeeRate, + ctx, tc.sweeps, c, tc.minRelayFeeRate, &chaincfg.RegressionNetParams, ) switch { @@ -580,11 +596,11 @@ func TestEnsurePresigned(t *testing.T) { t, tc.wantInputAmt, c.recordedInputAmt, ) require.Equal( - t, chainfee.FeePerKwFloor, + t, tc.minRelayFeeRate, c.recordedMinRelayFee, ) require.Equal( - t, chainfee.FeePerKwFloor, + t, tc.minRelayFeeRate, c.recordedFeeRate, ) require.True(t, c.recordedLoadOnly) @@ -664,6 +680,7 @@ func TestPresign(t *testing.T) { sweeps []sweep destAddr btcutil.Address nextBlockFeeRate chainfee.SatPerKWeight + minRelayFeeRate chainfee.SatPerKWeight wantErr string wantOutputs []btcutil.Amount wantLockTimes []uint32 @@ -680,6 +697,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "presigner is not installed", }, @@ -689,6 +707,7 @@ func TestPresign(t *testing.T) { presigner: &mockPresigner{}, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "there are no sweeps", }, @@ -704,6 +723,7 @@ func TestPresign(t *testing.T) { }, }, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "unsupported address type ", }, @@ -723,8 +743,30 @@ func TestPresign(t *testing.T) { timeout: 1000, }, }, - destAddr: destAddr, - wantErr: "nextBlockFeeRate is not set", + destAddr: destAddr, + minRelayFeeRate: chainfee.FeePerKwFloor, + wantErr: "nextBlockFeeRate is not set", + }, + + { + name: "error: zero minRelayFeeRate", + presigner: &mockPresigner{}, + primarySweepID: op1, + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + timeout: 1000, + }, + { + outpoint: op2, + value: 2_000_000, + timeout: 1000, + }, + }, + destAddr: destAddr, + nextBlockFeeRate: chainfee.FeePerKwFloor, + wantErr: "minRelayFeeRate is not set", }, { @@ -743,6 +785,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "timeout is invalid: 0", }, @@ -763,6 +806,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "not in tx", }, @@ -779,6 +823,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "not in tx", }, @@ -795,6 +840,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 999900, 999880, 999856, 999827, 999793, 999752, 999702, 999643, 999572, 999486, 999384, 999260, @@ -812,6 +858,34 @@ func TestPresign(t *testing.T) { }, }, + { + name: "higher minRelayFeeRate, fewer txns", + presigner: &mockPresigner{}, + primarySweepID: op1, + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + timeout: 1000, + }, + }, + destAddr: destAddr, + nextBlockFeeRate: 10 * chainfee.FeePerKwFloor, + minRelayFeeRate: 10 * chainfee.FeePerKwFloor, + wantOutputs: []btcutil.Amount{ + 998998, 998797, 998557, 998269, 997923, 997507, + 997009, 996411, 995694, 994833, 993800, 992560, + 991072, 989286, 987144, 984573, 981488, 977786, + 973343, 968012, 961614, 953937, 944725, 933670, + 920405, 904486, 885383, 862460, 834952, + }, + wantLockTimes: []uint32{ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 950, 950, + 950, 950, 950, 950, 950, 950, 950, 950, 950, + 950, 950, 950, 950, 950, + }, + }, + { name: "two sweeps", presigner: &mockPresigner{}, @@ -830,6 +904,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 2999841, 2999810, 2999773, 2999728, 2999673, 2999608, 2999530, 2999436, 2999323, 2999188, @@ -867,6 +942,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 2999841, 2999810, 2999773, 2999728, 2999673, 2999608, 2999530, 2999436, 2999323, 2999188, @@ -904,6 +980,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: 50 * chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 2999841, 2999810, 2999773, 2999728, 2999673, 2999608, 2999530, 2999436, 2999323, 2999188, @@ -940,6 +1017,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: 50 * chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 2999841, 2999810, 2999773, 2999728, 2999673, 2999608, 2999530, 2999436, 2999323, 2999188, @@ -976,6 +1054,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantOutputs: []btcutil.Amount{ 2841, 2810, 2773, 2728, 2673, 2608, 2530, 2436, 2400, @@ -1005,6 +1084,7 @@ func TestPresign(t *testing.T) { }, destAddr: destAddr, nextBlockFeeRate: chainfee.FeePerKwFloor, + minRelayFeeRate: chainfee.FeePerKwFloor, wantErr: "for feeRate 363 sat/kw", }, } @@ -1014,7 +1094,7 @@ func TestPresign(t *testing.T) { err := presign( ctx, tc.presigner, tc.destAddr, tc.primarySweepID, tc.sweeps, - tc.nextBlockFeeRate, minRelayFeeRate, + tc.nextBlockFeeRate, tc.minRelayFeeRate, ) if tc.wantErr != "" { require.Error(t, err) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 7b5e1542..bebe2118 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -1409,12 +1409,12 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, "fee: %w", err) } - // Ensure that batch amount exceeds the sum of change outputs and the - // fee, and that it is also greater than dust limit for the main - // output. + // Ensure that batch amount is equal or exceeds the sum of change + // outputs and the fee, and that it is also greater than dust limit + // for the main output. dustLimit := utils.DustLimitForPkScript(batchPkScript) if fee+btcutil.Amount(sumChange)+dustLimit > batchAmt { - return nil, 0, 0, 0, fmt.Errorf("batch amount %v is <= the "+ + return nil, 0, 0, 0, fmt.Errorf("batch amount %v is < the "+ "sum of change outputs %v plus fee %v and dust "+ "limit %v", batchAmt, btcutil.Amount(sumChange), fee, dustLimit) diff --git a/sweepbatcher/sweep_batch_test.go b/sweepbatcher/sweep_batch_test.go index 2b77f871..202e0a04 100644 --- a/sweepbatcher/sweep_batch_test.go +++ b/sweepbatcher/sweep_batch_test.go @@ -444,7 +444,7 @@ func TestConstructUnsignedTx(t *testing.T) { currentHeight: 800_000, feeRate: 1_000, minRelayFeeRate: 50, - wantErr: "batch amount 0.00100294 BTC is <= the sum " + + wantErr: "batch amount 0.00100294 BTC is < the sum " + "of change outputs 0.00100000 BTC plus fee " + "0.00000058 BTC and dust limit 0.00000330 BTC", }, @@ -570,7 +570,7 @@ func TestConstructUnsignedTx(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - relayFeeRate := minRelayFeeRate + relayFeeRate := chainfee.FeePerKwFloor if tc.minRelayFeeRate != 0 { relayFeeRate = tc.minRelayFeeRate } diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index d2907204..99a70fd4 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -16,10 +16,9 @@ import ( "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" "github.com/lightninglabs/loop/test" + "github.com/lightninglabs/loop/utils" "github.com/lightningnetwork/lnd/chainntnfs" - "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lntypes" - "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -574,7 +573,7 @@ func testPresigned_min_relay_fee(t *testing.T, require.NoError(t, lnd.NotifyHeight(602)) tx = <-lnd.TxPublishChannel gotFeeRate = presignedHelper.getTxFeerate(tx, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(523), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(520), gotFeeRate) // LockTime of a presigned tx is 0. require.Equal(t, uint32(0), tx.LockTime) @@ -585,7 +584,7 @@ func testPresigned_min_relay_fee(t *testing.T, tx2 = <-lnd.TxPublishChannel require.Equal(t, tx.TxHash(), tx2.TxHash()) gotFeeRate = presignedHelper.getTxFeerate(tx2, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(523), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(520), gotFeeRate) // LockTime of a presigned tx is 0. require.Equal(t, uint32(0), tx2.LockTime) @@ -595,7 +594,7 @@ func testPresigned_min_relay_fee(t *testing.T, tx3 := <-lnd.TxPublishChannel require.Equal(t, tx2.TxOut[0].Value, tx3.TxOut[0].Value) gotFeeRate = presignedHelper.getTxFeerate(tx3, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(523), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(520), gotFeeRate) require.Equal(t, uint32(604), tx3.LockTime) } @@ -1278,6 +1277,9 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, defer test.Guard(t)() + batchPkScript, err := txscript.PayToAddrScript(destAddr) + require.NoError(t, err) + lnd := test.NewMockLnd() ctx, cancel := context.WithCancel(context.Background()) @@ -1289,9 +1291,6 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, return chainfee.SatPerKWeight(100_000), nil } - // Set min relay fee low enough to pass the clampBatchFee check. - lnd.SetMinRelayFee(140) - presignedHelper := newMockPresignedHelper() batcher := NewBatcher( @@ -1320,9 +1319,23 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, Value: 1_000_000, }, } - dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + dustLimit := int64(utils.DustLimitForPkScript(batchPkScript)) + mainOutput := dustLimit + + // Let's solve an equation of what the fee should be so it is 20% + // (clamped fee) of fee (itself) + main output. + // fee / (fee+main) = 0.2 + // fee = 0.2 fee + 0.2 main + // 0.8 fee = 0.2 main + // fee = 0.25 main + clampedFee := mainOutput / 4 + + // Set min relay fee low enough to pass the clampBatchFee check. + lnd.SetMinRelayFee(166) + change := &wire.TxOut{ - Value: inputValue - dustLimit + 1, + // If "+1" is removed, the group would be added successfully. + Value: inputValue - dustLimit - clampedFee + 1, PkScript: []byte{0xaf, 0xfe}, } @@ -1332,13 +1345,13 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) // An attempt to presign must fail. - err := batcher.PresignSweepsGroup( + err = batcher.PresignSweepsGroup( ctx, group1, sweepTimeout, destAddr, change, ) require.EqualError(t, err, "failed to construct unsigned tx for "+ - "feeRate 253 sat/kw: batch amount 0.01000000 BTC is <= the "+ - "sum of change outputs 0.00999671 BTC plus fee "+ - "0.00000065 BTC and dust limit 0.00000294 BTC") + "feeRate 166 sat/kw: batch amount 0.01000000 BTC is < the "+ + "sum of change outputs 0.00999634 BTC plus fee "+ + "0.00000073 BTC and dust limit 0.00000294 BTC") // Add the sweep, triggering the publishing attempt. err = batcher.AddSweep(ctx, &SweepRequest{ @@ -1347,6 +1360,21 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, Notifier: &dummyNotifier, }) require.ErrorContains(t, err, "were not presigned") + + // Now let's verify that we found the edge value correctly. + change.Value-- + + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, change, + ) + require.NoError(t, err) + + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.NoError(t, err) } // testPresigned_presigned_group_with_dust_below_relay_fee passes a tx with a @@ -1357,6 +1385,9 @@ func testPresigned_presigned_group_with_dust_below_relay_fee(t *testing.T, defer test.Guard(t)() + batchPkScript, err := txscript.PayToAddrScript(destAddr) + require.NoError(t, err) + lnd := test.NewMockLnd() ctx, cancel := context.WithCancel(context.Background()) @@ -1396,9 +1427,10 @@ func testPresigned_presigned_group_with_dust_below_relay_fee(t *testing.T, Value: 1_000_000, }, } - dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + dustLimit := int64(utils.DustLimitForPkScript(batchPkScript)) change := &wire.TxOut{ - Value: inputValue - dustLimit + 1, + // Note that there is no space for fee. + Value: inputValue - dustLimit, PkScript: []byte{0xaf, 0xfe}, } @@ -1408,12 +1440,12 @@ func testPresigned_presigned_group_with_dust_below_relay_fee(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) // An attempt to presign must fail. - err := batcher.PresignSweepsGroup( + err = batcher.PresignSweepsGroup( ctx, group1, sweepTimeout, destAddr, change, ) require.EqualError(t, err, "failed to construct unsigned tx for "+ "feeRate 253 sat/kw: failed to clamp batch fee: clamped "+ - "fee rate 148 sat/kw is less than minimum relay fee 253 sat/kw") + "fee rate 132 sat/kw is less than minimum relay fee 253 sat/kw") // Add the sweep, triggering the publishing attempt. err = batcher.AddSweep(ctx, &SweepRequest{ @@ -1478,10 +1510,12 @@ func testPresigned_presigned_group_with_dust_change(t *testing.T, Value: 2_000_000, }, } - dustLimit := lnwallet.DustLimitForSize(input.P2TRSize) + changePkScript := []byte{0xaf, 0xfe} + dustLimit := utils.DustLimitForPkScript(changePkScript) change := &wire.TxOut{ + // If "-1" is removed, the group would be added successfully. Value: int64(dustLimit - 1), - PkScript: []byte{0xaf, 0xfe}, + PkScript: changePkScript, } presignedHelper.setChangeForPrimaryDeposit(op1, change) @@ -1495,7 +1529,7 @@ func testPresigned_presigned_group_with_dust_change(t *testing.T, ctx, group1, sweepTimeout, destAddr, change, ) require.EqualError(t, err, "failed to construct unsigned tx for "+ - "feeRate 253 sat/kw: output 0.00000329 BTC is below dust "+ + "feeRate 253 sat/kw: output 0.00000476 BTC is below dust "+ "limit 0.00000477 BTC") // Add the sweep, triggering the publishing attempt. @@ -1505,6 +1539,21 @@ func testPresigned_presigned_group_with_dust_change(t *testing.T, Notifier: &dummyNotifier, }) require.ErrorContains(t, err, "were not presigned") + + // Now let's verify that we found the edge value correctly. + change.Value++ + + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, change, + ) + require.NoError(t, err) + + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.NoError(t, err) } // wrappedStoreWithPresignedFlag wraps a SweepFetcher store adding IsPresigned