diff --git a/config_builder.go b/config_builder.go index 1cbf17348..1827587e8 100644 --- a/config_builder.go +++ b/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 +} diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index eeb8850f7..b19bae7fe 100644 --- a/contractcourt/channel_arbitrator.go +++ b/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 } } diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 3cda7a666..48aea1a12 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/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( diff --git a/contractcourt/utxonursery.go b/contractcourt/utxonursery.go index 3776a3546..a763679ed 100644 --- a/contractcourt/utxonursery.go +++ b/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 diff --git a/contractcourt/utxonursery_test.go b/contractcourt/utxonursery_test.go index 3f5026ddc..05cda32ca 100644 --- a/contractcourt/utxonursery_test.go +++ b/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) { diff --git a/docs/release-notes/release-notes-0.17.0.md b/docs/release-notes/release-notes-0.17.0.md index d14f0378a..272585947 100644 --- a/docs/release-notes/release-notes-0.17.0.md +++ b/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. diff --git a/go.mod b/go.mod index b72c58ed3..2e696353b 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 50e166e2d..9419edfc1 100644 --- a/go.sum +++ b/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.20230621165747-9c21f464ce13/go.mod h1:Hl4PP/tSNcgN6himfx/020mYSa19a1qkqTuqQBUU97w= +github.com/btcsuite/btcwallet v0.16.10-0.20230706223227-037580c66b74 h1:RanL5kPm5Gi9YPf+3aPfdd6gJuohoJMxcDv8R033RIo= +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.0/go.mod h1:pmjwElN/091TErtSE9Vd5W4hpxoG2/+xlb+HoPm9Gug= +github.com/lightninglabs/neutrino v0.15.1-0.20230710222839-9fd0fc551366 h1:++HuI+fNJ61HWobNkj0BvFs477R2Ar3TJABI0gendI8= +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= diff --git a/lnwallet/btcwallet/btcwallet.go b/lnwallet/btcwallet/btcwallet.go index 3050be03a..1535d434b 100644 --- a/lnwallet/btcwallet/btcwallet.go +++ b/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 -// network (either in the mempool or chain) no error will be returned. +// and mapped to the wallet's internal error types. If the transaction is +// 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 } diff --git a/lnwallet/interface.go b/lnwallet/interface.go index f27e6345d..d8dc9d5b8 100644 --- a/lnwallet/interface.go +++ b/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