diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index c9cdd0d1c..7b9b4310e 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -12,7 +12,7 @@ #include -const char* TransactionErrorString(const TransactionError err) +std::string TransactionErrorString(const TransactionError err) { switch (err) { case TransactionError::OK: @@ -33,22 +33,16 @@ const char* TransactionErrorString(const TransactionError err) return "PSBTs not compatible (different transactions)"; case TransactionError::SIGHASH_MISMATCH: return "Specified sighash value does not match existing value"; - - case TransactionError::UNKNOWN_ERROR: - default: break; + // no default case, so the compiler can warn about missing cases } - return "Unknown error"; + assert(false); } -bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, TransactionError& error, std::string& err_string, const bool allowhighfees) +TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, std::string& err_string, const CAmount& highfee) { std::promise promise; hashTx = tx->GetHash(); - CAmount nMaxRawTxFee = maxTxFee; - if (allowhighfees) - nMaxRawTxFee = 0; - { // cs_main scope LOCK(cs_main); CCoinsViewCache &view = *pcoinsTip; @@ -63,19 +57,16 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction CValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, - nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) { + nullptr /* plTxnReplaced */, false /* bypass_limits */, highfee)) { if (state.IsInvalid()) { err_string = FormatStateMessage(state); - error = TransactionError::MEMPOOL_REJECTED; - return false; + return TransactionError::MEMPOOL_REJECTED; } else { if (fMissingInputs) { - error = TransactionError::MISSING_INPUTS; - return false; + return TransactionError::MISSING_INPUTS; } err_string = FormatStateMessage(state); - error = TransactionError::MEMPOOL_ERROR; - return false; + return TransactionError::MEMPOOL_ERROR; } } else { // If wallet is enabled, ensure that the wallet has been made aware @@ -88,8 +79,7 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction }); } } else if (fHaveChain) { - error = TransactionError::ALREADY_IN_CHAIN; - return false; + return TransactionError::ALREADY_IN_CHAIN; } else { // Make sure we don't block forever if re-sending // a transaction already in mempool. @@ -100,16 +90,14 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction promise.get_future().wait(); - if(!g_connman) { - error = TransactionError::P2P_DISABLED; - return false; + if (!g_connman) { + return TransactionError::P2P_DISABLED; } CInv inv(MSG_TX, hashTx); - g_connman->ForEachNode([&inv](CNode* pnode) - { + g_connman->ForEachNode([&inv](CNode* pnode) { pnode->PushInventory(inv); }); - return true; - } + return TransactionError::OK; +} diff --git a/src/node/transaction.h b/src/node/transaction.h index 3b0cbba98..3457ececa 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -1,17 +1,16 @@ -// Copyright (c) 2017-2018 The Bitcoin Core developers +// Copyright (c) 2017-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. #ifndef BITCOIN_NODE_TRANSACTION_H #define BITCOIN_NODE_TRANSACTION_H +#include #include #include enum class TransactionError { - OK = 0, - UNKNOWN_ERROR, - + OK, //!< No error MISSING_INPUTS, ALREADY_IN_CHAIN, P2P_DISABLED, @@ -20,24 +19,19 @@ enum class TransactionError { INVALID_PSBT, PSBT_MISMATCH, SIGHASH_MISMATCH, - - ERROR_COUNT }; -#define TRANSACTION_ERR_LAST TransactionError::ERROR_COUNT - -const char* TransactionErrorString(const TransactionError error); +std::string TransactionErrorString(const TransactionError error); /** * Broadcast a transaction * * @param[in] tx the transaction to broadcast * @param[out] &txid the txid of the transaction, if successfully broadcast - * @param[out] &error reference to UniValue to fill with error info on failure * @param[out] &err_string reference to std::string to fill with error string if available - * @param[in] allowhighfees whether to allow fees exceeding maxTxFee - * return true on success, false on error (and fills in `error`) + * @param[in] highfee Reject txs with fees higher than this (if 0, accept any fee) + * return error */ -bool BroadcastTransaction(CTransactionRef tx, uint256& txid, TransactionError& error, std::string& err_string, bool allowhighfees = false); +NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/psbt.cpp b/src/psbt.cpp index 81633c0cc..f0e177a64 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -263,21 +263,19 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti return true; } -bool CombinePSBTs(PartiallySignedTransaction& out, TransactionError& error, const std::vector& psbtxs) +TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector& psbtxs) { out = psbtxs[0]; // Copy the first one // Merge for (auto it = std::next(psbtxs.begin()); it != psbtxs.end(); ++it) { if (!out.Merge(*it)) { - error = TransactionError::PSBT_MISMATCH; - return false; + return TransactionError::PSBT_MISMATCH; } } if (!out.IsSane()) { - error = TransactionError::INVALID_PSBT; - return false; + return TransactionError::INVALID_PSBT; } - return true; + return TransactionError::OK; } diff --git a/src/psbt.h b/src/psbt.h index e18790322..efa3d5849 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -565,10 +565,9 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti * Combines PSBTs with the same underlying transaction, resulting in a single PSBT with all partial signatures from each input. * * @param[out] &out the combined PSBT, if successful - * @param[out] &error reference to TransactionError to fill with error info on failure * @param[in] psbtxs the PSBTs to combine - * @return True if we successfully combined the transactions, false if they were not compatible + * @return error (OK if we successfully combined the transactions, other error if they were not compatible) */ -bool CombinePSBTs(PartiallySignedTransaction& out, TransactionError& error, const std::vector& psbtxs); +NODISCARD TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector& psbtxs); #endif // BITCOIN_PSBT_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 7fe73e56d..44cac1422 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1050,10 +1050,11 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) bool allowhighfees = false; if (!request.params[1].isNull()) allowhighfees = request.params[1].get_bool(); + const CAmount highfee{allowhighfees ? 0 : ::maxTxFee}; uint256 txid; - TransactionError err; std::string err_string; - if (!BroadcastTransaction(tx, txid, err, err_string, allowhighfees)) { + const TransactionError err = BroadcastTransaction(tx, txid, err_string, highfee); + if (TransactionError::OK != err) { throw JSONRPCTransactionError(err, err_string); } @@ -1478,8 +1479,8 @@ UniValue combinepsbt(const JSONRPCRequest& request) } PartiallySignedTransaction merged_psbt; - TransactionError error; - if (!CombinePSBTs(merged_psbt, error, psbtxs)) { + const TransactionError error = CombinePSBTs(merged_psbt, psbtxs); + if (error != TransactionError::OK) { throw JSONRPCTransactionError(error); } diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index 761e7b7dd..1b17b0976 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -4,7 +4,7 @@ #include -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, TransactionError& error, bool& complete, int sighash_type, bool sign, bool bip32derivs) +TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs) { LOCK(pwallet->cs_wallet); // Get all of the previous transactions @@ -19,8 +19,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. if (!input.IsSane()) { - error = TransactionError::INVALID_PSBT; - return false; + return TransactionError::INVALID_PSBT; } // If we have no utxo, grab it from the wallet. @@ -37,8 +36,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac // Get the Sighash type if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) { - error = TransactionError::SIGHASH_MISMATCH; - return false; + return TransactionError::SIGHASH_MISMATCH; } complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), psbtx, i, sighash_type); @@ -58,5 +56,5 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac psbt_out.FromSignatureData(sigdata); } - return true; + return TransactionError::OK; } diff --git a/src/wallet/psbtwallet.h b/src/wallet/psbtwallet.h index b679f5c6b..a24a0967d 100644 --- a/src/wallet/psbtwallet.h +++ b/src/wallet/psbtwallet.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -18,16 +18,14 @@ * * @param[in] pwallet pointer to a wallet * @param[in] &psbtx reference to PartiallySignedTransaction to fill in - * @param[out] &error reference to UniValue to fill with error info on failure * @param[out] &complete indicates whether the PSBT is now complete * @param[in] sighash_type the sighash type to use when signing (if PSBT does not specify) * @param[in] sign whether to sign or not * @param[in] bip32derivs whether to fill in bip32 derivation information if available - * return true on success, false on error (and fills in `error`) + * return error */ -bool FillPSBT(const CWallet* pwallet, +NODISCARD TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, - TransactionError& error, bool& complete, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9e362cf16..4211eeac8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3995,8 +3995,8 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? false : request.params[3].get_bool(); bool complete = true; - TransactionError err; - if (!FillPSBT(pwallet, psbtx, err, complete, nHashType, sign, bip32derivs)) { + const TransactionError err = FillPSBT(pwallet, psbtx, complete, nHashType, sign, bip32derivs); + if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -4113,8 +4113,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? false : request.params[4].get_bool(); bool complete = true; - TransactionError err; - if (!FillPSBT(pwallet, psbtx, err, complete, 1, false, bip32derivs)) { + const TransactionError err = FillPSBT(pwallet, psbtx, complete, 1, false, bip32derivs); + if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index e89d4121b..d1a9741ca 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -61,9 +61,8 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) ssData >> psbtx; // Fill transaction with our data - TransactionError err; bool complete = true; - FillPSBT(&m_wallet, psbtx, err, complete, SIGHASH_ALL, false, true); + BOOST_REQUIRE_EQUAL(TransactionError::OK, FillPSBT(&m_wallet, psbtx, complete, SIGHASH_ALL, false, true)); // Get the final tx CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);