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

Specify proposer when constructing validator set in light client #706

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

romac
Copy link
Member

@romac romac commented Nov 26, 2020

Close: #705

This PR also refactors the constructors of validator::Set and precomputes the total voting power in the set.

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG.md

ancazamfir
ancazamfir previously approved these changes Dec 1, 2020
Copy link
Contributor

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Looks good. Also tried it with some changes in ibc-rs and all is fine. Thanks!

pub fn new(
validators: Vec<Info>,
proposer: Option<Info>,
total_voting_power: vote::Power,
total_voting_power: Option<vote::Power>,
Copy link
Contributor

Choose a reason for hiding this comment

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

why would someone need to pass total_voting_power if it can be computed?

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 I wonder about this as well. Maybe @ebuchman knows why it was done like this?

Copy link
Member

Choose a reason for hiding this comment

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

they do not I've been campaigning against total voting power and proposer fields in this type for a few weeks now! I suspect it was done without really thinking about it ...

light-client/src/components/io.rs Show resolved Hide resolved
Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thanethomson thanethomson merged commit 56d7e49 into master Dec 14, 2020
@thanethomson thanethomson deleted the romac/proposer-validator-set branch December 14, 2020 15:46
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.

Specify the proposer in the validator set of fetched light blocks
5 participants