Merge #15408: Remove unused TransactionError constants

fa9b60c842 Remove unused TransactionError constants (MarcoFalke)

Pull request description:

  Fixup to #14978, which introduced a bunch of unused enum values, such as `UNKNOWN_ERROR`, `ERROR_COUNT` and `TRANSACTION_ERR_LAST`. None of those have a meaning in the context of an `enum class`, where the compiler can infer if all cases have been covered in a switch-case.

  Also, move the global `::maxTxFee` back to the rpc caller, so it can be set on a per call basis (in the future).

Tree-SHA512: 7f1e2d795f1c1278ecd54ddab2b92c2a862f3c637b482d1d008208925befa1c9dd4b3c4bb1bfcbc5ca4b66a41004aaf01ea96ea95236f944250b8a6cf99ff173

1 files changed, 14 insertions, 26 deletions

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 <future>
 
-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<void> 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;
+}