wallet: Avoid translating RPC errors when creating txs

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.

Author: MarcoFalke

src/interfaces/wallet.cpp

@@ -219,7 +219,7 @@ class WalletImpl : public Wallet
         bool sign,
         int& change_pos,
         CAmount& fee,
-        std::string& fail_reason) override
+        bilingual_str& fail_reason) override
     {
         LOCK(m_wallet->cs_wallet);
         CTransactionRef tx;
@@ -248,7 +248,7 @@ class WalletImpl : public Wallet
     }
     bool createBumpTransaction(const uint256& txid,
         const CCoinControl& coin_control,
-        std::vector<std::string>& errors,
+        std::vector<bilingual_str>& errors,
         CAmount& old_fee,
         CAmount& new_fee,
         CMutableTransaction& mtx) override
@@ -258,7 +258,7 @@ class WalletImpl : public Wallet
     bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
     bool commitBumpTransaction(const uint256& txid,
         CMutableTransaction&& mtx,
-        std::vector<std::string>& errors,
+        std::vector<bilingual_str>& errors,
         uint256& bumped_txid) override
     {
         return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==

src/interfaces/wallet.h

@@ -31,6 +31,7 @@ enum class TransactionError;
 enum isminetype : unsigned int;
 struct CRecipient;
 struct PartiallySignedTransaction;
+struct bilingual_str;
 typedef uint8_t isminefilter;
 
 namespace interfaces {
@@ -136,7 +137,7 @@ class Wallet
         bool sign,
         int& change_pos,
         CAmount& fee,
-        std::string& fail_reason) = 0;
+        bilingual_str& fail_reason) = 0;
 
     //! Commit transaction.
     virtual void commitTransaction(CTransactionRef tx,
@@ -155,7 +156,7 @@ class Wallet
     //! Create bump transaction.
     virtual bool createBumpTransaction(const uint256& txid,
         const CCoinControl& coin_control,
-        std::vector<std::string>& errors,
+        std::vector<bilingual_str>& errors,
         CAmount& old_fee,
         CAmount& new_fee,
         CMutableTransaction& mtx) = 0;
@@ -166,7 +167,7 @@ class Wallet
     //! Commit bump transaction.
     virtual bool commitBumpTransaction(const uint256& txid,
         CMutableTransaction&& mtx,
-        std::vector<std::string>& errors,
+        std::vector<bilingual_str>& errors,
         uint256& bumped_txid) = 0;
 
     //! Get a transaction.

src/qt/walletmodel.cpp

@@ -24,6 +24,7 @@
 #include <psbt.h>
 #include <ui_interface.h>
 #include <util/system.h> // for GetBoolArg
+#include <util/translation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/wallet.h> // for CRecipient
 
@@ -185,10 +186,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
     {
         CAmount nFeeRequired = 0;
         int nChangePosRet = -1;
-        std::string strFailReason;
+        bilingual_str error;
 
         auto& newTx = transaction.getWtx();
-        newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, strFailReason);
+        newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
         transaction.setTransactionFee(nFeeRequired);
         if (fSubtractFeeFromAmount && newTx)
             transaction.reassignAmounts(nChangePosRet);
@@ -199,8 +200,8 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
             {
                 return SendCoinsReturn(AmountWithFeeExceedsBalance);
             }
-            Q_EMIT message(tr("Send Coins"), QString::fromStdString(strFailReason),
-                         CClientUIInterface::MSG_ERROR);
+            Q_EMIT message(tr("Send Coins"), QString::fromStdString(error.translated),
+                CClientUIInterface::MSG_ERROR);
             return TransactionCreationFailed;
         }
 
@@ -482,14 +483,14 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
 {
     CCoinControl coin_control;
     coin_control.m_signal_bip125_rbf = true;
-    std::vector<std::string> errors;
+    std::vector<bilingual_str> errors;
     CAmount old_fee;
     CAmount new_fee;
     CMutableTransaction mtx;
     if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
         QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
-            (errors.size() ? QString::fromStdString(errors[0]) : "") +")");
-         return false;
+            (errors.size() ? QString::fromStdString(errors[0].translated) : "") +")");
+        return false;
     }
 
     const bool create_psbt = m_wallet->privateKeysDisabled();
@@ -551,8 +552,8 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
     // commit the bumped transaction
     if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, new_hash)) {
         QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Could not commit transaction") + "<br />(" +
-            QString::fromStdString(errors[0])+")");
-         return false;
+            QString::fromStdString(errors[0].translated)+")");
+        return false;
     }
     return true;
 }

src/wallet/feebumper.cpp

@@ -3,52 +3,53 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <interfaces/chain.h>
-#include <wallet/coincontrol.h>
-#include <wallet/feebumper.h>
-#include <wallet/fees.h>
-#include <wallet/wallet.h>
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
 #include <util/system.h>
+#include <util/translation.h>
+#include <wallet/coincontrol.h>
+#include <wallet/feebumper.h>
+#include <wallet/fees.h>
+#include <wallet/wallet.h>
 
 //! Check whether transaction has descendant in wallet or mempool, or has been
 //! mined, or conflicts with a mined transaction. Return a feebumper::Result.
-static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     if (wallet.HasWalletSpend(wtx.GetHash())) {
-        errors.push_back("Transaction has descendants in the wallet");
+        errors.push_back(Untranslated("Transaction has descendants in the wallet"));
         return feebumper::Result::INVALID_PARAMETER;
     }
 
     {
         if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
-            errors.push_back("Transaction has descendants in the mempool");
+            errors.push_back(Untranslated("Transaction has descendants in the mempool"));
             return feebumper::Result::INVALID_PARAMETER;
         }
     }
 
     if (wtx.GetDepthInMainChain() != 0) {
-        errors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
+        errors.push_back(Untranslated("Transaction has been mined, or is conflicted with a mined transaction"));
         return feebumper::Result::WALLET_ERROR;
     }
 
     if (!SignalsOptInRBF(*wtx.tx)) {
-        errors.push_back("Transaction is not BIP 125 replaceable");
+        errors.push_back(Untranslated("Transaction is not BIP 125 replaceable"));
         return feebumper::Result::WALLET_ERROR;
     }
 
     if (wtx.mapValue.count("replaced_by_txid")) {
-        errors.push_back(strprintf("Cannot bump transaction %s which was already bumped by transaction %s", wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
+        errors.push_back(strprintf(Untranslated("Cannot bump transaction %s which was already bumped by transaction %s"), wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
         return feebumper::Result::WALLET_ERROR;
     }
 
     // check that original tx consists entirely of our inputs
     // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
     isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
     if (!wallet.IsAllFromMe(*wtx.tx, filter)) {
-        errors.push_back("Transaction contains inputs that don't belong to this wallet");
+        errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
         return feebumper::Result::WALLET_ERROR;
     }
 
@@ -57,7 +58,8 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
 }
 
 //! Check if the user provided a valid feeRate
-static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) {
+static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<bilingual_str>& errors)
+{
     // check that fee rate is higher than mempool's minimum fee
     // (no point in bumping fee if we know that the new tx won't be accepted to the mempool)
     // This may occur if the user set fee_rate or paytxfee too low, if fallbackfee is too low, or, perhaps,
@@ -67,7 +69,7 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt
 
     if (newFeerate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
         errors.push_back(strprintf(
-            "New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- ",
+            Untranslated("New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- "),
             FormatMoney(newFeerate.GetFeePerK()),
             FormatMoney(minMempoolFeeRate.GetFeePerK())));
         return feebumper::Result::WALLET_ERROR;
@@ -86,23 +88,23 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt
     CAmount minTotalFee = nOldFeeRate.GetFee(maxTxSize) + incrementalRelayFee.GetFee(maxTxSize);
 
     if (new_total_fee < minTotalFee) {
-        errors.push_back(strprintf("Insufficient total fee %s, must be at least %s (oldFee %s + incrementalFee %s)",
+        errors.push_back(strprintf(Untranslated("Insufficient total fee %s, must be at least %s (oldFee %s + incrementalFee %s)"),
             FormatMoney(new_total_fee), FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxTxSize)), FormatMoney(incrementalRelayFee.GetFee(maxTxSize))));
         return feebumper::Result::INVALID_PARAMETER;
     }
 
     CAmount requiredFee = GetRequiredFee(wallet, maxTxSize);
     if (new_total_fee < requiredFee) {
-        errors.push_back(strprintf("Insufficient total fee (cannot be less than required fee %s)",
+        errors.push_back(strprintf(Untranslated("Insufficient total fee (cannot be less than required fee %s)"),
             FormatMoney(requiredFee)));
         return feebumper::Result::INVALID_PARAMETER;
     }
 
     // Check that in all cases the new fee doesn't violate maxTxFee
     const CAmount max_tx_fee = wallet.m_default_max_tx_fee;
     if (new_total_fee > max_tx_fee) {
-        errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)",
-                            FormatMoney(new_total_fee), FormatMoney(max_tx_fee)));
+        errors.push_back(strprintf(Untranslated("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)"),
+            FormatMoney(new_total_fee), FormatMoney(max_tx_fee)));
         return feebumper::Result::WALLET_ERROR;
     }
 
@@ -144,12 +146,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
     const CWalletTx* wtx = wallet.GetWalletTx(txid);
     if (wtx == nullptr) return false;
 
-    std::vector<std::string> errors_dummy;
+    std::vector<bilingual_str> errors_dummy;
     feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
     return res == feebumper::Result::OK;
 }
 
-Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<std::string>& errors,
+Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
                                  CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
 {
     // We are going to modify coin control later, copy to re-use
@@ -159,7 +161,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     errors.clear();
     auto it = wallet.mapWallet.find(txid);
     if (it == wallet.mapWallet.end()) {
-        errors.push_back("Invalid or non-wallet transaction id");
+        errors.push_back(Untranslated("Invalid or non-wallet transaction id"));
         return Result::INVALID_ADDRESS_OR_KEY;
     }
     const CWalletTx& wtx = it->second;
@@ -216,9 +218,9 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     CTransactionRef tx_new = MakeTransactionRef();
     CAmount fee_ret;
     int change_pos_in_out = -1; // No requested location for change
-    std::string fail_reason;
+    bilingual_str fail_reason;
     if (!wallet.CreateTransaction(recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
-        errors.push_back("Unable to create transaction: " + fail_reason);
+        errors.push_back(Untranslated("Unable to create transaction: ") + Untranslated(" ") + fail_reason);
         return Result::WALLET_ERROR;
     }
 
@@ -242,15 +244,15 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
     return wallet.SignTransaction(mtx);
 }
 
-Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid)
+Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, uint256& bumped_txid)
 {
     LOCK(wallet.cs_wallet);
     if (!errors.empty()) {
         return Result::MISC_ERROR;
     }
     auto it = txid.IsNull() ? wallet.mapWallet.end() : wallet.mapWallet.find(txid);
     if (it == wallet.mapWallet.end()) {
-        errors.push_back("Invalid or non-wallet transaction id");
+        errors.push_back(Untranslated("Invalid or non-wallet transaction id"));
         return Result::MISC_ERROR;
     }
     CWalletTx& oldWtx = it->second;
@@ -275,7 +277,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti
         // along with an exception. It would be good to return information about
         // wtxBumped to the caller even if marking the original transaction
         // replaced does not succeed for some reason.
-        errors.push_back("Created new bumpfee transaction but could not mark the original transaction as replaced");
+        errors.push_back(Untranslated("Created new bumpfee transaction but could not mark the original transaction as replaced"));
     }
     return Result::OK;
 }

src/wallet/feebumper.h

@@ -12,6 +12,7 @@ class CWalletTx;
 class uint256;
 class CCoinControl;
 enum class FeeEstimateMode;
+struct bilingual_str;
 
 namespace feebumper {
 
@@ -30,12 +31,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
 
 //! Create bumpfee transaction based on feerate estimates.
 Result CreateRateBumpTransaction(CWallet& wallet,
-                         const uint256& txid,
-                         const CCoinControl& coin_control,
-                         std::vector<std::string>& errors,
-                         CAmount& old_fee,
-                         CAmount& new_fee,
-                         CMutableTransaction& mtx);
+    const uint256& txid,
+    const CCoinControl& coin_control,
+    std::vector<bilingual_str>& errors,
+    CAmount& old_fee,
+    CAmount& new_fee,
+    CMutableTransaction& mtx);
 
 //! Sign the new transaction,
 //! @return false if the tx couldn't be found or if it was
@@ -47,10 +48,10 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx);
 //! but sets errors if the tx could not be added to the mempool (will try later)
 //! or if the old transaction could not be marked as replaced.
 Result CommitTransaction(CWallet& wallet,
-                         const uint256& txid,
-                         CMutableTransaction&& mtx,
-                         std::vector<std::string>& errors,
-                         uint256& bumped_txid);
+    const uint256& txid,
+    CMutableTransaction&& mtx,
+    std::vector<bilingual_str>& errors,
+    uint256& bumped_txid);
 
 } // namespace feebumper