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

Permit fullnodes to advertise a different address than what they bind to #485

Merged
merged 3 commits into from
Jun 29, 2018

Conversation

mvines
Copy link
Member

@mvines mvines commented Jun 28, 2018

This permits GCP-based nodes to talk to each other over public IP addresses (provided firewall ingress rules are setup correctly for the nodes).

The leader/validator need to advertise their public IP so they can be found, but cannot bind to their public IP as they only see their private IP.

The issue of NAT traversal for client nodes is not addressed here.

@aeyakovenko
Copy link
Member

wooooohoooo!!!!

@@ -126,30 +126,48 @@ fn main() {
}
}

let mut local_gossip_addr = bind_addr.clone();
Copy link
Contributor

Choose a reason for hiding this comment

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

very clever reuse of bind_addr 👍

@@ -156,6 +156,17 @@ impl ThinClient {
}
self.process_response(resp);
}
match self.recv_response() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete that if let Ok above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah crap. Yeah, rebase error I think. Thanks for catching

@@ -126,30 +126,48 @@ fn main() {
}
}

let mut local_gossip_addr = bind_addr.clone();
local_gossip_addr.set_port(repl_data.gossip_addr.port());
Copy link
Contributor

Choose a reason for hiding this comment

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

So ugly! You thinking ReplicatedData should separate IP and ports into separate fields?

Copy link
Member Author

Choose a reason for hiding this comment

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

🙈 yeah something like that would totally help. That was more Rust than I'm comfortable busting out at this point, you're ok with this as is for now?

@mvines
Copy link
Member Author

mvines commented Jun 29, 2018

Fixes applied, @garious r?

@mvines mvines added the automerge Merge this Pull Request automatically once CI passes label Jun 29, 2018
@solana-grimes solana-grimes merged commit 47917d0 into solana-labs:master Jun 29, 2018
@@ -1,11 +1,54 @@
#!/bin/bash

num_tokens=${1:-1000000000}
Copy link
Contributor

Choose a reason for hiding this comment

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

?

EOF
}

while getopts "h?n:p:P" opt; do
Copy link
Contributor

Choose a reason for hiding this comment

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

there are some error cases you might handle here, but it's not a gating issue, IMHO

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh like what? I’ll fix

Copy link
Contributor

Choose a reason for hiding this comment

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

I always have to go look at my old getopts() calls along with the bash manpage...

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/cornholington/autocross/blob/master/results/esc.sh captures my latest thinking on getopts(). note the initial ':' in the string and the unknown opt case statement

@mvines mvines deleted the ip branch July 8, 2018 03:10
CriesofCarrots pushed a commit that referenced this pull request Feb 28, 2020
mvines pushed a commit to mvines/solana that referenced this pull request Jun 15, 2020
mvines pushed a commit that referenced this pull request Jun 15, 2020
vkomenda pushed a commit to vkomenda/solana that referenced this pull request Aug 29, 2021
Bumps [rollup](https://github.com/rollup/rollup) from 2.27.1 to 2.28.1.
- [Release notes](https://github.com/rollup/rollup/releases)
- [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md)
- [Commits](rollup/rollup@v2.27.1...v2.28.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
brooksprumo added a commit to brooksprumo/solana that referenced this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants