Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix getpeerinfo doc #24691

Merged
merged 1 commit into from
Mar 28, 2022
Merged

Fix getpeerinfo doc #24691

merged 1 commit into from
Mar 28, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Mar 28, 2022

@maflcko
Copy link
Member Author

maflcko commented Mar 28, 2022

diff --git a/getpeerinfo b/getpeerinfo
index ae40ea4..40b0d63 100644
--- a/getpeerinfo
+++ b/getpeerinfo
@@ -1,6 +1,6 @@
 getpeerinfo
 
-Returns data about each connected network node as a json array of objects.
+Returns data about each connected network peer as a json array of objects.
 
 Result:
 [                                         (json array)
@@ -17,7 +17,7 @@ Result:
       "str",                              (string) the service name if it is recognised
       ...
     ],
-    "relaytxes" : true|false,             (boolean) Whether peer has asked us to relay transactions to it
+    "relaytxes" : true|false,             (boolean, optional) Whether peer has asked us to relay transactions to it
     "lastsend" : xxx,                     (numeric) The UNIX epoch time of the last send
     "lastrecv" : xxx,                     (numeric) The UNIX epoch time of the last receive
     "last_transaction" : xxx,             (numeric) The UNIX epoch time of the last valid transaction received from this peer
@@ -55,7 +55,7 @@ Result:
                                           
       ...
     ],
-    "minfeefilter" : n,                   (numeric) The minimum fee rate for transactions this peer accepts
+    "minfeefilter" : n,                   (numeric, optional) The minimum fee rate for transactions this peer accepts
     "bytessent_per_msg" : {               (json object)
       "msg" : n,                          (numeric) The total bytes sent aggregated by message type
                                           When a message type is not listed in this json object, the bytes sent are 0.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faac877

@fanquake fanquake merged commit 56c4ac5 into bitcoin:master Mar 28, 2022
@jonatack
Copy link
Member

Post-merge ACK

@maflcko maflcko deleted the 2203-doc-🎲 branch March 28, 2022 11:16
fanquake added a commit that referenced this pull request Mar 28, 2022
fc892c3 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke)
f4bc4a7 rpc: Add m_skip_type_check to RPCResult (MarcoFalke)

Pull request description:

  This avoids documentation shortcomings such as the ones fixed in commit e7b6272, 138d55e, 577bd51, f8c84e0, 0ee9a00, 13f4185, or faecb2e

ACKs for top commit:
  fanquake:
    ACK fc892c3 - tested that this catches issue, i.e #24691:

Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
@jonatack jonatack mentioned this pull request Mar 28, 2022
fanquake added a commit that referenced this pull request Mar 31, 2022
174af33 util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov)
ded10fe build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov)
26c2f23 build: Fix Boost.Process detection on macOS arm64 (Hennadii Stepanov)
85f85c7 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake)
eaa0419 contrib: fix signet miner (sighash mismatch) (Sebastian Falbesoner)
235b042 rpc: Exclude descriptor when address is excluded (MarcoFalke)
b05a59b ci: Temporarily use clang-13 to work around clang-14 TSan bug (MarcoFalke)
65b9667 doc, init: add links to doc/cjdns.md (Jon Atack)
7a553d4 doc: update i2p.md with cjdns, improve local addresses section (Jon Atack)
4148396 doc: update tor.md with cjdns and getnodeaddresses, fix tor grep, (Jon Atack)
4690e8a doc: create initial doc/cjdns.md for cjdns how-to documentation (Jon Atack)
5d24f61 Clarify in -maxtimeadjustment that only outbound peers influence time data (Jon Atack)
b1646f1 test: set segwit height back to 0 on regtest (Martin Zumsande)
ef6a37b rpc: rename getdeploymentinfo status-next to status_next (Jon Atack)
2a6fcf9 init, doc: improve -onlynet help and tor/i2p documentation (Jon Atack)

Pull request description:

  Backport the following to 23.x:

  - #24468
  - #24528
  - #24527
  - #24609
  - #24555
  - #24663
  - #24572
  - #24636
  - #24553
  - #24659
  - #24521
  - #24523
  - #24690
  - #24710

  Possibly also:
  - #24579
  - #24691

ACKs for top commit:
  laanwj:
    List-of-commits ACK 174af33, I think we should merge this and move forward with rc3..
  hebasto:
    ACK 174af33

Tree-SHA512: 5a493e1652b780b527767d6ca9e67012abd2fa5573496e85e0d8aa4bed3eb332bfcd72610b8dfb954ff274d42450623233c96c479de2085b9c8344ba5abf1935
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 3, 2022
faac877 doc: Fix getpeerinfo doc (MarcoFalke)

Pull request description:

  * Replace `node` with `peer`
  * Remove unused `\n`
  * Mark optional fields optional. See commit 9344697, found by bitcoin#23083

ACKs for top commit:
  fanquake:
    ACK faac877

Tree-SHA512: ae4d52a0dcf8e132d9084e632d65fa835b1e7d0ed5c3d45a360570414d1e20bc7fb6500ff9be94b784af1dec5badcd1304153b1a4a59a6c484a87d8afd88b8bd
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 3, 2022
…d JSON

fc892c3 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke)
f4bc4a7 rpc: Add m_skip_type_check to RPCResult (MarcoFalke)

Pull request description:

  This avoids documentation shortcomings such as the ones fixed in commit e7b6272, 138d55e, 577bd51, f8c84e0, 0ee9a00, 13f4185, or faecb2e

ACKs for top commit:
  fanquake:
    ACK fc892c3 - tested that this catches issue, i.e bitcoin#24691:

Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
@luke-jr
Copy link
Member

luke-jr commented Apr 13, 2022

Note: This change would be incorrect in 23.x.

maflcko pushed a commit to bitcoin-core/gui that referenced this pull request May 23, 2022
…issing getpeerinfo#relaytxes

a17c5e9 Rename NetinfoRequestHandler::is_block_relay data member to is_tx_relay (Jon Atack)
f0bb7db Fix frequent -netinfo JSON errors from null getpeerinfo#relaytxes (Jon Atack)

Pull request description:

  CLI -netinfo frequently returns "error: JSON value is not a boolean as expected" since the merge of #21160, which moved fRelayTxes (renamed to m_relay_txs in that pull) from CNodeStats to CNodeStateStats.

  This change made getpeerinfo "relaytxes" an optional field that can return UniValue IsNull(). It is the only optional field consumed by -netinfo where the latter didn't already handle that case. See also bitcoin/bitcoin#24691.

  Also rename the NetinfoRequestHandler::is_block_relay data member to is_tx_relay and inverse its boolean logic. The naming is out of date and incorrect, as lack of request of tx relay does not imply block relay, and a preference for tx relay doesn't imply that block relay isn't happening. Thanks to Marco Falke and Martin Zumsande for their feedback on this.

  (I may look at reducing the number of optional node stats fields via refactoring at the net processing level, but ongoing refactoring there may make that slow or complicated and this is a one-line fix that works now.)

ACKs for top commit:
  mzumsande:
    Code review ACK a17c5e9

Tree-SHA512: dc54ce80b78122874a6794555f99e5b328a1574b52bb3e7f974c699c2b759a60ea0807a6483c5bc0414a950d853c0eeeb13dcc1b790d3917c6ee4c9c99fe159f
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
…etpeerinfo#relaytxes

a17c5e9 Rename NetinfoRequestHandler::is_block_relay data member to is_tx_relay (Jon Atack)
f0bb7db Fix frequent -netinfo JSON errors from null getpeerinfo#relaytxes (Jon Atack)

Pull request description:

  CLI -netinfo frequently returns "error: JSON value is not a boolean as expected" since the merge of bitcoin#21160, which moved fRelayTxes (renamed to m_relay_txs in that pull) from CNodeStats to CNodeStateStats.

  This change made getpeerinfo "relaytxes" an optional field that can return UniValue IsNull(). It is the only optional field consumed by -netinfo where the latter didn't already handle that case. See also bitcoin#24691.

  Also rename the NetinfoRequestHandler::is_block_relay data member to is_tx_relay and inverse its boolean logic. The naming is out of date and incorrect, as lack of request of tx relay does not imply block relay, and a preference for tx relay doesn't imply that block relay isn't happening. Thanks to Marco Falke and Martin Zumsande for their feedback on this.

  (I may look at reducing the number of optional node stats fields via refactoring at the net processing level, but ongoing refactoring there may make that slow or complicated and this is a one-line fix that works now.)

ACKs for top commit:
  mzumsande:
    Code review ACK a17c5e9

Tree-SHA512: dc54ce80b78122874a6794555f99e5b328a1574b52bb3e7f974c699c2b759a60ea0807a6483c5bc0414a950d853c0eeeb13dcc1b790d3917c6ee4c9c99fe159f
@bitcoin bitcoin locked and limited conversation to collaborators Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants