[wallet] Return success from CommitTransaction()

CommitTransaction returns a bool to indicate success, but since commit
b3a7410 it only returns true, even if the transaction was not
successfully broadcast. This commit fixes CommitTransaction() to return
whether or not the transaction broadcast was successful.

A couple of call sites are updated to maintain existing behavior.

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();
 }
 

src/wallet/wallet.cpp

@@ -3312,12 +3312,17 @@ 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) {
+        state.Error("Wallet not broadcasting");
+        WalletLogPrintf("CommitTransaction(): transaction not submitted to mempool because wallet is not broadcasting transactions!\n");
+        return false;
+    }
+
+    std::string err_string;
+    if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) {
+        WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
+        return false;
+        // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
     }
 
     return true;