refactor: Error message bilingual_str consistency

- Move the decision whether to translate an error message to where it is
  defined. This simplifies call sites: no more `InitError(Untranslated(...))`.

- Make all functions in `util/error.h` consistently return a
  `bilingual_str`. We've decided to use this as error message type so
  let's roll with it.

This has no functional changes: no messages are changed, no new
translation messages are defined.

Author: laanwj

src/init.cpp

@@ -1465,7 +1465,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
         if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
             AddLocal(addrLocal, LOCAL_MANUAL);
         else
-            return InitError(Untranslated(ResolveErrMsg("externalip", strAddr)));
+            return InitError(ResolveErrMsg("externalip", strAddr));
     }
 
     // Read asmap file if configured
@@ -1904,21 +1904,21 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
     for (const std::string& strBind : gArgs.GetArgs("-bind")) {
         CService addrBind;
         if (!Lookup(strBind, addrBind, GetListenPort(), false)) {
-            return InitError(Untranslated(ResolveErrMsg("bind", strBind)));
+            return InitError(ResolveErrMsg("bind", strBind));
         }
         connOptions.vBinds.push_back(addrBind);
     }
     for (const std::string& strBind : gArgs.GetArgs("-whitebind")) {
         NetWhitebindPermissions whitebind;
-        std::string error;
-        if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(Untranslated(error));
+        bilingual_str error;
+        if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error);
         connOptions.vWhiteBinds.push_back(whitebind);
     }
 
     for (const auto& net : gArgs.GetArgs("-whitelist")) {
         NetWhitelistPermissions subnet;
-        std::string error;
-        if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(Untranslated(error));
+        bilingual_str error;
+        if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error);
         connOptions.vWhitelistedRange.push_back(subnet);
     }
 

src/net_permissions.cpp

@@ -17,7 +17,7 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{
 };
 
 // The parse the following format "perm1,perm2@xxxxxx"
-bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, std::string& error)
+bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, bilingual_str& error)
 {
     NetPermissionFlags flags = PF_NONE;
     const auto atSeparator = str.find('@');
@@ -48,15 +48,15 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output,
             else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY);
             else if (permission.length() == 0); // Allow empty entries
             else {
-                error = strprintf(_("Invalid P2P permission: '%s'").translated, permission);
+                error = strprintf(_("Invalid P2P permission: '%s'"), permission);
                 return false;
             }
         }
         readen++;
     }
 
     output = flags;
-    error = "";
+    error = Untranslated("");
     return true;
 }
 
@@ -71,7 +71,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags)
     return strings;
 }
 
-bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error)
+bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error)
 {
     NetPermissionFlags flags;
     size_t offset;
@@ -84,17 +84,17 @@ bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermis
         return false;
     }
     if (addrBind.GetPort() == 0) {
-        error = strprintf(_("Need to specify a port with -whitebind: '%s'").translated, strBind);
+        error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind);
         return false;
     }
 
     output.m_flags = flags;
     output.m_service = addrBind;
-    error = "";
+    error = Untranslated("");
     return true;
 }
 
-bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error)
+bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error)
 {
     NetPermissionFlags flags;
     size_t offset;
@@ -104,12 +104,12 @@ bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermis
     CSubNet subnet;
     LookupSubNet(net, subnet);
     if (!subnet.IsValid()) {
-        error = strprintf(_("Invalid netmask specified in -whitelist: '%s'").translated, net);
+        error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net);
         return false;
     }
 
     output.m_flags = flags;
     output.m_subnet = subnet;
-    error = "";
+    error = Untranslated("");
     return true;
 }

src/net_permissions.h

@@ -10,6 +10,8 @@
 #ifndef BITCOIN_NET_PERMISSIONS_H
 #define BITCOIN_NET_PERMISSIONS_H
 
+struct bilingual_str;
+
 extern const std::vector<std::string> NET_PERMISSIONS_DOC;
 
 enum NetPermissionFlags
@@ -54,14 +56,14 @@ class NetPermissions
 class NetWhitebindPermissions : public NetPermissions
 {
 public:
-    static bool TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error);
+    static bool TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error);
     CService m_service;
 };
 
 class NetWhitelistPermissions : public NetPermissions
 {
 public:
-    static bool TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error);
+    static bool TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error);
     CSubNet m_subnet;
 };
 

src/rpc/util.cpp

@@ -10,6 +10,7 @@
 #include <tinyformat.h>
 #include <util/strencodings.h>
 #include <util/string.h>
+#include <util/translation.h>
 
 #include <tuple>
 
@@ -285,7 +286,7 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s
     if (err_string.length() > 0) {
         return JSONRPCError(RPCErrorFromTransactionError(terr), err_string);
     } else {
-        return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr));
+        return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr).original);
     }
 }
 

src/test/fuzz/kitchen_sink.cpp

@@ -7,6 +7,7 @@
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
 #include <util/error.h>
+#include <util/translation.h>
 
 #include <cstdint>
 #include <vector>

src/test/fuzz/net_permissions.cpp

@@ -6,6 +6,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <util/translation.h>
 
 #include <cassert>
 #include <cstdint>
@@ -29,7 +30,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
                                                                                          static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
 
     NetWhitebindPermissions net_whitebind_permissions;
-    std::string error_net_whitebind_permissions;
+    bilingual_str error_net_whitebind_permissions;
     if (NetWhitebindPermissions::TryParse(s, net_whitebind_permissions, error_net_whitebind_permissions)) {
         (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags);
         (void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags);
@@ -39,7 +40,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     }
 
     NetWhitelistPermissions net_whitelist_permissions;
-    std::string error_net_whitelist_permissions;
+    bilingual_str error_net_whitelist_permissions;
     if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, error_net_whitelist_permissions)) {
         (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags);
         (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags);

src/test/netbase_tests.cpp

@@ -6,6 +6,7 @@
 #include <netbase.h>
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
+#include <util/translation.h>
 
 #include <string>
 
@@ -325,15 +326,15 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork)
 
 BOOST_AUTO_TEST_CASE(netpermissions_test)
 {
-    std::string error;
+    bilingual_str error;
     NetWhitebindPermissions whitebindPermissions;
     NetWhitelistPermissions whitelistPermissions;
 
     // Detect invalid white bind
     BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
-    BOOST_CHECK(error.find("Cannot resolve -whitebind address") != std::string::npos);
+    BOOST_CHECK(error.original.find("Cannot resolve -whitebind address") != std::string::npos);
     BOOST_CHECK(!NetWhitebindPermissions::TryParse("127.0.0.1", whitebindPermissions, error));
-    BOOST_CHECK(error.find("Need to specify a port with -whitebind") != std::string::npos);
+    BOOST_CHECK(error.original.find("Need to specify a port with -whitebind") != std::string::npos);
     BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
 
     // If no permission flags, assume backward compatibility
@@ -377,11 +378,11 @@ BOOST_AUTO_TEST_CASE(netpermissions_test)
 
     // Detect invalid flag
     BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,[email protected]:32", whitebindPermissions, error));
-    BOOST_CHECK(error.find("Invalid P2P permission") != std::string::npos);
+    BOOST_CHECK(error.original.find("Invalid P2P permission") != std::string::npos);
 
     // Check whitelist error
     BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,[email protected]:32", whitelistPermissions, error));
-    BOOST_CHECK(error.find("Invalid netmask specified in -whitelist") != std::string::npos);
+    BOOST_CHECK(error.original.find("Invalid netmask specified in -whitelist") != std::string::npos);
 
     // Happy path for whitelist parsing
     BOOST_CHECK(NetWhitelistPermissions::TryParse("[email protected]", whitelistPermissions, error));

src/util/error.cpp

@@ -8,37 +8,37 @@
 #include <util/system.h>
 #include <util/translation.h>
 
-std::string TransactionErrorString(const TransactionError err)
+bilingual_str TransactionErrorString(const TransactionError err)
 {
     switch (err) {
         case TransactionError::OK:
-            return "No error";
+            return Untranslated("No error");
         case TransactionError::MISSING_INPUTS:
-            return "Missing inputs";
+            return Untranslated("Missing inputs");
         case TransactionError::ALREADY_IN_CHAIN:
-            return "Transaction already in block chain";
+            return Untranslated("Transaction already in block chain");
         case TransactionError::P2P_DISABLED:
-            return "Peer-to-peer functionality missing or disabled";
+            return Untranslated("Peer-to-peer functionality missing or disabled");
         case TransactionError::MEMPOOL_REJECTED:
-            return "Transaction rejected by AcceptToMemoryPool";
+            return Untranslated("Transaction rejected by AcceptToMemoryPool");
         case TransactionError::MEMPOOL_ERROR:
-            return "AcceptToMemoryPool failed";
+            return Untranslated("AcceptToMemoryPool failed");
         case TransactionError::INVALID_PSBT:
-            return "PSBT is not sane";
+            return Untranslated("PSBT is not sane");
         case TransactionError::PSBT_MISMATCH:
-            return "PSBTs not compatible (different transactions)";
+            return Untranslated("PSBTs not compatible (different transactions)");
         case TransactionError::SIGHASH_MISMATCH:
-            return "Specified sighash value does not match existing value";
+            return Untranslated("Specified sighash value does not match existing value");
         case TransactionError::MAX_FEE_EXCEEDED:
-            return "Fee exceeds maximum configured by -maxtxfee";
+            return Untranslated("Fee exceeds maximum configured by -maxtxfee");
         // no default case, so the compiler can warn about missing cases
     }
     assert(false);
 }
 
-std::string ResolveErrMsg(const std::string& optname, const std::string& strBind)
+bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind)
 {
-    return strprintf(_("Cannot resolve -%s address: '%s'").translated, optname, strBind);
+    return strprintf(_("Cannot resolve -%s address: '%s'"), optname, strBind);
 }
 
 bilingual_str AmountHighWarn(const std::string& optname)

src/util/error.h

@@ -32,9 +32,9 @@ enum class TransactionError {
     MAX_FEE_EXCEEDED,
 };
 
-std::string TransactionErrorString(const TransactionError error);
+bilingual_str TransactionErrorString(const TransactionError error);
 
-std::string ResolveErrMsg(const std::string& optname, const std::string& strBind);
+bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind);
 
 bilingual_str AmountHighWarn(const std::string& optname);
 

src/wallet/wallet.cpp

@@ -2997,7 +2997,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
     }
 
     if (nFeeRet > m_default_max_tx_fee) {
-        error = Untranslated(TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED));
+        error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
         return false;
     }