Skip to content

rpc, cli: describe quality/recency filtering in getnodeaddresses and -addrinfo #24370

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

Jump to bottom
Merged
maflcko merged 2 commits into from
Feb 21, 2022
Merged

rpc, cli: describe quality/recency filtering in getnodeaddresses and -addrinfo #24370

maflcko merged 2 commits into from
Feb 21, 2022

Conversation

@jonatack
Copy link
Member

@jonatack jonatack commented Feb 17, 2022 •
edited
Loading

Addresses #24278.

$ bitcoin-cli help getnodeaddresses
getnodeaddresses ( count "network" )

Return known addresses, after filtering for quality and recency.
These can potentially be used to find new peers in the network.
The total number of addresses known to the node may be higher.

$ bitcoin-cli -help | grep -A3 addrinfo
  -addrinfo
       Get the number of addresses known to the node, per network and total,
       after filtering for quality and recency. The total number of
       addresses known to the node may be higher.

mzumsande
mzumsande reviewed Feb 17, 2022
View reviewed changes
Copy link
Contributor

@mzumsande mzumsande left a comment

Choose a reason for hiding this comment

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

Concept ACK, mentioning this distinction makes sense to me.

@jonatack jonatack changed the title doc: mention quality filter in getnodeaddresses and -addrinfo helps doc: mention quality/recency filtering in getnodeaddresses and -addrinfo helps Feb 17, 2022
@jonatack jonatack force-pushed the mention-quality-filter-in-getnodeaddresses-and-addrinfo branch from c89cdd8 to 2ed2388 Compare February 17, 2022 22:25
ghost
ghost approved these changes Feb 17, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

ACK 2ed2388

jarolrod
jarolrod reviewed Feb 18, 2022
View reviewed changes
Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK 2ed23881d9e51bfff06fe89ffdb396bd1577bff8

./src/bitcoin-cli help getnodeaddresses
getnodeaddresses ( count "network" )

Return known addresses, after filtering for quality and recency.
These can potentially be used to find new peers in the network.
The total number of addresses known to the node may be higher.

...

./src/bitcoin-cli -help | grep -A3 addrinfo
  -addrinfo
       Get the number of addresses known to the node, per network and total,
       after filtering for quality and recency. The total number of
       addresses known to the node may be higher.

theStack
theStack approved these changes Feb 20, 2022
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 2ed23881d9e51bfff06fe89ffdb396bd1577bff8

@jonatack jonatack changed the title doc: mention quality/recency filtering in getnodeaddresses and -addrinfo helps rpc, cli: describe quality/recency filtering in getnodeaddresses and -addrinfo Feb 21, 2022
@jonatack
Copy link
Member Author

jonatack commented Feb 21, 2022

Rebased.

@jonatack jonatack force-pushed the mention-quality-filter-in-getnodeaddresses-and-addrinfo branch from 2ed2388 to ce69084 Compare February 21, 2022 13:37
ghost
ghost approved these changes Feb 21, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

reACK ce69084

@maflcko
Copy link
Member

maflcko commented Feb 21, 2022

@mzumsande Did you want to re-review this before merge, since you left a comment? If not, then it can be merged now.

@mzumsande
Copy link
Contributor

mzumsande commented Feb 21, 2022

Did you want to re-review this before merge, since you left a comment? If not, then it can be merged now.

Thanks, Code Review ACK ce69084

@maflcko maflcko merged commit 48a90c6 into bitcoin:master Feb 21, 2022
@jonatack jonatack deleted the mention-quality-filter-in-getnodeaddresses-and-addrinfo branch February 21, 2022 17:11
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 22, 2022
Merge bitcoin#24370: rpc, cli: describe quality/recency filtering in …
f24cfd1 
…getnodeaddresses and -addrinfo

ce69084 cli: describe quality/recency filtering in -addrinfo (Jon Atack)
7c97561 rpc: describe quality/recency filtering in getnodeaddresses (Jon Atack)

Pull request description:

  Addresses bitcoin#24278.

  ```
  $ bitcoin-cli help getnodeaddresses
  getnodeaddresses ( count "network" )

  Return known addresses, after filtering for quality and recency.
  These can potentially be used to find new peers in the network.
  The total number of addresses known to the node may be higher.
  ```
  ```
  $ bitcoin-cli -help | grep -A3 addrinfo
    -addrinfo
         Get the number of addresses known to the node, per network and total,
         after filtering for quality and recency. The total number of
         addresses known to the node may be higher.
  ```

ACKs for top commit:
  mzumsande:
    Thanks, Code Review ACK ce69084
  prayank23:
    reACK bitcoin@ce69084

Tree-SHA512: 82d23b15e64a99411eb8e70d7267a1b4f23182fabe072e824277569d9677e392b466be63f00e3d157d7db94bbe032d53f12ad4ab30b55b7b8a629c37d80d1d8c
@stratospher stratospher mentioned this pull request Jan 29, 2023
Open
@bitcoin bitcoin locked and limited conversation to collaborators Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@jarolrod jarolrod jarolrod left review comments

@mzumsande mzumsande mzumsande left review comments

@theStack theStack theStack approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RPC/REST/ZMQ Utils/log/libs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jonatack @maflcko @mzumsande @theStack @jarolrod @DrahtBot