Merge bitcoin/bitcoin#30395: rpc: Use untranslated error strings in loadtxoutset

fa5b892 rpc: Use untranslated error strings in loadtxoutset (MarcoFalke)
fa45865 refactor: Use named arguments to get path arg in loadtxoutset (MarcoFalke)

Pull request description:

  Motivation:
  * Some are not translated at all, anyway. See bitcoin/bitcoin#30267 (comment)
  * For others translation is not yet needed, because they are not called by the GUI (yet)
  * For others translations will never be needed, because they are RPC code. See bitcoin/bitcoin#30267 (comment)

  Also, while touching this:
  * Remove the trailing `\n`. See bitcoin/bitcoin#30267 (comment)
  * Add back the path. See bitcoin/bitcoin#30267 (comment)
  * Use named args to get the path.

ACKs for top commit:
  fjahr:
    re-ACK fa5b892
  tdb3:
    ACK fa5b892
  ryanofsky:
    Code review ACK fa5b892

Tree-SHA512: 46504dc5fd55a6274ef885dbe071aa9efb25bca247cd68cd86fb2ff066d70d295e0522e1fe42e63f1fdf7e4c89bd696220edaf06e33b804aba746492eafd852e

Author: ryanofsky

src/rpc/blockchain.cpp

@@ -2814,7 +2814,7 @@ static RPCHelpMan loadtxoutset()
 {
     NodeContext& node = EnsureAnyNodeContext(request.context);
     ChainstateManager& chainman = EnsureChainman(node);
-    fs::path path{AbsPathForConfigVal(EnsureArgsman(node), fs::u8path(request.params[0].get_str()))};
+    const fs::path path{AbsPathForConfigVal(EnsureArgsman(node), fs::u8path(self.Arg<std::string>("path")))};
 
     FILE* file{fsbridge::fopen(path, "rb")};
     AutoFile afile{file};
@@ -2833,7 +2833,7 @@ static RPCHelpMan loadtxoutset()
 
     auto activation_result{chainman.ActivateSnapshot(afile, metadata, false)};
     if (!activation_result) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf(_("Unable to load UTXO snapshot: %s\n"), util::ErrorString(activation_result)).original);
+        throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Unable to load UTXO snapshot: %s. (%s)", util::ErrorString(activation_result).original, path.utf8string()));
     }
 
     UniValue result(UniValue::VOBJ);

src/validation.cpp

@@ -5657,7 +5657,7 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
     int base_blockheight = metadata.m_base_blockheight;
 
     if (this->SnapshotBlockhash()) {
-        return util::Error{_("Can't activate a snapshot-based chainstate more than once")};
+        return util::Error{Untranslated("Can't activate a snapshot-based chainstate more than once")};
     }
 
     {
@@ -5666,26 +5666,26 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
         if (!GetParams().AssumeutxoForBlockhash(base_blockhash).has_value()) {
             auto available_heights = GetParams().GetAvailableSnapshotHeights();
             std::string heights_formatted = util::Join(available_heights, ", ", [&](const auto& i) { return util::ToString(i); });
-            return util::Error{strprintf(_("assumeutxo block hash in snapshot metadata not recognized (hash: %s, height: %s). The following snapshot heights are available: %s."),
+            return util::Error{strprintf(Untranslated("assumeutxo block hash in snapshot metadata not recognized (hash: %s, height: %s). The following snapshot heights are available: %s"),
                 base_blockhash.ToString(),
                 base_blockheight,
                 heights_formatted)};
         }
 
         CBlockIndex* snapshot_start_block = m_blockman.LookupBlockIndex(base_blockhash);
         if (!snapshot_start_block) {
-            return util::Error{strprintf(_("The base block header (%s) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again."),
+            return util::Error{strprintf(Untranslated("The base block header (%s) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again"),
                           base_blockhash.ToString())};
         }
 
         bool start_block_invalid = snapshot_start_block->nStatus & BLOCK_FAILED_MASK;
         if (start_block_invalid) {
-            return util::Error{strprintf(_("The base block header (%s) is part of an invalid chain."), base_blockhash.ToString())};
+            return util::Error{strprintf(Untranslated("The base block header (%s) is part of an invalid chain"), base_blockhash.ToString())};
         }
 
         auto mempool{m_active_chainstate->GetMempool()};
         if (mempool && mempool->size() > 0) {
-            return util::Error{_("Can't activate a snapshot when mempool not empty.")};
+            return util::Error{Untranslated("Can't activate a snapshot when mempool not empty")};
         }
     }
 
@@ -5734,7 +5734,7 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
             static_cast<size_t>(current_coinstip_cache_size * SNAPSHOT_CACHE_PERC));
     }
 
-    auto cleanup_bad_snapshot = [&](const char* reason) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+    auto cleanup_bad_snapshot = [&](bilingual_str&& reason) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         this->MaybeRebalanceCaches();
 
         // PopulateAndValidateSnapshot can return (in error) before the leveldb datadir
@@ -5750,12 +5750,12 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
                     "Manually remove it before restarting.\n"), fs::PathToString(*snapshot_datadir)));
             }
         }
-        return util::Error{_(reason)};
+        return util::Error{std::move(reason)};
     };
 
     if (!this->PopulateAndValidateSnapshot(*snapshot_chainstate, coins_file, metadata)) {
         LOCK(::cs_main);
-        return cleanup_bad_snapshot("population failed");
+        return cleanup_bad_snapshot(Untranslated("population failed"));
     }
 
     LOCK(::cs_main);  // cs_main required for rest of snapshot activation.
@@ -5764,13 +5764,13 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
     // work chain than the active chainstate; a user could have loaded a snapshot
     // very late in the IBD process, and we wouldn't want to load a useless chainstate.
     if (!CBlockIndexWorkComparator()(ActiveTip(), snapshot_chainstate->m_chain.Tip())) {
-        return cleanup_bad_snapshot("work does not exceed active chainstate");
+        return cleanup_bad_snapshot(Untranslated("work does not exceed active chainstate"));
     }
     // If not in-memory, persist the base blockhash for use during subsequent
     // initialization.
     if (!in_memory) {
         if (!node::WriteSnapshotBaseBlockhash(*snapshot_chainstate)) {
-            return cleanup_bad_snapshot("could not write base blockhash");
+            return cleanup_bad_snapshot(Untranslated("could not write base blockhash"));
         }
     }