wallet, refactor: Replace GetDisplayName() with LogName()

ryanofsky · ryanofsky · commit db225cea56b0 · 2025-06-02T08:46:03.000-05:00
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.
diff --git 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
@@ -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 @@ class ScriptPubKeyMan
     template <typename... Params>
     void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> 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
@@ -905,14 +905,14 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     /** 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 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     template <typename... Params>
     void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> 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 */

Original file line number	Diff line number	Diff line change
`@@ -996,7 +996,7 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size)`
`996`	`996`	`WalletBatch batch(m_storage.GetDatabase());`
`997`	`997`	`if (!batch.TxnBegin()) return false;`
`998`	`998`	`bool res = TopUpWithDB(batch, size);`
`999`		`- if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName()));`
	`999`	`+ if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet [%s]", m_storage.LogName()));`
`1000`	`1000`	`return res;`
`1001`	`1001`	`}`
`1002`	`1002`