add `(none)` in -getinfo `Warnings:` if no warning returned #24632

pull ghost wants to merge 1 commits into bitcoin:master from changing 1 files +3 −1
  1. ghost commented at 11:53 PM on March 21, 2022: none

    Adds (none) in warnings when no warnings returned by -getinfo

    Reviewers can test this by making the following change in /src/warnings.cpp:

    bilingual_str GetWarnings(bool verbose)
{
    bilingual_str warnings_concise;
    std::vector<bilingual_str> warnings_verbose;

    LOCK(g_warnings_mutex);

    // Pre-release build warning
    if (!CLIENT_VERSION_IS_RELEASE) {
-        warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");;
+        warnings_concise = _("");;

    Before this pull request:

    $ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10

Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000

Warnings:

    After this pull request:

    $ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10

Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000

Warnings: (none)
  2. DrahtBot added the label Utils/log/libs on Mar 22, 2022
  3. jessebarton approved
  4. jessebarton commented at 2:43 AM on March 22, 2022: contributor

    Concept ACK 1249aa6

    Confirmed Warning is being thrown even if there are no Warnings The version is showing 220000 I think because of the version of bitcoind im running, I could be wrong though.

    Blocks: 728446
Headers: 728446
Verification progress: 99.9998%
Difficulty: 27452707696466.39

Network: in 8, out 10, total 18
Version: 220000
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000

Warnings:

    Compiled and tested fix with no issues with the desired outcome of not throwing empty warnings.

    Chain: main
Blocks: 728448
Headers: 728448
Verification progress: 99.9999%
Difficulty: 27452707696466.39

Network: in 15, out 10, total 25
Version: 220000
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000
  5. unknown force-pushed on Mar 22, 2022
  6. ghost commented at 7:58 AM on March 22, 2022: none

    The version is showing 220000 I think because of the version of bitcoind im running, I could be wrong though. @jessebarton I see Version: 230000 when running RC2. It prints server version from getnetworkinfo so you are right that its 220000 because of bitcoind in your output.

  7. fanquake requested review from jonatack on Mar 22, 2022
  8. in src/bitcoin-cli.cpp:1067 in 2258b0172b outdated 
    1058 | @@ -1059,7 +1059,10 @@ static void ParseGetInfoResult(UniValue& result)
1059 |          result_string += "\n";
1060 |      }
1061 |  
1062 | -    result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, result["warnings"].getValStr());
1063 | +    if (result["warnings"].getValStr()!="")
1064 | +    {
1065 | +        result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, result["warnings"].getValStr());
1066 | +    }
    jonatack commented at 9:17 AM on March 22, 2022:

    suggestions

    • don't add extra line breaks when no warning is present
    • avoid running getValStr() twice
    • use empty()
    • clang-format of brackets in the conditional
    --- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -1038,7 +1038,7 @@ static void ParseGetInfoResult(UniValue& result)
         result_string += strprintf("Transaction fee rate (-paytxfee) (%s/kvB): %s\n\n", CURRENCY_UNIT, result["paytxfee"].getValStr());
     }
     if (!result["balance"].isNull()) {
-        result_string += strprintf("%sBalance:%s %s\n\n", CYAN, RESET, result["balance"].getValStr());
+        result_string += strprintf("%sBalance:%s %s\n", CYAN, RESET, result["balance"].getValStr());
     }
 
     if (!result["balances"].isNull()) {
@@ -1056,13 +1056,13 @@ static void ParseGetInfoResult(UniValue& result)
                                        result["balances"][wallet].getValStr(),
                                        wallet.empty() ? "\"\"" : wallet);
         }
-        result_string += "\n";
     }
 
-    if (result["warnings"].getValStr()!="")
-    {
-        result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, result["warnings"].getValStr());
+    const std::string warnings{result["warnings"].getValStr()};
+    if (!warnings.empty()) {
+        result_string += strprintf("\n%sWarnings:%s %s", YELLOW, RESET, warnings);
     }
+
     result.setStr(result_string);
 }

    With this suggestion, I think there is still an extra line break after the balance/balances, if you can look into it.

    unknown commented at 11:57 AM on March 22, 2022:

    @jonatack thanks for review and suggestion. Made changes, tested and it looks good. Let me know if there are any issues with line breaks:

    With warning: https://pastebin.com/raw/eN3SGjiX Without warning: https://pastebin.com/raw/frfnqfvU

    jonatack commented at 5:22 PM on March 22, 2022:

    As I mentioned, there is still an extra line, and the commits should be squashed, as the extra line is created by the changes here.

    unknown commented at 6:57 PM on March 22, 2022:

    Done. Removed extra lines: https://pastebin.com/raw/bZk4YNj5 and squashed commits.

  9. jonatack commented at 9:28 AM on March 22, 2022: member

    Concept ACK. Previously, -getinfo returned a JSON-RPC-like response with the "warnings" field returned by getnetworkinfo after calling GetWarnings() in src/warnings.{h,cpp}. With #21832, -getinfo was converted to a human-readable format without taking into account that the field, when empty, could appear odd. This was likely overlooked because in pre-release development the tester will always see Warnings: This is a pre-release test build - use at your own risk - do not use for mining or merchant applications.

  10. unknown force-pushed on Mar 22, 2022
  11. unknown force-pushed on Mar 22, 2022
  12. jonatack commented at 1:10 PM on March 23, 2022: member

    Thanks for updating.

    • It looks like there is now (a) an extra line between "Balances" and the balances, and (b) a line missing before the warnings, with both one balance and multiple balances. Example:
    ₿ ./src/bitcoin-cli -signet -getinfo

.../...

Min tx relay fee rate (BTC/kvB): 0.00001000

Balances

  1.10997653 default settings
  0.00000000 disable private keys and blank
  0.00000000 disable private keys
  0.00000000 encrypted blank descriptor
  0.00000000 blank
498.77584341 ""
Warnings: This is a pre-release test build - use at your own risk - do not use for mining or merchant applications
    • Could you please update the PR and commit titles to include "-getinfo"? i.e. something like print -getinfo warnings only if warnings are present
  13. theStack commented at 12:58 PM on March 25, 2022: member

    Concept ACK

  14. ghost commented at 11:18 PM on March 31, 2022: none
    • It looks like there is now (a) an extra line between "Balances" and the balances, and (b) a line missing before the warnings, with both one balance and multiple balances. Example:

    Sorry, missed this. Will do it this weekend.

    Could you please update the PR and commit titles to include "-getinfo"? i.e. something like print -getinfo warnings only if warnings are present

    Done

  15. unknown renamed this:
    print `Warnings:` only if warning returned
    print -getinfo `Warnings:` only if warning returned
    on Mar 31, 2022
  16. DrahtBot commented at 4:36 AM on April 5, 2022: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  17. laanwj commented at 11:01 AM on April 6, 2022: member

    I don't know about leaving it out entirely. It might be reassuring to the user that there are, explicitly, no warnings. Or less reassuring (but important) that there could be warnings, but are none. Could format it different like Warnings: (none) or something like that if the empty field bothers you.

  18. jonatack commented at 7:05 AM on April 7, 2022: member

    I don't know about leaving it out entirely. It might be reassuring to the user that there are, explicitly, no warnings. Or less reassuring (but important) that there could be warnings, but are none. Could format it different like Warnings: (none) or something like that if the empty field bothers you.

    Good point (and probably a simpler change to implement).

  19. unknown renamed this:
    print -getinfo `Warnings:` only if warning returned
    add `(none)` in -getinfo `Warnings:` if no warning returned
    on Apr 8, 2022
  20. unknown force-pushed on Apr 8, 2022
  21. ghost commented at 4:01 AM on April 8, 2022: none

    I don't know about leaving it out entirely. It might be reassuring to the user that there are, explicitly, no warnings. Or less reassuring (but important) that there could be warnings, but are none. Could format it different like Warnings: (none) or something like that if the empty field bothers you.

    Done in last commit. Updated pull request title and description.

  22. print `(none)` if no warnings in -getinfo 0cea7b10f1
  23. in src/bitcoin-cli.cpp:1064 in 8b2c124215 outdated 
    1058 | @@ -1059,7 +1059,13 @@ static void ParseGetInfoResult(UniValue& result)
1059 |          result_string += "\n";
1060 |      }
1061 |  
1062 | -    result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, result["warnings"].getValStr());
1063 | +    const std::string warnings{result["warnings"].getValStr()};
1064 | +    if (warnings.empty()) {
1065 | +        result_string += strprintf("%sWarnings: %s(none)", YELLOW, RESET);
    jonatack commented at 10:36 AM on April 8, 2022: 
            result_string += strprintf("%sWarnings:%s (none)", YELLOW, RESET);

    alternatively, could simplify

    -    if (warnings.empty()) {
-        result_string += strprintf("%sWarnings: %s(none)", YELLOW, RESET);
-    } else {
-        result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, warnings);
-    }
+    result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, warnings.empty() ? "(none)" : warnings);
    unknown commented at 3:05 PM on April 8, 2022:

    Done in last commit.

  24. unknown force-pushed on Apr 8, 2022
  25. jonatack commented at 4:06 PM on April 8, 2022: member

    ACK 0cea7b10f1180e9993c14473e1a3b6525ef6ba01

    image

  26. laanwj commented at 7:47 PM on April 13, 2022: member

    Tested ACK 0cea7b10f1180e9993c14473e1a3b6525ef6ba01

  27. laanwj merged this on Apr 13, 2022
  28. laanwj closed this on Apr 13, 2022
  29. sidhujag referenced this in commit e77af4bc9e on Apr 14, 2022
  30. DrahtBot locked this on Apr 13, 2023
Contributors
ghostjessebartonjonatacktheStackDrahtBotlaanwj
Labels
Utils/log/libs
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me