Merge pull request #7746 from ziggie1984/min-relay-fee

Handle Min Mempool Fee error so that lnd will startup correctly

config_builder.go

@@ -3,6 +3,7 @@ package lnd
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -719,6 +720,10 @@ func (d *DefaultWalletImpl) BuildChainControl(
 				partialChainControl.ChainNotifier,
 			),
 			RebroadcastInterval: pushtx.DefaultRebroadcastInterval,
+			// In case the backend is different from neutrino we
+			// make sure that broadcast backend errors are mapped
+			// to the neutrino broadcastErr.
+			MapCustomBroadcastError: broadcastErrorMapper,
 		}
 
 		lnWalletConfig.Rebroadcaster = newWalletReBroadcaster(
@@ -1420,3 +1425,50 @@ func parseHeaderStateAssertion(state string) (*headerfs.FilterHeader, error) {
 		FilterHash: *hash,
 	}, nil
 }
+
+// broadcastErrorMapper maps errors from bitcoin backends other than neutrino to
+// the neutrino BroadcastError which allows the Rebroadcaster which currently
+// resides in the neutrino package to use all of its functionalities.
+func broadcastErrorMapper(err error) error {
+	returnErr := wallet.MapBroadcastBackendError(err)
+
+	// We only filter for specific backend errors which are relevant for the
+	// Rebroadcaster.
+	var errAlreadyConfirmed *wallet.ErrAlreadyConfirmed
+	var errInMempool *wallet.ErrInMempool
+	var errMempoolFee *wallet.ErrMempoolFee
+
+	switch {
+	// This makes sure the tx is removed from the rebroadcaster once it is
+	// confirmed.
+	case errors.As(returnErr, &errAlreadyConfirmed):
+		returnErr = &pushtx.BroadcastError{
+			Code:   pushtx.Confirmed,
+			Reason: returnErr.Error(),
+		}
+
+	// Transactions which are still in mempool but might fall out because
+	// of low fees are rebroadcasted despite of their backend error.
+	case errors.As(returnErr, &errInMempool):
+		returnErr = &pushtx.BroadcastError{
+			Code:   pushtx.Mempool,
+			Reason: returnErr.Error(),
+		}
+
+	// Transactions which are not accepted into mempool because of low fees
+	// 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.As(returnErr, &errMempoolFee):
+		ltndLog.Warnf("Error while broadcasting transaction: %v",
+			returnErr)
+
+		returnErr = &pushtx.BroadcastError{
+			Code:   pushtx.Mempool,
+			Reason: returnErr.Error(),
+		}
+	}
+
+	return returnErr
+}

contractcourt/channel_arbitrator.go

@@ -553,7 +553,7 @@ func (c *ChannelArbitrator) Start(state *chanArbStartState) error {
 		// StateWaitingFullResolution after we've transitioned from
 		// StateContractClosed which can only be triggered by the local
 		// or remote close trigger. This trigger is only fired when we
-		// receive a chain event from the chain watcher than the
+		// receive a chain event from the chain watcher that the
 		// commitment has been confirmed on chain, and before we
 		// advance our state step, we call InsertConfirmedCommitSet.
 		err := c.relaunchResolvers(state.commitSet, triggerHeight)
@@ -990,11 +990,18 @@ func (c *ChannelArbitrator) stateStep(
 		label := labels.MakeLabel(
 			labels.LabelTypeChannelClose, &c.cfg.ShortChanID,
 		)
-
 		if err := c.cfg.PublishTx(closeTx, label); err != nil {
 			log.Errorf("ChannelArbitrator(%v): unable to broadcast "+
 				"close tx: %v", c.cfg.ChanPoint, err)
-			if err != lnwallet.ErrDoubleSpend {
+
+			// This makes sure we don't fail at startup if the
+			// commitment transaction has too low fees to make it
+			// into mempool. The rebroadcaster makes sure this
+			// transaction is republished regularly until confirmed
+			// or replaced.
+			if !errors.Is(err, lnwallet.ErrDoubleSpend) &&
+				!errors.Is(err, lnwallet.ErrMempoolFee) {
+
 				return StateError, closeTx, err
 			}
 		}

contractcourt/channel_arbitrator_test.go

@@ -2686,6 +2686,91 @@ func TestChannelArbitratorAnchors(t *testing.T) {
 	)
 }
 
+// TestChannelArbitratorStartAfterCommitmentRejected tests that when we run into
+// the case where our commitment tx is rejected by our bitcoin backend we still
+// continue to startup the arbitrator for a specific set of errors.
+func TestChannelArbitratorStartAfterCommitmentRejected(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+
+		// The specific error during broadcasting the transaction.
+		broadcastErr error
+
+		// expected state when the startup of the arbitrator succeeds.
+		expectedState ArbitratorState
+
+		expectedStartup bool
+	}{
+		{
+			name: "Commitment is rejected because of low mempool " +
+				"fees",
+			broadcastErr:    lnwallet.ErrMempoolFee,
+			expectedState:   StateCommitmentBroadcasted,
+			expectedStartup: true,
+		},
+		{
+			// We map a rejected rbf transaction to ErrDoubleSpend
+			// in lnd.
+			name: "Commitment is rejected because of a " +
+				"rbf transaction not succeeding",
+			broadcastErr:    lnwallet.ErrDoubleSpend,
+			expectedState:   StateCommitmentBroadcasted,
+			expectedStartup: true,
+		},
+		{
+			name: "Commitment is rejected with an " +
+				"unmatched error",
+			broadcastErr:    fmt.Errorf("Reject Commitment Tx"),
+			expectedState:   StateBroadcastCommit,
+			expectedStartup: false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			// We'll create the arbitrator and its backing log
+			// to signal that it's already in the process of being
+			// force closed.
+			log := &mockArbitratorLog{
+				newStates: make(chan ArbitratorState, 5),
+				state:     StateBroadcastCommit,
+			}
+			chanArbCtx, err := createTestChannelArbitrator(t, log)
+			require.NoError(t, err, "unable to create "+
+				"ChannelArbitrator")
+
+			chanArb := chanArbCtx.chanArb
+
+			// Customize the PublishTx function of the arbitrator.
+			chanArb.cfg.PublishTx = func(*wire.MsgTx,
+				string) error {
+
+				return test.broadcastErr
+			}
+			err = chanArb.Start(nil)
+			if !test.expectedStartup {
+				require.ErrorIs(t, err, test.broadcastErr)
+				return
+			}
+			require.NoError(t, err)
+
+			t.Cleanup(func() {
+				require.NoError(t, chanArb.Stop())
+			})
+
+			// In case the startup succeeds we check that the state
+			// is as expected.
+			chanArbCtx.AssertStateTransitions(test.expectedState)
+		})
+	}
+}
+
 // putResolverReportInChannel returns a put report function which will pipe
 // reports into the channel provided.
 func putResolverReportInChannel(reports chan *channeldb.ResolverReport) func(

contractcourt/utxonursery.go

@@ -3,6 +3,7 @@ package contractcourt
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"sync"
@@ -872,7 +873,13 @@ func (u *UtxoNursery) sweepCribOutput(classHeight uint32, baby *babyOutput) erro
 	// confirmed before transitioning it to kindergarten.
 	label := labels.MakeLabel(labels.LabelTypeSweepTransaction, nil)
 	err := u.cfg.PublishTransaction(baby.timeoutTx, label)
-	if err != nil && err != lnwallet.ErrDoubleSpend {
+
+	// In case the tx does not meet mempool fee requirements we continue
+	// because the tx is rebroadcasted in the background and there is
+	// nothing we can do to bump this transaction anyways.
+	if err != nil && !errors.Is(err, lnwallet.ErrDoubleSpend) &&
+		!errors.Is(err, lnwallet.ErrMempoolFee) {
+
 		utxnLog.Errorf("Unable to broadcast baby tx: "+
 			"%v, %v", err, spew.Sdump(baby.timeoutTx))
 		return err

contractcourt/utxonursery_test.go

@@ -504,8 +504,7 @@ func createNurseryTestContext(t *testing.T,
 			/// Restart nursery.
 			nurseryCfg.SweepInput = ctx.sweeper.sweepInput
 			ctx.nursery = NewUtxoNursery(&nurseryCfg)
-			ctx.nursery.Start()
+			require.NoError(t, ctx.nursery.Start())
-
 		})
 	}
 
@@ -646,6 +645,109 @@ func incubateTestOutput(t *testing.T, nursery *UtxoNursery,
 	return outgoingRes
 }
 
+// TestRejectedCribTransaction makes sure that our nursery does not fail to
+// start up in case a Crib transaction (htlc-timeout) is rejected by the
+// bitcoin backend for some excepted reasons.
+func TestRejectedCribTransaction(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+
+		// The specific error during broadcasting the transaction.
+		broadcastErr error
+
+		// expectErr specifies whether the rejection of the transaction
+		// fails the nursery engine.
+		expectErr bool
+	}{
+		{
+			name: "Crib tx is rejected because of low mempool " +
+				"fees",
+			broadcastErr: lnwallet.ErrMempoolFee,
+		},
+		{
+			// We map a rejected rbf transaction to ErrDoubleSpend
+			// in lnd.
+			name: "Crib tx is rejected because of a " +
+				"rbf transaction not succeeding",
+			broadcastErr: lnwallet.ErrDoubleSpend,
+		},
+		{
+			name: "Crib tx is rejected with an " +
+				"unmatched error",
+			broadcastErr: fmt.Errorf("Reject Commitment Tx"),
+			expectErr:    true,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			// The checkStartStop function just calls the callback
+			// here to make sure the restart routine works
+			// correctly.
+			ctx := createNurseryTestContext(t,
+				func(callback func()) bool {
+					callback()
+					return true
+				})
+
+			outgoingRes := createOutgoingRes(true)
+
+			ctx.nursery.cfg.PublishTransaction =
+				func(tx *wire.MsgTx, source string) error {
+					log.Tracef("Publishing tx %v "+
+						"by %v", tx.TxHash(), source)
+					return test.broadcastErr
+				}
+			ctx.notifyEpoch(125)
+
+			// Hand off to nursery.
+			err := ctx.nursery.IncubateOutputs(
+				testChanPoint,
+				[]lnwallet.OutgoingHtlcResolution{*outgoingRes},
+				nil, 0,
+			)
+			if test.expectErr {
+				require.ErrorIs(t, err, test.broadcastErr)
+				return
+			}
+			require.NoError(t, err)
+
+			// Make sure that a restart is not affected by the
+			// rejected Crib transaction.
+			ctx.restart()
+
+			// Confirm the timeout tx. This should promote the
+			// HTLC to KNDR state.
+			timeoutTxHash := outgoingRes.SignedTimeoutTx.TxHash()
+			err = ctx.notifier.ConfirmTx(&timeoutTxHash, 126)
+			require.NoError(t, err)
+
+			// Wait for output to be promoted in store to KNDR.
+			select {
+			case <-ctx.store.cribToKinderChan:
+			case <-time.After(defaultTestTimeout):
+				t.Fatalf("output not promoted to KNDR")
+			}
+
+			// Notify arrival of block where second level HTLC
+			// unlocks.
+			ctx.notifyEpoch(128)
+
+			// Check final sweep into wallet.
+			testSweepHtlc(t, ctx)
+
+			// Cleanup utxonursery.
+			ctx.finish()
+		})
+	}
+}
+
 func assertNurseryReport(t *testing.T, nursery *UtxoNursery,
 	expectedNofHtlcs int, expectedStage uint32,
 	expectedLimboBalance btcutil.Amount) {

docs/release-notes/release-notes-0.17.0.md

@@ -177,6 +177,11 @@ unlock or create.
 
 * Fixed a memory leak found in mempool management handled by
   [`btcwallet`](https://github.com/lightningnetwork/lnd/pull/7767).
+  
+* Make sure lnd starts up as normal in case a transaction does not meet min
+  mempool fee requirements. [Handle min mempool fee backend error when a
+  transaction fails to be broadcasted by the 
+  bitcoind backend](https://github.com/lightningnetwork/lnd/pull/7746)
 
 * [Updated bbolt to v1.3.7](https://github.com/lightningnetwork/lnd/pull/7796)
   in order to address mmap issues affecting certain older iPhone devices.

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/btcsuite/btcd/btcutil/psbt v1.1.8
 	github.com/btcsuite/btcd/chaincfg/chainhash v1.0.2
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
-	github.com/btcsuite/btcwallet v0.16.10-0.20230621165747-9c21f464ce13
+	github.com/btcsuite/btcwallet v0.16.10-0.20230706223227-037580c66b74
 	github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2
 	github.com/btcsuite/btcwallet/wallet/txrules v1.2.0
 	github.com/btcsuite/btcwallet/walletdb v1.4.0
@@ -29,7 +29,7 @@ require (
 	github.com/jessevdk/go-flags v1.4.0
 	github.com/jrick/logrotate v1.0.0
 	github.com/kkdai/bstream v1.0.0
-	github.com/lightninglabs/neutrino v0.15.0
+	github.com/lightninglabs/neutrino v0.15.1-0.20230710222839-9fd0fc551366
 	github.com/lightninglabs/neutrino/cache v1.1.1
 	github.com/lightningnetwork/lightning-onion v1.2.1-0.20221202012345-ca23184850a1
 	github.com/lightningnetwork/lnd/cert v1.2.1

go.sum

@@ -91,8 +91,8 @@ github.com/btcsuite/btcd/chaincfg/chainhash v1.0.2/go.mod h1:7SFka0XMvUgj3hfZtyd
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
 github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
-github.com/btcsuite/btcwallet v0.16.10-0.20230621165747-9c21f464ce13 h1:7i0CzK+PP4+Dth9ia/eIBFRw8+K6MT8MfoFqBH43Xts=
+github.com/btcsuite/btcwallet v0.16.10-0.20230706223227-037580c66b74 h1:RanL5kPm5Gi9YPf+3aPfdd6gJuohoJMxcDv8R033RIo=
-github.com/btcsuite/btcwallet v0.16.10-0.20230621165747-9c21f464ce13/go.mod h1:Hl4PP/tSNcgN6himfx/020mYSa19a1qkqTuqQBUU97w=
+github.com/btcsuite/btcwallet v0.16.10-0.20230706223227-037580c66b74/go.mod h1:Hl4PP/tSNcgN6himfx/020mYSa19a1qkqTuqQBUU97w=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 h1:etuLgGEojecsDOYTII8rYiGHjGyV5xTqsXi+ZQ715UU=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2/go.mod h1:Zpk/LOb2sKqwP2lmHjaZT9AdaKsHPSbNLm2Uql5IQ/0=
 github.com/btcsuite/btcwallet/wallet/txrules v1.2.0 h1:BtEN5Empw62/RVnZ0VcJaVtVlBijnLlJY+dwjAye2Bg=
@@ -390,8 +390,8 @@ github.com/lib/pq v1.10.3 h1:v9QZf2Sn6AmjXtQeFpdoq/eaNtYP6IN+7lcrygsIAtg=
 github.com/lib/pq v1.10.3/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf h1:HZKvJUHlcXI/f/O0Avg7t8sqkPo78HFzjmeYFl6DPnc=
 github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf/go.mod h1:vxmQPeIQxPf6Jf9rM8R+B4rKBqLA2AjttNxkFBL2Plk=
-github.com/lightninglabs/neutrino v0.15.0 h1:yr3uz36fLAq8hyM0TRUVlef1TRNoWAqpmmNlVtKUDtI=
+github.com/lightninglabs/neutrino v0.15.1-0.20230710222839-9fd0fc551366 h1:++HuI+fNJ61HWobNkj0BvFs477R2Ar3TJABI0gendI8=
-github.com/lightninglabs/neutrino v0.15.0/go.mod h1:pmjwElN/091TErtSE9Vd5W4hpxoG2/+xlb+HoPm9Gug=
+github.com/lightninglabs/neutrino v0.15.1-0.20230710222839-9fd0fc551366/go.mod h1:pmjwElN/091TErtSE9Vd5W4hpxoG2/+xlb+HoPm9Gug=
 github.com/lightninglabs/neutrino/cache v1.1.1 h1:TllWOSlkABhpgbWJfzsrdUaDH2fBy/54VSIB4vVqV8M=
 github.com/lightninglabs/neutrino/cache v1.1.1/go.mod h1:XJNcgdOw1LQnanGjw8Vj44CvguYA25IMKjWFZczwZuo=
 github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display h1:pRdza2wleRN1L2fJXd6ZoQ9ZegVFTAb2bOQfruJPKcY=

lnwallet/btcwallet/btcwallet.go

@@ -1192,8 +1192,9 @@ func (b *BtcWallet) ListUnspentWitness(minConfs, maxConfs int32,
 // PublishTransaction performs cursory validation (dust checks, etc), then
 // finally broadcasts the passed transaction to the Bitcoin network. If
 // publishing the transaction fails, an error describing the reason is returned
-// (currently ErrDoubleSpend). If the transaction is already published to the
+// and mapped to the wallet's internal error types. If the transaction is
-// network (either in the mempool or chain) no error will be returned.
+// already published to the network (either in the mempool or chain) no error
+// will be returned.
 func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 	if err := b.wallet.PublishTransaction(tx, label); err != nil {
 		// If we failed to publish the transaction, check whether we
@@ -1210,6 +1211,13 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 		case *base.ErrReplacement:
 			return lnwallet.ErrDoubleSpend
 
+		// 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 *base.ErrMempoolFee:
+			return fmt.Errorf("%w: %v", lnwallet.ErrMempoolFee,
+				err.Error())
+
 		default:
 			return err
 		}

lnwallet/interface.go

@@ -69,6 +69,12 @@ var (
 	// ErrNotMine is an error denoting that a WalletController instance is
 	// unable to spend a specified output.
 	ErrNotMine = errors.New("the passed output doesn't belong to the wallet")
+
+	// ErrMempoolFee is returned from PublishTransaction in case the tx
+	// being published is not accepted into mempool because the fee
+	// requirements of the mempool backend are not met.
+	ErrMempoolFee = errors.New("transaction rejected by the mempool " +
+		"because of low fees")
 )
 
 // ErrNoOutputs is returned if we try to create a transaction with no outputs