From a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f Mon Sep 17 00:00:00 2001
From: Ryan Ofsky <ryan@ofsky.org>
Date: Mon, 18 Nov 2024 10:29:55 -0500
Subject: [PATCH] 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 |  4 ++--
 src/wallet/scriptpubkeyman.h   |  4 ++--
 src/wallet/wallet.h            | 12 ++++++------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index cc115008442..d98a6d12a9f 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -1317,7 +1317,7 @@ bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize)
             return false;
         }
     }
-    if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName()));
+    if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during keypool top up. Cannot commit changes for wallet [%s]", m_storage.LogName()));
     NotifyCanGetAddressesChanged();
     // Note: Unlike with DescriptorSPKM, LegacySPKM does not need to call
     // m_storage.TopUpCallback() as we do not know what new scripts the LegacySPKM is
@@ -2230,7 +2230,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 9b62a35beda..83eac5d0329 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;
@@ -257,7 +257,7 @@ public:
     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...));
     };
 
     /** Watch-only address added */
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 4c7e55bd4b3..943733b215d 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -922,14 +922,14 @@ public:
     /** Determine if we are a legacy wallet */
     bool IsLegacy() const;
 
-    /** 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()};
@@ -940,7 +940,7 @@ public:
     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 */