Skip to content

rpc, doc: getpeerinfo updates #26109

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.

Sign up for GitHub

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

Merged
achow101 merged 4 commits into from
Oct 13, 2022
Merged

rpc, doc: getpeerinfo updates #26109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cd6682
Make getpeerinfo field order consistent with its help (for v24 backport)
jonatack Sep 16, 2022
1f44854
Always return getpeerinfo "minfeefilter" field (for v24 backport)
jonatack Sep 16, 2022
df660dd
Update getpeerinfo/-netinfo/TxRelay#m_relay_txs relaytxes docs (for v…
jonatack Aug 24, 2022
a3789c7
Improve getpeerinfo pingtime, minping, and pingwait help docs
jonatack Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bitcoin-cli.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
" send Time since last message sent to the peer, in seconds\n"
" recv Time since last message received from the peer, in seconds\n"
" txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes\n"
" \"*\" - the peer requested we not relay transactions to it (relaytxes is false)\n"
" \"*\" - whether we relay transactions to this peer (relaytxes is false)\n"
Copy link
Member

@maflcko maflcko Oct 14, 2022

Choose a reason for hiding this comment

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

This doesn't seem right. Even if relaytxes is true, this might be false (for example this is an outbound block-relay-only connection?)

Copy link
Member

@maflcko maflcko Oct 14, 2022

Choose a reason for hiding this comment

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

unrelated: It seems the cli will default to true if the field is not available, which might also be confusing if an outbound block-relay-only connection is spun up.

Copy link
Member Author

@jonatack jonatack Oct 17, 2022

Choose a reason for hiding this comment

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

Good catch. Perhaps: we don't relay transactions to this peer (relaytxes is false or not available)

Copy link
Member Author

@jonatack jonatack Oct 17, 2022

Choose a reason for hiding this comment

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

Proposed a fixup in #26328.

Copy link
Member

@maflcko maflcko Jan 11, 2023

Choose a reason for hiding this comment

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

Oh, I think I read this as version.relaytxes is false, but this means getpeerinfo.relaytxes is false? The two are different, so for one interpretation this comment is right and for the other it is wrong?

Copy link
Member Author

@jonatack jonatack Jan 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Will clarify next time I touch the file that it's the getpeerinfo "relaytxes" field (what is version.relaytxes?)

Copy link
Member

@maflcko maflcko Jan 11, 2023

Choose a reason for hiding this comment

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

what is version.relaytxes?

https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki#extensions-to-existing-messages

Copy link
Member Author

@jonatack jonatack Jan 11, 2023

Choose a reason for hiding this comment

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

Thanks! The various namings for tx relay can be a little confusing.

" blk Time since last novel block passing initial validity checks received from the peer, in minutes\n"
" hb High-bandwidth BIP152 compact block relay\n"
" \".\" (to) - we selected the peer as a high-bandwidth peer\n"
Expand Down
16 changes: 6 additions & 10 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,7 @@ struct Peer {

struct TxRelay {
mutable RecursiveMutex m_bloom_filter_mutex;
/** Whether the peer wishes to receive transaction announcements.
*
* This is initially set based on the fRelay flag in the received
* `version` message. If initially set to false, it can only be flipped
* to true if we have offered the peer NODE_BLOOM services and it sends
* us a `filterload` or `filterclear` message. See BIP37. */
/** Whether we relay transactions to this peer. */
bool m_relay_txs GUARDED_BY(m_bloom_filter_mutex){false};
/** A bloom filter for which transactions to announce to the peer. See BIP37. */
std::unique_ptr<CBloomFilter> m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr};
Expand Down Expand Up @@ -3247,10 +3242,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
peer->m_starting_height = starting_height;

// We only initialize the m_tx_relay data structure if:
// - this isn't an outbound block-relay-only connection; and
// - fRelay=true or we're offering NODE_BLOOM to this peer
// (NODE_BLOOM means that the peer may turn on tx relay later)
// We only initialize the Peer::TxRelay m_relay_txs data structure if:
// - this isn't an outbound block-relay-only connection, and
// - fRelay=true (the peer wishes to receive transaction announcements)
// or we're offering NODE_BLOOM to this peer. NODE_BLOOM means that
// the peer may turn on transaction relay later.
if (!pfrom.IsBlockOnlyConn() &&
(fRelay || (peer->m_our_services & NODE_BLOOM))) {
auto* const tx_relay = peer->SetTxRelay();
Expand Down
16 changes: 9 additions & 7 deletions src/rpc/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static RPCHelpMan getpeerinfo()
{
{RPCResult::Type::STR, "SERVICE_NAME", "the service name if it is recognised"}
}},
{RPCResult::Type::BOOL, "relaytxes", /*optional=*/true, "Whether peer has asked us to relay transactions to it"},
{RPCResult::Type::BOOL, "relaytxes", /*optional=*/true, "Whether we relay transactions to this peer"},
{RPCResult::Type::NUM_TIME, "lastsend", "The " + UNIX_EPOCH_TIME + " of the last send"},
{RPCResult::Type::NUM_TIME, "lastrecv", "The " + UNIX_EPOCH_TIME + " of the last receive"},
{RPCResult::Type::NUM_TIME, "last_transaction", "The " + UNIX_EPOCH_TIME + " of the last valid transaction received from this peer"},
Expand All @@ -123,9 +123,9 @@ static RPCHelpMan getpeerinfo()
{RPCResult::Type::NUM, "bytesrecv", "The total bytes received"},
{RPCResult::Type::NUM_TIME, "conntime", "The " + UNIX_EPOCH_TIME + " of the connection"},
{RPCResult::Type::NUM, "timeoffset", "The time offset in seconds"},
{RPCResult::Type::NUM, "pingtime", /*optional=*/true, "ping time (if available)"},
{RPCResult::Type::NUM, "minping", /*optional=*/true, "minimum observed ping time (if any at all)"},
{RPCResult::Type::NUM, "pingwait", /*optional=*/true, "ping wait (if non-zero)"},
{RPCResult::Type::NUM, "pingtime", /*optional=*/true, "The last ping time in milliseconds (ms), if any"},
{RPCResult::Type::NUM, "minping", /*optional=*/true, "The minimum observed ping time in milliseconds (ms), if any"},
{RPCResult::Type::NUM, "pingwait", /*optional=*/true, "The duration in milliseconds (ms) of an outstanding ping (if non-zero)"},
{RPCResult::Type::NUM, "version", "The peer version, such as 70001"},
{RPCResult::Type::STR, "subver", "The string version"},
{RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"},
Expand All @@ -146,7 +146,7 @@ static RPCHelpMan getpeerinfo()
{
{RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"},
}},
{RPCResult::Type::NUM, "minfeefilter", /*optional=*/true, "The minimum fee rate for transactions this peer accepts"},
{RPCResult::Type::NUM, "minfeefilter", "The minimum fee rate for transactions this peer accepts"},
{RPCResult::Type::OBJ_DYN, "bytessent_per_msg", "",
{
{RPCResult::Type::NUM, "msg", "The total bytes sent aggregated by message type\n"
Expand Down Expand Up @@ -200,6 +200,9 @@ static RPCHelpMan getpeerinfo()
ServiceFlags services{fStateStats ? statestats.their_services : ServiceFlags::NODE_NONE};
obj.pushKV("services", strprintf("%016x", services));
obj.pushKV("servicesnames", GetServicesNames(services));
if (fStateStats) {
obj.pushKV("relaytxes", statestats.m_relay_txs);
}
obj.pushKV("lastsend", count_seconds(stats.m_last_send));
obj.pushKV("lastrecv", count_seconds(stats.m_last_recv));
obj.pushKV("last_transaction", count_seconds(stats.m_last_tx_time));
Expand Down Expand Up @@ -235,8 +238,6 @@ static RPCHelpMan getpeerinfo()
heights.push_back(height);
}
obj.pushKV("inflight", heights);
obj.pushKV("relaytxes", statestats.m_relay_txs);
obj.pushKV("minfeefilter", ValueFromAmount(statestats.m_fee_filter_received));
obj.pushKV("addr_relay_enabled", statestats.m_addr_relay_enabled);
obj.pushKV("addr_processed", statestats.m_addr_processed);
obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited);
Expand All @@ -246,6 +247,7 @@ static RPCHelpMan getpeerinfo()
permissions.push_back(permission);
}
obj.pushKV("permissions", permissions);
obj.pushKV("minfeefilter", fStateStats ? ValueFromAmount(statestats.m_fee_filter_received) : 0);

UniValue sendPerMsgType(UniValue::VOBJ);
for (const auto& i : stats.mapSendBytesPerMsgType) {
Expand Down