Skip to content

Conversation

amitiuttarwar
Copy link
Contributor

two commits addressing small followups from #19725

@@ -108,6 +108,8 @@ will trigger BIP 125 (replace-by-fee) opt-in. (#11413)
- The `getpeerinfo` RPC now returns a `connection_type` field. This indicates
the type of connection established with the peer. It will return one of six
options. For more information, see the `getpeerinfo` help documentation.
Please note that this string output is unlikely to be stable in upcoming
releases as we iterate to best capture connection behaviors.
Copy link
Member

Choose a reason for hiding this comment

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

This is a generally correct statement, that could go into the RPC help instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

src/net.h Outdated
"manual (added via addnode RPC or -addnode/-connect configuration options)",
"addr-fetch (short-lived automatic connection for soliciting addresses)",
"feeler (short-lived automatic connection for testing addresses)"};
extern const std::vector<std::string> CONNECTION_TYPE_DOC;
Copy link
Contributor

Choose a reason for hiding this comment

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

f476fbd

not sure if this was asked before, CONNECTION_TYPE_DOC is used only in src/rpc/net.cpp, why not just have it there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, good point!

history: Marco suggested extracting in #19725 (comment). I thought a benefit was to keep CONNECTION_TYPE_DOC next to ConnectionType definition for future updates, but now we don't have that regardless. I've moved into src/rpc/net.cpp and added a comment on the ConnectionType enum for future code changes.

@amitiuttarwar amitiuttarwar force-pushed the 2020-09-getpeerinfo-conn-type-release-notes branch from f476fbd to 41dca08 Compare October 9, 2020 23:12
@maflcko maflcko added this to the 0.21.0 milestone Oct 12, 2020
@achow101
Copy link
Member

ACK 41dca08

@laanwj
Copy link
Member

laanwj commented Oct 15, 2020

Code review ACK 41dca08

@laanwj laanwj merged commit 9ad7cd2 into bitcoin:master Oct 15, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 16, 2020
…on type field

41dca08 [trivial] Extract connection type doc into file where it is used. (Amiti Uttarwar)
3069b56 [doc] Improve help for getpeerinfo connection_type field. (Amiti Uttarwar)

Pull request description:

  two commits addressing small followups from bitcoin#19725

  * first commit adds a clarification in the release notes that this field shouldn't be expected to be stable (suggested by sdaftuar in bitcoin#19725 (comment))

  * second commit moves the `CONNECTION_TYPE_DOC` object out of the header file to reduce the size of the binary (suggested by MarcoFalke in bitcoin#19725 (comment), he tested and found a decrease of 10kB)

ACKs for top commit:
  achow101:
    ACK 41dca08
  laanwj:
    Code review ACK 41dca08

Tree-SHA512: a555df978b4341fbe05deeb40a8a655f0d3c5c1c0adcc1737fd2cf61b204a5a24a301ca0c2b5a3616554d4abf8c57074d22dbda5a50d8450bc22c57679424985
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants