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

Dynamically bind to available UDP ports in Fullnode #920

Merged
merged 2 commits into from
Aug 25, 2018

Conversation

pgarg66
Copy link
Contributor

@pgarg66 pgarg66 commented Aug 9, 2018

No description provided.

@pgarg66 pgarg66 added the work in progress This isn't quite right yet label Aug 9, 2018
@pgarg66 pgarg66 requested review from mvines and rob-solana August 9, 2018 21:37
src/crdt.rs Outdated
@@ -1353,6 +1354,55 @@ impl TestNode {
},
}
}
pub fn new_with_external_ip(pubkey: Pubkey, ip: IpAddr) -> TestNode {
fn bind() -> (u16, UdpSocket) {
match udp_random_bind(8000, 10000, 5) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we use 8001, or maybe 8100, instead of 8000? 8000 is hard coded in places like

let bind_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 8000);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it to 8100 for now. I think, going forward, only NCP port for leader needs to be hardcoded. If someday we add a DNS entry with SRV record, we can potentially remove that as well.

@pgarg66 pgarg66 force-pushed the dynamic-ports branch 2 times, most recently from 4bdc510 to 6f13fa0 Compare August 9, 2018 22:23
@pgarg66 pgarg66 force-pushed the dynamic-ports branch 2 times, most recently from 19b3276 to 1f24fe9 Compare August 9, 2018 22:43
@pgarg66
Copy link
Contributor Author

pgarg66 commented Aug 9, 2018

It needs more work. Looks like wallet is also reading config files to figure out the ports.

@pgarg66 pgarg66 removed the work in progress This isn't quite right yet label Aug 24, 2018
- Also removed hard coding of port range from CRDT
@pgarg66 pgarg66 requested review from garious and aeyakovenko August 24, 2018 16:58
@pgarg66
Copy link
Contributor Author

pgarg66 commented Aug 24, 2018

@garious could you pls review when you get a chance?

@pgarg66 pgarg66 merged commit d3fac8a into solana-labs:master Aug 25, 2018
@pgarg66 pgarg66 deleted the dynamic-ports branch August 25, 2018 17:24
vkomenda pushed a commit to vkomenda/solana that referenced this pull request Aug 29, 2021
…olana-labs#920)

Bumps [@rollup/plugin-commonjs](https://github.com/rollup/plugins) from 16.0.0 to 17.0.0.
- [Release notes](https://github.com/rollup/plugins/releases)
- [Commits](rollup/plugins@commonjs-v16.0.0...commonjs-v17.0.0)

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

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants