Merge pull request #10189 from ziggie1984/fix-fee-estimation

bugfix error matching sweeper

chainntnfs/txnotifier.go

@@ -356,11 +356,22 @@ func NewSpendRequest(op *wire.OutPoint, pkScript []byte) (SpendRequest, error) {
 
 // String returns the string representation of the SpendRequest.
 func (r SpendRequest) String() string {
-	if r.OutPoint != ZeroOutPoint {
-		return fmt.Sprintf("outpoint=%v, script=%v", r.OutPoint,
-			r.PkScript)
+	var (
+		outpointStr = fmt.Sprintf("%v", r.OutPoint)
+		scriptStr   = fmt.Sprintf("%v", r.PkScript)
+	)
+
+	if r.OutPoint == ZeroOutPoint {
+		outpointStr = "<zero>"
 	}
-	return fmt.Sprintf("outpoint=<zero>, script=%v", r.PkScript)
+
+	// If the pk script is all zeros, we blank the pk script.
+	// Currently we do not support taproot pk scripts for notifications.
+	if r.PkScript == ZeroTaprootPkScript {
+		scriptStr = "<zero> (taproot pk script not supported)"
+	}
+
+	return fmt.Sprintf("outpoint=%s, script=%s", outpointStr, scriptStr)
 }
 
 // MatchesTx determines whether the given transaction satisfies the spend

config_builder.go

@@ -1807,8 +1807,11 @@ func broadcastErrorMapper(err error) error {
 	// in the first place are rebroadcasted despite of their backend error.
 	// Mempool conditions change over time so it makes sense to retry
 	// publishing the transaction. Moreover we log the detailed error so the
-	// user can intervene and increase the size of his mempool.
-	case errors.Is(err, chain.ErrMempoolMinFeeNotMet):
+	// user can intervene and increase the size of his mempool or increase
+	// his min relay fee configuration.
+	case errors.Is(err, chain.ErrMempoolMinFeeNotMet),
+		errors.Is(err, chain.ErrMinRelayFeeNotMet):
+
 		ltndLog.Warnf("Error while broadcasting transaction: %v", err)
 
 		returnErr = &pushtx.BroadcastError{

docs/release-notes/release-notes-0.20.0.md

@@ -42,6 +42,10 @@
   send unnecessary `channel_announcement` and `node_announcement` messages when
   replying to a `gossip_timestamp_filter` query.
 
+- [Fixed](https://github.com/lightningnetwork/lnd/pull/10189) a case in the
+  sweeper where some outputs would not be resolved due to an error string
+  mismatch.
+
 # New Features
  
 * Use persisted [nodeannouncement](https://github.com/lightningnetwork/lnd/pull/8825) 

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0
 	github.com/btcsuite/btclog v0.0.0-20241003133417-09c4e92e319c
 	github.com/btcsuite/btclog/v2 v2.0.1-0.20250728225537-6090e87c6c5b
-	github.com/btcsuite/btcwallet v0.16.15-0.20250811092146-05b3a40651e6
+	github.com/btcsuite/btcwallet v0.16.17
 	github.com/btcsuite/btcwallet/wallet/txauthor v1.3.5
 	github.com/btcsuite/btcwallet/wallet/txrules v1.2.2
 	github.com/btcsuite/btcwallet/walletdb v1.5.1

go.sum

@@ -64,8 +64,8 @@ github.com/btcsuite/btclog v0.0.0-20241003133417-09c4e92e319c/go.mod h1:w7xnGOhw
 github.com/btcsuite/btclog/v2 v2.0.1-0.20250728225537-6090e87c6c5b h1:MQ+Q6sDy37V1wP1Yu79A5KqJutolqUGwA99UZWQDWZM=
 github.com/btcsuite/btclog/v2 v2.0.1-0.20250728225537-6090e87c6c5b/go.mod h1:XItGUfVOxotJL8kkuk2Hj3EVow5KCugXl3wWfQ6K0AE=
 github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
-github.com/btcsuite/btcwallet v0.16.15-0.20250811092146-05b3a40651e6 h1:s6NCipDdvDK5rBrC4dIlni1iHsuDOKdfwpL32I3b6Tw=
-github.com/btcsuite/btcwallet v0.16.15-0.20250811092146-05b3a40651e6/go.mod h1:H6dfoZcWPonM2wbVsR2ZBY0PKNZKdQyLAmnX8vL9JFA=
+github.com/btcsuite/btcwallet v0.16.17 h1:1N6lHznRdcjDopBvcofxaIHknArkJ/EcVKgLKfGL4Dg=
+github.com/btcsuite/btcwallet v0.16.17/go.mod h1:YO+W745BAH8n/Rpgj68QsLR6eLlgM4W2do4RejT0buo=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.5 h1:Rr0njWI3r341nhSPesKQ2JF+ugDSzdPoeckS75SeDZk=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.5/go.mod h1:+tXJ3Ym0nlQc/iHSwW1qzjmPs3ev+UVWMbGgfV1OZqU=
 github.com/btcsuite/btcwallet/wallet/txrules v1.2.2 h1:YEO+Lx1ZJJAtdRrjuhXjWrYsmAk26wLTlNzxt2q0lhk=

lnwallet/btcwallet/btcwallet.go

@@ -1133,7 +1133,9 @@ func mapRpcclientError(err error) error {
 	// If the wallet reports that fee requirements for accepting the tx
 	// into mempool are not met, convert it to our internal ErrMempoolFee
 	// and return.
-	case errors.Is(err, chain.ErrMempoolMinFeeNotMet):
+	case errors.Is(err, chain.ErrMempoolMinFeeNotMet),
+		errors.Is(err, chain.ErrMinRelayFeeNotMet):
+
 		return fmt.Errorf("%w: %v", lnwallet.ErrMempoolFee, err.Error())
 	}
 

sweep/fee_bumper.go

@@ -565,7 +565,10 @@ func (t *TxPublisher) createRBFCompliantTx(
 
 		// If the error indicates the fees paid is not enough, we will
 		// ask the fee function to increase the fee rate and retry.
-		case errors.Is(err, lnwallet.ErrMempoolFee):
+		case errors.Is(err, lnwallet.ErrMempoolFee),
+			errors.Is(err, chain.ErrMinRelayFeeNotMet),
+			errors.Is(err, chain.ErrMempoolMinFeeNotMet):
+
 			// We should at least start with a feerate above the
 			// mempool min feerate, so if we get this error, it
 			// means something is wrong earlier in the pipeline.
@@ -574,7 +577,8 @@ func (t *TxPublisher) createRBFCompliantTx(
 
 			fallthrough
 
-		// We are not paying enough fees so we increase it.
+		// We are not paying enough fees to RBF a previous tx, so we
+		// increase it.
 		case errors.Is(err, chain.ErrInsufficientFee):
 			increased := false
 
@@ -1668,6 +1672,16 @@ func prepareSweepTx(inputs []input.Input, changePkScript lnwallet.AddrWithKey,
 		return 0, noChange, noLocktime, err
 	}
 
+	// We also add the extra change output to the change pk scripts.
+	//
+	// NOTE: The weight estimation will not be quite accurate because the
+	// witness data is greater when overlay channels are used. But that
+	// shouldn't be a problem since we will increase the fee rate
+	// incrementally via the fee function.
+	extraChangeOut.WhenSome(func(o SweepOutput) {
+		changePkScripts = append(changePkScripts, o.TxOut.PkScript)
+	})
+
 	// Creating a weight estimator with nil outputs and zero max fee rate.
 	// We don't allow adding customized outputs in the sweeping tx, and the
 	// fee rate is already being managed before we get here.

sweep/fee_bumper_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/fn/v2"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/keychain"
+	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/stretchr/testify/mock"
@@ -2109,3 +2110,200 @@ func createTestSpendEvent(tx *wire.MsgTx) *chainntnfs.SpendEvent {
 		Cancel: func() {},
 	}
 }
+
+// TestPrepareSweepTx tests the prepareSweepTx function behavior.
+func TestPrepareSweepTx(t *testing.T) {
+	t.Parallel()
+
+	// Create test inputs with different values.
+	inp1 := createTestInput(1000000, input.WitnessKeyHash)
+	inp2 := createTestInput(2000000, input.WitnessKeyHash)
+
+	// Test fee rate and height.
+	feeRate := chainfee.SatPerKWeight(1000)
+	currentHeight := int32(800000)
+
+	testCases := []struct {
+		name           string
+		inputs         []input.Input
+		changePkScript lnwallet.AddrWithKey
+		feeRate        chainfee.SatPerKWeight
+		currentHeight  int32
+		auxSweeper     fn.Option[AuxSweeper]
+		expectedErr    error
+		checkResults   func(t *testing.T, fee btcutil.Amount,
+			changeOuts fn.Option[[]SweepOutput],
+			locktime fn.Option[int32])
+	}{
+		{
+			name: "successful sweep with change - no " +
+				"extra output",
+			inputs:         []input.Input{&inp1, &inp2},
+			changePkScript: changePkScript,
+			feeRate:        feeRate,
+			currentHeight:  currentHeight,
+			auxSweeper:     fn.None[AuxSweeper](),
+			expectedErr:    nil,
+			checkResults: func(t *testing.T, fee btcutil.Amount,
+				changeOuts fn.Option[[]SweepOutput],
+				locktime fn.Option[int32]) {
+
+				// Calculate expected weight - only regular
+				// change output, no extra.
+				expectedWeight, err := calcSweepTxWeight(
+					[]input.Input{&inp1, &inp2},
+					[][]byte{
+						changePkScript.DeliveryAddress,
+					},
+				)
+				require.NoError(t, err)
+
+				// Expected fee based on fee rate and weight.
+				expectedFee := feeRate.FeeForWeight(
+					expectedWeight,
+				)
+
+				require.Equal(t, fee, expectedFee)
+			},
+		},
+		{
+			name:           "successful sweep with extra output",
+			inputs:         []input.Input{&inp1, &inp2},
+			changePkScript: changePkScript,
+			feeRate:        feeRate,
+			currentHeight:  currentHeight,
+			auxSweeper:     fn.Some[AuxSweeper](&MockAuxSweeper{}),
+			expectedErr:    nil,
+			checkResults: func(t *testing.T, fee btcutil.Amount,
+				changeOuts fn.Option[[]SweepOutput],
+				locktime fn.Option[int32]) {
+
+				// Calculate expected weight - includes both
+				// regular change and extra output.
+				expectedWeight, err := calcSweepTxWeight(
+					[]input.Input{&inp1, &inp2},
+					[][]byte{changePkScript.DeliveryAddress,
+						changePkScript.DeliveryAddress},
+				)
+				require.NoError(t, err)
+
+				// Expected fee based on fee rate and weight.
+				expectedFee := feeRate.FeeForWeight(
+					expectedWeight,
+				)
+
+				require.Equal(t, fee, expectedFee)
+
+				// Should have change outputs (both regular
+				// and extra).
+				require.True(t, changeOuts.IsSome())
+				outputs := changeOuts.UnwrapOr([]SweepOutput{})
+				require.Equal(t, 2, len(outputs))
+
+				// Check if extra output is present.
+				hasExtra := false
+				for _, out := range outputs {
+					if out.IsExtra {
+						hasExtra = true
+						break
+					}
+				}
+				require.True(
+					t, hasExtra, "Should have extra output",
+				)
+
+				// Locktime should be None since no inputs
+				// require locktime.
+				require.True(t, locktime.IsNone())
+			},
+		},
+		{
+			name:           "insufficient inputs",
+			inputs:         []input.Input{},
+			changePkScript: changePkScript,
+			feeRate:        feeRate,
+			currentHeight:  currentHeight,
+			auxSweeper:     fn.None[AuxSweeper](),
+			expectedErr:    ErrNotEnoughInputs,
+		},
+		{
+			name: "high fee rate causes insufficient " +
+				"inputs",
+			inputs:         []input.Input{&inp1},
+			changePkScript: changePkScript,
+			feeRate:        chainfee.SatPerKWeight(10000000),
+			currentHeight:  currentHeight,
+			auxSweeper:     fn.None[AuxSweeper](),
+			expectedErr:    ErrNotEnoughInputs,
+		},
+		{
+			name: "immature locktime",
+			inputs: []input.Input{
+				createTestInputWithLocktime(
+					1000000, input.WitnessKeyHash,
+					uint32(currentHeight+100),
+				),
+			},
+			changePkScript: changePkScript,
+			feeRate:        feeRate,
+			currentHeight:  currentHeight,
+			auxSweeper:     fn.None[AuxSweeper](),
+			expectedErr:    ErrLocktimeImmature,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			fee, changeOuts, locktime, err := prepareSweepTx(
+				tc.inputs,
+				tc.changePkScript,
+				tc.feeRate,
+				tc.currentHeight,
+				tc.auxSweeper,
+			)
+
+			// Check error expectations.
+			if tc.expectedErr != nil {
+				require.ErrorIs(t, err, tc.expectedErr)
+				return
+			}
+
+			// For successful cases, run additional checks.
+			require.NoError(t, err)
+			if tc.checkResults != nil {
+				tc.checkResults(t, fee, changeOuts, locktime)
+			}
+		})
+	}
+}
+
+// createTestInputWithLocktime creates a test input with a specific locktime
+// requirement.
+func createTestInputWithLocktime(value int64, witnessType input.WitnessType,
+	locktime uint32) *input.BaseInput {
+
+	// Create a unique test identifier based on input count.
+	hash := chainhash.Hash{}
+	hash[lntypes.HashSize-1] = byte(testInputCount.Add(1))
+
+	// Use NewCsvInputWithCltv to create an input with locktime requirement.
+	inp := input.NewCsvInputWithCltv(
+		&wire.OutPoint{
+			Hash: hash,
+		},
+		witnessType,
+		&input.SignDescriptor{
+			Output: &wire.TxOut{
+				Value: value,
+			},
+			KeyDesc: keychain.KeyDescriptor{
+				PubKey: testPubKey,
+			},
+		},
+		1, 0, locktime,
+	)
+
+	return inp
+}

sweep/mock_test.go

@@ -338,7 +338,7 @@ func (m *MockAuxSweeper) DeriveSweepAddr(_ []input.Input,
 			Value:    123,
 			PkScript: changePkScript.DeliveryAddress,
 		},
-		IsExtra:     false,
+		IsExtra:     true,
 		InternalKey: fn.None[keychain.KeyDescriptor](),
 	})
 }