Merge 0.18 PRs: #14268, #15408, #14094, #14935 and #14969

src/node/transaction.cpp

@@ -13,7 +13,7 @@
 
 #include <future>
 
-const char* TransactionErrorString(const TransactionError err)
+std::string TransactionErrorString(const TransactionError err)
 {
     switch (err) {
         case TransactionError::OK:
@@ -34,22 +34,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, const bool bypass_limits)
+TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, std::string& err_string, const CAmount& highfee, const bool bypass_limits)
 {
     std::promise<void> promise;
     hashTx = tx->GetHash();
 
-    CAmount nMaxRawTxFee = maxTxFee;
-    if (allowhighfees)
-        nMaxRawTxFee = 0;
-
     { // cs_main scope
     LOCK(cs_main);
     CCoinsViewCache &view = *pcoinsTip;
@@ -64,19 +58,16 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction
         CValidationState state;
         bool fMissingInputs;
         if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs,
-                                bypass_limits, nMaxRawTxFee)) {
+                                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
@@ -89,8 +80,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.
@@ -101,12 +91,11 @@ 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;
     }
 
     g_connman->RelayTransaction(*tx);
 
-    return true;
-    }
+    return TransactionError::OK;
+}

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 <attributes.h>
 #include <primitives/transaction.h>
 #include <uint256.h>
 
 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(const CTransactionRef tx, uint256& txid, TransactionError& error, std::string& err_string, const bool allowhighfees = false, const bool bypass_limits = false);
+[[nodiscard]] TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee, const bool bypass_limits = false);
 
 #endif // BITCOIN_NODE_TRANSACTION_H

src/psbt.cpp

@@ -220,21 +220,19 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
     return true;
 }
 
-bool CombinePSBTs(PartiallySignedTransaction& out, TransactionError& error, const std::vector<PartiallySignedTransaction>& psbtxs)
+TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& 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;
 }

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.
 
@@ -497,10 +497,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<PartiallySignedTransaction>& psbtxs);
+[[nodiscard]] TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs);
 
 #endif // BITCOIN_PSBT_H

src/rpc/rawtransaction.cpp

@@ -1046,10 +1046,11 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
     if (!request.params[1].isNull()) allowhighfees = request.params[1].get_bool();
     bool bypass_limits = false;
     if (!request.params[3].isNull()) bypass_limits = request.params[3].get_bool();
+    const CAmount highfee{allowhighfees ? 0 : ::maxTxFee};
     uint256 txid;
-    TransactionError err;
     std::string err_string;
-    if (!BroadcastTransaction(tx, txid, err, err_string, allowhighfees, bypass_limits)) {
+    const TransactionError err = BroadcastTransaction(tx, txid, err_string, highfee, bypass_limits);
+    if (TransactionError::OK != err) {
         throw JSONRPCTransactionError(err, err_string);
     }
 
@@ -1426,8 +1427,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);
     }