/bitcoin

Populate services in GetLocalAddress #10424

Merged
merged 1 commit into from May 22, 2017
+1 −1

Conversation

Reviewers

@gmaxwell gmaxwell

Assignees
No one assigned
Labels
P2P
Projects
None yet
Milestone
No milestone
7 participants
@morcos @fanquake @theuni @laanwj @sipa @TheBlueMatt @gmaxwell
@morcos
Contributor

morcos commented May 18, 2017

Previously if we didn't have any local addresses, GetLocalAddress would return
0.0.0.0 and then we'd swap in a peer's notion of our address in AdvertiseLocal,
but then nServices would never get set.

This would prevent a node behind a NAT from having its nServices set in addr messages gossiped between peers and could lead to no incoming connections.

This should be backported to 0.14 (and 0.13 if we do that)

I can't promise that I've fully understood all the implications here.
@morcos morcos Populate services in GetLocalAddress 
Previously if we didn't have any local addresses, GetLocalAddress would return
0.0.0.0 and then we'd swap in a peer's notion of our address in AdvertiseLocal,
but then nServices would never get set.
3070134
@gmaxwell

gmaxwell approved these changes May 18, 2017
View changes

utACK.

@fanquake fanquake added P2P Needs backport labels May 18, 2017

@fanquake
Contributor

fanquake commented May 19, 2017

Restarted the failing test.
@theuni
Member

theuni commented May 19, 2017

utACK 3070134. Failing test was unrelated. I'm working on a fix for it, though.
@laanwj
Owner

laanwj commented May 20, 2017

utACK 3070134
@sipa
Owner

sipa commented May 20, 2017

utACK 3070134

@laanwj laanwj merged commit 3070134 into bitcoin:master May 22, 2017
1 check passed

1 check passed

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

@laanwj laanwj added a commit that referenced this pull request May 22, 2017

@laanwj laanwj Merge #10424: Populate services in GetLocalAddress 
3070134 Populate services in GetLocalAddress (Alex Morcos)

Tree-SHA512: b822d7e898ccb5b959ccb1b1d0f159f27190c2105fbf8f5b67ae54debab6fa6a0723d65a66e7341f55cd0d80398c3fbb39a41e067b9f4e0bfa2c1cd366032404
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
6512913

@laanwj laanwj added a commit that referenced this pull request May 22, 2017

@morcos @laanwj morcos + laanwj Populate services in GetLocalAddress 
Previously if we didn't have any local addresses, GetLocalAddress would return
0.0.0.0 and then we'd swap in a peer's notion of our address in AdvertiseLocal,
but then nServices would never get set.

Github-Pull: #10424
Rebased-From: 3070134
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
37a8fc5

@laanwj laanwj added a commit that referenced this pull request May 22, 2017

@morcos @laanwj morcos + laanwj Populate services in GetLocalAddress 
Previously if we didn't have any local addresses, GetLocalAddress would return
0.0.0.0 and then we'd swap in a peer's notion of our address in AdvertiseLocal,
but then nServices would never get set.

Github-Pull: #10424
Rebased-From: 3070134
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
b654842

laanwj removed the Needs backport label May 22, 2017

@TheBlueMatt
Contributor

TheBlueMatt commented May 29, 2017

Postumous utACK

@nomnombtc nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Jul 17, 2017

@morcos @nomnombtc morcos + nomnombtc Populate services in GetLocalAddress 
Previously if we didn't have any local addresses, GetLocalAddress would return
0.0.0.0 and then we'd swap in a peer's notion of our address in AdvertiseLocal,
but then nServices would never get set.

Github-Pull: #10424
Rebased-From: 3070134
 2ab7d60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment