#36719 doc: clarify RPC addnode 'add' vs 'onetry' connection limits
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36719 doc: clarify RPC addnode 'add' vs 'onetry' connection limits | Ready | 1 | Positive | 5 | |||
|
|
|
|
| |||
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
Clarifies in the addnode RPC help text that MAX_ADDNODE_CONNECTIONS applies only to persistent 'add' connections, not to 'onetry' attempts. In bitcoin-cli -netinfo, widens the manual peer counter from uint8_t to uint16_t to match the other peer counters and prevent display wraparound.
Problem: The addnode RPC help text does not distinguish between 'add' and 'onetry' limits, leaving users with the impression that onetry connections count against MAX_ADDNODE_CONNECTIONS. Because onetry connections are not capped at 8, nodes with many manual connections can exceed 255 manual peers, causing the uint8_t counter in -netinfo to wrap and display incorrect counts or drop the column.
Evidence of importance:
- Commit e46bcd40bb: 'The other -netinfo peer counts are uint16_t, but the manual peer count was uint8_t and wraps at 256. Manual peers aren't limited to the 8 -addnode slots ... so the count can reach that and the "manual" column then disappears or shows a wrapped value.'
- Author PR description notes that 'add' entries are capped by MAX_ADDNODE_CONNECTIONS while 'onetry' connections are not.
Risk of a mistake: low (rpc_api, tooling, docs). The changes only affect RPC help strings and a CLI display counter, with no impact on node operation or consensus.
Changed since the previous assessment: The PR now also updates the addnode RPC help documentation in src/rpc/net.cpp to clarify connection limits; the previous version touched only bitcoin-cli. Additionally, in bitcoin-cli only m_manual_peers_count is widened to uint16_t (m_block_relay_peers_count remains uint8_t).
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- nothing: jonatack ACKed and author addressed PR title and description requests 2026-11-01
Agreement
Positive
support without stated reasons, no open objection (jonatack)
Thread notes: jonatack objected to widening the block-relay peer counter and suggested clarifying the addnode RPC help text. Author reverted the block-relay change, added the doc clarification, and jonatack ACKed.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| jonatack (member) | suggestion, usefulness | State in the PR description why the manual peer counter might exceed 255 to justify the change | accepted · author updated description to explain onetry bypasses the cap and jonatack ACKed | answered | no | 2026-11-01: 'the PR description should state why the manual peer counter might exceed 255 to justify the change' Status from: 2026-11-01: jonatack: 'ACK 4c4970a69646231a1b443caa3c0c57aa012e4d89' |
| jonatack (member) | objection, scope | The block-relay peer counter should not be widened because block-relay connections are limited to around 2 in steady state | accepted · fix pushed · author reverted the block-relay counter and jonatack ACKed | changed | yes | 2026-11-01: 'NACK on the block relay one' Status from: 2026-11-01: jonatack: 'ACK 4c4970a69646231a1b443caa3c0c57aa012e4d89' |
| jonatack (member) | suggestion, interface | Clarify the addnode RPC help regarding separate connection limits for add and onetry | accepted · fix pushed · author adopted the suggested help text clarification and jonatack ACKed | answered | yes | 2026-11-01: 'The RPC addnode help could perhaps be clarified about the separate limits for add and onetry' Status from: 2026-11-01: jonatack: 'ACK 4c4970a69646231a1b443caa3c0c57aa012e4d89' |
Support:
- jonatack (member) (ACK, 2026-11-01): (no reason given) [not substantive]
Participants:
- pinheadmz (member, neutral, 1 statements 2026-10-31 to 2026-10-31): asked author to pause opening PRs so reviewers can digest them
- jonatack (member, support, 4 statements 2026-11-01 to 2026-11-01): ACKed after block-relay counter change was dropped and RPC help text clarified
Files
0 lines under test/bench/ci.
- src/rpc/net.cpp +4/-3
- src/bitcoin-cli.cpp +1/-1
Card
The short text the ranking pass compares across PRs.
Clarifies the addnode RPC help documentation regarding connection limits and prevents an integer overflow in bitcoin-cli -netinfo. The RPC help now specifies that MAX_ADDNODE_CONNECTIONS applies only to persistent 'add' connections, while 'onetry' connections are not subject to this limit. In bitcoin-cli, the manual peer counter is widened from uint8_t to uint16_t, preventing the manual peer column from wrapping at 256 or disappearing when many 'onetry' connections are active.
Data
thread read through 2026-11-01 (8 statements), head 4c4970a696
labels: Scripts and tools
Categories
RPC / REST / ZMQ
P4 · bug fix
Fixes a minor display wraparound bug in bitcoin-cli -netinfo and clarifies RPC documentation. The manual peer counter in -netinfo was uint8_t and could wrap at 256, and the addnode help text did not specify that the connection cap applies only to persistent add entries rather than onetry.
Membership: Changes RPC help text in src/rpc/net.cpp and fixes a counter wraparound in bitcoin-cli in src/bitcoin-cli.cpp
Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 0