From 01737883b3ff8051253c961b7dde50d055104ef9 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 15 Nov 2024 11:54:49 -0500 Subject: [PATCH 1/2] wallet: Translate [default wallet] string in progress messages Noticed while reviewing #31287 (https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843809721) that the [default wallet] part of progress messages remains untranslated while the rest of the string is translated. Fix this in all places where Wallet::ShowProgress (which has a cancel button) and chain::showProgress (which doesn't have a cancel button) are called by making "default wallet" into a translated string. To minimize scope of this bugfix, this introduces a new wallet DisplayName() method which behaves differently than the existing GetDisplayName() method. The existing method will be cleaned up in the following commit. --- src/wallet/wallet.cpp | 6 +++--- src/wallet/wallet.h | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6542abc23df..ede6188c68d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1817,7 +1817,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc fast_rescan_filter ? "fast variant using block filters" : "slow variant inspecting all blocks"); fAbortRescan = false; - ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption) + ShowProgress(strprintf("[%s] %s", DisplayName(), _("Rescanning…")), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption) uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash()); uint256 end_hash = tip_hash; if (max_height) chain().findAncestorByHeight(tip_hash, *max_height, FoundBlock().hash(end_hash)); @@ -1832,7 +1832,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc m_scanning_progress = 0; } if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) { - ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); + ShowProgress(strprintf("[%s] %s", DisplayName(), _("Rescanning…")), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); } bool next_interval = reserver.now() >= current_time + INTERVAL_TIME; @@ -1937,7 +1937,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Scanning current mempool transactions.\n"); WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); } - ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), 100); // hide progress dialog in GUI + ShowProgress(strprintf("[%s] %s", DisplayName(), _("Rescanning…")), 100); // hide progress dialog in GUI if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current); result.status = ScanResult::USER_ABORT; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0c4f89ccf5c..fa2833827c4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -912,6 +912,13 @@ public: return strprintf("[%s]", wallet_name); }; + /** Return wallet name for display, translating "default wallet" string if returned. */ + std::string DisplayName() const + { + std::string name{GetName()}; + return name.empty() ? _("default wallet") : name; + }; + /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ template void WalletLogPrintf(util::ConstevalFormatString wallet_fmt, const Params&... params) const From db225cea56b0531cc42d4b89dc61b02890f432ff Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 18 Nov 2024 10:29:55 -0500 Subject: [PATCH 2/2] wallet, refactor: Replace GetDisplayName() with LogName() The GetDisplayName() method name was confusing because it suggested the return value could be used for display, while documentation and implementation indicated it only meant to be used for logging. Also the name didn't suggest that it was formatting the wallet names, which made it harder understand how messages were formatted in the places it was called. Fix these issues by splitting up the GetDisplayName() method and replacing it with LogName() / DisplayName() methods. This commit is a refactoring that does not change any behavior. --- src/wallet/scriptpubkeyman.cpp | 2 +- src/wallet/scriptpubkeyman.h | 4 ++-- src/wallet/wallet.h | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d2ac171f8d3..40e017600d1 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -996,7 +996,7 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) WalletBatch batch(m_storage.GetDatabase()); if (!batch.TxnBegin()) return false; bool res = TopUpWithDB(batch, size); - if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName())); + if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet [%s]", m_storage.LogName())); return res; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index e41ab6d669d..33c6ea3dcad 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -43,7 +43,7 @@ class WalletStorage { public: virtual ~WalletStorage() = default; - virtual std::string GetDisplayName() const = 0; + virtual std::string LogName() const = 0; virtual WalletDatabase& GetDatabase() const = 0; virtual bool IsWalletFlagSet(uint64_t) const = 0; virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; @@ -162,7 +162,7 @@ public: template void WalletLogPrintf(util::ConstevalFormatString wallet_fmt, const Params&... params) const { - LogInfo("%s %s", m_storage.GetDisplayName(), tfm::format(wallet_fmt, params...)); + LogInfo("[%s] %s", m_storage.LogName(), tfm::format(wallet_fmt, params...)); }; /** Keypool has new keys */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fa2833827c4..fc4c55fb7e9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -905,14 +905,14 @@ public: /** Loads the flags into the wallet. (used by LoadWallet) */ bool LoadWalletFlags(uint64_t flags); - /** Returns a bracketed wallet name for displaying in logs, will return [default wallet] if the wallet has no name */ - std::string GetDisplayName() const override + /** Return wallet name for use in logs, will return "default wallet" if the wallet has no name. */ + std::string LogName() const override { - std::string wallet_name = GetName().length() == 0 ? "default wallet" : GetName(); - return strprintf("[%s]", wallet_name); + std::string name{GetName()}; + return name.empty() ? "default wallet" : name; }; - /** Return wallet name for display, translating "default wallet" string if returned. */ + /** Return wallet name for display, like LogName() but translates "default wallet" string. */ std::string DisplayName() const { std::string name{GetName()}; @@ -923,7 +923,7 @@ public: template void WalletLogPrintf(util::ConstevalFormatString wallet_fmt, const Params&... params) const { - LogInfo("%s %s", GetDisplayName(), tfm::format(wallet_fmt, params...)); + LogInfo("[%s] %s", LogName(), tfm::format(wallet_fmt, params...)); }; /** Upgrade the wallet */