From bb00c95c16f50c5dfab1aa8fbb6c873318a6acc8 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Tue, 6 Feb 2018 18:47:51 -0500 Subject: [PATCH] Consistently use FormatStateMessage in RPC error output This will include the error code and debug output as well as the reason string. See #11955 for the motivation. --- src/rpc/blockchain.cpp | 8 ++++---- src/rpc/mining.cpp | 4 ++-- src/rpc/rawtransaction.cpp | 4 ++-- src/wallet/feebumper.cpp | 3 +-- src/wallet/rpcwallet.cpp | 4 ++-- src/wallet/wallet.cpp | 2 +- test/functional/feature_nulldummy.py | 2 +- test/functional/mempool_limit.py | 2 +- test/functional/mining_prioritisetransaction.py | 2 +- 9 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 4276ad9ee..c880991b5 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1434,7 +1434,7 @@ UniValue preciousblock(const JSONRPCRequest& request) PreciousBlock(state, Params(), pblockindex); if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, state.GetRejectReason()); + throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); } return NullUniValue; @@ -1472,7 +1472,7 @@ UniValue invalidateblock(const JSONRPCRequest& request) } if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, state.GetRejectReason()); + throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); } return NullUniValue; @@ -1509,7 +1509,7 @@ UniValue reconsiderblock(const JSONRPCRequest& request) ActivateBestChain(state, Params()); if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, state.GetRejectReason()); + throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); } return NullUniValue; @@ -1563,7 +1563,7 @@ UniValue getchaintxstats(const JSONRPCRequest& request) pindex = chainActive.Tip(); } } - + assert(pindex != nullptr); if (request.params[0].isNull()) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 69d0d12e8..32f8ca0c7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -264,11 +264,11 @@ static UniValue BIP22ValidationResult(const CValidationState& state) if (state.IsValid()) return NullUniValue; - std::string strRejectReason = state.GetRejectReason(); if (state.IsError()) - throw JSONRPCError(RPC_VERIFY_ERROR, strRejectReason); + throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state)); if (state.IsInvalid()) { + std::string strRejectReason = state.GetRejectReason(); if (strRejectReason.empty()) return "rejected"; return strRejectReason; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 00e73675b..85709256f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -981,12 +981,12 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) { if (state.IsInvalid()) { - throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); + throw JSONRPCError(RPC_TRANSACTION_REJECTED, FormatStateMessage(state)); } else { if (fMissingInputs) { throw JSONRPCError(RPC_TRANSACTION_ERROR, "Missing inputs"); } - throw JSONRPCError(RPC_TRANSACTION_ERROR, state.GetRejectReason()); + throw JSONRPCError(RPC_TRANSACTION_ERROR, FormatStateMessage(state)); } } else { // If wallet is enabled, ensure that the wallet has been made aware diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 9bfcab54a..ebf30fe03 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -267,7 +267,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti CValidationState state; if (!wallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. - errors.push_back(strprintf("The transaction was rejected: %s", state.GetRejectReason())); + errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); return Result::WALLET_ERROR; } @@ -290,4 +290,3 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti } } // namespace feebumper - diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 261d9b37f..a82afcf2a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -435,7 +435,7 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA } CValidationState state; if (!pwallet->CommitTransaction(wtxNew, reservekey, g_connman.get(), state)) { - strError = strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason()); + strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } } @@ -1155,7 +1155,7 @@ UniValue sendmany(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; if (!pwallet->CommitTransaction(wtx, keyChange, g_connman.get(), state)) { - strFailReason = strprintf("Transaction commit failed:: %s", state.GetRejectReason()); + strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1b3a3ee50..4a022fa56 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3094,7 +3094,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon { // Broadcast if (!wtx.AcceptToMemoryPool(maxTxFee, state)) { - LogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", state.GetRejectReason()); + LogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", FormatStateMessage(state)); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. } else { wtx.RelayWalletTransaction(connman); diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index e4f413cc2..740c498ce 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -21,7 +21,7 @@ from test_framework.script import CScript from io import BytesIO import time -NULLDUMMY_ERROR = "64: non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)" +NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero) (code 64)" def trueDummy(tx): scriptSig = CScript(tx.vin[0].scriptSig) diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 1385271e6..7e01663c9 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -58,7 +58,7 @@ class MempoolLimitTest(BitcoinTestFramework): # specifically fund this tx with a fee < mempoolminfee, >= than minrelaytxfee txF = self.nodes[0].fundrawtransaction(tx, {'feeRate': relayfee}) txFS = self.nodes[0].signrawtransaction(txF['hex']) - assert_raises_rpc_error(-26, "66: mempool min fee not met", self.nodes[0].sendrawtransaction, txFS['hex']) + assert_raises_rpc_error(-26, "mempool min fee not met, 166 < 411 (code 66)", self.nodes[0].sendrawtransaction, txFS['hex']) if __name__ == '__main__': MempoolLimitTest().main() diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 57954ce32..8cea9c278 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -120,7 +120,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): tx_id = self.nodes[0].decoderawtransaction(tx_hex)["txid"] # This will raise an exception due to min relay fee not being met - assert_raises_rpc_error(-26, "66: min relay fee not met", self.nodes[0].sendrawtransaction, tx_hex) + assert_raises_rpc_error(-26, "min relay fee not met (code 66)", self.nodes[0].sendrawtransaction, tx_hex) assert(tx_id not in self.nodes[0].getrawmempool()) # This is a less than 1000-byte transaction, so just set the fee