Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

improve quality of vote_collector module #7984

Merged
merged 1 commit into from
Feb 27, 2018
Merged

Conversation

debris
Copy link
Collaborator

@debris debris commented Feb 23, 2018

I was just reading the code to understand what's going on over there and added several insubstantial improvements

@debris debris added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust. labels Feb 23, 2018
@5chdn 5chdn added this to the 1.10 milestone Feb 23, 2018
@rphmeier
Copy link
Contributor

rphmeier commented Feb 23, 2018

I think eventually we can replace this module with the implementation here for vote accumulation. https://github.com/paritytech/polkadot/blob/master/substrate/bft/src/generic/accumulator.rs

There are a few small improvements like a generalization of justifications (to support stuff like BLS-threshold signatures) and misbehavior generation, but otherwise it serves the same purpose in a (IMO) clearer way.

FWIW that entire module contains a futures-based implementation of something pretty close to tendermint, so if we had generate_seal return a future (desired for AuraV2 also) we could probably just plug the whole thing in.

@debris
Copy link
Collaborator Author

debris commented Feb 24, 2018

@rphmeier I created a new issue for that ;)

Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

LGTM!

@andresilva andresilva merged commit e32b600 into master Feb 27, 2018
@5chdn 5chdn deleted the random_style_improvement branch February 27, 2018 19:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants