Skip to content

Commit 23c926d

Browse files
laanwjlaanwj
committed
Merge #18699: wallet: Avoid translating RPC errors
fa2cce4 wallet: Remove trailing whitespace from potential translation strings (MarcoFalke) fa59cc1 wallet: Report full error message in wallettool (MarcoFalke) fae7776 wallet: Avoid translating RPC errors when creating txs (MarcoFalke) fae51a5 wallet: Avoid translating RPC errors when loading wallets (MarcoFalke) Pull request description: Common errors and warnings should be translated when displayed in the GUI, but not translated when displayed elsewhere. The wallet method `CreateWalletFromFile` does not know its caller, so this commit changes it to return a `bilingual_str` to the caller. Fixes #17072 ACKs for top commit: laanwj: ACK fa2cce4, checked that no new translation messages are added compared to master. hebasto: ACK fa2cce4 Tree-SHA512: c6a943ae9c3689ea3c48c20d26de6e4970de0257a1f1eec57a2bded67a4af9dcc5c45b2d64659d6fb4c4bc4d8103e28483ea3d14bb850df8db0ff9e8e5c77ee2
2 parents 42fd503 + fa2cce4 commit 23c926d

26 files changed

+260
-221
lines changed

‎src/dummywallet.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
class CWallet;
1010
enum class WalletCreationStatus;
11+
struct bilingual_str;
1112

1213
namespace interfaces {
1314
class Chain;
@@ -72,12 +73,12 @@ std::vector<std::shared_ptr<CWallet>> GetWallets()
7273
throw std::logic_error("Wallet function called in non-wallet build.");
7374
}
7475

75-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector<std::string>& warnings)
76+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings)
7677
{
7778
throw std::logic_error("Wallet function called in non-wallet build.");
7879
}
7980

80-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, std::shared_ptr<CWallet>& result)
81+
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
8182
{
8283
throw std::logic_error("Wallet function called in non-wallet build.");
8384
}

‎src/interfaces/node.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ class CWallet;
4343
fs::path GetWalletDir();
4444
std::vector<fs::path> ListWalletDir();
4545
std::vector<std::shared_ptr<CWallet>> GetWallets();
46-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector<std::string>& warnings);
47-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, std::shared_ptr<CWallet>& result);
46+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings);
47+
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result);
4848
std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet);
4949

5050
namespace interfaces {
@@ -259,11 +259,11 @@ class NodeImpl : public Node
259259
}
260260
return wallets;
261261
}
262-
std::unique_ptr<Wallet> loadWallet(const std::string& name, std::string& error, std::vector<std::string>& warnings) override
262+
std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
263263
{
264264
return MakeWallet(LoadWallet(*m_context.chain, name, error, warnings));
265265
}
266-
std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, WalletCreationStatus& status) override
266+
std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override
267267
{
268268
std::shared_ptr<CWallet> wallet;
269269
status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet);

‎src/interfaces/node.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ class Coin;
2727
class RPCTimerInterface;
2828
class UniValue;
2929
class proxyType;
30+
enum class WalletCreationStatus;
3031
struct CNodeStateStats;
3132
struct NodeContext;
32-
enum class WalletCreationStatus;
33+
struct bilingual_str;
3334

3435
namespace interfaces {
3536
class Handler;
@@ -201,10 +202,10 @@ class Node
201202
//! Attempts to load a wallet from file or directory.
202203
//! The loaded wallet is also notified to handlers previously registered
203204
//! with handleLoadWallet.
204-
virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, std::string& error, std::vector<std::string>& warnings) = 0;
205+
virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
205206

206207
//! Create a wallet from file
207-
virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::vector<std::string>& warnings, WalletCreationStatus& status) = 0;
208+
virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) = 0;
208209

209210
//! Register handler for init messages.
210211
using InitMessageFn = std::function<void(const std::string& message)>;

‎src/interfaces/wallet.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class WalletImpl : public Wallet
219219
bool sign,
220220
int& change_pos,
221221
CAmount& fee,
222-
std::string& fail_reason) override
222+
bilingual_str& fail_reason) override
223223
{
224224
LOCK(m_wallet->cs_wallet);
225225
CTransactionRef tx;
@@ -248,7 +248,7 @@ class WalletImpl : public Wallet
248248
}
249249
bool createBumpTransaction(const uint256& txid,
250250
const CCoinControl& coin_control,
251-
std::vector<std::string>& errors,
251+
std::vector<bilingual_str>& errors,
252252
CAmount& old_fee,
253253
CAmount& new_fee,
254254
CMutableTransaction& mtx) override
@@ -258,7 +258,7 @@ class WalletImpl : public Wallet
258258
bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
259259
bool commitBumpTransaction(const uint256& txid,
260260
CMutableTransaction&& mtx,
261-
std::vector<std::string>& errors,
261+
std::vector<bilingual_str>& errors,
262262
uint256& bumped_txid) override
263263
{
264264
return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==

‎src/interfaces/wallet.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ enum class TransactionError;
3131
enum isminetype : unsigned int;
3232
struct CRecipient;
3333
struct PartiallySignedTransaction;
34+
struct bilingual_str;
3435
typedef uint8_t isminefilter;
3536

3637
namespace interfaces {
@@ -136,7 +137,7 @@ class Wallet
136137
bool sign,
137138
int& change_pos,
138139
CAmount& fee,
139-
std::string& fail_reason) = 0;
140+
bilingual_str& fail_reason) = 0;
140141

141142
//! Commit transaction.
142143
virtual void commitTransaction(CTransactionRef tx,
@@ -155,7 +156,7 @@ class Wallet
155156
//! Create bump transaction.
156157
virtual bool createBumpTransaction(const uint256& txid,
157158
const CCoinControl& coin_control,
158-
std::vector<std::string>& errors,
159+
std::vector<bilingual_str>& errors,
159160
CAmount& old_fee,
160161
CAmount& new_fee,
161162
CMutableTransaction& mtx) = 0;
@@ -166,7 +167,7 @@ class Wallet
166167
//! Commit bump transaction.
167168
virtual bool commitBumpTransaction(const uint256& txid,
168169
CMutableTransaction&& mtx,
169-
std::vector<std::string>& errors,
170+
std::vector<bilingual_str>& errors,
170171
uint256& bumped_txid) = 0;
171172

172173
//! Get a transaction.

‎src/qt/walletcontroller.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <interfaces/handler.h>
1515
#include <interfaces/node.h>
1616
#include <util/string.h>
17+
#include <util/translation.h>
1718
#include <wallet/wallet.h>
1819

1920
#include <algorithm>
@@ -244,10 +245,10 @@ void CreateWalletActivity::finish()
244245
{
245246
destroyProgressDialog();
246247

247-
if (!m_error_message.empty()) {
248-
QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message));
248+
if (!m_error_message.original.empty()) {
249+
QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message.translated));
249250
} else if (!m_warning_message.empty()) {
250-
QMessageBox::warning(m_parent_widget, tr("Create wallet warning"), QString::fromStdString(Join(m_warning_message, "\n")));
251+
QMessageBox::warning(m_parent_widget, tr("Create wallet warning"), QString::fromStdString(Join(m_warning_message, "\n", OpTranslated)));
251252
}
252253

253254
if (m_wallet_model) Q_EMIT created(m_wallet_model);
@@ -285,10 +286,10 @@ void OpenWalletActivity::finish()
285286
{
286287
destroyProgressDialog();
287288

288-
if (!m_error_message.empty()) {
289-
QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message));
289+
if (!m_error_message.original.empty()) {
290+
QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message.translated));
290291
} else if (!m_warning_message.empty()) {
291-
QMessageBox::warning(m_parent_widget, tr("Open wallet warning"), QString::fromStdString(Join(m_warning_message, "\n")));
292+
QMessageBox::warning(m_parent_widget, tr("Open wallet warning"), QString::fromStdString(Join(m_warning_message, "\n", OpTranslated)));
292293
}
293294

294295
if (m_wallet_model) Q_EMIT opened(m_wallet_model);

‎src/qt/walletcontroller.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <qt/sendcoinsrecipient.h>
99
#include <support/allocators/secure.h>
1010
#include <sync.h>
11+
#include <util/translation.h>
1112

1213
#include <map>
1314
#include <memory>
@@ -104,8 +105,8 @@ class WalletControllerActivity : public QObject
104105
QWidget* const m_parent_widget;
105106
QProgressDialog* m_progress_dialog{nullptr};
106107
WalletModel* m_wallet_model{nullptr};
107-
std::string m_error_message;
108-
std::vector<std::string> m_warning_message;
108+
bilingual_str m_error_message;
109+
std::vector<bilingual_str> m_warning_message;
109110
};
110111

111112

‎src/qt/walletmodel.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <psbt.h>
2525
#include <ui_interface.h>
2626
#include <util/system.h> // for GetBoolArg
27+
#include <util/translation.h>
2728
#include <wallet/coincontrol.h>
2829
#include <wallet/wallet.h> // for CRecipient
2930

@@ -185,10 +186,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
185186
{
186187
CAmount nFeeRequired = 0;
187188
int nChangePosRet = -1;
188-
std::string strFailReason;
189+
bilingual_str error;
189190

190191
auto& newTx = transaction.getWtx();
191-
newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, strFailReason);
192+
newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
192193
transaction.setTransactionFee(nFeeRequired);
193194
if (fSubtractFeeFromAmount && newTx)
194195
transaction.reassignAmounts(nChangePosRet);
@@ -199,8 +200,8 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
199200
{
200201
return SendCoinsReturn(AmountWithFeeExceedsBalance);
201202
}
202-
Q_EMIT message(tr("Send Coins"), QString::fromStdString(strFailReason),
203-
CClientUIInterface::MSG_ERROR);
203+
Q_EMIT message(tr("Send Coins"), QString::fromStdString(error.translated),
204+
CClientUIInterface::MSG_ERROR);
204205
return TransactionCreationFailed;
205206
}
206207

@@ -482,14 +483,14 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
482483
{
483484
CCoinControl coin_control;
484485
coin_control.m_signal_bip125_rbf = true;
485-
std::vector<std::string> errors;
486+
std::vector<bilingual_str> errors;
486487
CAmount old_fee;
487488
CAmount new_fee;
488489
CMutableTransaction mtx;
489490
if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
490491
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
491-
(errors.size() ? QString::fromStdString(errors[0]) : "") +")");
492-
return false;
492+
(errors.size() ? QString::fromStdString(errors[0].translated) : "") +")");
493+
return false;
493494
}
494495

495496
const bool create_psbt = m_wallet->privateKeysDisabled();
@@ -551,8 +552,8 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
551552
// commit the bumped transaction
552553
if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, new_hash)) {
553554
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Could not commit transaction") + "<br />(" +
554-
QString::fromStdString(errors[0])+")");
555-
return false;
555+
QString::fromStdString(errors[0].translated)+")");
556+
return false;
556557
}
557558
return true;
558559
}

‎src/util/translation.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@ struct bilingual_str {
1818
std::string translated;
1919
};
2020

21+
inline bilingual_str operator+(const bilingual_str& lhs, const bilingual_str& rhs)
22+
{
23+
return bilingual_str{
24+
lhs.original + rhs.original,
25+
lhs.translated + rhs.translated};
26+
}
27+
28+
/** Mark a bilingual_str as untranslated */
29+
inline static bilingual_str Untranslated(std::string original) { return {original, original}; }
30+
/** Unary operator to return the original */
31+
inline static std::string OpOriginal(const bilingual_str& b) { return b.original; }
32+
/** Unary operator to return the translation */
33+
inline static std::string OpTranslated(const bilingual_str& b) { return b.translated; }
34+
2135
namespace tinyformat {
2236
template <typename... Args>
2337
bilingual_str format(const bilingual_str& fmt, const Args&... args)

‎src/wallet/db.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, boo
393393
return fSuccess;
394394
}
395395

396-
bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, std::string& errorStr)
396+
bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr)
397397
{
398398
std::string walletFile;
399399
std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile);
@@ -403,14 +403,14 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, std::string& er
403403
LogPrintf("Using wallet %s\n", file_path.string());
404404

405405
if (!env->Open(true /* retry */)) {
406-
errorStr = strprintf(_("Error initializing wallet database environment %s!").translated, walletDir);
406+
errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir);
407407
return false;
408408
}
409409

410410
return true;
411411
}
412412

413-
bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<std::string>& warnings, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc)
413+
bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<bilingual_str>& warnings, bilingual_str& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc)
414414
{
415415
std::string walletFile;
416416
std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile);
@@ -425,12 +425,12 @@ bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, std::vector<st
425425
warnings.push_back(strprintf(_("Warning: Wallet file corrupt, data salvaged!"
426426
" Original %s saved as %s in %s; if"
427427
" your balance or transactions are incorrect you should"
428-
" restore from a backup.").translated,
428+
" restore from a backup."),
429429
walletFile, backup_filename, walletDir));
430430
}
431431
if (r == BerkeleyEnvironment::VerifyResult::RECOVER_FAIL)
432432
{
433-
errorStr = strprintf(_("%s corrupt, salvage failed").translated, walletFile);
433+
errorStr = strprintf(_("%s corrupt, salvage failed"), walletFile);
434434
return false;
435435
}
436436
}

0 commit comments

Comments
 (0)