Skip to content

Commit 6ace0b1

Browse files
stariushieblmi
starius
authored and
hieblmi
committed
sweepbatcher: subtle adjustments for change
 - ensurePresigned: use passed minRelayFeeRate instead of chainfee.FeePerKwFloor - presign: use minRelayFeeRate for start and minRelayFee - presign: make sure minRelayFeeRate is set - add tests for presign to test this new behavior; make sure the number of transactions is lower if minRelayFeeRate is higher - update error message in constructUnsignedTx: use <, not <= (more accurate) - use utils.DustLimitForPkScript instead of lnwallet.DustLimitForSize in tests - in tests adjust amounts to edge values, add controls
1 parent 1b31b88 commit 6ace0b1

File tree

5 files changed

+181
-51
lines changed

5 files changed

+181
-51
lines changed

‎sweepbatcher/presigned.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep,
7878
const currentHeight = 0
7979

8080
// Check if we can sign with minimum fee rate.
81-
const feeRate = chainfee.FeePerKwFloor
81+
feeRate := minRelayFeeRate
8282

8383
tx, _, _, _, err := constructUnsignedTx(
8484
sweeps, destAddr, currentHeight, feeRate, minRelayFeeRate,
@@ -330,6 +330,10 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
330330
return fmt.Errorf("nextBlockFeeRate is not set")
331331
}
332332

333+
if minRelayFeeRate == 0 {
334+
return fmt.Errorf("minRelayFeeRate is not set")
335+
}
336+
333337
// Keep track of the total amount this batch is sweeping back.
334338
batchAmt := btcutil.Amount(0)
335339
for _, sweep := range sweeps {
@@ -345,9 +349,9 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
345349
return fmt.Errorf("timeout is invalid: %d", timeout)
346350
}
347351

348-
// Go from the floor (1.01 sat/vbyte) to 2k sat/vbyte with step of 1.2x.
352+
// Go from minRelayFeeRate to 2k sat/vbyte with step of 1.2x.
353+
var start = minRelayFeeRate
349354
const (
350-
start = chainfee.FeePerKwFloor
351355
stop = chainfee.AbsoluteFeePerKwFloor * 2_000
352356
factorPPM = 1_200_000
353357
timeoutThreshold = 50
@@ -384,12 +388,9 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
384388
}
385389

386390
// Try to presign this transaction.
387-
const (
388-
loadOnly = false
389-
minRelayFee = chainfee.AbsoluteFeePerKwFloor
390-
)
391+
const loadOnly = false
391392
_, err = presigner.SignTx(
392-
ctx, primarySweepID, tx, batchAmt, minRelayFee, fr,
393+
ctx, primarySweepID, tx, batchAmt, minRelayFeeRate, fr,
393394
loadOnly,
394395
)
395396
if err != nil {

‎sweepbatcher/presigned_test.go‎

Lines changed: 96 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@ import (
1515
"github.com/stretchr/testify/require"
1616
)
1717

18-
const (
19-
minRelayFeeRate = chainfee.FeePerKwFloor
20-
)
21-
2218
// TestOrderedSweeps checks that methods batch.getOrderedSweeps and
2319
// batch.getSweepsGroups works properly.
2420
func TestOrderedSweeps(t *testing.T) {
@@ -490,6 +486,7 @@ func TestEnsurePresigned(t *testing.T) {
490486
primarySweepID wire.OutPoint
491487
sweeps []*sweep
492488
destPkScript []byte
489+
minRelayFeeRate chainfee.SatPerKWeight
493490
wantInputAmt btcutil.Amount
494491
destPkScriptErr error
495492
signedTxErr error
@@ -504,8 +501,24 @@ func TestEnsurePresigned(t *testing.T) {
504501
timeout: 1000,
505502
},
506503
},
507-
destPkScript: batchPkScript,
508-
wantInputAmt: 1_000_000,
504+
destPkScript: batchPkScript,
505+
minRelayFeeRate: chainfee.FeePerKwFloor,
506+
wantInputAmt: 1_000_000,
507+
},
508+
509+
{
510+
name: "one input, higher minRelayFeeRate",
511+
primarySweepID: op1,
512+
sweeps: []*sweep{
513+
{
514+
outpoint: op1,
515+
value: 1_000_000,
516+
timeout: 1000,
517+
},
518+
},
519+
destPkScript: batchPkScript,
520+
minRelayFeeRate: 1000,
521+
wantInputAmt: 1_000_000,
509522
},
510523

511524
{
@@ -523,8 +536,9 @@ func TestEnsurePresigned(t *testing.T) {
523536
timeout: 1000,
524537
},
525538
},
526-
destPkScript: batchPkScript,
527-
wantInputAmt: 3_000_000,
539+
destPkScript: batchPkScript,
540+
minRelayFeeRate: chainfee.FeePerKwFloor,
541+
wantInputAmt: 3_000_000,
528542
},
529543

530544
{
@@ -537,6 +551,7 @@ func TestEnsurePresigned(t *testing.T) {
537551
timeout: 1000,
538552
},
539553
},
554+
minRelayFeeRate: chainfee.FeePerKwFloor,
540555
destPkScriptErr: fmt.Errorf("test DestPkScript error"),
541556
},
542557

@@ -550,8 +565,9 @@ func TestEnsurePresigned(t *testing.T) {
550565
timeout: 1000,
551566
},
552567
},
553-
destPkScript: batchPkScript,
554-
signedTxErr: fmt.Errorf("test SignTx error"),
568+
destPkScript: batchPkScript,
569+
minRelayFeeRate: chainfee.FeePerKwFloor,
570+
signedTxErr: fmt.Errorf("test SignTx error"),
555571
},
556572
}
557573

@@ -565,7 +581,7 @@ func TestEnsurePresigned(t *testing.T) {
565581
}
566582

567583
err := ensurePresigned(
568-
ctx, tc.sweeps, c, minRelayFeeRate,
584+
ctx, tc.sweeps, c, tc.minRelayFeeRate,
569585
&chaincfg.RegressionNetParams,
570586
)
571587
switch {
@@ -580,11 +596,11 @@ func TestEnsurePresigned(t *testing.T) {
580596
t, tc.wantInputAmt, c.recordedInputAmt,
581597
)
582598
require.Equal(
583-
t, chainfee.FeePerKwFloor,
599+
t, tc.minRelayFeeRate,
584600
c.recordedMinRelayFee,
585601
)
586602
require.Equal(
587-
t, chainfee.FeePerKwFloor,
603+
t, tc.minRelayFeeRate,
588604
c.recordedFeeRate,
589605
)
590606
require.True(t, c.recordedLoadOnly)
@@ -664,6 +680,7 @@ func TestPresign(t *testing.T) {
664680
sweeps []sweep
665681
destAddr btcutil.Address
666682
nextBlockFeeRate chainfee.SatPerKWeight
683+
minRelayFeeRate chainfee.SatPerKWeight
667684
wantErr string
668685
wantOutputs []btcutil.Amount
669686
wantLockTimes []uint32
@@ -680,6 +697,7 @@ func TestPresign(t *testing.T) {
680697
},
681698
destAddr: destAddr,
682699
nextBlockFeeRate: chainfee.FeePerKwFloor,
700+
minRelayFeeRate: chainfee.FeePerKwFloor,
683701
wantErr: "presigner is not installed",
684702
},
685703

@@ -689,6 +707,7 @@ func TestPresign(t *testing.T) {
689707
presigner: &mockPresigner{},
690708
destAddr: destAddr,
691709
nextBlockFeeRate: chainfee.FeePerKwFloor,
710+
minRelayFeeRate: chainfee.FeePerKwFloor,
692711
wantErr: "there are no sweeps",
693712
},
694713

@@ -704,6 +723,7 @@ func TestPresign(t *testing.T) {
704723
},
705724
},
706725
nextBlockFeeRate: chainfee.FeePerKwFloor,
726+
minRelayFeeRate: chainfee.FeePerKwFloor,
707727
wantErr: "unsupported address type <nil>",
708728
},
709729

@@ -723,8 +743,30 @@ func TestPresign(t *testing.T) {
723743
timeout: 1000,
724744
},
725745
},
726-
destAddr: destAddr,
727-
wantErr: "nextBlockFeeRate is not set",
746+
destAddr: destAddr,
747+
minRelayFeeRate: chainfee.FeePerKwFloor,
748+
wantErr: "nextBlockFeeRate is not set",
749+
},
750+
751+
{
752+
name: "error: zero minRelayFeeRate",
753+
presigner: &mockPresigner{},
754+
primarySweepID: op1,
755+
sweeps: []sweep{
756+
{
757+
outpoint: op1,
758+
value: 1_000_000,
759+
timeout: 1000,
760+
},
761+
{
762+
outpoint: op2,
763+
value: 2_000_000,
764+
timeout: 1000,
765+
},
766+
},
767+
destAddr: destAddr,
768+
nextBlockFeeRate: chainfee.FeePerKwFloor,
769+
wantErr: "minRelayFeeRate is not set",
728770
},
729771

730772
{
@@ -743,6 +785,7 @@ func TestPresign(t *testing.T) {
743785
},
744786
destAddr: destAddr,
745787
nextBlockFeeRate: chainfee.FeePerKwFloor,
788+
minRelayFeeRate: chainfee.FeePerKwFloor,
746789
wantErr: "timeout is invalid: 0",
747790
},
748791

@@ -763,6 +806,7 @@ func TestPresign(t *testing.T) {
763806
},
764807
destAddr: destAddr,
765808
nextBlockFeeRate: chainfee.FeePerKwFloor,
809+
minRelayFeeRate: chainfee.FeePerKwFloor,
766810
wantErr: "not in tx",
767811
},
768812

@@ -779,6 +823,7 @@ func TestPresign(t *testing.T) {
779823
},
780824
destAddr: destAddr,
781825
nextBlockFeeRate: chainfee.FeePerKwFloor,
826+
minRelayFeeRate: chainfee.FeePerKwFloor,
782827
wantErr: "not in tx",
783828
},
784829

@@ -795,6 +840,7 @@ func TestPresign(t *testing.T) {
795840
},
796841
destAddr: destAddr,
797842
nextBlockFeeRate: chainfee.FeePerKwFloor,
843+
minRelayFeeRate: chainfee.FeePerKwFloor,
798844
wantOutputs: []btcutil.Amount{
799845
999900, 999880, 999856, 999827, 999793, 999752,
800846
999702, 999643, 999572, 999486, 999384, 999260,
@@ -812,6 +858,34 @@ func TestPresign(t *testing.T) {
812858
},
813859
},
814860

861+
{
862+
name: "higher minRelayFeeRate, fewer txns",
863+
presigner: &mockPresigner{},
864+
primarySweepID: op1,
865+
sweeps: []sweep{
866+
{
867+
outpoint: op1,
868+
value: 1_000_000,
869+
timeout: 1000,
870+
},
871+
},
872+
destAddr: destAddr,
873+
nextBlockFeeRate: 10 * chainfee.FeePerKwFloor,
874+
minRelayFeeRate: 10 * chainfee.FeePerKwFloor,
875+
wantOutputs: []btcutil.Amount{
876+
998998, 998797, 998557, 998269, 997923, 997507,
877+
997009, 996411, 995694, 994833, 993800, 992560,
878+
991072, 989286, 987144, 984573, 981488, 977786,
879+
973343, 968012, 961614, 953937, 944725, 933670,
880+
920405, 904486, 885383, 862460, 834952,
881+
},
882+
wantLockTimes: []uint32{
883+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 950, 950,
884+
950, 950, 950, 950, 950, 950, 950, 950, 950,
885+
950, 950, 950, 950, 950,
886+
},
887+
},
888+
815889
{
816890
name: "two sweeps",
817891
presigner: &mockPresigner{},
@@ -830,6 +904,7 @@ func TestPresign(t *testing.T) {
830904
},
831905
destAddr: destAddr,
832906
nextBlockFeeRate: chainfee.FeePerKwFloor,
907+
minRelayFeeRate: chainfee.FeePerKwFloor,
833908
wantOutputs: []btcutil.Amount{
834909
2999841, 2999810, 2999773, 2999728, 2999673,
835910
2999608, 2999530, 2999436, 2999323, 2999188,
@@ -867,6 +942,7 @@ func TestPresign(t *testing.T) {
867942
},
868943
destAddr: destAddr,
869944
nextBlockFeeRate: chainfee.FeePerKwFloor,
945+
minRelayFeeRate: chainfee.FeePerKwFloor,
870946
wantOutputs: []btcutil.Amount{
871947
2999841, 2999810, 2999773, 2999728, 2999673,
872948
2999608, 2999530, 2999436, 2999323, 2999188,
@@ -904,6 +980,7 @@ func TestPresign(t *testing.T) {
904980
},
905981
destAddr: destAddr,
906982
nextBlockFeeRate: 50 * chainfee.FeePerKwFloor,
983+
minRelayFeeRate: chainfee.FeePerKwFloor,
907984
wantOutputs: []btcutil.Amount{
908985
2999841, 2999810, 2999773, 2999728, 2999673,
909986
2999608, 2999530, 2999436, 2999323, 2999188,
@@ -940,6 +1017,7 @@ func TestPresign(t *testing.T) {
9401017
},
9411018
destAddr: destAddr,
9421019
nextBlockFeeRate: 50 * chainfee.FeePerKwFloor,
1020+
minRelayFeeRate: chainfee.FeePerKwFloor,
9431021
wantOutputs: []btcutil.Amount{
9441022
2999841, 2999810, 2999773, 2999728, 2999673,
9451023
2999608, 2999530, 2999436, 2999323, 2999188,
@@ -976,6 +1054,7 @@ func TestPresign(t *testing.T) {
9761054
},
9771055
destAddr: destAddr,
9781056
nextBlockFeeRate: chainfee.FeePerKwFloor,
1057+
minRelayFeeRate: chainfee.FeePerKwFloor,
9791058
wantOutputs: []btcutil.Amount{
9801059
2841, 2810, 2773, 2728, 2673, 2608, 2530, 2436,
9811060
2400,
@@ -1005,6 +1084,7 @@ func TestPresign(t *testing.T) {
10051084
},
10061085
destAddr: destAddr,
10071086
nextBlockFeeRate: chainfee.FeePerKwFloor,
1087+
minRelayFeeRate: chainfee.FeePerKwFloor,
10081088
wantErr: "for feeRate 363 sat/kw",
10091089
},
10101090
}
@@ -1014,7 +1094,7 @@ func TestPresign(t *testing.T) {
10141094
err := presign(
10151095
ctx, tc.presigner, tc.destAddr,
10161096
tc.primarySweepID, tc.sweeps,
1017-
tc.nextBlockFeeRate, minRelayFeeRate,
1097+
tc.nextBlockFeeRate, tc.minRelayFeeRate,
10181098
)
10191099
if tc.wantErr != "" {
10201100
require.Error(t, err)

‎sweepbatcher/sweep_batch.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,12 +1358,12 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address,
13581358
"fee: %w", err)
13591359
}
13601360

1361-
// Ensure that batch amount exceeds the sum of change outputs and the
1362-
// fee, and that it is also greater than dust limit for the main
1363-
// output.
1361+
// Ensure that batch amount is equal or exceeds the sum of change
1362+
// outputs and the fee, and that it is also greater than dust limit
1363+
// for the main output.
13641364
dustLimit := utils.DustLimitForPkScript(batchPkScript)
13651365
if fee+btcutil.Amount(sumChange)+dustLimit > batchAmt {
1366-
return nil, 0, 0, 0, fmt.Errorf("batch amount %v is <= the "+
1366+
return nil, 0, 0, 0, fmt.Errorf("batch amount %v is < the "+
13671367
"sum of change outputs %v plus fee %v and dust "+
13681368
"limit %v", batchAmt, btcutil.Amount(sumChange),
13691369
fee, dustLimit)

‎sweepbatcher/sweep_batch_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ func TestConstructUnsignedTx(t *testing.T) {
444444
currentHeight: 800_000,
445445
feeRate: 1_000,
446446
minRelayFeeRate: 50,
447-
wantErr: "batch amount 0.00100294 BTC is <= the sum " +
447+
wantErr: "batch amount 0.00100294 BTC is < the sum " +
448448
"of change outputs 0.00100000 BTC plus fee " +
449449
"0.00000058 BTC and dust limit 0.00000330 BTC",
450450
},
@@ -570,7 +570,7 @@ func TestConstructUnsignedTx(t *testing.T) {
570570

571571
for _, tc := range cases {
572572
t.Run(tc.name, func(t *testing.T) {
573-
relayFeeRate := minRelayFeeRate
573+
relayFeeRate := chainfee.FeePerKwFloor
574574
if tc.minRelayFeeRate != 0 {
575575
relayFeeRate = tc.minRelayFeeRate
576576
}

0 commit comments

Comments
 (0)