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

[RPC][Net] Add getnodeaddresses RPC command #2503

Merged
merged 12 commits into from
Aug 12, 2021

Conversation

Fuzzbawls
Copy link
Collaborator

Implement a new RPC command (getnodeaddresses) that provides RPC access to the node's addrman. It may be useful for PIVX wallets to broadcast their transactions over tor for improved privacy without using the centralized DNS seeds.

I've included upstream improvements to this feature here as well, and this RPC command was used to scrape the Tor v3 hardcoded seed nodes added in #2502


Based on the following upstream PRs:

chris-belcher and others added 3 commits August 11, 2021 08:19
New getnodeaddresses call gives access via RPC to the peers known by
the node. It may be useful for PIVX wallets to broadcast their
transactions over tor for improved privacy without using the
centralized DNS seeds. getnodeaddresses is very similar to the getaddr
p2p method.

Tests the new rpc call by feeding IP address to a test node via the p2p
protocol, then obtaining someone of those addresses with
getnodeaddresses and checking that they are a subset.
CAddrMan.GetAddr() would previously limit the number and percentage of
addresses returned (to ADDRMAN_GETADDR_MAX (1000) and
ADDRMAN_GETADDR_MAX_PCT (23) respectively). Instead, make it the callers
responsibility to specify the maximum addresses and percentage they want
returned.

For net_processing, the maximums are MAX_ADDR_TO_SEND (1000) and
MAX_PCT_ADDR_TO_SEND (23). For rpc/net, the maximum is specified by the
client.
@Fuzzbawls Fuzzbawls added this to the 5.3.0 milestone Aug 12, 2021
@Fuzzbawls Fuzzbawls self-assigned this Aug 12, 2021
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

There's a bug with missing return value in CConnman::AddNewAddresses which causes UB. Otherwise looking good.

@Fuzzbawls Fuzzbawls force-pushed the 2021_rpc-getnodeaddresses branch from 4dc14d9 to e9d28da Compare August 12, 2021 10:21
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

all good, tested ACK e9d28da.
Have found +10 nodes running v3 onion addresses in testnet at the moment (different to mainnet that my node is alone there).

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK e9d28da and merging...

@random-zebra random-zebra merged commit 69979ce into PIVX-Project:master Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants