From 94168c3fe521cabdcaee6d693e872dd40925bf41 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Thu, 17 Jul 2025 15:08:11 +0200 Subject: [PATCH 1/8] utils: function to get dust limit for a pkscript. --- utils/dust_limit.go | 15 +++++++++++++++ utils/dust_limit_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 utils/dust_limit.go create mode 100644 utils/dust_limit_test.go diff --git a/utils/dust_limit.go b/utils/dust_limit.go new file mode 100644 index 000000000..ba9b567a0 --- /dev/null +++ b/utils/dust_limit.go @@ -0,0 +1,15 @@ +package utils + +import ( + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/mempool" + "github.com/btcsuite/btcd/wire" +) + +// DustLimitForPkScript returns the dust limit for a given pkScript. An output +// must be greater or equal to this value. +func DustLimitForPkScript(pkscript []byte) btcutil.Amount { + return btcutil.Amount(mempool.GetDustThreshold(&wire.TxOut{ + PkScript: pkscript, + })) +} diff --git a/utils/dust_limit_test.go b/utils/dust_limit_test.go new file mode 100644 index 000000000..be6d84bbd --- /dev/null +++ b/utils/dust_limit_test.go @@ -0,0 +1,32 @@ +package utils + +import ( + "testing" + + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet" + "github.com/stretchr/testify/require" +) + +type pkScriptGetter func([]byte) ([]byte, error) + +// TestDustLimitForPkScript checks that the dust limit for a given script size +// matches the calculation in lnwallet.DustLimitForSize. +func TestDustLimitForPkScript(t *testing.T) { + getScripts := map[int]pkScriptGetter{ + input.P2WPKHSize: input.WitnessPubKeyHash, + input.P2WSHSize: input.WitnessScriptHash, + input.P2SHSize: input.GenerateP2SH, + input.P2PKHSize: input.GenerateP2PKH, + } + + for scriptSize, getPkScript := range getScripts { + pkScript, err := getPkScript([]byte{}) + require.NoError(t, err, "failed to generate pkScript") + + require.Equal( + t, lnwallet.DustLimitForSize(scriptSize), + DustLimitForPkScript(pkScript), + ) + } +} From f30db74756f7f5d60082273664524aa008afe25b Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Fri, 18 Jul 2025 11:05:01 +0200 Subject: [PATCH 2/8] sweepbatcher: optional change in sweep struct This commit adds an optional change output to the sweep struct. --- sweepbatcher/sweep_batch.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 3eb65faf3..50ecafbbb 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -125,6 +125,9 @@ type sweep struct { // presigned is set, if the sweep should be handled in presigned mode. presigned bool + + // change is the optional change output of the sweep. + change *wire.TxOut } // batchState is the state of the batch. From 8399a63091b3495f6346d2c825bd9b710cb0c3eb Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Fri, 18 Jul 2025 11:06:42 +0200 Subject: [PATCH 3/8] sweepbatcher: consider optional change in greedy selection --- sweepbatcher/greedy_batch_selection.go | 7 ++ sweepbatcher/greedy_batch_selection_test.go | 88 +++++++++++++++++++-- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go index 1d6726e86..8b910dbcb 100644 --- a/sweepbatcher/greedy_batch_selection.go +++ b/sweepbatcher/greedy_batch_selection.go @@ -210,6 +210,13 @@ func estimateBatchWeight(batch *batch) (feeDetails, error) { err) } + // Add change output weights. + for _, s := range batch.sweeps { + if s.change != nil { + weight.AddOutput(s.change.PkScript) + } + } + // Add inputs. for _, sweep := range batch.sweeps { if sweep.nonCoopHint || sweep.coopFailed { diff --git a/sweepbatcher/greedy_batch_selection_test.go b/sweepbatcher/greedy_batch_selection_test.go index cc47f1ad5..51dd70d29 100644 --- a/sweepbatcher/greedy_batch_selection_test.go +++ b/sweepbatcher/greedy_batch_selection_test.go @@ -6,6 +6,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/lightninglabs/loop/swap" "github.com/lightningnetwork/lnd/input" @@ -16,24 +17,28 @@ import ( // Useful constants for tests. const ( - lowFeeRate = chainfee.FeePerKwFloor - highFeeRate = chainfee.SatPerKWeight(30000) + lowFeeRate = chainfee.FeePerKwFloor + mediumFeeRate = lowFeeRate + 200 + highFeeRate = chainfee.SatPerKWeight(30000) coopInputWeight = lntypes.WeightUnit(230) + batchOutputWeight = lntypes.WeightUnit(343) nonCoopInputWeight = lntypes.WeightUnit(393) nonCoopPenalty = nonCoopInputWeight - coopInputWeight coopNewBatchWeight = lntypes.WeightUnit(444) nonCoopNewBatchWeight = coopNewBatchWeight + nonCoopPenalty + changeOutputWeight = lntypes.WeightUnit(input.P2TROutputSize) // p2pkhDiscount is weight discount P2PKH output has over P2TR output. p2pkhDiscount = lntypes.WeightUnit( input.P2TROutputSize-input.P2PKHOutputSize, ) * 4 - coopTwoSweepBatchWeight = coopNewBatchWeight + coopInputWeight - nonCoopTwoSweepBatchWeight = coopTwoSweepBatchWeight + 2*nonCoopPenalty - v2v3BatchWeight = nonCoopTwoSweepBatchWeight - 25 - mixedTwoSweepBatchWeight = coopTwoSweepBatchWeight + nonCoopPenalty + coopTwoSweepBatchWeight = coopNewBatchWeight + coopInputWeight + coopSingleSweepChangeBatchWeight = coopInputWeight + batchOutputWeight + changeOutputWeight + nonCoopTwoSweepBatchWeight = coopTwoSweepBatchWeight + 2*nonCoopPenalty + v2v3BatchWeight = nonCoopTwoSweepBatchWeight - 25 + mixedTwoSweepBatchWeight = coopTwoSweepBatchWeight + nonCoopPenalty ) // testHtlcV2SuccessEstimator adds weight of non-cooperative input to estimator @@ -265,6 +270,13 @@ func TestEstimateBatchWeight(t *testing.T) { se3 := testHtlcV3SuccessEstimator trAddr := (*btcutil.AddressTaproot)(nil) + changeAddr := "bc1pdx9ggvtjjcpaqfqk375qhdmzx9xu8dcu7w94lqfcxhh0rj" + + "lwyyeq5ryn6r" + changeAddress, err := btcutil.DecodeAddress(changeAddr, nil) + require.NoError(t, err) + changePkscript, err := txscript.PayToAddrScript(changeAddress) + require.NoError(t, err) + cases := []struct { name string batch *batch @@ -290,6 +302,29 @@ func TestEstimateBatchWeight(t *testing.T) { }, }, + { + name: "one sweep regular batch with change", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[wire.OutPoint]sweep{ + outpoint1: { + htlcSuccessEstimator: se3, + change: &wire.TxOut{ + PkScript: changePkscript, + }, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + Weight: coopSingleSweepChangeBatchWeight, + }, + }, + { name: "two sweeps regular batch", batch: &batch{ @@ -778,6 +813,47 @@ func TestSelectBatches(t *testing.T) { }, wantBestBatchesIds: []int32{1, newBatchSignal}, }, + + { + name: "low fee change sweep, placed in new batch", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: mediumFeeRate, + Weight: coopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + Weight: coopInputWeight + changeOutputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + Weight: coopNewBatchWeight, + }, + wantBestBatchesIds: []int32{newBatchSignal, 1}, + }, + + { + name: "low fee sweep without change, placed in " + + "existing batch", + batches: []feeDetails{ + { + BatchId: 1, + FeeRate: mediumFeeRate, + Weight: coopNewBatchWeight, + }, + }, + sweep: feeDetails{ + FeeRate: lowFeeRate, + Weight: coopInputWeight, + }, + oneSweepBatch: feeDetails{ + FeeRate: lowFeeRate, + Weight: coopNewBatchWeight, + }, + wantBestBatchesIds: []int32{1, newBatchSignal}, + }, } for _, tc := range cases { From bc7d155e69c2ad74559d88be02d482ca7346901f Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Fri, 18 Jul 2025 11:07:40 +0200 Subject: [PATCH 4/8] sweepbatcher: consider change in presigning and batch tx Presigning sweeps takes change outputs into account. Each primary deposit id of a sweep group points to an optional change output. sweepbatcher.presign scans all passed sweeps for change outputs and passes them to constructUnsignedTx. Optional change of a swap is encoded in its sweeps as a pointer to the same change output. This change is taken into account when constructing the unsigned batch transaction when it comes to tx weight and outputs. --- sweepbatcher/presigned.go | 39 ++- sweepbatcher/presigned_test.go | 151 +++++++- sweepbatcher/sweep_batch.go | 94 ++++- sweepbatcher/sweep_batch_test.go | 241 ++++++++++++- sweepbatcher/sweep_batcher.go | 19 +- sweepbatcher/sweep_batcher_presigned_test.go | 343 ++++++++++++++++++- sweepbatcher/sweep_batcher_test.go | 118 +++---- 7 files changed, 900 insertions(+), 105 deletions(-) diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index 385f34cf9..d49506e2f 100644 --- a/sweepbatcher/presigned.go +++ b/sweepbatcher/presigned.go @@ -51,6 +51,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep, outpoint: s.outpoint, value: s.value, presigned: s.presigned, + change: s.change, } } @@ -493,10 +494,12 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, signedFeeRate := chainfee.NewSatPerKWeight(fee, realWeight) numSweeps := len(tx.TxIn) + numChange := len(tx.TxOut) - 1 b.Infof("attempting to publish custom signed tx=%v, desiredFeerate=%v,"+ - " signedFeeRate=%v, weight=%v, fee=%v, sweeps=%d, destAddr=%s", + " signedFeeRate=%v, weight=%v, fee=%v, sweeps=%d, "+ + "changeOutputs=%d, destAddr=%s", txHash, feeRate, signedFeeRate, realWeight, fee, numSweeps, - address) + numChange, address) b.debugLogTx("serialized batch", tx) // Publish the transaction. @@ -593,23 +596,31 @@ func CheckSignedTx(unsignedTx, signedTx *wire.MsgTx, inputAmt btcutil.Amount, } // Compare outputs. - if len(unsignedTx.TxOut) != 1 { - return fmt.Errorf("unsigned tx has %d outputs, want 1", - len(unsignedTx.TxOut)) - } - if len(signedTx.TxOut) != 1 { - return fmt.Errorf("the signed tx has %d outputs, want 1", + if len(unsignedTx.TxOut) != len(signedTx.TxOut) { + return fmt.Errorf("unsigned tx has %d outputs, signed tx has "+ + "%d outputs, should be equal", len(unsignedTx.TxOut), len(signedTx.TxOut)) } - unsignedOut := unsignedTx.TxOut[0] - signedOut := signedTx.TxOut[0] - if !bytes.Equal(unsignedOut.PkScript, signedOut.PkScript) { - return fmt.Errorf("mismatch of output pkScript: %x, %x", - unsignedOut.PkScript, signedOut.PkScript) + for i, o := range unsignedTx.TxOut { + if !bytes.Equal(o.PkScript, signedTx.TxOut[i].PkScript) { + return fmt.Errorf("mismatch of output pkScript: %x, %x", + o.PkScript, signedTx.TxOut[i].PkScript) + } + if i != 0 && o.Value != signedTx.TxOut[i].Value { + return fmt.Errorf("mismatch of output value: %d, %d", + o.Value, signedTx.TxOut[i].Value) + } + } + + // Calculate the total value of all outputs to help determine the + // transaction fee. + totalOutputValue := btcutil.Amount(0) + for _, o := range signedTx.TxOut { + totalOutputValue += btcutil.Amount(o.Value) } // Find the feerate of signedTx. - fee := inputAmt - btcutil.Amount(signedOut.Value) + fee := inputAmt - totalOutputValue weight := lntypes.WeightUnit( blockchain.GetTransactionWeight(btcutil.NewTx(signedTx)), ) diff --git a/sweepbatcher/presigned_test.go b/sweepbatcher/presigned_test.go index 629ff1de4..d4f593737 100644 --- a/sweepbatcher/presigned_test.go +++ b/sweepbatcher/presigned_test.go @@ -1460,7 +1460,8 @@ func TestCheckSignedTx(t *testing.T) { }, inputAmt: 3_000_000, minRelayFee: 253, - wantErr: "unsigned tx has 2 outputs, want 1", + wantErr: "unsigned tx has 2 outputs, signed tx " + + "has 1 outputs, should be equal", }, { @@ -1517,7 +1518,153 @@ func TestCheckSignedTx(t *testing.T) { }, inputAmt: 3_000_000, minRelayFee: 253, - wantErr: "the signed tx has 2 outputs, want 1", + wantErr: "unsigned tx has 1 outputs, signed tx " + + "has 2 outputs, should be equal", + }, + + { + name: "pkscript mismatch", + unsignedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 2999374, + PkScript: batchPkScript, + }, + }, + LockTime: 800_000, + }, + signedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + Witness: wire.TxWitness{ + []byte("test"), + }, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 2999374, + PkScript: []byte{0xaf, 0xfe}, // Just to make it different. + }, + }, + LockTime: 799_999, + }, + inputAmt: 3_000_000, + minRelayFee: 253, + wantErr: "mismatch of output pkScript", + }, + + { + name: "value mismatch, first output", + unsignedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 2999374, + PkScript: batchPkScript, + }, + }, + LockTime: 800_000, + }, + signedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + Witness: wire.TxWitness{ + []byte("test"), + }, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 1_337_000, // Just to make it different. + PkScript: batchPkScript, + }, + }, + LockTime: 799_999, + }, + inputAmt: 3_000_000, + minRelayFee: 253, + wantErr: "", + }, + + { + name: "value mismatch, change output", + unsignedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + }, + { + PreviousOutPoint: op1, + Sequence: 2, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 2999374, + PkScript: batchPkScript, + }, + { + Value: 1_337_000, + PkScript: batchPkScript, + }, + }, + LockTime: 800_000, + }, + signedTx: &wire.MsgTx{ + Version: 2, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + Sequence: 2, + Witness: wire.TxWitness{ + []byte("test"), + }, + }, + { + PreviousOutPoint: op1, + Sequence: 2, + Witness: wire.TxWitness{ + []byte("test"), + }, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 2_493_300, + PkScript: batchPkScript, + }, + { + Value: 1_338, // Just to make it different. + PkScript: batchPkScript, + }, + }, + LockTime: 799_999, + }, + inputAmt: 3_000_000, + minRelayFee: 253, + wantErr: "mismatch of output value", }, { diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 50ecafbbb..98f576df0 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -26,6 +26,7 @@ import ( "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" sweeppkg "github.com/lightninglabs/loop/sweep" + "github.com/lightninglabs/loop/utils" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/input" @@ -1290,10 +1291,14 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte, } // constructUnsignedTx creates unsigned tx from the sweeps, paying to the addr. -// It also returns absolute fee (from weight and clamped). +// It also returns absolute fee (from weight and clamped). The main output is +// the first output of the transaction, followed by an optional list of change +// outputs. If the main output value is below dust limit this function will +// return an error. func constructUnsignedTx(sweeps []sweep, address btcutil.Address, - currentHeight int32, feeRate chainfee.SatPerKWeight) (*wire.MsgTx, - lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, error) { + currentHeight int32, feeRate chainfee.SatPerKWeight) ( + *wire.MsgTx, lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, + error) { // Sanity check, there should be at least 1 sweep in this batch. if len(sweeps) == 0 { @@ -1306,6 +1311,13 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, LockTime: uint32(currentHeight), } + var changeOutputs []*wire.TxOut + for _, sweep := range sweeps { + if sweep.change != nil { + changeOutputs = append(changeOutputs, sweep.change) + } + } + // Add transaction inputs and estimate its weight. var weightEstimate input.TxWeightEstimator for _, sweep := range sweeps { @@ -1351,6 +1363,11 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, "failed: %w", err) } + // Add the optional change outputs to weight estimates. + for _, o := range changeOutputs { + weightEstimate.AddOutput(o.PkScript) + } + // Keep track of the total amount this batch is sweeping back. batchAmt := btcutil.Amount(0) for _, sweep := range sweeps { @@ -1368,15 +1385,78 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, feeForWeight++ } + // Add the batch transaction output, which excludes the fees paid to + // miners. Reduce the amount by the sum of change outputs, if any. + var sumChange int64 + for _, change := range changeOutputs { + sumChange += change.Value + } + + // Ensure that the batch amount is greater than the sum of change. + if batchAmt <= btcutil.Amount(sumChange) { + return nil, 0, 0, 0, fmt.Errorf("batch amount %v is <= the "+ + "sum of change outputs %v", batchAmt, + btcutil.Amount(sumChange)) + } + // Clamp the calculated fee to the max allowed fee amount for the batch. - fee := clampBatchFee(feeForWeight, batchAmt) + fee := clampBatchFee(feeForWeight, batchAmt-btcutil.Amount(sumChange)) - // Add the batch transaction output, which excludes the fees paid to - // miners. + // 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. + dustLimit := utils.DustLimitForPkScript(batchPkScript) + if fee+btcutil.Amount(sumChange)+dustLimit > batchAmt { + 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) + } + + // Add the main output first. batchTx.AddTxOut(&wire.TxOut{ PkScript: batchPkScript, - Value: int64(batchAmt - fee), + Value: int64(batchAmt-fee) - sumChange, }) + // Then add change outputs. + for _, txOut := range changeOutputs { + batchTx.AddTxOut(&wire.TxOut{ + PkScript: txOut.PkScript, + Value: txOut.Value, + }) + } + + // Check that for each swap, inputs exceed the change outputs. + if len(changeOutputs) != 0 { + swap2Inputs := make(map[lntypes.Hash]btcutil.Amount) + swap2Change := make(map[lntypes.Hash]btcutil.Amount) + for _, sweep := range sweeps { + swap2Inputs[sweep.swapHash] += sweep.value + if sweep.change != nil { + swap2Change[sweep.swapHash] += + btcutil.Amount(sweep.change.Value) + } + } + + for swapHash, inputs := range swap2Inputs { + change := swap2Change[swapHash] + if inputs <= change { + return nil, 0, 0, 0, fmt.Errorf(""+ + "inputs %v <= change %v for swap %x", + inputs, change, swapHash[:6]) + } + } + } + + // Ensure that each output is above dust limit. + for _, txOut := range batchTx.TxOut { + dustLimit = utils.DustLimitForPkScript(txOut.PkScript) + if btcutil.Amount(txOut.Value) < dustLimit { + return nil, 0, 0, 0, fmt.Errorf("output %v is below "+ + "dust limit %v", btcutil.Amount(txOut.Value), + dustLimit) + } + } return batchTx, weight, feeForWeight, fee, nil } diff --git a/sweepbatcher/sweep_batch_test.go b/sweepbatcher/sweep_batch_test.go index 570565b0d..785278744 100644 --- a/sweepbatcher/sweep_batch_test.go +++ b/sweepbatcher/sweep_batch_test.go @@ -29,6 +29,10 @@ func TestConstructUnsignedTx(t *testing.T) { Hash: chainhash.Hash{2, 2, 2}, Index: 2, } + op3 := wire.OutPoint{ + Hash: chainhash.Hash{3, 3, 3}, + Index: 3, + } batchPkScript, err := txscript.PayToAddrScript(destAddr) require.NoError(t, err) @@ -40,6 +44,28 @@ func TestConstructUnsignedTx(t *testing.T) { p2trPkScript, err := txscript.PayToAddrScript(p2trAddress) require.NoError(t, err) + change1Addr := "bc1pdx9ggvtjjcpaqfqk375qhdmzx9xu8dcu7w94lqfcxhh0rj" + + "lwyyeq5ryn6r" + change1Address, err := btcutil.DecodeAddress(change1Addr, nil) + require.NoError(t, err) + change1Pkscript, err := txscript.PayToAddrScript(change1Address) + require.NoError(t, err) + change1 := &wire.TxOut{ + Value: 100_000, + PkScript: change1Pkscript, + } + + change2Addr := "bc1psw0nrrulq4pgyuyk09a3wsutygltys4gxjjw3zl2uz4ep8pa" + + "r2vsvntfe0" + change2Address, err := btcutil.DecodeAddress(change2Addr, nil) + require.NoError(t, err) + change2Pkscript, err := txscript.PayToAddrScript(change2Address) + require.NoError(t, err) + change2 := &wire.TxOut{ + Value: 200_000, + PkScript: change2Pkscript, + } + serializedPubKey := []byte{ 0x02, 0x19, 0x2d, 0x74, 0xd0, 0xcb, 0x94, 0x34, 0x4c, 0x95, 0x69, 0xc2, 0xe7, 0x79, 0x01, 0x57, 0x3d, 0x8d, 0x79, 0x03, @@ -70,12 +96,15 @@ func TestConstructUnsignedTx(t *testing.T) { return fmt.Errorf("weight estimator test failure") } + dustLimit := utils.DustLimitForPkScript(batchPkScript) + cases := []struct { name string sweeps []sweep address btcutil.Address currentHeight int32 feeRate chainfee.SatPerKWeight + minRelayFeeRate chainfee.SatPerKWeight wantErr string wantTx *wire.MsgTx wantWeight lntypes.WeightUnit @@ -223,7 +252,7 @@ func TestConstructUnsignedTx(t *testing.T) { }, TxOut: []*wire.TxOut{ { - Value: 2400000, + Value: 2_400_000, PkScript: batchPkScript, }, }, @@ -265,7 +294,7 @@ func TestConstructUnsignedTx(t *testing.T) { }, TxOut: []*wire.TxOut{ { - Value: 2999211, + Value: 2_999_211, PkScript: batchPkScript, }, }, @@ -275,6 +304,208 @@ func TestConstructUnsignedTx(t *testing.T) { wantFee: 789, }, + { + name: "single sweep with change", + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + change: change1, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op1, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 899_384, + PkScript: p2trPkScript, + }, + { + Value: change1.Value, + PkScript: change1.PkScript, + }, + }, + }, + wantWeight: 616, + wantFeeForWeight: 616, + wantFee: 616, + }, + + { + name: "all sweeps different change outputs", + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + }, + { + outpoint: op2, + value: 2_000_000, + change: change1, + }, + { + outpoint: op3, + value: 3_000_000, + change: change2, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op1, + }, + { + PreviousOutPoint: op2, + }, + { + PreviousOutPoint: op3, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 5_698_752, + PkScript: p2trPkScript, + }, + { + Value: change1.Value, + PkScript: change1.PkScript, + }, + { + Value: change2.Value, + PkScript: change2.PkScript, + }, + }, + }, + wantWeight: 1248, + wantFeeForWeight: 1248, + wantFee: 1248, + }, + + { + name: "change exceeds input value", + sweeps: []sweep{ + { + outpoint: op2, + value: btcutil.Amount(change1.Value - 1), + change: change1, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op2, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: change1.Value, + PkScript: change1.PkScript, + }, + }, + }, + wantErr: "batch amount 0.00099999 BTC is <= the sum " + + "of change outputs 0.00100000 BTC", + }, + + { + name: "main output dust, batch amount less than " + + "change+fee+dust", + sweeps: []sweep{ + { + outpoint: op1, + value: dustLimit, + }, + { + outpoint: op2, + value: btcutil.Amount(change1.Value), + change: change1, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1, + wantErr: "batch amount 0.00100294 BTC is <= the sum " + + "of change outputs 0.00100000 BTC plus fee " + + "0.00000001 BTC and dust limit 0.00000330 BTC", + }, + + { + name: "change output is dust", + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + change: &wire.TxOut{ + Value: int64(dustLimit - 1), + PkScript: []byte{0xaf, 0xfe}, + }, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantErr: "output 0.00000293 BTC is below dust limit " + + "0.00000477 BTC", + }, + + { + name: "clamp fee to max fee to swap amount ratio", + sweeps: []sweep{ + { + outpoint: op1, + value: btcutil.SatoshiPerBitcoin, + change: &wire.TxOut{ + Value: btcutil.SatoshiPerBitcoin * 0.9, + PkScript: change1Pkscript, + }, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 10000000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op1, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: btcutil.SatoshiPerBitcoin * 0.08, + PkScript: p2trPkScript, + }, + { + Value: btcutil.SatoshiPerBitcoin * 0.9, + PkScript: change1.PkScript, + }, + }, + }, + wantWeight: 616, + wantFeeForWeight: 6_160_000, + wantFee: 2_000_000, + }, + { name: "weight estimator fails", sweeps: []sweep{ @@ -338,9 +569,13 @@ func TestConstructUnsignedTx(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + relayFeeRate := minRelayFeeRate + if tc.minRelayFeeRate != 0 { + relayFeeRate = tc.minRelayFeeRate + } tx, weight, feeForW, fee, err := constructUnsignedTx( tc.sweeps, tc.address, tc.currentHeight, - tc.feeRate, + tc.feeRate, relayFeeRate, ) if tc.wantErr != "" { require.Error(t, err) diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index dcd987efe..2afeb3273 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -125,6 +125,9 @@ type SweepInfo struct { // value should be stable for a sweep. Currently presigned and // non-presigned sweeps never appear in the same batch. IsPresigned bool + + // Change is an optional change output of the sweep. + Change *wire.TxOut } // SweepFetcher is used to get details of a sweep. @@ -168,7 +171,10 @@ type PresignedHelper interface { // SignTx signs an unsigned transaction or returns a pre-signed tx. // It must satisfy the following invariants: // - the set of inputs is the same, though the order may change; - // - the output is the same, but its amount may be different; + // - the main output is the same, but its amount may be different; + // - the main output is the first output in the transaction; + // - an optional set of change outputs may be added, the values and + // pkscripts must be preserved. // - feerate is higher or equal to minRelayFee; // - LockTime may be decreased; // - transaction version must be the same; @@ -177,6 +183,7 @@ type PresignedHelper interface { // When choosing a presigned transaction, a transaction with fee rate // closer to the fee rate passed is selected. If loadOnly is set, it // doesn't try to sign the transaction and only loads a presigned tx. + // These rules are enforced by CheckSignedTx function. SignTx(ctx context.Context, primarySweepID wire.OutPoint, tx *wire.MsgTx, inputAmt btcutil.Amount, minRelayFee, feeRate chainfee.SatPerKWeight, @@ -711,9 +718,11 @@ func (b *Batcher) Run(ctx context.Context) error { // group. This method must be called prior to AddSweep if presigned mode is // enabled, otherwise AddSweep will fail. All the sweeps must belong to the same // swap. The order of sweeps is important. The first sweep serves as -// primarySweepID if the group starts a new batch. +// primarySweepID if the group starts a new batch. The change output may be nil +// to indicate that the sweep group does not create a change output. func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, - sweepTimeout int32, destAddress btcutil.Address) error { + sweepTimeout int32, destAddress btcutil.Address, + changeOutput *wire.TxOut) error { if len(inputs) == 0 { return fmt.Errorf("no inputs passed to PresignSweepsGroup") @@ -745,6 +754,9 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, } } + // Set the change output on the primary group sweep. + sweeps[0].change = changeOutput + // The sweeps are ordered inside the group, the first one is the primary // outpoint in the batch. primarySweepID := sweeps[0].outpoint @@ -1548,6 +1560,7 @@ func (b *Batcher) loadSweep(ctx context.Context, swapHash lntypes.Hash, minFeeRate: minFeeRate, nonCoopHint: s.NonCoopHint, presigned: s.IsPresigned, + change: s.Change, }, nil } diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index 36f11f750..fd28bc7d8 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -17,7 +17,9 @@ import ( "github.com/lightninglabs/loop/swap" "github.com/lightninglabs/loop/test" "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" @@ -30,6 +32,9 @@ type mockPresignedHelper struct { // participating in presigning. onlineOutpoints map[wire.OutPoint]bool + // changeOutputs is a map of change outputs for a given primary deposit. + changeOutputs map[wire.OutPoint]*wire.TxOut + // presignedBatches is the collection of presigned batches. The key is // primarySweepID. presignedBatches map[wire.OutPoint][]*wire.MsgTx @@ -46,6 +51,7 @@ type mockPresignedHelper struct { func newMockPresignedHelper() *mockPresignedHelper { return &mockPresignedHelper{ onlineOutpoints: make(map[wire.OutPoint]bool), + changeOutputs: make(map[wire.OutPoint]*wire.TxOut), presignedBatches: make(map[wire.OutPoint][]*wire.MsgTx), cleanupCalled: make(chan struct{}), } @@ -59,6 +65,16 @@ func (h *mockPresignedHelper) SetOutpointOnline(op wire.OutPoint, online bool) { h.onlineOutpoints[op] = online } +// setChangeForPrimaryDeposit sets the change output of a primary deposit sweep. +func (h *mockPresignedHelper) setChangeForPrimaryDeposit(op wire.OutPoint, + change *wire.TxOut) { + + h.mu.Lock() + defer h.mu.Unlock() + + h.changeOutputs[op] = change +} + // offlineInputs returns inputs of a tx which are offline. func (h *mockPresignedHelper) offlineInputs(tx *wire.MsgTx) []wire.OutPoint { offline := make([]wire.OutPoint, 0, len(tx.TxIn)) @@ -113,7 +129,7 @@ func (h *mockPresignedHelper) DestPkScript(ctx context.Context, } // SignTx tries to sign the transaction. If all the inputs are online, it signs -// the exact transaction passed and adds it to presignedBatches. Otherwise it +// the exact transaction passed and adds it to presignedBatches. Otherwise, it // looks for a transaction in presignedBatches satisfying the criteria. func (h *mockPresignedHelper) SignTx(ctx context.Context, primarySweepID wire.OutPoint, tx *wire.MsgTx, inputAmt btcutil.Amount, @@ -211,6 +227,9 @@ func (h *mockPresignedHelper) FetchSweep(_ context.Context, // Find IsPresigned. _, isPresigned := h.onlineOutpoints[utxo] + // Find change. + change := h.changeOutputs[utxo] + return &SweepInfo{ // Set Timeout to prevent warning messages about timeout=0. Timeout: sweepTimeout, @@ -220,6 +239,7 @@ func (h *mockPresignedHelper) FetchSweep(_ context.Context, HTLC: swap.Htlc{ PkScript: []byte{10, 11, 12}, }, + Change: change, }, nil } @@ -273,7 +293,7 @@ func testPresigned_forgotten_presign(t *testing.T, presignedHelper.SetOutpointOnline(op1, false) err := batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: 1_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.Error(t, err) require.ErrorContains(t, err, "offline") @@ -350,7 +370,7 @@ func testPresigned_input1_offline_then_input2(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: 1_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) @@ -413,7 +433,7 @@ func testPresigned_input1_offline_then_input2(t *testing.T, presignedHelper.SetOutpointOnline(op2, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op2, Value: 2_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) @@ -520,7 +540,7 @@ func testPresigned_min_relay_fee(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) err := batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: inputAmt}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) @@ -644,7 +664,7 @@ func testPresigned_two_inputs_one_goes_offline(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: 1_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) require.NoError(t, batcher.AddSweep(ctx, &sweepReq1)) @@ -670,7 +690,7 @@ func testPresigned_two_inputs_one_goes_offline(t *testing.T, presignedHelper.SetOutpointOnline(op2, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op2, Value: 2_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) require.NoError(t, batcher.AddSweep(ctx, &sweepReq2)) @@ -780,7 +800,7 @@ func testPresigned_first_publish_fails(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: 1_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) presignedHelper.SetOutpointOnline(op1, false) @@ -903,7 +923,7 @@ func testPresigned_locktime(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op1, Value: 1_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) presignedHelper.SetOutpointOnline(op1, false) @@ -994,14 +1014,18 @@ func testPresigned_presigned_group(t *testing.T, presignedHelper.SetOutpointOnline(op2, false) // An attempt to presign must fail. - err = batcher.PresignSweepsGroup(ctx, group1, sweepTimeout, destAddr) + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, nil, + ) require.ErrorContains(t, err, "some outpoint is offline") // Enable both outpoints. presignedHelper.SetOutpointOnline(op2, true) // An attempt to presign must succeed. - err = batcher.PresignSweepsGroup(ctx, group1, sweepTimeout, destAddr) + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, nil, + ) require.NoError(t, err) // Add the sweep, triggering the publish attempt. @@ -1053,7 +1077,9 @@ func testPresigned_presigned_group(t *testing.T, presignedHelper.SetOutpointOnline(op4, true) // An attempt to presign must succeed. - err = batcher.PresignSweepsGroup(ctx, group2, sweepTimeout, destAddr) + err = batcher.PresignSweepsGroup( + ctx, group2, sweepTimeout, destAddr, nil, + ) require.NoError(t, err) // Add the sweep. It should go to the same batch. @@ -1107,7 +1133,9 @@ func testPresigned_presigned_group(t *testing.T, presignedHelper.SetOutpointOnline(op6, true) // An attempt to presign must succeed. - err = batcher.PresignSweepsGroup(ctx, group3, sweepTimeout, destAddr) + err = batcher.PresignSweepsGroup( + ctx, group3, sweepTimeout, destAddr, nil, + ) require.NoError(t, err) // Add the sweep. It should go to the same batch. @@ -1135,6 +1163,271 @@ func testPresigned_presigned_group(t *testing.T, require.Equal(t, batchPkScript, tx.TxOut[0].PkScript) } +// testPresigned_presigned_group_with_change 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. +func testPresigned_presigned_group_with_change(t *testing.T, + batcherStore testBatcherStore) { + + defer test.Guard(t)() + + batchPkScript, err := txscript.PayToAddrScript(destAddr) + require.NoError(t, err) + + 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) + }() + + // Create a swap of two sweeps. + swapHash1 := lntypes.Hash{1, 1, 1} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + op2 := wire.OutPoint{ + Hash: chainhash.Hash{2, 2}, + Index: 2, + } + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + { + Outpoint: op2, + Value: 2_000_000, + }, + } + change := &wire.TxOut{ + Value: 500_000, + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + presignedHelper.SetOutpointOnline(op2, true) + + // An attempt to presign must fail. + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, change, + ) + require.NoError(t, err) + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.NoError(t, err) + + // Since a batch was created we check that it registered for its primary + // sweep's spend. + <-lnd.RegisterSpendChannel + + // Wait for a transactions to be published. + tx := <-lnd.TxPublishChannel + require.Len(t, tx.TxIn, 2) + require.Len(t, tx.TxOut, 2) + require.ElementsMatch( + t, []wire.OutPoint{op1, op2}, + []wire.OutPoint{ + tx.TxIn[0].PreviousOutPoint, + tx.TxIn[1].PreviousOutPoint, + }, + ) + require.Equal(t, int64(2_493_300), tx.TxOut[0].Value) + require.Equal(t, change.Value, tx.TxOut[1].Value) + require.Equal(t, batchPkScript, tx.TxOut[0].PkScript) + require.Equal(t, change.PkScript, tx.TxOut[1].PkScript) + + // Mine a blocks to trigger republishing. + require.NoError(t, lnd.NotifyHeight(601)) +} + +// testPresigned_presigned_group_with_dust_main_output tests passing multiple +// sweeps to the method PresignSweepsGroup. It tests that a dust change output of +// a primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. +func testPresigned_presigned_group_with_dust_main_output(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) + }() + + // Create a swap of two sweeps. + swapHash1 := lntypes.Hash{1, 1, 1} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + inputValue := int64(1_000_000) + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + } + dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + change := &wire.TxOut{ + Value: inputValue - dustLimit + 1, + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + + // An attempt to presign must fail. + 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") + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.ErrorContains(t, err, "were not presigned") +} + +// testPresigned_presigned_group_with_dust_change tests passing multiple sweeps +// to the method PresignSweepsGroup. It tests that a dust change output of a +// primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. +func testPresigned_presigned_group_with_dust_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) + }() + + // Create a swap of two sweeps. + swapHash1 := lntypes.Hash{1, 1, 1} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + op2 := wire.OutPoint{ + Hash: chainhash.Hash{2, 2}, + Index: 2, + } + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + { + Outpoint: op2, + Value: 2_000_000, + }, + } + dustLimit := lnwallet.DustLimitForSize(input.P2TRSize) + change := &wire.TxOut{ + Value: int64(dustLimit - 1), + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + presignedHelper.SetOutpointOnline(op2, true) + + // An attempt to presign must fail. + err := batcher.PresignSweepsGroup( + 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 "+ + "limit 0.00000477 BTC") + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.ErrorContains(t, err, "were not presigned") +} + // wrappedStoreWithPresignedFlag wraps a SweepFetcher store adding IsPresigned // flag to the returned sweeps, taking it from mockPresignedHelper. type wrappedStoreWithPresignedFlag struct { @@ -1304,7 +1597,7 @@ func testPresigned_presigned_and_regular_sweeps(t *testing.T, store testStore, presignedHelper.SetOutpointOnline(op2, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op2, Value: 2_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) require.NoError(t, batcher.AddSweep(ctx, &sweepReq2)) @@ -1399,7 +1692,7 @@ func testPresigned_presigned_and_regular_sweeps(t *testing.T, store testStore, presignedHelper.SetOutpointOnline(op4, true) err = batcher.PresignSweepsGroup( ctx, []Input{{Outpoint: op4, Value: 3_000_000}}, - sweepTimeout, destAddr, + sweepTimeout, destAddr, nil, ) require.NoError(t, err) require.NoError(t, batcher.AddSweep(ctx, &sweepReq4)) @@ -1520,7 +1813,7 @@ func testPresigned_purging(t *testing.T, numSwaps, numConfirmedSwaps int, // An attempt to presign must succeed. err := batcher.PresignSweepsGroup( - ctx, group, sweepTimeout, destAddr, + ctx, group, sweepTimeout, destAddr, nil, ) require.NoError(t, err) @@ -1584,11 +1877,11 @@ func testPresigned_purging(t *testing.T, numSwaps, numConfirmedSwaps int, // An attempt to presign must succeed. err := batcher.PresignSweepsGroup( - ctx, group, sweepTimeout, destAddr, + ctx, group, sweepTimeout, destAddr, nil, ) require.NoError(t, err) - // Add the sweep, triggering the publish attempt. + // Add the sweep, triggering the publishing attempt. require.NoError(t, batcher.AddSweep(ctx, &SweepRequest{ SwapHash: swapHash, Inputs: group, @@ -1799,6 +2092,20 @@ func TestPresigned(t *testing.T) { testPresigned_presigned_group(t, NewStoreMock()) }) + t.Run("change", func(t *testing.T) { + testPresigned_presigned_group_with_change(t, NewStoreMock()) + }) + + t.Run("dust_main_output", func(t *testing.T) { + testPresigned_presigned_group_with_dust_main_output( + t, NewStoreMock(), + ) + }) + + t.Run("dust_change", func(t *testing.T) { + testPresigned_presigned_group_with_dust_change(t, NewStoreMock()) + }) + t.Run("presigned_and_regular_sweeps", func(t *testing.T) { runTests(t, testPresigned_presigned_and_regular_sweeps) }) diff --git a/sweepbatcher/sweep_batcher_test.go b/sweepbatcher/sweep_batcher_test.go index 126724435..f74307818 100644 --- a/sweepbatcher/sweep_batcher_test.go +++ b/sweepbatcher/sweep_batcher_test.go @@ -229,7 +229,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -238,7 +238,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -276,7 +276,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 222, + Value: 2222, Outpoint: op2, }}, Notifier: &dummyNotifier, @@ -285,7 +285,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, swap2 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance - 1, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -322,7 +322,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, sweepReq3 := SweepRequest{ SwapHash: lntypes.Hash{3, 3, 3}, Inputs: []Input{{ - Value: 333, + Value: 3333, Outpoint: op3, }}, Notifier: &dummyNotifier, @@ -331,7 +331,7 @@ func testSweepBatcherBatchCreation(t *testing.T, store testStore, swap3 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance + 1, - AmountRequested: 333, + AmountRequested: 3333, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -539,7 +539,7 @@ func testTxLabeler(t *testing.T, store testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -548,7 +548,7 @@ func testTxLabeler(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -692,7 +692,7 @@ func testPublishErrorHandler(t *testing.T, store testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{1, 1}, Index: 1, @@ -704,7 +704,7 @@ func testPublishErrorHandler(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -774,7 +774,7 @@ func testSweepBatcherSimpleLifecycle(t *testing.T, store testStore, Index: 1, } const ( - inputValue = 111 + inputValue = 1111 outputValue = 50 fee = inputValue - outputValue ) @@ -1209,7 +1209,7 @@ func testSweepBatcherSkippedTxns(t *testing.T, store testStore, } swapHash := lntypes.Hash{1, 1, 1} const ( - inputValue = 111 + inputValue = 1111 initiationHeight = 550 ) @@ -1419,7 +1419,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { sweepReq := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -1428,7 +1428,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 1000, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -1712,7 +1712,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{2, 2}, Index: 2, @@ -1727,7 +1727,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { // CltvExpiry is not urgent, but close. CltvExpiry: 600 + blocksInDelay*2 + 5, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -1795,7 +1795,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { sweepReq3 := SweepRequest{ SwapHash: lntypes.Hash{3, 3, 3}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{3, 3}, Index: 3, @@ -1808,7 +1808,7 @@ func testDelays(t *testing.T, store testStore, batcherStore testBatcherStore) { // CltvExpiry is urgent. CltvExpiry: 600 + blocksInDelay*2 - 5, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -1871,8 +1871,8 @@ func testCustomDelays(t *testing.T, store testStore, swapHash2 := lntypes.Hash{2, 2, 2} const ( - swapSize1 = 111 - swapSize2 = 222 + swapSize1 = 1111 + swapSize2 = 2222 ) // initialDelay returns initialDelay depending of batch size (sats). @@ -1945,7 +1945,7 @@ func testCustomDelays(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 1000, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -2013,7 +2013,7 @@ func testCustomDelays(t *testing.T, store testStore, swap2 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 1000, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2152,7 +2152,7 @@ func testMaxSweepsPerBatch(t *testing.T, store testStore, sweepReq := SweepRequest{ SwapHash: swapHash, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: outpoint, }}, Notifier: &dummyNotifier, @@ -2161,7 +2161,7 @@ func testMaxSweepsPerBatch(t *testing.T, store testStore, swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 1000, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2269,7 +2269,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, Hash: chainhash.Hash{1, 1}, Index: 1, } - value1 := btcutil.Amount(111) + value1 := btcutil.Amount(1111) sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ @@ -2282,7 +2282,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -2298,7 +2298,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 222, + Value: 2222, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{2, 2}, Index: 2, @@ -2310,7 +2310,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, swap2 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2329,7 +2329,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, sweepReq3 := SweepRequest{ SwapHash: lntypes.Hash{3, 3, 3}, Inputs: []Input{{ - Value: 333, + Value: 3333, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{3, 3}, Index: 3, @@ -2341,7 +2341,7 @@ func testSweepBatcherSweepReentry(t *testing.T, store testStore, swap3 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 333, + AmountRequested: 3333, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2536,7 +2536,7 @@ func testSweepBatcherGroup(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -2555,11 +2555,11 @@ func testSweepBatcherGroup(t *testing.T, store testStore, Inputs: []Input{ { Outpoint: outpoint1, - Value: 111, + Value: 1111, }, { Outpoint: outpoint2, - Value: 222, + Value: 2222, }, }, Notifier: &dummyNotifier, @@ -2621,7 +2621,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -2630,7 +2630,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -2668,7 +2668,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 222, + Value: 2222, Outpoint: op2, }}, Notifier: &dummyNotifier, @@ -2677,7 +2677,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, swap2 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance - 1, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2714,7 +2714,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, sweepReq3 := SweepRequest{ SwapHash: lntypes.Hash{3, 3, 3}, Inputs: []Input{{ - Value: 333, + Value: 3333, Outpoint: op3, }}, Notifier: &dummyNotifier, @@ -2723,7 +2723,7 @@ func testSweepBatcherNonWalletAddr(t *testing.T, store testStore, swap3 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance + 1, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2799,6 +2799,8 @@ func testSweepBatcherComposite(t *testing.T, store testStore, ctx, cancel := context.WithCancel(context.Background()) defer cancel() + lnd.SetMinRelayFee(200) + sweepStore, err := NewSweepFetcherFromSwapStore(store, lnd.ChainParams) require.NoError(t, err) @@ -2839,7 +2841,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -2848,7 +2850,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, swap1 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -2866,7 +2868,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 222, + Value: 2222, Outpoint: op2, }}, Notifier: &dummyNotifier, @@ -2875,7 +2877,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, swap2 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance - 1, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -2896,7 +2898,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, sweepReq3 := SweepRequest{ SwapHash: lntypes.Hash{3, 3, 3}, Inputs: []Input{{ - Value: 333, + Value: 3333, Outpoint: op3, }}, Notifier: &dummyNotifier, @@ -2905,7 +2907,7 @@ func testSweepBatcherComposite(t *testing.T, store testStore, swap3 := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111 + defaultMaxTimeoutDistance - 3, - AmountRequested: 333, + AmountRequested: 3333, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, @@ -3242,7 +3244,7 @@ func testRestoringEmptyBatch(t *testing.T, store testStore, sweepReq := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op, }}, Notifier: &dummyNotifier, @@ -3251,7 +3253,7 @@ func testRestoringEmptyBatch(t *testing.T, store testStore, swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -3426,7 +3428,7 @@ func testHandleSweepTwice(t *testing.T, backend testStore, sweepReq1 := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op1, }}, Notifier: &dummyNotifier, @@ -3439,7 +3441,7 @@ func testHandleSweepTwice(t *testing.T, backend testStore, Contract: &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: shortCltv, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -3456,7 +3458,7 @@ func testHandleSweepTwice(t *testing.T, backend testStore, sweepReq2 := SweepRequest{ SwapHash: lntypes.Hash{2, 2, 2}, Inputs: []Input{{ - Value: 222, + Value: 2222, Outpoint: op2, }}, Notifier: &dummyNotifier, @@ -3469,7 +3471,7 @@ func testHandleSweepTwice(t *testing.T, backend testStore, Contract: &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: longCltv, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -3524,7 +3526,7 @@ func testHandleSweepTwice(t *testing.T, backend testStore, Contract: &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: shortCltv, - AmountRequested: 222, + AmountRequested: 2222, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -3628,7 +3630,7 @@ func testRestoringPreservesConfTarget(t *testing.T, store testStore, sweepReq := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: op, }}, Notifier: &dummyNotifier, @@ -3637,7 +3639,7 @@ func testRestoringPreservesConfTarget(t *testing.T, store testStore, swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, @@ -3955,7 +3957,7 @@ func testSweepBatcherCloseDuringAdding(t *testing.T, store testStore, swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, // Make preimage unique to pass SQL constraints. Preimage: lntypes.Preimage{i}, @@ -3981,7 +3983,7 @@ func testSweepBatcherCloseDuringAdding(t *testing.T, store testStore, sweepReq := SweepRequest{ SwapHash: lntypes.Hash{i, i, i}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{i, i}, Index: 1, @@ -4066,7 +4068,7 @@ func testCustomSignMuSig2(t *testing.T, store testStore, sweepReq := SweepRequest{ SwapHash: lntypes.Hash{1, 1, 1}, Inputs: []Input{{ - Value: 111, + Value: 1111, Outpoint: wire.OutPoint{ Hash: chainhash.Hash{1, 1}, Index: 1, @@ -4078,7 +4080,7 @@ func testCustomSignMuSig2(t *testing.T, store testStore, swap := &loopdb.LoopOutContract{ SwapContract: loopdb.SwapContract{ CltvExpiry: 111, - AmountRequested: 111, + AmountRequested: 1111, ProtocolVersion: loopdb.ProtocolVersionMuSig2, HtlcKeys: htlcKeys, }, From 29348a94df3626d65a0eb6606706e1601eed9051 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 22 Jul 2025 11:23:58 +0200 Subject: [PATCH 5/8] sweepbatcher: consider min relay fee when constructing batch tx if constructUnsignedTx constructs a batch transaction that is below the minimum relay fee, an error is returned. --- loopout_test.go | 1 + sweepbatcher/presigned.go | 38 +++++-- sweepbatcher/presigned_test.go | 8 +- sweepbatcher/sweep_batch.go | 57 +++++++---- sweepbatcher/sweep_batch_test.go | 9 +- sweepbatcher/sweep_batcher.go | 14 ++- sweepbatcher/sweep_batcher_presigned_test.go | 101 +++++++++++++++++-- 7 files changed, 184 insertions(+), 44 deletions(-) diff --git a/loopout_test.go b/loopout_test.go index 1cf90a4b8..254ccea86 100644 --- a/loopout_test.go +++ b/loopout_test.go @@ -280,6 +280,7 @@ func testCustomSweepConfTarget(t *testing.T) { // yields a much higher fee rate. ctx.Lnd.SetFeeEstimate(testReq.SweepConfTarget, 250) ctx.Lnd.SetFeeEstimate(DefaultSweepConfTarget, 10000) + ctx.Lnd.SetMinRelayFee(250) cfg := newSwapConfig( &lnd.LndServices, loopdb.NewStoreMock(t), server, nil, diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index d49506e2f..9485815f0 100644 --- a/sweepbatcher/presigned.go +++ b/sweepbatcher/presigned.go @@ -28,8 +28,14 @@ func (b *batch) ensurePresigned(ctx context.Context, newSweeps []*sweep, "adding to an empty batch") } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFee: %w", err) + } + return ensurePresigned( - ctx, newSweeps, b.cfg.presignedHelper, b.cfg.chainParams, + ctx, newSweeps, b.cfg.presignedHelper, minRelayFeeRate, + b.cfg.chainParams, ) } @@ -43,6 +49,7 @@ type presignedTxChecker interface { // inputs of this group only. func ensurePresigned(ctx context.Context, newSweeps []*sweep, presignedTxChecker presignedTxChecker, + minRelayFeeRate chainfee.SatPerKWeight, chainParams *chaincfg.Params) error { sweeps := make([]sweep, len(newSweeps)) @@ -74,7 +81,7 @@ func ensurePresigned(ctx context.Context, newSweeps []*sweep, const feeRate = chainfee.FeePerKwFloor tx, _, _, _, err := constructUnsignedTx( - sweeps, destAddr, currentHeight, feeRate, + sweeps, destAddr, currentHeight, feeRate, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to construct unsigned tx "+ @@ -218,6 +225,14 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error { b.Infof("nextBlockFeeRate is %v", nextBlockFeeRate) + // Find the minRelayFeeRate. + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFeeRate: %w", err) + } + + b.Infof("minRelayFeeRate is %v", minRelayFeeRate) + // We need to restore previously added groups. We can do it by reading // all the sweeps from DB (they must be ordered) and grouping by swap. groups, err := b.getSweepsGroups(ctx) @@ -258,7 +273,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error { err = presign( ctx, b.cfg.presignedHelper, destAddr, primarySweepID, - sweeps, nextBlockFeeRate, + sweeps, nextBlockFeeRate, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to presign a transaction "+ @@ -300,7 +315,8 @@ type presigner interface { // 10x of the current next block feerate. func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, primarySweepID wire.OutPoint, sweeps []sweep, - nextBlockFeeRate chainfee.SatPerKWeight) error { + nextBlockFeeRate chainfee.SatPerKWeight, + minRelayFeeRate chainfee.SatPerKWeight) error { if presigner == nil { return fmt.Errorf("presigner is not installed") @@ -354,7 +370,7 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address, for fr := start; fr <= stop; fr = (fr * factorPPM) / 1_000_000 { // Construct an unsigned transaction for this fee rate. tx, _, feeForWeight, fee, err := constructUnsignedTx( - sweeps, destAddr, currentHeight, fr, + sweeps, destAddr, currentHeight, fr, minRelayFeeRate, ) if err != nil { return fmt.Errorf("failed to construct unsigned tx "+ @@ -411,15 +427,15 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, } // Determine the current minimum relay fee based on our chain backend. - minRelayFee, err := b.wallet.MinRelayFee(ctx) + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) if err != nil { - return 0, fmt.Errorf("failed to get minRelayFee: %w", err), + return 0, fmt.Errorf("failed to get minRelayFeeRate: %w", err), false } // Cache current height and desired feerate of the batch. currentHeight := b.currentHeight - feeRate := max(b.rbfCache.FeeRate, minRelayFee) + feeRate := max(b.rbfCache.FeeRate, minRelayFeeRate) // Append this sweep to an array of sweeps. This is needed to keep the // order of sweeps stored, as iterating the sweeps map does not @@ -441,7 +457,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Construct unsigned batch transaction. tx, weight, _, fee, err := constructUnsignedTx( - sweeps, address, currentHeight, feeRate, + sweeps, address, currentHeight, feeRate, minRelayFeeRate, ) if err != nil { return 0, fmt.Errorf("failed to construct tx: %w", err), @@ -460,7 +476,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Get a pre-signed transaction. const loadOnly = false signedTx, err := b.cfg.presignedHelper.SignTx( - ctx, b.primarySweepID, tx, batchAmt, minRelayFee, feeRate, + ctx, b.primarySweepID, tx, batchAmt, minRelayFeeRate, feeRate, loadOnly, ) if err != nil { @@ -470,7 +486,7 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error, // Run sanity checks to make sure presignedHelper.SignTx complied with // all the invariants. - err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFee) + err = CheckSignedTx(tx, signedTx, batchAmt, minRelayFeeRate) if err != nil { return 0, fmt.Errorf("signed tx doesn't correspond the "+ "unsigned tx: %w", err), false diff --git a/sweepbatcher/presigned_test.go b/sweepbatcher/presigned_test.go index d4f593737..35fe8bf2e 100644 --- a/sweepbatcher/presigned_test.go +++ b/sweepbatcher/presigned_test.go @@ -15,6 +15,10 @@ 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) { @@ -561,7 +565,7 @@ func TestEnsurePresigned(t *testing.T) { } err := ensurePresigned( - ctx, tc.sweeps, c, + ctx, tc.sweeps, c, minRelayFeeRate, &chaincfg.RegressionNetParams, ) switch { @@ -1010,7 +1014,7 @@ func TestPresign(t *testing.T) { err := presign( ctx, tc.presigner, tc.destAddr, tc.primarySweepID, tc.sweeps, - tc.nextBlockFeeRate, + tc.nextBlockFeeRate, minRelayFeeRate, ) if tc.wantErr != "" { require.Error(t, err) diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index 98f576df0..7b5e15422 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -1296,9 +1296,9 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte, // outputs. If the main output value is below dust limit this function will // return an error. func constructUnsignedTx(sweeps []sweep, address btcutil.Address, - currentHeight int32, feeRate chainfee.SatPerKWeight) ( - *wire.MsgTx, lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, - error) { + currentHeight int32, feeRate chainfee.SatPerKWeight, + minRelayFeeRate chainfee.SatPerKWeight) (*wire.MsgTx, + lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, error) { // Sanity check, there should be at least 1 sweep in this batch. if len(sweeps) == 0 { @@ -1400,7 +1400,14 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, } // Clamp the calculated fee to the max allowed fee amount for the batch. - fee := clampBatchFee(feeForWeight, batchAmt-btcutil.Amount(sumChange)) + fee, err := clampBatchFee( + feeForWeight, batchAmt-btcutil.Amount(sumChange), + minRelayFeeRate, weight, + ) + if err != nil { + return nil, 0, 0, 0, fmt.Errorf("failed to clamp batch "+ + "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 @@ -1516,15 +1523,21 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, // known in advance to be non-cooperative (nonCoopHint) and not failed // to sign cooperatively in previous rounds (coopFailed). If any of them // fails, the sweep is excluded from all following rounds and another - // round is attempted. Otherwise the cycle completes and we sign the + // round is attempted. Otherwise, the cycle completes and we sign the // remaining sweeps non-cooperatively. var ( - tx *wire.MsgTx - weight lntypes.WeightUnit - feeForWeight btcutil.Amount - fee btcutil.Amount - coopInputs int + tx *wire.MsgTx + weight lntypes.WeightUnit + feeForWeight btcutil.Amount + fee btcutil.Amount + minRelayFeeRate chainfee.SatPerKWeight + coopInputs int ) + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return 0, fmt.Errorf("failed to get min relay fee: %w", err), + false + } for attempt := 1; ; attempt++ { b.Infof("Attempt %d of collecting cooperative signatures.", attempt) @@ -1533,6 +1546,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, var err error tx, weight, feeForWeight, fee, err = constructUnsignedTx( sweeps, address, b.currentHeight, b.rbfCache.FeeRate, + minRelayFeeRate, ) if err != nil { return 0, fmt.Errorf("failed to construct tx: %w", err), @@ -1584,7 +1598,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, // If there was any failure of cooperative signing, we need to // update weight estimates (since non-cooperative signing has // larger witness) and hence update the whole transaction and - // all the signatures. Otherwise we complete cooperative part. + // all the signatures. Otherwise, we complete cooperative part. if !newCoopFailures { break } @@ -1720,7 +1734,7 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, } // Publish the transaction. - err := b.wallet.PublishTransaction( + err = b.wallet.PublishTransaction( ctx, tx, b.cfg.txLabeler(b.id), ) if err != nil { @@ -2583,16 +2597,25 @@ func (b *batch) persistSweep(ctx context.Context, sweep sweep, // clampBatchFee takes the fee amount and total amount of the sweeps in the // batch and makes sure the fee is not too high. If the fee is too high, it is -// clamped to the maximum allowed fee. -func clampBatchFee(fee btcutil.Amount, - totalAmount btcutil.Amount) btcutil.Amount { +// clamped to the maximum allowed fee. If the clamped fee results in a fee rate +// below the minimum relay fee, an error is returned. +func clampBatchFee(fee btcutil.Amount, totalAmount btcutil.Amount, + minRelayFeeRate chainfee.SatPerKWeight, + weight lntypes.WeightUnit) (btcutil.Amount, error) { maxFeeAmount := btcutil.Amount(float64(totalAmount) * maxFeeToSwapAmtRatio) + clampedFee := fee if fee > maxFeeAmount { - return maxFeeAmount + clampedFee = maxFeeAmount + } + + clampedFeeRate := chainfee.NewSatPerKWeight(clampedFee, weight) + if clampedFeeRate < minRelayFeeRate { + return 0, fmt.Errorf("clamped fee rate %v is less than "+ + "minimum relay fee %v", clampedFeeRate, minRelayFeeRate) } - return fee + return clampedFee, nil } diff --git a/sweepbatcher/sweep_batch_test.go b/sweepbatcher/sweep_batch_test.go index 785278744..2b77f8713 100644 --- a/sweepbatcher/sweep_batch_test.go +++ b/sweepbatcher/sweep_batch_test.go @@ -440,12 +440,13 @@ func TestConstructUnsignedTx(t *testing.T) { change: change1, }, }, - address: p2trAddress, - currentHeight: 800_000, - feeRate: 1, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1_000, + minRelayFeeRate: 50, wantErr: "batch amount 0.00100294 BTC is <= the sum " + "of change outputs 0.00100000 BTC plus fee " + - "0.00000001 BTC and dust limit 0.00000330 BTC", + "0.00000058 BTC and dust limit 0.00000330 BTC", }, { diff --git a/sweepbatcher/sweep_batcher.go b/sweepbatcher/sweep_batcher.go index 2afeb3273..f3b7265b4 100644 --- a/sweepbatcher/sweep_batcher.go +++ b/sweepbatcher/sweep_batcher.go @@ -736,6 +736,10 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, if err != nil { return fmt.Errorf("failed to get nextBlockFeeRate: %w", err) } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get minRelayFeeRate: %w", err) + } destPkscript, err := txscript.PayToAddrScript(destAddress) if err != nil { return fmt.Errorf("txscript.PayToAddrScript failed: %w", err) @@ -763,7 +767,7 @@ func (b *Batcher) PresignSweepsGroup(ctx context.Context, inputs []Input, return presign( ctx, b.presignedHelper, destAddress, primarySweepID, sweeps, - nextBlockFeeRate, + nextBlockFeeRate, minRelayFeeRate, ) } @@ -818,12 +822,18 @@ func (b *Batcher) AddSweep(ctx context.Context, sweepReq *SweepRequest) error { } } + minRelayFeeRate, err := b.wallet.MinRelayFee(ctx) + if err != nil { + return fmt.Errorf("failed to get min relay fee: %w", err) + } + // If this is a presigned mode, make sure PresignSweepsGroup was called. // We skip the check for reorg-safely confirmed sweeps, because their // presigned transactions were already cleaned up from the store. if sweep.presigned && !fullyConfirmed { err := ensurePresigned( - ctx, sweeps, b.presignedHelper, b.chainParams, + ctx, sweeps, b.presignedHelper, minRelayFeeRate, + b.chainParams, ) if err != nil { return fmt.Errorf("inputs with primarySweep %v were "+ diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index fd28bc7d8..d29072044 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -519,7 +519,7 @@ func testPresigned_min_relay_fee(t *testing.T, }() // Set high min_relay_fee. - lnd.SetMinRelayFee(400) + lnd.SetMinRelayFee(252) // Create the first sweep. swapHash1 := lntypes.Hash{1, 1, 1} @@ -554,16 +554,16 @@ func testPresigned_min_relay_fee(t *testing.T, // Wait for a transactions to be published. tx := <-lnd.TxPublishChannel gotFeeRate := presignedHelper.getTxFeerate(tx, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate) // Now decrease min_relay_fee and make sure fee rate doesn't decrease. // The only difference of tx2 is a higher lock_time. - lnd.SetMinRelayFee(300) + lnd.SetMinRelayFee(150) require.NoError(t, lnd.NotifyHeight(601)) tx2 := <-lnd.TxPublishChannel require.Equal(t, tx.TxOut[0].Value, tx2.TxOut[0].Value) gotFeeRate = presignedHelper.getTxFeerate(tx2, inputAmt) - require.Equal(t, chainfee.SatPerKWeight(402), gotFeeRate) + require.Equal(t, chainfee.SatPerKWeight(253), gotFeeRate) require.Equal(t, uint32(601), tx2.LockTime) // Set a higher min_relay_fee, turn off the client and try presigned tx. @@ -1269,9 +1269,10 @@ func testPresigned_presigned_group_with_change(t *testing.T, require.NoError(t, lnd.NotifyHeight(601)) } -// testPresigned_presigned_group_with_dust_main_output tests passing multiple -// sweeps to the method PresignSweepsGroup. It tests that a dust change output of -// a primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. +// testPresigned_presigned_group_with_dust_main_output passes a dust main output +// and a change output to PresignSweepsGroup. It will fail because of the dust +// main output. Note that the min relay fee is set low enough to pass the +// clampBatchFee check, so the error is not related to the fee rate. func testPresigned_presigned_group_with_dust_main_output(t *testing.T, batcherStore testBatcherStore) { @@ -1285,9 +1286,12 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, customFeeRate := func(_ context.Context, _ lntypes.Hash, _ wire.OutPoint) (chainfee.SatPerKWeight, error) { - return chainfee.SatPerKWeight(10_000), nil + return chainfee.SatPerKWeight(100_000), nil } + // Set min relay fee low enough to pass the clampBatchFee check. + lnd.SetMinRelayFee(140) + presignedHelper := newMockPresignedHelper() batcher := NewBatcher( @@ -1345,6 +1349,81 @@ func testPresigned_presigned_group_with_dust_main_output(t *testing.T, require.ErrorContains(t, err, "were not presigned") } +// testPresigned_presigned_group_with_dust_below_relay_fee passes a tx with a +// dust main output and one change output to PresignSweepsGroup. The tx fee rate +// is below the min relay fee, hence clampBatchFee will return an error. +func testPresigned_presigned_group_with_dust_below_relay_fee(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(100_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) + }() + + // Create a swap of two sweeps. + swapHash1 := lntypes.Hash{1, 1, 1} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + inputValue := int64(1_000_000) + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + } + dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + change := &wire.TxOut{ + Value: inputValue - dustLimit + 1, + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + + // An attempt to presign must fail. + 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") + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.ErrorContains(t, err, "were not presigned") +} + // testPresigned_presigned_group_with_dust_change tests passing multiple sweeps // to the method PresignSweepsGroup. It tests that a dust change output of a // primary deposit sweep is rejected by PresignSweepsGroup and AddSweep. @@ -2102,6 +2181,12 @@ func TestPresigned(t *testing.T) { ) }) + t.Run("dust_main_output_below_min_relay_fee", func(t *testing.T) { + testPresigned_presigned_group_with_dust_below_relay_fee( + t, NewStoreMock(), + ) + }) + t.Run("dust_change", func(t *testing.T) { testPresigned_presigned_group_with_dust_change(t, NewStoreMock()) }) From 54652dc6415cf202993f73a9224a6fbbb2dd4fcc Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Tue, 22 Jul 2025 23:48:19 -0300 Subject: [PATCH 6/8] 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 --- sweepbatcher/presigned.go | 17 +-- sweepbatcher/presigned_test.go | 112 ++++++++++++++++--- sweepbatcher/sweep_batch.go | 8 +- sweepbatcher/sweep_batch_test.go | 4 +- sweepbatcher/sweep_batcher_presigned_test.go | 91 +++++++++++---- 5 files changed, 181 insertions(+), 51 deletions(-) diff --git a/sweepbatcher/presigned.go b/sweepbatcher/presigned.go index 9485815f0..0e75240b7 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 35fe8bf2e..06220e696 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 7b5e15422..bebe2118e 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 2b77f8713..202e0a043 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 d29072044..99a70fd4f 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 From 245a66610bd2a2969b22d2a21f603b0d6341b8e6 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 29 Jul 2025 18:02:57 +0200 Subject: [PATCH 7/8] utils: bip69 output sorting --- utils/tx_sort.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 utils/tx_sort.go diff --git a/utils/tx_sort.go b/utils/tx_sort.go new file mode 100644 index 000000000..bc31e6a1b --- /dev/null +++ b/utils/tx_sort.go @@ -0,0 +1,15 @@ +package utils + +import ( + "bytes" + + "github.com/btcsuite/btcd/wire" +) + +// Bip69Less is taken from btcd in btcutil/txsort/txsort.go. +func Bip69Less(output1, output2 *wire.TxOut) bool { + if output1.Value == output2.Value { + return bytes.Compare(output1.PkScript, output2.PkScript) < 0 + } + return output1.Value < output2.Value +} From d00b78ef6be3105f1efb212a9abfaca0dae37d17 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Mon, 28 Jul 2025 12:20:11 +0200 Subject: [PATCH 8/8] sweepbatcher: consolidate identical change pkscripts batch separate change outputs with identical pkscripts are tallied up and consolidated to a single change output. --- sweepbatcher/greedy_batch_selection.go | 16 ++- sweepbatcher/greedy_batch_selection_test.go | 30 +++++ sweepbatcher/sweep_batch.go | 43 ++++-- sweepbatcher/sweep_batch_test.go | 113 ++++++++++++++++ sweepbatcher/sweep_batcher_presigned_test.go | 134 ++++++++++++++++++- 5 files changed, 324 insertions(+), 12 deletions(-) diff --git a/sweepbatcher/greedy_batch_selection.go b/sweepbatcher/greedy_batch_selection.go index 8b910dbcb..1ecda9b0b 100644 --- a/sweepbatcher/greedy_batch_selection.go +++ b/sweepbatcher/greedy_batch_selection.go @@ -210,11 +210,21 @@ func estimateBatchWeight(batch *batch) (feeDetails, error) { err) } - // Add change output weights. + // Add change output weights. Change outputs with identical pkscript + // will be consolidated into a single output. + changeOutputs := make(map[string]struct{}) for _, s := range batch.sweeps { - if s.change != nil { - weight.AddOutput(s.change.PkScript) + if s.change == nil { + continue } + + pkScriptString := string(s.change.PkScript) + if _, has := changeOutputs[pkScriptString]; has { + continue + } + + weight.AddOutput(s.change.PkScript) + changeOutputs[pkScriptString] = struct{}{} } // Add inputs. diff --git a/sweepbatcher/greedy_batch_selection_test.go b/sweepbatcher/greedy_batch_selection_test.go index 51dd70d29..97c087f18 100644 --- a/sweepbatcher/greedy_batch_selection_test.go +++ b/sweepbatcher/greedy_batch_selection_test.go @@ -36,6 +36,7 @@ const ( coopTwoSweepBatchWeight = coopNewBatchWeight + coopInputWeight coopSingleSweepChangeBatchWeight = coopInputWeight + batchOutputWeight + changeOutputWeight + coopDoubleSweepChangeBatchWeight = 2*coopInputWeight + batchOutputWeight + changeOutputWeight nonCoopTwoSweepBatchWeight = coopTwoSweepBatchWeight + 2*nonCoopPenalty v2v3BatchWeight = nonCoopTwoSweepBatchWeight - 25 mixedTwoSweepBatchWeight = coopTwoSweepBatchWeight + nonCoopPenalty @@ -325,6 +326,35 @@ func TestEstimateBatchWeight(t *testing.T) { }, }, + { + name: "two sweeps regular batch with identical change", + batch: &batch{ + id: 1, + rbfCache: rbfCache{ + FeeRate: lowFeeRate, + }, + sweeps: map[wire.OutPoint]sweep{ + outpoint1: { + htlcSuccessEstimator: se3, + change: &wire.TxOut{ + PkScript: changePkscript, + }, + }, + outpoint2: { + htlcSuccessEstimator: se3, + change: &wire.TxOut{ + PkScript: changePkscript, + }, + }, + }, + }, + wantBatchFeeDetails: feeDetails{ + BatchId: 1, + FeeRate: lowFeeRate, + Weight: coopDoubleSweepChangeBatchWeight, + }, + }, + { name: "two sweeps regular batch", batch: &batch{ diff --git a/sweepbatcher/sweep_batch.go b/sweepbatcher/sweep_batch.go index bebe2118e..47deb2be0 100644 --- a/sweepbatcher/sweep_batch.go +++ b/sweepbatcher/sweep_batch.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "math" + "sort" "strings" "sync" "sync/atomic" @@ -1311,10 +1312,22 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, LockTime: uint32(currentHeight), } - var changeOutputs []*wire.TxOut - for _, sweep := range sweeps { - if sweep.change != nil { - changeOutputs = append(changeOutputs, sweep.change) + // Consolidate change outputs with identical pkscript. + changeOutputs := make(map[string]*wire.TxOut) + for _, s := range sweeps { + if s.change == nil { + continue + } + + stringPkScript := string(s.change.PkScript) + if _, has := changeOutputs[stringPkScript]; has { + changeOutputs[stringPkScript].Value += s.change.Value + continue + } + + changeOutputs[stringPkScript] = &wire.TxOut{ + Value: s.change.Value, + PkScript: s.change.PkScript, } } @@ -1425,11 +1438,25 @@ func constructUnsignedTx(sweeps []sweep, address btcutil.Address, PkScript: batchPkScript, Value: int64(batchAmt-fee) - sumChange, }) - // Then add change outputs. - for _, txOut := range changeOutputs { + // Then add change outputs. Sort the keys first to make tests + // deterministic. + sortedChangeOutputs := make([]*wire.TxOut, 0, len(changeOutputs)) + for _, output := range changeOutputs { + sortedChangeOutputs = append(sortedChangeOutputs, output) + } + + // Sort the keys + sort.Slice(sortedChangeOutputs, func(i, j int) bool { + return utils.Bip69Less( + sortedChangeOutputs[i], sortedChangeOutputs[j], + ) + }) + + // Add change outputs orderly. + for _, output := range sortedChangeOutputs { batchTx.AddTxOut(&wire.TxOut{ - PkScript: txOut.PkScript, - Value: txOut.Value, + PkScript: output.PkScript, + Value: output.Value, }) } diff --git a/sweepbatcher/sweep_batch_test.go b/sweepbatcher/sweep_batch_test.go index 202e0a043..34cdb87e6 100644 --- a/sweepbatcher/sweep_batch_test.go +++ b/sweepbatcher/sweep_batch_test.go @@ -55,6 +55,17 @@ func TestConstructUnsignedTx(t *testing.T) { PkScript: change1Pkscript, } + change1PrimeAddr := "bc1pdx9ggvtjjcpaqfqk375qhdmzx9xu8dcu7w94lqfcxhh0rj" + + "lwyyeq5ryn6r" + change1PrimeAddress, err := btcutil.DecodeAddress(change1PrimeAddr, nil) + require.NoError(t, err) + change1PrimePkscript, err := txscript.PayToAddrScript(change1PrimeAddress) + require.NoError(t, err) + change1Prime := &wire.TxOut{ + Value: 200_000, + PkScript: change1PrimePkscript, + } + change2Addr := "bc1psw0nrrulq4pgyuyk09a3wsutygltys4gxjjw3zl2uz4ep8pa" + "r2vsvntfe0" change2Address, err := btcutil.DecodeAddress(change2Addr, nil) @@ -395,6 +406,108 @@ func TestConstructUnsignedTx(t *testing.T) { wantFee: 1248, }, + { + name: "identical change pkscripts", + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + }, + { + outpoint: op2, + value: 2_000_000, + change: change1, + }, + { + outpoint: op3, + value: 3_000_000, + change: change1, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op1, + }, + { + PreviousOutPoint: op2, + }, + { + PreviousOutPoint: op3, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 5_798_924, + PkScript: p2trPkScript, + }, + { + Value: 2 * change1.Value, + PkScript: change1.PkScript, + }, + }, + }, + wantWeight: 1076, + wantFeeForWeight: 1076, + wantFee: 1076, + }, + + { + name: "identical change pkscripts different values", + sweeps: []sweep{ + { + outpoint: op1, + value: 1_000_000, + }, + { + outpoint: op2, + value: 2_000_000, + change: change1, + }, + { + outpoint: op3, + value: 3_000_000, + change: change1Prime, + }, + }, + address: p2trAddress, + currentHeight: 800_000, + feeRate: 1000, + wantTx: &wire.MsgTx{ + Version: 2, + LockTime: 800_000, + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: op1, + }, + { + PreviousOutPoint: op2, + }, + { + PreviousOutPoint: op3, + }, + }, + TxOut: []*wire.TxOut{ + { + Value: 5_698_924, + PkScript: p2trPkScript, + }, + { + Value: change1.Value + change1Prime.Value, + PkScript: change1.PkScript, + }, + }, + }, + wantWeight: 1076, + wantFeeForWeight: 1076, + wantFee: 1076, + }, + { name: "change exceeds input value", sweeps: []sweep{ diff --git a/sweepbatcher/sweep_batcher_presigned_test.go b/sweepbatcher/sweep_batcher_presigned_test.go index 99a70fd4f..a38496279 100644 --- a/sweepbatcher/sweep_batcher_presigned_test.go +++ b/sweepbatcher/sweep_batcher_presigned_test.go @@ -1230,7 +1230,7 @@ func testPresigned_presigned_group_with_change(t *testing.T, presignedHelper.SetOutpointOnline(op1, true) presignedHelper.SetOutpointOnline(op2, true) - // An attempt to presign must fail. + // An attempt to presign shouldn't fail. err = batcher.PresignSweepsGroup( ctx, group1, sweepTimeout, destAddr, change, ) @@ -1268,6 +1268,134 @@ func testPresigned_presigned_group_with_change(t *testing.T, require.NoError(t, lnd.NotifyHeight(601)) } +// 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. +func testPresigned_presigned_group_with_identical_change_pkscript(t *testing.T, + batcherStore testBatcherStore) { + + defer test.Guard(t)() + + batchPkScript, err := txscript.PayToAddrScript(destAddr) + require.NoError(t, err) + + 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) + }() + + // Create two swaps of a single sweep + swapHash1 := lntypes.Hash{1, 1, 1} + swapHash2 := lntypes.Hash{2, 2, 2} + op1 := wire.OutPoint{ + Hash: chainhash.Hash{1, 1}, + Index: 1, + } + op2 := wire.OutPoint{ + Hash: chainhash.Hash{2, 2}, + Index: 2, + } + group1 := []Input{ + { + Outpoint: op1, + Value: 1_000_000, + }, + } + change1 := &wire.TxOut{ + Value: 500_000, + PkScript: []byte{0xaf, 0xfe}, + } + group2 := []Input{ + { + Outpoint: op2, + Value: 2_000_000, + }, + } + change2 := &wire.TxOut{ + Value: 600_000, + PkScript: []byte{0xaf, 0xfe}, + } + + presignedHelper.setChangeForPrimaryDeposit(op1, change1) + presignedHelper.setChangeForPrimaryDeposit(op2, change2) + + // Enable only one of the sweeps. + presignedHelper.SetOutpointOnline(op1, true) + presignedHelper.SetOutpointOnline(op2, true) + + // An attempt to presign group1 shouldn't fail. + err = batcher.PresignSweepsGroup( + ctx, group1, sweepTimeout, destAddr, change1, + ) + require.NoError(t, err) + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash1, + Inputs: group1, + Notifier: &dummyNotifier, + }) + require.NoError(t, err) + + // Since a batch was created we check that it registered for its primary + // sweep's spend. + <-lnd.RegisterSpendChannel + + // An attempt to presign group2 shouldn't fail. + err = batcher.PresignSweepsGroup( + ctx, group2, sweepTimeout, destAddr, change2, + ) + require.NoError(t, err) + + // Add the sweep, triggering the publishing attempt. + err = batcher.AddSweep(ctx, &SweepRequest{ + SwapHash: swapHash2, + Inputs: group2, + Notifier: &dummyNotifier, + }) + require.NoError(t, err) + + // Wait for a transactions to be published. + tx := <-lnd.TxPublishChannel + require.Len(t, tx.TxIn, 2) + require.Len(t, tx.TxOut, 2) + require.ElementsMatch( + t, []wire.OutPoint{op1, op2}, + []wire.OutPoint{ + tx.TxIn[0].PreviousOutPoint, + tx.TxIn[1].PreviousOutPoint, + }, + ) + require.Equal(t, int64(1_893_300), tx.TxOut[0].Value) + require.Equal(t, change1.Value+change2.Value, tx.TxOut[1].Value) + require.Equal(t, batchPkScript, tx.TxOut[0].PkScript) + require.Equal(t, change1.PkScript, tx.TxOut[1].PkScript) + + // Mine a blocks to trigger republishing. + require.NoError(t, lnd.NotifyHeight(601)) +} + // testPresigned_presigned_group_with_dust_main_output passes a dust main output // and a change output to PresignSweepsGroup. It will fail because of the dust // main output. Note that the min relay fee is set low enough to pass the @@ -2224,6 +2352,10 @@ func TestPresigned(t *testing.T) { testPresigned_presigned_group_with_change(t, NewStoreMock()) }) + t.Run("identical change pkscript", func(t *testing.T) { + testPresigned_presigned_group_with_identical_change_pkscript(t, NewStoreMock()) + }) + t.Run("dust_main_output", func(t *testing.T) { testPresigned_presigned_group_with_dust_main_output( t, NewStoreMock(),