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

Add WeightInfo to Babe and Grandpa Pallet #7155

Merged
4 commits merged into from
Sep 21, 2020
Merged

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Sep 21, 2020

This PR makes no logical changes to the Babe or Grandpa Pallet. It simply refactors the custom weight formula into the WeightInfo pattern we have used across the other pallets.

polkadot companion: paritytech/polkadot#1736

@shawntabrizi shawntabrizi added A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Sep 21, 2020
@shawntabrizi shawntabrizi changed the title Add WeightInfo to Babe Pallet Add WeightInfo to Babe and Grandpa Pallet Sep 21, 2020
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

lgtm

// calculate the weight but we set a floor of 100 validators.
let validator_count = validator_count.max(100) as u64;

// worst case we are considering is that the given offender
Copy link
Contributor

Choose a reason for hiding this comment

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

@andresilva you should keep an eye on this: https://github.com/paritytech/substrate/issues/6641

TLDR; It is really nonsensical that we store all backers of a validators and slash all of them but reward only a portion of them (top 64). I am considering making this always top 64, so here you can always ensure that MAX_NOMINATORS = 64;

Copy link
Contributor

Choose a reason for hiding this comment

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

Good! That way we can drop this assumption and have a strict upper bound on the number of nominators a slash will affect.

frame/grandpa/src/default_weights.rs Outdated Show resolved Hide resolved
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
@shawntabrizi
Copy link
Member Author

bot merge

@ghost
Copy link

ghost commented Sep 21, 2020

Trying merge.

@ghost ghost merged commit 93b2c36 into master Sep 21, 2020
@ghost ghost deleted the shawntabrizi-babe-weightinfo branch September 21, 2020 13:31
rakanalh pushed a commit to rakanalh/substrate that referenced this pull request Sep 22, 2020
* Add `WeightInfo` to Babe Pallet

* Also grandpa

* Update frame/grandpa/src/default_weights.rs

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants