From fc12e5e8fcad9074c232f9cbad371eedab17f013 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 15 Mar 2024 05:11:28 +0800 Subject: [PATCH 1/2] mod: update `btcd` package --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 4236823ea1..d9abbc76e4 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/btcsuite/btcwallet require ( - 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.2.2 github.com/btcsuite/btcd/btcutil v1.1.5 github.com/btcsuite/btcd/btcutil/psbt v1.1.8 diff --git a/go.sum b/go.sum index 6cf4615290..3abc568806 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13P github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c/go.mod h1:tjmYdS6MLJ5/s0Fj4DbLgSbDHbEqLJrtnHecBFkdz5M= github.com/btcsuite/btcd v0.22.0-beta.0.20220207191057-4dc4ff7963b4/go.mod h1:7alexyj/lHlOtr2PJK7L/+HDJZpcGDn/pAU98r7DY08= 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.2.2 h1:5uxe5YjoCq+JeOpg0gZSNHuFgeogrocBYxvg6w9sAgc= From e7771392c502c89a65a218a35236c810ce99bbbd Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 15 Mar 2024 05:01:54 +0800 Subject: [PATCH 2/2] chain+wallet: use the structured RPC-errors --- chain/neutrino.go | 2 +- wallet/wallet.go | 243 +++--------------------------------------- wallet/wallet_test.go | 65 ----------- 3 files changed, 15 insertions(+), 295 deletions(-) diff --git a/chain/neutrino.go b/chain/neutrino.go index 0cfa1c4b6d..61e4d5f428 100644 --- a/chain/neutrino.go +++ b/chain/neutrino.go @@ -220,7 +220,7 @@ func (s *NeutrinoClient) SendRawTransaction(tx *wire.MsgTx, allowHighFees bool) *chainhash.Hash, error) { err := s.CS.SendTransaction(tx) if err != nil { - return nil, err + return nil, rpcclient.MapRPCErr(err) } hash := tx.TxHash() return &hash, nil diff --git a/wallet/wallet.go b/wallet/wallet.go index 7ac328d3bd..2ab8646cbb 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "sort" - "strings" "sync" "sync/atomic" "time" @@ -23,6 +22,7 @@ import ( "github.com/btcsuite/btcd/btcutil/hdkeychain" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/rpcclient" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/chain" @@ -3591,25 +3591,6 @@ func (e *ErrDoubleSpend) Unwrap() error { return e.backendError } -// ErrReplacement is an error returned from PublishTransaction in case the -// published transaction failed to propagate since it was double spending a -// replacable transaction but did not satisfy the requirements to replace it. -type ErrReplacement struct { - backendError error -} - -// Error returns the string representation of ErrReplacement. -// -// NOTE: Satisfies the error interface. -func (e *ErrReplacement) Error() string { - return fmt.Sprintf("unable to replace transaction: %v", e.backendError) -} - -// Unwrap returns the underlying error returned from the backend. -func (e *ErrReplacement) Unwrap() error { - return e.backendError -} - // ErrMempoolFee is an error returned from PublishTransaction in case the // published transaction failed to propagate since it did not match the // current mempool fee requirement. @@ -3760,27 +3741,27 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) { return nil, err } - var ( - txid = tx.TxHash() - returnErr error - ) - + txid := tx.TxHash() _, err = chainClient.SendRawTransaction(tx, false) if err == nil { return &txid, nil } - returnErr = MapBroadcastBackendError(err) - - var errInMempool *ErrInMempool - var errAlreadyConfirmed *ErrAlreadyConfirmed + // Map the error to an RPC-specific error type. + // + // NOTE: all the errors returned here are mapped to an error type + // defined in `rpcclient` package, where the error strings are taken + // from bitcoind. + rpcErr := rpcclient.MapRPCErr(err) switch { - case errors.As(returnErr, &errInMempool): + case errors.Is(rpcErr, rpcclient.ErrTxAlreadyInMempool): log.Infof("%v: tx already in mempool", txid) return &txid, nil - case errors.As(returnErr, &errAlreadyConfirmed): + case errors.Is(rpcErr, rpcclient.ErrTxAlreadyKnown), + errors.Is(rpcErr, rpcclient.ErrTxAlreadyConfirmed): + dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey) txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now()) @@ -3801,7 +3782,7 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) { } // Log the causing error, even if we know how to handle it. - log.Infof("%v: broadcast failed because of: %v", txid, returnErr) + log.Infof("%v: broadcast failed because of: %v", txid, rpcErr) // If the transaction was rejected for whatever other reason, then // we'll remove it from the transaction store, as otherwise, we'll @@ -3838,7 +3819,7 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) { } } - return nil, returnErr + return nil, rpcErr } // ChainParams returns the network parameters for the blockchain the wallet @@ -4052,199 +4033,3 @@ func OpenWithRetry(db walletdb.DB, pubPass []byte, cbs *waddrmgr.OpenCallbacks, return w, nil } - -// matchBitcoindErr takes an error returned from bitcoind RPC client and -// matches it against the specified string. If the expected string pattern is -// found in the error passed, return true. -func matchBitcoindErr(err error, s string) bool { - // Replace all dashes found in the error string with spaces. - strippedErrStr := strings.Replace(err.Error(), "-", " ", -1) - - // Replace all dashes found in the error string with spaces. - strippedMatchStr := strings.Replace(s, "-", " ", -1) - - // Match against the lowercase. - return strings.Contains( - strings.ToLower(strippedErrStr), - strings.ToLower(strippedMatchStr), - ) -} - -// MapBroadcastBackendError maps the different backend errors when broadcasting -// a transaction to the bitcoin network to an internal error type. -func MapBroadcastBackendError(err error) error { - // Determine if this was an RPC error thrown due to the transaction - // already confirming. - var rpcTxConfirmed bool - if rpcErr, ok := err.(*btcjson.RPCError); ok { - rpcTxConfirmed = rpcErr.Code == btcjson.ErrRPCTxAlreadyInChain - } - - var returnErr error - - switch { - case err == nil: - return nil - - // Since we have different backends that can be used with the wallet, - // we'll need to check specific errors for each one. - // - // If the transaction is already in the mempool, we can just return now. - // - // This error is returned when broadcasting/sending a transaction to a - // btcd node that already has it in their mempool. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L953 - case matchBitcoindErr(err, "already have transaction"): - fallthrough - - // This error is returned when broadcasting a transaction to a bitcoind - // node that already has it in their mempool. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L590 - case matchBitcoindErr(err, "txn-already-in-mempool"): - return &ErrInMempool{ - backendError: err, - } - - // If the transaction has already confirmed, we can safely remove it - // from the unconfirmed store as it should already exist within the - // confirmed store. We'll avoid returning an error as the broadcast was - // in a sense successful. - // - // This error is returned when sending a transaction that has already - // confirmed to a btcd/bitcoind node over RPC. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/rpcserver.go#L3355 - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/node/transaction.cpp#L36 - case rpcTxConfirmed: - fallthrough - - // This error is returned when broadcasting a transaction that has - // already confirmed to a btcd node over the P2P network. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L1036 - case matchBitcoindErr(err, "transaction already exists"): - fallthrough - - // This error is returned when broadcasting a transaction that has - // already confirmed to a bitcoind node over the P2P network. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L648 - case matchBitcoindErr(err, "txn-already-known"): - return &ErrAlreadyConfirmed{ - backendError: err, - } - - // If the transactions is invalid since it attempts to double spend a - // transaction already in the mempool or in the chain, we'll remove it - // from the store and return an error. - // - // This error is returned from btcd when there is already a transaction - // not signaling replacement in the mempool that spends one of the - // referenced outputs. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L591 - case matchBitcoindErr(err, "already spent"): - fallthrough - - // This error is returned from btcd when a referenced output cannot be - // found, meaning it etiher has been spent or doesn't exist. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/blockchain/chain.go#L405 - case matchBitcoindErr(err, "already been spent"): - fallthrough - - // This error is returned from btcd when a transaction is spending - // either output that is missing or already spent, and orphans aren't - // allowed. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L1409 - case matchBitcoindErr(err, "orphan transaction"): - fallthrough - - // Error returned from bitcoind when output was spent by other - // non-replacable transaction already in the mempool. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L622 - case matchBitcoindErr(err, "txn-mempool-conflict"): - fallthrough - - // Returned by bitcoind on the RPC when broadcasting a transaction that - // is spending either output that is missing or already spent. - // - // https://github.com/bitcoin/bitcoin/blob/3ba8de1b704d590fa4e1975620bd21d830d11666/test/functional/mempool_accept.py#L163C1-L163C1 - case matchBitcoindErr(err, "missing-inputs") || - matchBitcoindErr(err, "bad-txns-inputs-missingorspent"): - - returnErr = &ErrDoubleSpend{ - backendError: err, - } - - // Returned by bitcoind if the transaction spends outputs that would be - // replaced by it. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L790 - case matchBitcoindErr(err, "bad-txns-spends-conflicting-tx"): - fallthrough - - // Returned by bitcoind when a replacement transaction did not have - // enough fee. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L830 - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L894 - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L904 - case matchBitcoindErr(err, "insufficient fee"): - fallthrough - - // Returned by bitcoind in case the transaction would replace too many - // transaction in the mempool. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L858 - case matchBitcoindErr(err, "too many potential replacements"): - fallthrough - - // Returned by bitcoind if the transaction spends an output that is - // unconfimed and not spent by the transaction it replaces. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L882 - case matchBitcoindErr(err, "replacement-adds-unconfirmed"): - fallthrough - - // Returned by btcd when replacement transaction was rejected for - // whatever reason. - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L841 - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L854 - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L875 - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L896 - // https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L913 - case matchBitcoindErr(err, "replacement transaction"): - returnErr = &ErrReplacement{ - backendError: err, - } - - // Returned by bitcoind when a transaction does not meet the fee - // requirements to be accepted into mempool. This happens when the - // mempool reached its limits and is now purging low fee transactions. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L510 - case matchBitcoindErr(err, "mempool min fee not met"): - fallthrough - - // Returned by btcd when a transaction does not meet the fee - // requirements to be accepted into mempool. - // https://github.com/btcsuite/btcd/blob/9c16d23918b15c468c5647c388b9b7db3bc48dc7/mempool/mempool.go#L1151 - case matchBitcoindErr(err, "fees which is under the required amount"): - fallthrough - - // Returned by btcd when a transaction does not meet the fee - // requirements to be accepted into mempool. The error states priority - // but decreasing the min relay fee prevents checking for the priority - // in the first place therefore we consider this a mempool fee error. - // https://github.com/btcsuite/btcd/blob/9c16d23918b15c468c5647c388b9b7db3bc48dc7/mempool/mempool.go#L1162 - case matchBitcoindErr(err, "has insufficient priority"): - fallthrough - - // Returned by bitcoind when a transaction does not meet the fee - // requirements to be accepted into mempool because the policy of the - // mempool has a higher min relay fee. Default value for bitcoind is - // 1000 sat/kvbyte but this is configurable. - // https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L514 - case matchBitcoindErr(err, "min relay fee not met"): - returnErr = &ErrMempoolFee{ - backendError: err, - } - - // We received an error not matching any of the above cases. - default: - returnErr = fmt.Errorf("unmatched backend error: %w", err) - } - - return returnErr -} diff --git a/wallet/wallet_test.go b/wallet/wallet_test.go index c7920e06de..bf479d7735 100644 --- a/wallet/wallet_test.go +++ b/wallet/wallet_test.go @@ -2,7 +2,6 @@ package wallet import ( "encoding/hex" - "errors" "testing" "time" @@ -306,67 +305,3 @@ func TestGetTransaction(t *testing.T) { }) } } - -// TestMatchBitcoindErr checks that `matchBitcoindErr` can correctly replace -// the dashes with spaces and turn title cases into lowercases for a given -// error and match it against the specified string pattern. -func TestMatchBitcoindErr(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - bitcoindErr error - matchStr string - matched bool - }{ - { - name: "error without dashes", - bitcoindErr: errors.New("missing input"), - matchStr: "missing input", - matched: true, - }, - { - name: "match str without dashes", - bitcoindErr: errors.New("missing-input"), - matchStr: "missing input", - matched: true, - }, - { - name: "error with dashes", - bitcoindErr: errors.New("missing-input"), - matchStr: "missing input", - matched: true, - }, - { - name: "match str with dashes", - bitcoindErr: errors.New("missing-input"), - matchStr: "missing-input", - matched: true, - }, - { - name: "error with title case and dash", - bitcoindErr: errors.New("Missing-Input"), - matchStr: "missing input", - matched: true, - }, - { - name: "match str with title case and dash", - bitcoindErr: errors.New("missing-input"), - matchStr: "Missing-Input", - matched: true, - }, - { - name: "unmatched error", - bitcoindErr: errors.New("missing input"), - matchStr: "missingorspent", - matched: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - matched := matchBitcoindErr(tc.bitcoindErr, tc.matchStr) - require.Equal(t, tc.matched, matched) - }) - } -}