[wallet] Remove return value from CommitTransaction()

CommitTransaction returns a bool to indicate success, but since commit
b3a74100b8 it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.

All dead code in `if (!CommitTransaction())` branches has been removed.

5 files changed, 14 insertions, 24 deletions

diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index fbcf8d4de7..c9cd042b03 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -394,11 +394,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti
     mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
 
     CValidationState state;
-    if (!wallet.CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, state)) {
-        // NOTE: CommitTransaction never returns false, so this should never happen.
-        errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state)));
-        return Result::WALLET_ERROR;
-    }
+    wallet.CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, state);
 
     bumped_txid = tx->GetHash();
     if (state.IsInvalid()) {
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index debc3fecda..62066ddcc7 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -343,10 +343,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
         throw JSONRPCError(RPC_WALLET_ERROR, strError);
     }
     CValidationState state;
-    if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state)) {
-        strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state));
-        throw JSONRPCError(RPC_WALLET_ERROR, strError);
-    }
+    pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state);
     return tx;
 }
 
@@ -928,11 +925,7 @@ static UniValue sendmany(const JSONRPCRequest& request)
     if (!fCreated)
         throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
     CValidationState state;
-    if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state)) {
-        strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state));
-        throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
-    }
-
+    pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, state);
     return tx->GetHash().GetHex();
 }
 
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 73523ca36d..f31cd1e1b6 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -452,7 +452,7 @@ public:
             BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, fee, changePos, error, dummy));
         }
         CValidationState state;
-        BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, state));
+        wallet->CommitTransaction(tx, {}, {}, state);
         CMutableTransaction blocktx;
         {
             LOCK(wallet->cs_wallet);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index a9ea56b6ab..9c5de45c28 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3286,7 +3286,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
     return true;
 }
 
-bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state)
+void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state)
 {
     auto locked_chain = chain().lock();
     LOCK(cs_wallet);
@@ -3314,15 +3314,16 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
     // fInMempool flag is cached properly
     CWalletTx& wtx = mapWallet.at(wtxNew.GetHash());
 
-    if (fBroadcastTransactions) {
-        std::string err_string;
-        if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) {
-            WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
-            // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
-        }
+    if (!fBroadcastTransactions) {
+        // Don't submit tx to the mempool
+        return;
     }
 
-    return true;
+    std::string err_string;
+    if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) {
+        WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
+        // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
+    }
 }
 
 DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index d12c6077bf..8113e60aa6 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1157,7 +1157,7 @@ public:
      * @param orderForm[in] BIP 70 / BIP 21 order form details to be set on the transaction.
      * @param state[in,out] CValidationState object returning information about whether the transaction was accepted
      */
-    bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state);
+    void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CValidationState& state);
 
     bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const
     {