Skip to content

rpc: getaddrmaninfo followups #28565

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
merged 3 commits into from
Oct 16, 2023
Merged

rpc: getaddrmaninfo followups #28565

merged 3 commits into from
Oct 16, 2023

Conversation

stratospher
Copy link
Contributor

@stratospher stratospher commented Oct 3, 2023 •
edited
Loading

  • make getaddrmaninfo RPC public since it's not for development purposes only and regular users might find it useful. #26988 (comment)
  • add missing all_networks key to RPC help. #27511 (comment)
  • fix clang format spacing
  • add and use EnsureAddrman in RPC code. #27511 (comment)

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 3, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK theStack, pablomartin4btc, 0xB10C

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

This was referenced Oct 3, 2023
Merged
Open
kevkevinpal
kevkevinpal reviewed Oct 3, 2023
View reviewed changes
@stratospher stratospher force-pushed the getaddrmaninfo_followups branch from 0538579 to b62f7d6 Compare October 3, 2023 05:59
@0xB10C
Copy link
Contributor

0xB10C commented Oct 3, 2023

Code Review ACK b62f7d6

The changes look good to me. I think it makes sense to un-hide this RPC.

Reviewers might want to use --ignore-all-space on the first commit.

@DrahtBot DrahtBot mentioned this pull request Oct 3, 2023
Merged
@stratospher
Copy link
Contributor Author

added a new commit to include refactor suggestion in #27511 (comment).

@0xB10C
Copy link
Contributor

0xB10C commented Oct 3, 2023

added a new commit to include refactor suggestion in #27511 (comment).

That's useful for #28523 too. Can be included there in a follow up.

maflcko
maflcko reviewed Oct 3, 2023
View reviewed changes
@stratospher
rpc: getaddrmaninfo followups
3931e6a 
- make `getaddrmaninfo` RPC public since it's not for development
  purposes only and regular users might find it useful
- add missing `all_networks` key to RPC help
- use clang format spacing
@stratospher
doc: add release notes for bitcoin#27511
bf589a5
@stratospher
refactor: add and use EnsureAnyAddrman in rpc
e6e444c
@stratospher stratospher force-pushed the getaddrmaninfo_followups branch from 00421fe to e6e444c Compare October 4, 2023 03:25
@stratospher
Copy link
Contributor Author

That's useful for #28523 too. Can be included there in a follow up.

Rebased and included the EnsureAnyAddrman refactor in getrawaddrman too.

kevkevinpal
kevkevinpal reviewed Oct 4, 2023
View reviewed changes
theStack
theStack approved these changes Oct 10, 2023
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.

Code-review ACK e6e444c

@DrahtBot DrahtBot requested a review from 0xB10C October 10, 2023 10:42
pablomartin4btc
pablomartin4btc reviewed Oct 16, 2023
View reviewed changes
Copy link
Member

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

tested ACK e6e444c

Verified that the rpc command `getaddrmaninfo` is now public (not hidden anymore),

./src/bitcoin-cli -signet help
...
image
...

RPC Help shows `all_networks`

image

and RPC Help doesn't display "This RPC is for testing only." anymore before

image

after

image

@0xB10C
Copy link
Contributor

0xB10C commented Oct 16, 2023 •
edited
Loading

Code Review re-ACK e6e444c

@DrahtBot DrahtBot removed the request for review from 0xB10C October 16, 2023 08:11
@maflcko maflcko added this to the 27.0 milestone Oct 16, 2023
@fanquake fanquake merged commit 22fa1f4 into bitcoin:master Oct 16, 2023
@maflcko maflcko removed this from the 27.0 milestone Oct 16, 2023
luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Oct 19, 2023
@stratospher @luke-jr
rpc: getaddrmaninfo followups
b9afa91 
- make `getaddrmaninfo` RPC public since it's not for development
  purposes only and regular users might find it useful
- add missing `all_networks` key to RPC help
- use clang format spacing

Github-Pull: bitcoin#28565
Rebased-From: 3931e6a
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2023
@fanquake
Merge bitcoin#28565: rpc: getaddrmaninfo followups
60d9eb1
@stratospher stratospher deleted the getaddrmaninfo_followups branch July 17, 2024 06:39
@bitcoin bitcoin locked and limited conversation to collaborators Jul 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
4 more reviewers

@maflcko maflcko maflcko left review comments

@kevkevinpal kevkevinpal kevkevinpal left review comments

@pablomartin4btc pablomartin4btc pablomartin4btc 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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@stratospher @DrahtBot @0xB10C @theStack @maflcko @kevkevinpal @pablomartin4btc @fanquake