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: Add listen address to incoming connections in `getpeerinfo` #10478

Merged
merged 2 commits into from Jun 5, 2017

Conversation

Projects
None yet
3 participants
@laanwj
Member

laanwj commented May 30, 2017

This adds the listening address on which incoming connections were received to the CNode and CNodeStats structures.

The address is reported in getpeerinfo.

This can be useful for distinguishing connections received on different listening ports (e.g. when using a different listening port for Tor hidden service connections) or different networks.

@ryanofsky

Slightly tested ACK 63c7158.

I would maybe consider renaming addrlisten to addrbind and not just restricting it to incoming connections.

Show outdated Hide outdated src/net.cpp Outdated
Show outdated Hide outdated src/rpc/net.cpp Outdated
Show outdated Hide outdated src/net.h Outdated
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 30, 2017

Member

I would maybe consider renaming addrlisten to addrbind and not just restricting it to incoming connections.

Good point. We could also call getsockname for for outgoing connections, though the meaning is somewhat more obscure.

We use both bind and listen in command line options so I wasn't sure what to call it, but you're right it correlates more with bind's meaning.

Member

laanwj commented May 30, 2017

I would maybe consider renaming addrlisten to addrbind and not just restricting it to incoming connections.

Good point. We could also call getsockname for for outgoing connections, though the meaning is somewhat more obscure.

We use both bind and listen in command line options so I wasn't sure what to call it, but you're right it correlates more with bind's meaning.

@ryanofsky

utACK a501d01 with one comment. Thanks for taking some of my suggestions.

Show outdated Hide outdated src/rpc/net.cpp Outdated
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 31, 2017

Member

@ryanofsky yes, should have gotten most of them, I still intend to add a python tests. Now that both incoming and outgoing connections have the addrbind it's possible to correlate 100% the connection ids between local nodes.

Member

laanwj commented May 31, 2017

@ryanofsky yes, should have gotten most of them, I still intend to add a python tests. Now that both incoming and outgoing connections have the addrbind it's possible to correlate 100% the connection ids between local nodes.

laanwj added some commits May 30, 2017

rpc: Add listen address to incoming connections in `getpeerinfo`
This adds the listening address on which incoming connections were received to the
CNode and CNodeStats structures.

The address is reported in `getpeerinfo`.

This can be useful for distinguishing connections received on different listening ports
(e.g. when using a different listening port for Tor hidden service connections)
or different networks.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 5, 2017

Member

Rebased, squashed, and added a basic python test.

Member

laanwj commented Jun 5, 2017

Rebased, squashed, and added a basic python test.

@laanwj laanwj merged commit 3457331 into bitcoin:master Jun 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jun 5, 2017

Merge #10478: rpc: Add listen address to incoming connections in `get…
…peerinfo`

3457331 test: Add test for `getpeerinfo` `bindaddr` field (Wladimir J. van der Laan)
a7e3c28 rpc: Add listen address to incoming connections in `getpeerinfo` (Wladimir J. van der Laan)

Tree-SHA512: bcd58bca2d35fc9698e958e22a7cf8268a6c731a3a309df183f43fc5e725a88ae09f006290fde7aa03cee9a403e2e25772097409677cedbce8f267e01e9040f6

@laanwj laanwj referenced this pull request Jun 5, 2017

Closed

TODO for release notes 0.15.0 #9889

12 of 12 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment