wallet: Avoid translating RPC errors when creating txs

Summary:
Also, mark feebumper bilingual_str as Untranslated

They are technical and have previously not been translated either.
It is questionable whether they can even appear in the GUI.

This is a partial backport of Cor [[bitcoin/bitcoin#18699 | PR18699]] : bitcoin/bitcoin@fae7776

Test Plan:
  ninja all check

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7703

Author: MarcoFalke

src/interfaces/wallet.cpp

@@ -235,7 +235,7 @@ namespace {
         createTransaction(const std::vector<CRecipient> &recipients,
                           const CCoinControl &coin_control, bool sign,
                           int &change_pos, Amount &fee,
-                          std::string &fail_reason) override {
+                          bilingual_str &fail_reason) override {
             auto locked_chain = m_wallet->chain().lock();
             LOCK(m_wallet->cs_wallet);
             CTransactionRef tx;

src/interfaces/wallet.h

@@ -35,6 +35,7 @@ typedef uint8_t isminefilter;
 enum class OutputType;
 struct CRecipient;
 struct TxId;
+struct bilingual_str;
 
 namespace interfaces {
 
@@ -156,7 +157,7 @@ class Wallet {
     createTransaction(const std::vector<CRecipient> &recipients,
                       const CCoinControl &coin_control, bool sign,
                       int &change_pos, Amount &fee,
-                      std::string &fail_reason) = 0;
+                      bilingual_str &fail_reason) = 0;
 
     //! Commit transaction.
     virtual void commitTransaction(CTransactionRef tx, WalletValueMap value_map,

src/qt/walletmodel.cpp

@@ -15,6 +15,7 @@
 #include <qt/transactiontablemodel.h>
 #include <ui_interface.h>
 #include <util/system.h> // for GetBoolArg
+#include <util/translation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/wallet.h>
 
@@ -197,12 +198,11 @@ WalletModel::prepareTransaction(WalletModelTransaction &transaction,
 
     Amount nFeeRequired = Amount::zero();
     int nChangePosRet = -1;
-    std::string strFailReason;
+    bilingual_str error;
 
     auto &newTx = transaction.getWtx();
-    newTx =
-        m_wallet->createTransaction(vecSend, coinControl, true /* sign */,
-                                    nChangePosRet, nFeeRequired, strFailReason);
+    newTx = m_wallet->createTransaction(vecSend, coinControl, true /* sign */,
+                                        nChangePosRet, nFeeRequired, error);
     transaction.setTransactionFee(nFeeRequired);
     if (fSubtractFeeFromAmount && newTx) {
         transaction.reassignAmounts(nChangePosRet);
@@ -212,7 +212,8 @@ WalletModel::prepareTransaction(WalletModelTransaction &transaction,
         if (!fSubtractFeeFromAmount && (total + nFeeRequired) > nBalance) {
             return SendCoinsReturn(AmountWithFeeExceedsBalance);
         }
-        Q_EMIT message(tr("Send Coins"), QString::fromStdString(strFailReason),
+        Q_EMIT message(tr("Send Coins"),
+                       QString::fromStdString(error.translated),
                        CClientUIInterface::MSG_ERROR);
         return TransactionCreationFailed;
     }

src/wallet/rpcwallet.cpp

@@ -377,7 +377,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock &locked_chain,
 
     // Create and send the transaction
     Amount nFeeRequired;
-    std::string strError;
+    bilingual_str error;
     std::vector<CRecipient> vecSend;
     int nChangePosRet = -1;
     CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
@@ -386,13 +386,13 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock &locked_chain,
     CCoinControl coinControl;
     CTransactionRef tx;
     if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, nFeeRequired,
-                                    nChangePosRet, strError, coinControl)) {
+                                    nChangePosRet, error, coinControl)) {
         if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) {
-            strError = strprintf("Error: This transaction requires a "
-                                 "transaction fee of at least %s",
-                                 FormatMoney(nFeeRequired));
+            error = strprintf(Untranslated("Error: This transaction requires a "
+                                           "transaction fee of at least %s"),
+                              FormatMoney(nFeeRequired));
         }
-        throw JSONRPCError(RPC_WALLET_ERROR, strError);
+        throw JSONRPCError(RPC_WALLET_ERROR, error.original);
     }
     pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */);
     return tx;
@@ -1065,14 +1065,14 @@ static UniValue sendmany(const Config &config, const JSONRPCRequest &request) {
     // Send
     Amount nFeeRequired = Amount::zero();
     int nChangePosRet = -1;
-    std::string strFailReason;
+    bilingual_str error;
     CTransactionRef tx;
     CCoinControl coinControl;
     bool fCreated =
         pwallet->CreateTransaction(*locked_chain, vecSend, tx, nFeeRequired,
-                                   nChangePosRet, strFailReason, coinControl);
+                                   nChangePosRet, error, coinControl);
     if (!fCreated) {
-        throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
+        throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
     }
     pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */);
     return tx->GetId().GetHex();
@@ -3670,12 +3670,12 @@ void FundTransaction(CWallet *const pwallet, CMutableTransaction &tx,
         setSubtractFeeFromOutputs.insert(pos);
     }
 
-    std::string strFailReason;
+    bilingual_str error;
 
-    if (!pwallet->FundTransaction(tx, fee_out, change_position, strFailReason,
+    if (!pwallet->FundTransaction(tx, fee_out, change_position, error,
                                   lockUnspents, setSubtractFeeFromOutputs,
                                   coinControl)) {
-        throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
+        throw JSONRPCError(RPC_WALLET_ERROR, error.original);
     }
 }
 

src/wallet/test/wallet_tests.cpp

@@ -9,6 +9,7 @@
 #include <node/context.h>
 #include <policy/policy.h>
 #include <rpc/server.h>
+#include <util/translation.h>
 #include <validation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/rpcdump.h>
@@ -521,7 +522,7 @@ class ListCoinsTestingSetup : public TestChain100Setup {
         CTransactionRef tx;
         Amount fee;
         int changePos = -1;
-        std::string error;
+        bilingual_str error;
         CCoinControl dummy;
         {
             auto locked_chain = m_chain->lock();

src/wallet/wallet.cpp

@@ -16,7 +16,6 @@
 #include <key_io.h>
 #include <policy/mempool.h>
 #include <policy/policy.h>
-#include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <random.h>
 #include <script/descriptor.h>
@@ -2736,7 +2735,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx) {
 }
 
 bool CWallet::FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
-                              int &nChangePosInOut, std::string &strFailReason,
+                              int &nChangePosInOut, bilingual_str &error,
                               bool lockUnspents,
                               const std::set<int> &setSubtractFeeFromOutputs,
                               CCoinControl coinControl) {
@@ -2763,8 +2762,7 @@ bool CWallet::FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
 
     CTransactionRef tx_new;
     if (!CreateTransaction(*locked_chain, vecSend, tx_new, nFeeRet,
-                           nChangePosInOut, strFailReason, coinControl,
-                           false)) {
+                           nChangePosInOut, error, coinControl, false)) {
         return false;
     }
 
@@ -2879,8 +2877,7 @@ CWallet::TransactionChangeType(OutputType change_type,
 bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                                 const std::vector<CRecipient> &vecSend,
                                 CTransactionRef &tx, Amount &nFeeRet,
-                                int &nChangePosInOut,
-                                std::string &strFailReason,
+                                int &nChangePosInOut, bilingual_str &error,
                                 const CCoinControl &coinControl, bool sign) {
     Amount nValue = Amount::zero();
     const OutputType change_type = TransactionChangeType(
@@ -2892,8 +2889,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
     unsigned int nSubtractFeeFromAmount = 0;
     for (const auto &recipient : vecSend) {
         if (nValue < Amount::zero() || recipient.nAmount < Amount::zero()) {
-            strFailReason =
-                _("Transaction amounts must not be negative").translated;
+            error = _("Transaction amounts must not be negative");
             return false;
         }
 
@@ -2905,8 +2901,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
     }
 
     if (vecSend.empty()) {
-        strFailReason =
-            _("Transaction must have at least one recipient").translated;
+        error = _("Transaction must have at least one recipient");
         return false;
     }
 
@@ -2946,18 +2941,14 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
 
             // Reserve a new key pair from key pool
             if (!CanGetAddresses(true)) {
-                strFailReason =
-                    _("Can't generate a change-address key. No keys in the "
-                      "internal keypool and can't generate any keys.")
-                        .translated;
+                error = _("Can't generate a change-address key. No keys in the "
+                          "internal keypool and can't generate any keys.");
                 return false;
             }
             CTxDestination dest;
             bool ret = reservedest.GetReservedDestination(dest, true);
             if (!ret) {
-                strFailReason =
-                    _("Keypool ran out, please call keypoolrefill first")
-                        .translated;
+                error = _("Keypool ran out, please call keypoolrefill first");
                 return false;
             }
 
@@ -3027,18 +3018,14 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                     if (recipient.fSubtractFeeFromAmount &&
                         nFeeRet > Amount::zero()) {
                         if (txout.nValue < Amount::zero()) {
-                            strFailReason = _("The transaction amount is too "
-                                              "small to pay the fee")
-                                                .translated;
+                            error = _("The transaction amount is too small to "
+                                      "pay the fee");
                         } else {
-                            strFailReason =
-                                _("The transaction amount is too small to "
-                                  "send after the fee has been deducted")
-                                    .translated;
+                            error = _("The transaction amount is too small to "
+                                      "send after the fee has been deducted");
                         }
                     } else {
-                        strFailReason =
-                            _("Transaction amount too small").translated;
+                        error = _("Transaction amount too small");
                     }
 
                     return false;
@@ -3065,7 +3052,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                         coin_selection_params.use_bnb = false;
                         continue;
                     } else {
-                        strFailReason = _("Insufficient funds").translated;
+                        error = _("Insufficient funds");
                         return false;
                     }
                 }
@@ -3090,8 +3077,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                         nChangePosInOut = GetRandInt(txNew.vout.size() + 1);
                     } else if ((unsigned int)nChangePosInOut >
                                txNew.vout.size()) {
-                        strFailReason =
-                            _("Change index out of range").translated;
+                        error = _("Change index out of range");
                         return false;
                     }
 
@@ -3113,7 +3099,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
             int nBytes = CalculateMaximumSignedTxSize(
                 txNewConst, this, coinControl.fAllowWatchOnly);
             if (nBytes < 0) {
-                strFailReason = _("Signing transaction failed").translated;
+                error = _("Signing transaction failed");
                 return false;
             }
 
@@ -3165,9 +3151,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                 // to pay for the new output and still meet nFeeNeeded.
                 // Or we should have just subtracted fee from recipients and
                 // nFeeNeeded should not have changed.
-                strFailReason =
-                    _("Transaction fee and change calculation failed")
-                        .translated;
+                error = _("Transaction fee and change calculation failed");
                 return false;
             }
 
@@ -3230,7 +3214,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                         MutableTransactionSignatureCreator(
                             &txNew, nIn, coin.txout.nValue, sigHashType),
                         scriptPubKey, sigdata)) {
-                    strFailReason = _("Signing transaction failed").translated;
+                    error = _("Signing transaction failed");
                     return false;
                 }
 
@@ -3244,23 +3228,22 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
 
         // Limit size.
         if (tx->GetTotalSize() > MAX_STANDARD_TX_SIZE) {
-            strFailReason = _("Transaction too large").translated;
+            error = _("Transaction too large");
             return false;
         }
     }
 
     if (nFeeRet > m_default_max_tx_fee) {
-        strFailReason =
-            TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
+        error = Untranslated(
+            TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED));
         return false;
     }
 
     if (gArgs.GetBoolArg("-walletrejectlongchains",
                          DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
         // Lastly, ensure this tx will pass the mempool's chain limits
         if (!chain().checkChainLimits(tx)) {
-            strFailReason =
-                _("Transaction has too long of a mempool chain").translated;
+            error = _("Transaction has too long of a mempool chain");
             return false;
         }
     }

src/wallet/wallet.h

@@ -1054,7 +1054,7 @@ class CWallet final : public WalletStorage,
      * CreateTransaction();
      */
     bool FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
-                         int &nChangePosInOut, std::string &strFailReason,
+                         int &nChangePosInOut, bilingual_str &error,
                          bool lockUnspents,
                          const std::set<int> &setSubtractFeeFromOutputs,
                          CCoinControl coinControl);
@@ -1070,7 +1070,7 @@ class CWallet final : public WalletStorage,
     bool CreateTransaction(interfaces::Chain::Lock &locked_chain,
                            const std::vector<CRecipient> &vecSend,
                            CTransactionRef &tx, Amount &nFeeRet,
-                           int &nChangePosInOut, std::string &strFailReason,
+                           int &nChangePosInOut, bilingual_str &error,
                            const CCoinControl &coin_control, bool sign = true);
     /**
      * Submit the transaction to the node's mempool and then relay to peers.