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

Make the number of nominations configurable #8368

Merged
53 commits merged into from
Mar 25, 2021

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Mar 16, 2021

Builds on top of #8113, and makes the number of votes per nominator configurable, while adding integrity checks to prevent footguns.

I don't intend to change the value to 24 in the companion yet. That can happen afterwards.

cc @wpank @gavofyork

This has already been audited (almost identical logic in #7929) and the main feedback was that this will need re-benchmarking.

polkadot companion: paritytech/polkadot#2683

kianenigma and others added 30 commits January 15, 2021 13:19
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
…om:paritytech/substrate into kiz-election-provider-2-two-phase-unsigned
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
…om:paritytech/substrate into kiz-election-provider-2-two-phase-unsigned
…/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs
…om:paritytech/substrate into kiz-election-provider-2-two-phase-unsigned
Base automatically changed from kiz-election-provider-21-enable-multi-phase to master March 20, 2021 08:43
@gui1117
Copy link
Contributor

gui1117 commented Mar 23, 2021

this needs to merge master to be easier to review

@kianenigma
Copy link
Contributor Author

Done, but it was quite a mess, I have to re-review myself and make sure I didn't mess things up.

@kianenigma
Copy link
Contributor Author

Does someone know a way to also remove duplicate commits, while keeping the PR? I could just generate diff from this branch, and apply it to a new branch to get cleaner commit history, but I prefer keeping the PR.

@coriolinus
Copy link
Contributor

git rebase -i HEAD~3 lets you omit / squash / rearrange the last 3 commits. Of course you can specify whatever number of them you want. Then just git push -f.

@kianenigma
Copy link
Contributor Author

cheers @coriolinus

frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved

// We only accept data provider who's maximum votes per voter matches our
// `T::CompactSolution`'s `LIMIT`.
assert_eq!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that the main reason why I need to add MAXIMUM_VOTES_PER_VOTER const is so that I can make this assertion -- otherwise this might be a foot gun.

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.

looks good to me

bin/node/runtime/src/lib.rs Show resolved Hide resolved
kianenigma added a commit to paritytech/polkadot that referenced this pull request Mar 24, 2021
@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Mar 25, 2021

Trying merge.

@ghost ghost merged commit 2f69b2d into master Mar 25, 2021
@ghost ghost deleted the kiz-election-provider-21-24-nominators branch March 25, 2021 09:15
ghost pushed a commit to paritytech/polkadot that referenced this pull request Mar 25, 2021
* Companion for paritytech/substrate#8368

* "Update Substrate"

Co-authored-by: parity-processbot <>
@kianenigma kianenigma added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Apr 13, 2021
hirschenberger pushed a commit to hirschenberger/substrate that referenced this pull request Apr 14, 2021
* Base features and traits.

* pallet and unsigned phase

* Undo bad formattings.

* some formatting cleanup.

* Small self-cleanup.

* Make it all build

* self-review

* Some doc tests.

* Some changes from other PR

* Fix session test

* Update Cargo.lock

* Update frame/election-provider-multi-phase/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Some review comments

* Rename + make encode/decode

* Do an assert as well, just in case.

* Fix build

* Update frame/election-provider-multi-phase/src/unsigned.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Las comment

* fix staking fuzzer.

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Add one last layer of feasibility check as well.

* Last fixes to benchmarks

* Some more docs.

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Some nits

* It all works

* Some self cleanup

* Update frame/staking/src/lib.rs

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

* remove most todos.

* Round of self-review.

* Fix migration

* clean macro

* Revert wrong merge

* Make the number of nominations configurable

* Self reivew

* renmae.

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
ggwpez pushed a commit to ggwpez/runtimes that referenced this pull request Mar 10, 2023
* Companion for paritytech/substrate#8368

* "Update Substrate"

Co-authored-by: parity-processbot <>
ggwpez pushed a commit to ggwpez/runtimes that referenced this pull request Jul 13, 2023
* Companion for paritytech/substrate#8368

* "Update Substrate"

Co-authored-by: parity-processbot <>
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. C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants