wallet: Use bilingual_str for errors #22337

achow101 commented at 6:15 PM on June 24, 2021: member

In a couple of places in the wallet, errors are std::string. In order for these errors to be translated, change them to use bilingual_str.

DrahtBot added the label RPC/REST/ZMQ on Jun 24, 2021

DrahtBot added the label Utils/log/libs on Jun 24, 2021

DrahtBot added the label Wallet on Jun 24, 2021

DrahtBot commented at 11:01 PM on June 24, 2021: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19602 (wallet: Migrate legacy wallets to descriptor wallets by achow101)

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.

in src/test/util/wallet.cpp:10 in b1eeae2458 outdated

   6 | @@ -7,6 +7,7 @@
   7 |  #include <key_io.h>
   8 |  #include <outputtype.h>
   9 |  #include <script/standard.h>
  10 | +#include <util/translation.h>

hebasto commented at 5:15 AM on June 28, 2021:

Move under #ifdef ENABLE_WALLET?

achow101 commented at 9:46 PM on June 28, 2021:

Done

in src/wallet/spend.cpp:623 in b1eeae2458 outdated

 617 | @@ -618,9 +618,9 @@ bool CWallet::CreateTransactionInternal(
 618 |          // Reserve a new key pair from key pool. If it fails, provide a dummy
 619 |          // destination in case we don't need change.
 620 |          CTxDestination dest;
 621 | -        std::string dest_err;
 622 | +        bilingual_str dest_err;
 623 |          if (!reservedest.GetReservedDestination(dest, true, dest_err)) {
 624 | -            error = strprintf(_("Transaction needs a change address, but we can't generate it. %s"), dest_err);
 625 | +            error = _("Transaction needs a change address, but we can't generate it. ") + dest_err;

hebasto commented at 5:17 AM on June 28, 2021:

The trailing space could be really confusing for translators. Could it be avoided?

achow101 commented at 9:46 PM on June 28, 2021:

Done

in src/script/sign.cpp:689 in b1eeae2458 outdated

 687 | -                input_errors[i] = "Unable to sign input, invalid stack size (possibly missing key)";
 688 | +                input_errors[i] = _("Unable to sign input, invalid stack size (possibly missing key)");
 689 |              } else if (serror == SCRIPT_ERR_SIG_NULLFAIL) {
 690 |                  // Verification failed (possibly due to insufficient signatures).
 691 | -                input_errors[i] = "CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)";
 692 | +                input_errors[i] = _("CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)");

hebasto commented at 5:22 AM on June 28, 2021:

Not sure if CHECK(MULTI)SIG should be a part of the translatable string.

MarcoFalke commented at 5:51 AM on June 28, 2021:

If something can't be hit in the gui, it is possible to mark it Untranslated(...) to avoid dealing with translations

achow101 commented at 9:46 PM on June 28, 2021:

Done

in src/rpc/rawtransaction_util.cpp:20 in b1eeae2458 outdated

  15 | @@ -16,6 +16,7 @@
  16 |  #include <script/signingprovider.h>
  17 |  #include <tinyformat.h>
  18 |  #include <univalue.h>
  19 | +#include <util/translation.h>
  20 |  #include <util/rbf.h>
  21 |  #include <util/strencodings.h>

hebasto commented at 5:25 AM on June 28, 2021:

nit, sorting:

#include <util/rbf.h>
#include <util/strencodings.h>
#include <util/translation.h>

achow101 commented at 9:46 PM on June 28, 2021:

Done

in src/script/sign.cpp:14 in b1eeae2458 outdated

  11 | @@ -12,6 +12,7 @@
  12 |  #include <script/standard.h>
  13 |  #include <uint256.h>
  14 |  #include <util/vector.h>
  15 | +#include <util/translation.h>

hebasto commented at 5:26 AM on June 28, 2021:

nit, sorting:

#include <util/translation.h>
#include <util/vector.h>

achow101 commented at 9:46 PM on June 28, 2021:

Done

hebasto commented at 5:28 AM on June 28, 2021: member

Approach ACK b1eeae2458f533712c78d4aef4ea4f9587f10633.

While touching this code, could string duplication be eliminated in wallet/scriptpubkeyman.cpp?

achow101 force-pushed on Jun 28, 2021

hebasto approved

hebasto commented at 2:43 AM on June 29, 2021: member

ACK ffc8ff57f0211603da7f0a1a9af5ef6fafaa5dd6

DrahtBot added the label Needs rebase on Jul 1, 2021

Add bilingual_str::clear() 9571c69b51

Use bilingual_str for address fetching functions

For GetNewDestination, GetNewChangeDestination, and
GetReservedDestination, use bilingual_str for
errors

171366e89b

Change SignTransaction's input_errors to use bilingual_str 92993aa5cf

achow101 force-pushed on Jul 1, 2021

DrahtBot removed the label Needs rebase on Jul 1, 2021

hebasto approved

hebasto commented at 5:49 PM on July 3, 2021: member

re-ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c, only rebased since my previous review, verified with

$ git range-diff master ffc8ff57f0211603da7f0a1a9af5ef6fafaa5dd6 92993aa5cf37995e65e68dfd6f129ecaf418e01c

klementtan commented at 1:31 PM on July 8, 2021: contributor

Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c

meshcollider commented at 2:13 AM on August 9, 2021: contributor

Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c

meshcollider merged this on Aug 9, 2021

meshcollider closed this on Aug 9, 2021

sidhujag referenced this in commit 1bcbe14ed6 on Aug 10, 2021

DrahtBot locked this on Aug 16, 2022