[init] Add transaction fee warnings #7374

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1601-init-fee-warn changing 4 files +35 −14

MarcoFalke commented at 7:48 PM on January 18, 2016: member

Introduce HIGH_AMOUNT_WARN_MSG and LOW_AMOUNT_WARN_MSG.
MarcoFalke force-pushed on Jan 18, 2016
MarcoFalke force-pushed on Jan 18, 2016
MarcoFalke force-pushed on Jan 18, 2016
in src/init.cpp:None in fa37684d88 outdated
```
 764 | @@ -765,9 +765,9 @@ static std::string ResolveErrMsg(const char * const optname, const std::string&
 765 |      return strprintf(_("Cannot resolve -%s address: '%s'"), optname, strBind);
 766 |  }
 767 |  
 768 | -static std::string AmountErrMsg(const char * const optname, const std::string& strValue)
 769 | +static std::string AmountErrMsg(const std::string& optname, const std::string& strValue)
 770 |  {
 771 | -    return strprintf(_("Invalid amount for -%s=<amount>: '%s'"), optname, strValue);
 772 | +    return strprintf(_("Invalid amount for %s."), strprintf("-%s (%s=%s)", optname, optname, strValue));
```
luke-jr commented at 12:18 AM on January 19, 2016:

Forces English grammar a bit much?

MarcoFalke commented at 7:45 AM on January 19, 2016:

I don't think you can ask someone to "translate" a collection of characters (-%s=<amount>: '%s') into a language.

Also, this seems to work with other grammar just fine:

Am I missing something?

luke-jr commented at 8:24 AM on January 19, 2016:

German is not the only language. What's wrong with "Invalid amount for -%s (%s=%s)."?

MarcoFalke commented at 1:26 PM on January 19, 2016:

There is nothing wrong but I'd rather not expose three %s to translators, considering that the syntax in the braces does not change. Also, this is only a short note to the user that a specific arg can't be parsed, so not worth to have perfect grammar in every possible language, imo.

Though, I am happy to revert, if that's wanted.

MarcoFalke commented at 2:57 PM on January 20, 2016:

Reverted
laanwj commented at 10:44 AM on January 19, 2016: member

IMO we should settle this code. What do we really want here?

It's been changed way too many times over recent times, I don't think this is warranted by the importance of these errors. At some point it just introduces unnecessary risk of breakage (we don't have good testing for parameter processing), review work, and forces other patches to rebase for no good reason.
MarcoFalke commented at 1:20 PM on January 19, 2016: member

@laanwj There is no high/low fee checks for most of the args right now. Currently you can set e.g. mintxfee arbitrarily without a warning.
MarcoFalke force-pushed on Jan 19, 2016
MarcoFalke renamed this:
~~[init] Rework transaction fee warnings~~
[init] Add high transaction fee warnings
on Jan 19, 2016
[tests] Extend util_ParseMoney test case 36d1823b55
MarcoFalke force-pushed on Jan 19, 2016
MarcoFalke renamed this:
~~[init] Add high transaction fee warnings~~
[init] Add transaction fee warnings
on Jan 19, 2016
jonasschnelli commented at 12:48 PM on January 20, 2016: contributor

Agree with @laanwj. It's already very heavy-to-follow if you want to see the coupling between vars (const, configurable).
jonasschnelli added the label Wallet on Jan 20, 2016
MarcoFalke force-pushed on Jan 20, 2016
MarcoFalke force-pushed on Jan 20, 2016
MarcoFalke force-pushed on Jan 20, 2016
[init] Rework transaction fee warnings fa69058bef
MarcoFalke force-pushed on Jan 20, 2016
MarcoFalke commented at 2:56 PM on January 20, 2016: member

Sorry for the odd diff. Now it should be clearer that it is only adding warnings in the appropriate places.
MarcoFalke commented at 8:44 PM on January 21, 2016: member

I will open a replacement pull next week.
MarcoFalke closed this on Jan 21, 2016
MarcoFalke deleted the branch on Jan 21, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Labels