Switch away from exceptions in refactored tx code

After refactoring general-purpose PSBT and transaction code out of RPC code,
for use in the GUI, it's no longer appropriate to throw exceptions. Instead we
now return bools for success, and take an output parameter for an error object.
We still use JSONRPCError() for the error objects, since only RPC callers
actually care about the error codes.

2 files changed, 75 insertions, 13 deletions

diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 47c0323f14..6c4efb3d26 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -5,7 +5,6 @@
 
 #include <consensus/validation.h>
 #include <net.h>
-#include <rpc/server.h>
 #include <txmempool.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -13,9 +12,36 @@
 
 #include <future>
 
-uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees) {
+const char* TransactionErrorString(const TransactionError err)
+{
+    switch (err) {
+        case TransactionError::OK:
+            return "No error";
+        case TransactionError::MISSING_INPUTS:
+            return "Missing inputs";
+        case TransactionError::ALREADY_IN_CHAIN:
+            return "Transaction already in block chain";
+        case TransactionError::P2P_DISABLED:
+            return "Peer-to-peer functionality missing or disabled";
+        case TransactionError::MEMPOOL_REJECTED:
+            return "Transaction rejected by AcceptToMemoryPool";
+        case TransactionError::MEMPOOL_ERROR:
+            return "AcceptToMemoryPool failed";
+        case TransactionError::INVALID_PSBT:
+            return "PSBT is not sane";
+        case TransactionError::SIGHASH_MISMATCH:
+            return "Specified sighash value does not match existing value";
+
+        case TransactionError::UNKNOWN_ERROR:
+        default: break;
+    }
+    return "Unknown error";
+}
+
+bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, TransactionError& error, std::string& err_string, const bool allowhighfees)
+{
     std::promise<void> promise;
-    const uint256& hashTx = tx->GetHash();
+    hashTx = tx->GetHash();
 
     CAmount nMaxRawTxFee = maxTxFee;
     if (allowhighfees)
@@ -37,12 +63,17 @@ uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees)
         if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs,
                                 nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) {
             if (state.IsInvalid()) {
-                throw JSONRPCError(RPC_TRANSACTION_REJECTED, FormatStateMessage(state));
+                err_string = FormatStateMessage(state);
+                error = TransactionError::MEMPOOL_REJECTED;
+                return false;
             } else {
                 if (fMissingInputs) {
-                    throw JSONRPCError(RPC_TRANSACTION_ERROR, "Missing inputs");
+                    error = TransactionError::MISSING_INPUTS;
+                    return false;
                 }
-                throw JSONRPCError(RPC_TRANSACTION_ERROR, FormatStateMessage(state));
+                err_string = FormatStateMessage(state);
+                error = TransactionError::MEMPOOL_ERROR;
+                return false;
             }
         } else {
             // If wallet is enabled, ensure that the wallet has been made aware
@@ -55,7 +86,8 @@ uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees)
             });
         }
     } else if (fHaveChain) {
-        throw JSONRPCError(RPC_TRANSACTION_ALREADY_IN_CHAIN, "transaction already in block chain");
+        error = TransactionError::ALREADY_IN_CHAIN;
+        return false;
     } else {
         // Make sure we don't block forever if re-sending
         // a transaction already in mempool.
@@ -66,8 +98,10 @@ uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees)
 
     promise.get_future().wait();
 
-    if(!g_connman)
-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
+    if(!g_connman) {
+        error = TransactionError::P2P_DISABLED;
+        return false;
+    }
 
     CInv inv(MSG_TX, hashTx);
     g_connman->ForEachNode([&inv](CNode* pnode)
@@ -75,5 +109,5 @@ uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees)
         pnode->PushInventory(inv);
     });
 
-    return hashTx;
-}
+    return true;
+    }
diff --git a/src/node/transaction.h b/src/node/transaction.h
index 1916c6db26..83354d9400 100644
--- a/src/node/transaction.h
+++ b/src/node/transaction.h
@@ -8,7 +8,35 @@
 #include <primitives/transaction.h>
 #include <uint256.h>
 
-/** Broadcast a transaction */
-uint256 BroadcastTransaction(CTransactionRef tx, bool allowhighfees = false);
+enum class TransactionError {
+    OK = 0,
+    UNKNOWN_ERROR,
+
+    MISSING_INPUTS,
+    ALREADY_IN_CHAIN,
+    P2P_DISABLED,
+    MEMPOOL_REJECTED,
+    MEMPOOL_ERROR,
+    INVALID_PSBT,
+    SIGHASH_MISMATCH,
+
+    ERROR_COUNT
+};
+
+#define TRANSACTION_ERR_LAST TransactionError::ERROR_COUNT
+
+const char* 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`)
+ */
+bool BroadcastTransaction(CTransactionRef tx, uint256& txid, TransactionError& error, std::string& err_string, bool allowhighfees = false);
 
 #endif // BITCOIN_NODE_TRANSACTION_H