rpc: Use untranslated error strings in loadtxoutset #30395

1. 
   maflcko commented at 8:17 AM on July 5, 2024: member

   Motivation:

   • Some are not translated at all, anyway. See #30267 (review)
   • 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 #30267 (review)

   Also, while touching this:

   • Remove the trailing \n. See #30267 (review)
   • Add back the path. See #30267 (review)
   • Use named args to get the path.
2. 
   DrahtBot commented at 8:17 AM on July 5, 2024: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	tdb3, fjahr, ryanofsky
   Concept ACK	TheCharlatan

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #30383 (util: Catch translation string errors at compile time by maflcko)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot renamed this:
   rpc: Use untranslated error strings in loadtxoutset
   rpc: Use untranslated error strings in loadtxoutset
   on Jul 5, 2024
4. DrahtBot added the label RPC/REST/ZMQ on Jul 5, 2024
5. 
   TheCharlatan commented at 8:20 AM on July 5, 2024: contributor

   Concept ACK

6. maflcko force-pushed on Jul 5, 2024
7. bitcoin deleted a comment on Jul 5, 2024
8. maflcko force-pushed on Jul 5, 2024
9. DrahtBot added the label CI failed on Jul 5, 2024
10. 
    DrahtBot commented at 9:00 AM on July 5, 2024: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    _{Debug: https://github.com/bitcoin/bitcoin/runs/27074813459}

11. refactor: Use named arguments to get path arg in loadtxoutset fa45865778
12. maflcko force-pushed on Jul 5, 2024
13. DrahtBot removed the label CI failed on Jul 5, 2024
14. 
    fjahr commented at 1:12 PM on July 5, 2024: contributor

    utACK fa22edc1ef4d4456dcde883acae6f38af142d7bf

15. DrahtBot requested review from TheCharlatan on Jul 5, 2024
16. in src/rpc/blockchain.cpp:2836 in fa22edc1ef outdated

    2832 | @@ -2833,7 +2833,7 @@ static RPCHelpMan loadtxoutset()
    2833 |  
    2834 |      auto activation_result{chainman.ActivateSnapshot(afile, metadata, false)};
    2835 |      if (!activation_result) {
    2836 | -        throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf(_("Unable to load UTXO snapshot: %s\n"), util::ErrorString(activation_result)).original);
    2837 | +        throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Unable to load UTXO snapshot: %s. (%s)", util::ErrorString(activation_result).original, path.utf8string()));
    

    ---

    ---

    

    tdb3 commented at 3:49 PM on July 5, 2024:

    nit: It looks like there might be an extraneous period for at least some error messages (between the error message and the parenthesis-wrapped path).

    Created a snapshot with dumptxoutset. Modified a byte in the snapshot to induce failure during load.

    $ src/bitcoin-cli loadtxoutset mytxoutset.dat
    error code: -32603
    error message:
    Unable to load UTXO snapshot: assumeutxo block hash in snapshot metadata not recognized (hash: 767fccc851802c0180a3ebf1631ab978d4d6a5ae1d452dc26e9c177850708e6c, height: 101). The following snapshot heights are available: 110, 299.. (/home/dev/.bitcoin/regtest/mytxoutset.dat)
    

    ---

    

    maflcko commented at 3:57 PM on July 5, 2024:

    Thx, removed ..

17. 
    tdb3 commented at 3:49 PM on July 5, 2024: contributor

    Approach ACK

18. rpc: Use untranslated error strings in loadtxoutset fa5b8920be
19. maflcko force-pushed on Jul 5, 2024
20. tdb3 approved
21. 
    tdb3 commented at 4:12 PM on July 5, 2024: contributor

    ACK fa5b8920be041380fbfa4c7b443918637423d7a0

    Re-ran check in #30395 (review) . No extra period seen. Ran unit/functional tests locally, including feature_assumeutxo (all passed).

22. DrahtBot requested review from fjahr on Jul 5, 2024
23. 
    fjahr commented at 7:04 PM on July 5, 2024: contributor

    re-ACK fa5b8920be041380fbfa4c7b443918637423d7a0

24. in src/validation.cpp:5735 in fa5b8920be

    5731 | @@ -5732,7 +5732,7 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
    5732 |              static_cast<size_t>(current_coinstip_cache_size * SNAPSHOT_CACHE_PERC));
    5733 |      }
    5734 |  
    5735 | -    auto cleanup_bad_snapshot = [&](const char* reason) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
    5736 | +    auto cleanup_bad_snapshot = [&](bilingual_str&& reason) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
    

    ---

    ---

    

    ryanofsky commented at 6:16 PM on July 9, 2024:

    In commit "rpc: Use untranslated error strings in loadtxoutset" (fa5b8920be041380fbfa4c7b443918637423d7a0)

    Not important, but you could change this from bilingual_str&& to just bilingual_str so it is simpler, and function can be called naturally with both lvalues and rvalues, not just rvalues. Rvalues will be moved in any case.

25. ryanofsky approved
26. 
    ryanofsky commented at 6:17 PM on July 9, 2024: contributor

    Code review ACK fa5b8920be041380fbfa4c7b443918637423d7a0

27. ryanofsky merged this on Jul 9, 2024
28. ryanofsky closed this on Jul 9, 2024

    ---

29. maflcko deleted the branch on Jul 10, 2024
30. 
    hebasto commented at 3:35 PM on July 10, 2024: member

    Post-merge ACK fa5b8920be041380fbfa4c7b443918637423d7a0.

31. maflcko referenced this in commit fa5c253b4e on Jul 22, 2024
32. maflcko referenced this in commit faa5c86dbf on Jul 25, 2024
33. janus referenced this in commit bf86a15598 on Jan 6, 2025
34. Fabcien referenced this in commit ef4fb88903 on May 7, 2025
35. bitcoin locked this on Jul 10, 2025

Contributors

maflckoDrahtBotTheCharlatanfjahrtdb3ryanofskyhebasto

Review Requested
fjahrTheCharlatan