Merge pull request #8554 from yyforyongyu/use-new-errors

lnwallet: use new errors returned from `rpcclient`

.github/workflows/main.yml

@@ -218,7 +218,7 @@ jobs:
         run: ./scripts/install_bitcoind.sh $BITCOIN_VERSION
 
       - name: run ${{ matrix.unit_type }}
-        run: make log="stdlog debug" ${{ matrix.unit_type }}
+        run: make ${{ matrix.unit_type }}
 
       - name: Send coverage
         uses: shogo82148/actions-goveralls@v1

config_builder.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btclog"
 	"github.com/btcsuite/btcwallet/waddrmgr"
@@ -1470,26 +1471,24 @@ func parseHeaderStateAssertion(state string) (*headerfs.FilterHeader, error) {
 // 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)
+	returnErr := rpcclient.MapRPCErr(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):
+	case errors.Is(returnErr, rpcclient.ErrTxAlreadyKnown),
+		errors.Is(err, rpcclient.ErrTxAlreadyConfirmed):
+
 		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):
+	case errors.Is(returnErr, rpcclient.ErrTxAlreadyInMempool):
 		returnErr = &pushtx.BroadcastError{
 			Code:   pushtx.Mempool,
 			Reason: returnErr.Error(),
@@ -1500,7 +1499,7 @@ func broadcastErrorMapper(err error) 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):
+	case errors.Is(err, rpcclient.ErrMempoolMinFeeNotMet):
 		ltndLog.Warnf("Error while broadcasting transaction: %v",
 			returnErr)
 

docs/release-notes/release-notes-0.18.0.md

@@ -240,6 +240,10 @@ bitcoin peers' feefilter values into account](https://github.com/lightningnetwor
   callbacks](https://github.com/lightningnetwork/lnd/pull/8504) which will
   execute whenever a healthcheck succeeds/fails.
 
+* `PublishTransaction` now [returns the error
+  types](https://github.com/lightningnetwork/lnd/pull/8554) defined in
+  `btcd/rpcclient`.
+
 ### Logging
 * [Add the htlc amount](https://github.com/lightningnetwork/lnd/pull/8156) to
   contract court logs in case of timed-out HTLCs in order to easily spot dust

go.mod

@@ -4,17 +4,17 @@ require (
 	github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82
 	github.com/Yawning/aez v0.0.0-20211027044916-e49e68abd344
 	github.com/andybalholm/brotli v1.0.3
-	github.com/btcsuite/btcd v0.24.1-0.20240301210420-1a2b599bf1af
+	github.com/btcsuite/btcd v0.24.1-0.20240318151728-2fc99e0496d2
 	github.com/btcsuite/btcd/btcec/v2 v2.3.2
 	github.com/btcsuite/btcd/btcutil v1.1.5
 	github.com/btcsuite/btcd/btcutil/psbt v1.1.8
 	github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
-	github.com/btcsuite/btcwallet v0.16.10-0.20240305014015-f7c216e78ee8
+	github.com/btcsuite/btcwallet v0.16.10-0.20240318155207-9a7dd2416f4d
 	github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4
 	github.com/btcsuite/btcwallet/wallet/txrules v1.2.1
-	github.com/btcsuite/btcwallet/walletdb v1.4.1
-	github.com/btcsuite/btcwallet/wtxmgr v1.5.1
+	github.com/btcsuite/btcwallet/walletdb v1.4.2
+	github.com/btcsuite/btcwallet/wtxmgr v1.5.3
 	github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f
 	github.com/davecgh/go-spew v1.1.1
 	github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1

go.sum

@@ -71,8 +71,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
 github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ=
 github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c/go.mod h1:tjmYdS6MLJ5/s0Fj4DbLgSbDHbEqLJrtnHecBFkdz5M=
 github.com/btcsuite/btcd v0.23.5-0.20231215221805-96c9fd8078fd/go.mod h1:nm3Bko6zh6bWP60UxwoT5LzdGJsQJaPo6HjduXq9p6A=
-github.com/btcsuite/btcd v0.24.1-0.20240301210420-1a2b599bf1af h1:F60A3wst4/fy9Yr1Vn8MYmFlfn7DNLxp8o8UTvhqgBE=
-github.com/btcsuite/btcd v0.24.1-0.20240301210420-1a2b599bf1af/go.mod h1:5C8ChTkl5ejr3WHj8tkQSCmydiMEPB0ZhQhehpq7Dgg=
+github.com/btcsuite/btcd v0.24.1-0.20240318151728-2fc99e0496d2 h1:b7EiiYEZypI2id3516ptqjzhUfFAgNfF4YVtxikAg6Y=
+github.com/btcsuite/btcd v0.24.1-0.20240318151728-2fc99e0496d2/go.mod h1:5C8ChTkl5ejr3WHj8tkQSCmydiMEPB0ZhQhehpq7Dgg=
 github.com/btcsuite/btcd/btcec/v2 v2.1.0/go.mod h1:2VzYrv4Gm4apmbVVsSq5bqf1Ec8v56E48Vt0Y/umPgA=
 github.com/btcsuite/btcd/btcec/v2 v2.1.3/go.mod h1:ctjw4H1kknNJmRN4iP1R7bTQ+v3GJkZBd6mui8ZsAZE=
 github.com/btcsuite/btcd/btcec/v2 v2.3.2 h1:5n0X6hX0Zk+6omWcihdYvdAlGf2DfasC0GMf7DClJ3U=
@@ -90,18 +90,18 @@ github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0/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.20240305014015-f7c216e78ee8 h1:fkPZ7LfOCm3Nn0Y/56LOqwooYaw9cV/4jZkdHw7XGik=
-github.com/btcsuite/btcwallet v0.16.10-0.20240305014015-f7c216e78ee8/go.mod h1:q+J5AzV+ymzUaBXkP099uuVh18yzCyHHXPc4rxi55mk=
+github.com/btcsuite/btcwallet v0.16.10-0.20240318155207-9a7dd2416f4d h1:ikEYkMZEEOwOEpp3DR3EwV6DbZRDPshNw0uJn7W8v7I=
+github.com/btcsuite/btcwallet v0.16.10-0.20240318155207-9a7dd2416f4d/go.mod h1:2C3Q/MhYAKmk7F+Tey6LfKtKRTdQsrCf8AAAzzDPmH4=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4 h1:poyHFf7+5+RdxNp5r2T6IBRD7RyraUsYARYbp/7t4D8=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.4/go.mod h1:GETGDQuyq+VFfH1S/+/7slLM/9aNa4l7P4ejX6dJfb0=
 github.com/btcsuite/btcwallet/wallet/txrules v1.2.1 h1:UZo7YRzdHbwhK7Rhv3PO9bXgTxiOH45edK5qdsdiatk=
 github.com/btcsuite/btcwallet/wallet/txrules v1.2.1/go.mod h1:MVSqRkju/IGxImXYPfBkG65FgEZYA4fXchheILMVl8g=
 github.com/btcsuite/btcwallet/wallet/txsizes v1.2.4 h1:nmcKAVTv/cmYrs0A4hbiC6Qw+WTLYy/14SmTt3mLnCo=
 github.com/btcsuite/btcwallet/wallet/txsizes v1.2.4/go.mod h1:YqJR8WAAHiKIPesZTr9Cx9Az4fRhRLcJ6GcxzRUZCAc=
-github.com/btcsuite/btcwallet/walletdb v1.4.1 h1:NGIGoxx3trpaWqmdOeuhju7KJKp5UM96mQL21idF6RY=
-github.com/btcsuite/btcwallet/walletdb v1.4.1/go.mod h1:7ZQ+BvOEre90YT7eSq8bLoxTsgXidUzA/mqbRS114CQ=
-github.com/btcsuite/btcwallet/wtxmgr v1.5.1 h1:2yXhMGa4DNz16Mi0e8dVoiFXKOznXlxiGLhB3hKj2uA=
-github.com/btcsuite/btcwallet/wtxmgr v1.5.1/go.mod h1:tO4FBSdann0xg/Jtm0grV7t1DzpQMK8nThYVtvSJo/8=
+github.com/btcsuite/btcwallet/walletdb v1.4.2 h1:zwZZ+zaHo4mK+FAN6KeK85S3oOm+92x2avsHvFAhVBE=
+github.com/btcsuite/btcwallet/walletdb v1.4.2/go.mod h1:7ZQ+BvOEre90YT7eSq8bLoxTsgXidUzA/mqbRS114CQ=
+github.com/btcsuite/btcwallet/wtxmgr v1.5.3 h1:QrWCio9Leh3DwkWfp+A1SURj8pYn3JuTLv3waP5uEro=
+github.com/btcsuite/btcwallet/wtxmgr v1.5.3/go.mod h1:M4nQpxGTXiDlSOODKXboXX7NFthmiBNjzAKKNS7Fhjg=
 github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd h1:R/opQEbFEy9JGkIguV40SvRY1uliPX8ifOvi6ICsFCw=
 github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd/go.mod h1:HHNXQzUsZCxOoE+CPiyCTO6x34Zs86zZUiwtpXoGdtg=
 github.com/btcsuite/golangcrypto v0.0.0-20150304025918-53f62d9b43e8/go.mod h1:tYvUd8KLhm/oXvUeSEs2VlLghFjQt9+ZaF9ghH0JNjc=

lnwallet/btcwallet/btcwallet.go

@@ -29,7 +29,6 @@ import (
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/keychain"
 	"github.com/lightningnetwork/lnd/kvdb"
-	"github.com/lightningnetwork/lnd/lnutils"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 )
@@ -1191,47 +1190,45 @@ func (b *BtcWallet) ListUnspentWitness(minConfs, maxConfs int32,
 	return witnessOutputs, nil
 }
 
+// mapRpcclientError maps an error from the rpcclient package to defined error
+// in this package.
+//
+// NOTE: we are mapping the errors returned from `sendrawtransaction` RPC or
+// the reject reason from `testmempoolaccept` RPC.
+func mapRpcclientError(err error) error {
+	// If we failed to publish the transaction, check whether we got an
+	// error of known type.
+	switch {
+	// If the wallet reports a double spend, convert it to our internal
+	// ErrDoubleSpend and return.
+	case errors.Is(err, rpcclient.ErrMempoolConflict),
+		errors.Is(err, rpcclient.ErrMissingInputs):
+
+		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 errors.Is(err, rpcclient.ErrMempoolMinFeeNotMet):
+		return fmt.Errorf("%w: %v", lnwallet.ErrMempoolFee, err.Error())
+	}
+
+	return err
+}
+
 // 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
 // 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 {
-	// handleErr is a helper closure that handles the error from
-	// PublishTransaction and TestMempoolAccept.
-	handleErr := func(err error) error {
-		// If we failed to publish the transaction, check whether we
-		// got an error of known type.
-		switch {
-		// If the wallet reports a double spend, convert it to our
-		// internal ErrDoubleSpend and return.
-		case lnutils.ErrorAs[*base.ErrDoubleSpend](err):
-			return lnwallet.ErrDoubleSpend
-
-		// If the wallet reports a replacement error, return
-		// ErrDoubleSpend, as we currently are never attempting to
-		// replace transactions.
-		case lnutils.ErrorAs[*base.ErrReplacement](err):
-			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 lnutils.ErrorAs[*base.ErrMempoolFee](err):
-			return fmt.Errorf("%w: %v", lnwallet.ErrMempoolFee,
-				err.Error())
-		}
-
-		return err
-	}
-
 	// For neutrino backend there's no mempool, so we return early by
 	// publishing the transaction.
 	if b.chain.BackEnd() == "neutrino" {
 		err := b.wallet.PublishTransaction(tx, label)
 
-		return handleErr(err)
+		return mapRpcclientError(err)
 	}
 
 	// For non-neutrino nodes, we will first check whether the transaction
@@ -1250,7 +1247,7 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 
 			err := b.wallet.PublishTransaction(tx, label)
 
-			return handleErr(err)
+			return mapRpcclientError(err)
 		}
 
 		return err
@@ -1269,7 +1266,7 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 	if result.Allowed {
 		err = b.wallet.PublishTransaction(tx, label)
 
-		return handleErr(err)
+		return mapRpcclientError(err)
 	}
 
 	// If the check failed, there's no need to publish it. We'll handle the
@@ -1279,7 +1276,7 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 
 	// We need to use the string to create an error type and map it to a
 	// btcwallet error.
-	err = base.MapBroadcastBackendError(errors.New(result.RejectReason))
+	err = rpcclient.MapRPCErr(errors.New(result.RejectReason))
 
 	//nolint:lll
 	// These two errors are ignored inside `PublishTransaction`:
@@ -1288,24 +1285,24 @@ func (b *BtcWallet) PublishTransaction(tx *wire.MsgTx, label string) error {
 	// returned from TestMempoolAccept.
 	//
 	// TODO(yy): since `LightningWallet.PublishTransaction` always publish
-	// the same tx twice, we'd always get ErrInMempool. We should instead
-	// create a new rebroadcaster that monitors the mempool, and only
-	// rebroadcast when the tx is evicted. This way we don't need to
+	// the same tx twice, we'd always get ErrTxAlreadyInMempool. We should
+	// instead create a new rebroadcaster that monitors the mempool, and
+	// only rebroadcast when the tx is evicted. This way we don't need to
 	// broadcast twice, and can instead return these errors here.
 	switch {
 	// NOTE: In addition to ignoring these errors, we need to call
 	// `PublishTransaction` again because we need to mark the label in the
 	// wallet. We can remove this exception once we have the above TODO
 	// fixed.
-	case lnutils.ErrorAs[*base.ErrInMempool](err),
-		lnutils.ErrorAs[*base.ErrAlreadyConfirmed](err):
+	case errors.Is(err, rpcclient.ErrTxAlreadyInMempool),
+		errors.Is(err, rpcclient.ErrTxAlreadyKnown),
+		errors.Is(err, rpcclient.ErrTxAlreadyConfirmed):
 
 		err := b.wallet.PublishTransaction(tx, label)
-
-		return handleErr(err)
+		return mapRpcclientError(err)
 	}
 
-	return handleErr(err)
+	return mapRpcclientError(err)
 }
 
 // LabelTransaction adds a label to a transaction. If the tx already

lnwallet/test/test_interface.go

@@ -1788,16 +1788,27 @@ func testPublishTransaction(r *rpctest.Harness,
 			require.NoError(t, err, "unable to obtain public key")
 
 			// Create a new transaction that spends the output from
-			// tx3, and that pays to a different address. We expect
-			// this to be rejected because it is a double spend.
+			// tx3, and that pays to a different address.
 			tx5, err := txFromOutput(
 				tx3, alice.Cfg.Signer, keyDesc.PubKey,
 				keyDesc2.PubKey, txFee, rbf,
 			)
 			require.NoError(t, err)
 
 			err = alice.PublishTransaction(tx5, labels.External)
-			require.ErrorIs(t, err, lnwallet.ErrDoubleSpend)
+
+			// If RBF is not enabled, we expect this to be rejected
+			// because it is a double spend.
+			expectedErr := lnwallet.ErrDoubleSpend
+
+			// If RBF is enabled, we expect it to be rejected
+			// because it doesn't pay enough fees.
+			if rbf {
+				expectedErr = rpcclient.ErrInsufficientFee
+			}
+
+			// Assert the expected error.
+			require.ErrorIsf(t, err, expectedErr, "has rbf=%v", rbf)
 
 			// Create another transaction that spends the same
 			// output, but has a higher fee. We expect also this tx
@@ -1875,21 +1886,8 @@ func testPublishTransaction(r *rpctest.Harness,
 
 		// Now broadcast the transaction, we should get an error that
 		// the weight is too large.
-		//
-		// TODO(roasbeef): we can't use Unwrap() here as TxRuleError
-		// doesn't define it
 		err := alice.PublishTransaction(testTx, labels.External)
-
-		errStr := "bad-txns-oversize"
-
-		// For neutrino backend, the error string in not mapped.
-		//
-		// TODO(yy): unify error matching in neutrino too.
-		if alice.BackEnd() == "neutrino" {
-			errStr = "serialized transaction is too big"
-		}
-
-		require.Contains(t, err.Error(), errStr)
+		require.ErrorIs(t, err, rpcclient.ErrOversizeTx)
 	})
 }