#36212 cli: Display warning arrays in -getinfo
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36212 cli: Display warning arrays in -getinfo | Ready | 0 | Strong | 4 + 11 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Fixes bitcoin-cli -getinfo parsing of warnings when getnetworkinfo returns a JSON array rather than a string. Array entries are joined with newlines while retaining fallback behavior for legacy string output, and the functional test helper is updated to parse multi-line warnings.
Problem: When warnings are returned as an array, bitcoin-cli -getinfo calls getValStr() which returns an empty string, causing active warnings to be swallowed and displaying Warnings: (none).
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- author: respond to review suggestions from stickies-v and clarify if still working on the PR 2026-10-31
- author: author response to review comments and inactivity ping 2026-10-31
Agreement
Strong
substantive support, no open objection (stickies-v)
Thread notes: Re-classified scope objections from stickies-v as suggestions per current schema rules, since they request dropping changes rather than naming costs. Added suggestions regarding layout consistency, type checking, and documentation. / Converted schema 1 claims from objections to suggestions since they did not name costs of merging. Added suggestions from stickies-v and the status inquiry from sedited.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| stickies-v (member) | suggestion, approach | update cli_get_info_string_to_dict to properly parse Warnings like is done for Balances | answered · fix pushed · author updated the helper in 59d0d24 | answered | yes | 2026-09-10: 'Would it be better to update `cli_get_info_string_to_dict` to properly parse `Warnings`, like we already do for `Balances`?' Status from: 2026-09-12: LittleYier: 'Tweaked the helper to grab the whole warnings section so colons and newlines don't get chopped.' |
| stickies-v (member) | suggestion, scope | drop test changes not relevant to testing the cli | open | answered | no | 2026-09-10: 'This doesn't seem all that relevant to testing the cli, makes sense to drop I think?' |
| stickies-v (member) | suggestion, scope | drop testing the cli in the versionbits warning functional test | fixed · fix pushed · author dropped the test check in 59d0d24 | answered | yes | 2026-09-10: 'I don't think testing the cli is relevant in this versionbits warning test?' Status from: 2026-09-12: LittleYier: 'Dropped the extra restart + cli check in versionbits test as suggested.' |
| stickies-v (member) | suggestion, scope | revert line processing and escaping changes in test helper, keeping only changes from L51 onwards | open | answered | no | 2026-09-16: 'I don't think this code that deals with line processing / escaping needs to be touched for the change you're trying to make? Would prefer keeping that part as-is' |
| stickies-v (member) | suggestion, maintenance | add a docstring indicating the code should be removed when deprecatedrpc is removed | open | answered | no | 2026-09-16: 'would be helpful to add a docstring here' |
| stickies-v (member) | suggestion, safety | use get_str() instead of getValStr() to ensure UniValue type checking | open | answered | no | 2026-09-16: 'if the original code had used `get_str()` with type checking, this issue probably wouldn't have stayed under the radar.' |
| stickies-v (member) | suggestion, interface | format warnings under a dedicated header like the Balances section, and omit the section entirely when empty | open | answered | no | 2026-09-16: 'perhaps it would be more consistent to follow the layout of the `Balances` section, which is to make a dedicated `Warnings` header and then print each item underneath it' |
| sedited (member) | question, maintenance | ask if the author is still working on the PR | open | answered | no | 2026-10-31: '@LittleYier are you still working on this?' |
Support:
- stickies-v (member) (Concept ACK, 2026-09-10): Concept ACK, thanks for catching this
Participants:
- stickies-v (member, support, 9 statements 2026-09-10 to 2026-09-16): Concept ACK; suggested layout improvements and dropping unrelated test modifications
- sedited (member, question, 1 statements 2026-10-31 to 2026-10-31): asked if author is still working on the PR
Files
16 lines under test/bench/ci.
- test/functional/interface_bitcoin_cli.py +11/-5
- src/bitcoin-cli.cpp +4/-1
Card
The short text the ranking pass compares across PRs.
This PR fixes a bug in bitcoin-cli -getinfo where warnings returned as a JSON array were evaluated as empty strings, displaying Warnings: (none) instead of actual node warnings. The fix joins array elements with newlines while preserving backwards compatibility for legacy string responses. The issue primarily affects operators relying on -getinfo to detect active warnings. Review is ready, with Concept ACK and minor layout suggestions from stickies-v.
Data
thread read through 2026-10-31 (11 statements), head 59d0d24d6c
labels: Scripts and tools
Categories
RPC / REST / ZMQ
P3 · bug fix (ranking pass: Moved down 8: real bug but confined to the -getinfo convenience display.)
Fixes a bug where `bitcoin-cli -getinfo` swallowed active network warnings and displayed `Warnings: (none)` when warnings were returned as an array. While this restores visibility into active node warnings for CLI operators, `-getinfo` is a convenience display rather than a programmatic RPC interface, making the issue worthwhile but deferrable.
Membership: Modifies `bitcoin-cli` in `src/bitcoin-cli.cpp` to correctly handle array-formatted warnings in `-getinfo`.
Factors: security/stability 1, bug 2, performance 0, user value 1, leverage 0