Merge pull request from yyforyongyu/use-new-errors

lnwallet: use new errors returned from `rpcclient`
This commit is contained in:
Oliver Gugger 2024-03-21 01:42:57 -06:00 committed by GitHub
commit 15c7686830
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 77 additions and 79 deletions

@ -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

@ -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,18 +1471,16 @@ 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(),
@ -1489,7 +1488,7 @@ func broadcastErrorMapper(err error) 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)

@ -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

8
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

16
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=

@ -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,6 +1190,32 @@ 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
@ -1198,40 +1223,12 @@ func (b *BtcWallet) ListUnspentWitness(minConfs, maxConfs int32,
// 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

@ -1788,8 +1788,7 @@ 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,
@ -1797,7 +1796,19 @@ func testPublishTransaction(r *rpctest.Harness,
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)
})
}