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

consensus: create new config for each authority #1916

Merged
merged 1 commit into from
May 12, 2022

Conversation

bmwill
Copy link
Contributor

@bmwill bmwill commented May 11, 2022

Create a new Narwhal configuration for each authority we want to start
in order to avoid binding to the exact same ports. Eventaully we'll want
this config to be a part of a larger ValidatorConfig.

Create a new Narwhal configuration for each authority we want to start
in order to avoid binding to the exact same ports. Eventaully we'll want
this config to be a part of a larger ValidatorConfig.
@bmwill bmwill requested review from lxfind and arun-koshy May 11, 2022 23:46
@huitseeker huitseeker requested a review from asonnino May 12, 2022 00:23
@asonnino
Copy link
Contributor

asonnino commented May 12, 2022

Create a new Narwhal configuration for each authority we want to start
in order to avoid binding to the exact same ports. Eventaully we'll want
this config to be a part of a larger ValidatorConfig.

The consensus parameters do not contain port information at all (those are found in the committee config). The parameters only contains things like block size, max block delays, etc and it is important (for performance) that every node has the same.

@bmwill
Copy link
Contributor Author

bmwill commented May 12, 2022

@asonnino while they may not contain network information for the primary/worker and transaction interfaces, they most certainly do contain network information for the interface @arun-koshy recently added.

https://github.com/MystenLabs/narwhal/blob/main/config/src/lib.rs#L110-L111

https://github.com/MystenLabs/narwhal/blob/main/config/src/lib.rs#L118-L119

@asonnino
Copy link
Contributor

@asonnino while they may not contain network information for the primary/worker and transaction interfaces, they most certainly do contain network information for the interface @arun-koshy recently added.

https://github.com/MystenLabs/narwhal/blob/main/config/src/lib.rs#L110-L111

https://github.com/MystenLabs/narwhal/blob/main/config/src/lib.rs#L118-L119

Ha I see

@bmwill
Copy link
Contributor Author

bmwill commented May 12, 2022

yeah, whether that was intended or not could be up for debate. I'm currently working on overhauling sui's configs. so i'm finding little quirks like this everywhere :)

@bmwill bmwill merged commit 6544875 into MystenLabs:main May 12, 2022
@bmwill bmwill deleted the fix-narwhal-port branch May 12, 2022 15:47
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.

None yet

3 participants