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.

10 files changed, 159 insertions, 24 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
diff --git a/src/psbt.h b/src/psbt.h
index fbe55ca100..ad6f003015 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -6,6 +6,7 @@
 #define BITCOIN_PSBT_H
 
 #include <attributes.h>
+#include <node/transaction.h>
 #include <primitives/transaction.h>
 #include <pubkey.h>
 #include <script/sign.h>
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 03e38739a1..bc836614ae 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1050,7 +1050,14 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request)
 
     bool allowhighfees = false;
     if (!request.params[1].isNull()) allowhighfees = request.params[1].get_bool();
-    return BroadcastTransaction(tx, allowhighfees).GetHex();
+    uint256 txid;
+    TransactionError err;
+    std::string err_string;
+    if (!BroadcastTransaction(tx, txid, err, err_string, allowhighfees)) {
+        throw JSONRPCTransactionError(err, err_string);
+    }
+
+    return txid.GetHex();
 }
 
 static UniValue testmempoolaccept(const JSONRPCRequest& request)
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index aa5076cd8e..9e825ac12a 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -141,6 +141,32 @@ unsigned int ParseConfirmTarget(const UniValue& value)
     return (unsigned int)target;
 }
 
+RPCErrorCode RPCErrorFromTransactionError(TransactionError terr)
+{
+    switch (terr) {
+        case TransactionError::MEMPOOL_REJECTED:
+            return RPC_TRANSACTION_REJECTED;
+        case TransactionError::ALREADY_IN_CHAIN:
+            return RPC_TRANSACTION_ALREADY_IN_CHAIN;
+        case TransactionError::P2P_DISABLED:
+            return RPC_CLIENT_P2P_DISABLED;
+        case TransactionError::INVALID_PSBT:
+        case TransactionError::SIGHASH_MISMATCH:
+            return RPC_DESERIALIZATION_ERROR;
+        default: break;
+    }
+    return RPC_TRANSACTION_ERROR;
+}
+
+UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string)
+{
+    if (err_string.length() > 0) {
+        return JSONRPCError(RPCErrorFromTransactionError(terr), err_string);
+    } else {
+        return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr));
+    }
+}
+
 struct Section {
     Section(const std::string& left, const std::string& right)
         : m_left{left}, m_right{right} {}
diff --git a/src/rpc/util.h b/src/rpc/util.h
index d34c9cfdbb..33fca79029 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_RPC_UTIL_H
 #define BITCOIN_RPC_UTIL_H
 
+#include <node/transaction.h>
 #include <pubkey.h>
 #include <script/standard.h>
 #include <univalue.h>
@@ -31,6 +32,9 @@ UniValue DescribeAddress(const CTxDestination& dest);
 //! Parse a confirm target option and raise an RPC error if it is invalid.
 unsigned int ParseConfirmTarget(const UniValue& value);
 
+RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
+UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = "");
+
 struct RPCArg {
     enum class Type {
         OBJ,
diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp
index a62ad08370..761e7b7dd7 100644
--- a/src/wallet/psbtwallet.cpp
+++ b/src/wallet/psbtwallet.cpp
@@ -2,14 +2,13 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <rpc/protocol.h>
 #include <wallet/psbtwallet.h>
 
-bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs)
+bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, TransactionError& error, bool& complete, int sighash_type, bool sign, bool bip32derivs)
 {
     LOCK(pwallet->cs_wallet);
     // Get all of the previous transactions
-    bool complete = true;
+    complete = true;
     for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
         const CTxIn& txin = psbtx.tx->vin[i];
         PSBTInput& input = psbtx.inputs.at(i);
@@ -20,7 +19,8 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sig
 
         // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness.
         if (!input.IsSane()) {
-            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "PSBT input is not sane.");
+            error = TransactionError::INVALID_PSBT;
+            return false;
         }
 
         // If we have no utxo, grab it from the wallet.
@@ -37,7 +37,8 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sig
 
         // Get the Sighash type
         if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) {
-            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Specified Sighash and sighash in PSBT do not match.");
+            error = TransactionError::SIGHASH_MISMATCH;
+            return false;
         }
 
         complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), psbtx, i, sighash_type);
@@ -56,5 +57,6 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sig
         ProduceSignature(HidingSigningProvider(pwallet, true, !bip32derivs), creator, out.scriptPubKey, sigdata);
         psbt_out.FromSignatureData(sigdata);
     }
-    return complete;
+
+    return true;
 }
diff --git a/src/wallet/psbtwallet.h b/src/wallet/psbtwallet.h
index 4f888a06ec..b679f5c6ba 100644
--- a/src/wallet/psbtwallet.h
+++ b/src/wallet/psbtwallet.h
@@ -5,10 +5,32 @@
 #ifndef BITCOIN_WALLET_PSBTWALLET_H
 #define BITCOIN_WALLET_PSBTWALLET_H
 
+#include <node/transaction.h>
 #include <psbt.h>
 #include <primitives/transaction.h>
 #include <wallet/wallet.h>
 
-bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false);
+/**
+ * Fills out a PSBT with information from the wallet. Fills in UTXOs if we have
+ * them. Tries to sign if sign=true. Sets `complete` if the PSBT is now complete
+ * (i.e. has all required signatures or signature-parts, and is ready to
+ * finalize.) Sets `error` and returns false if something goes wrong.
+ *
+ * @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`)
+ */
+bool FillPSBT(const CWallet* pwallet,
+              PartiallySignedTransaction& psbtx,
+              TransactionError& error,
+              bool& complete,
+              int sighash_type = 1 /* SIGHASH_ALL */,
+              bool sign = true,
+              bool bip32derivs = false);
 
 #endif // BITCOIN_WALLET_PSBTWALLET_H
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 905b183307..6811d927f2 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -13,6 +13,7 @@
 #include <validation.h>
 #include <key_io.h>
 #include <net.h>
+#include <node/transaction.h>
 #include <outputtype.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
@@ -4003,7 +4004,11 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request)
     // Fill transaction with our data and also sign
     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 = FillPSBT(pwallet, psbtx, nHashType, sign, bip32derivs);
+    bool complete = true;
+    TransactionError err;
+    if (!FillPSBT(pwallet, psbtx, err, complete, nHashType, sign, bip32derivs)) {
+        throw JSONRPCTransactionError(err);
+    }
 
     UniValue result(UniValue::VOBJ);
     CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
@@ -4117,7 +4122,11 @@ 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();
-    FillPSBT(pwallet, psbtx, 1, false, bip32derivs);
+    bool complete = true;
+    TransactionError err;
+    if (!FillPSBT(pwallet, psbtx, err, complete, 1, false, bip32derivs)) {
+        throw JSONRPCTransactionError(err);
+    }
 
     // Serialize the PSBT
     CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp
index 0c5b7c7e98..e89d4121bc 100644
--- a/src/wallet/test/psbt_wallet_tests.cpp
+++ b/src/wallet/test/psbt_wallet_tests.cpp
@@ -61,7 +61,9 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
     ssData >> psbtx;
 
     // Fill transaction with our data
-    FillPSBT(&m_wallet, psbtx, SIGHASH_ALL, false, true);
+    TransactionError err;
+    bool complete = true;
+    FillPSBT(&m_wallet, psbtx, err, complete, SIGHASH_ALL, false, true);
 
     // Get the final tx
     CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);