Skip to content

Commit 4435b08

Browse files
kianenigmashawntabrizigui1117Parity Benchmarking Botcoriolinus
authored andcommitted
Audit fixes for election/staking decoupling part 2 (paritytech#8167)
* 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 <[email protected]> * 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 <[email protected]> * 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 * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Fix doc * Mkae ci green * Audit fixes for election-provider: part 2 signed phase. * Fix weight * Some grumbles. * Try and weigh to get_npos_voters * Fix build * Fix line width * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Fix tests. * Fix build * Reorg some stuff * More reorg. * Reorg done. * Fix build * Another rename * Fix build * Update frame/election-provider-multi-phase/src/mock.rs Co-authored-by: Peter Goodspeed-Niklaus <[email protected]> * nit * better doc * Line width * Fix build * Self-review * Self-review * Fix wan * 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_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs * fix build and review comments. * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Shawn Tabrizi <[email protected]> * add comment Co-authored-by: Shawn Tabrizi <[email protected]> Co-authored-by: Guillaume Thiolliere <[email protected]> Co-authored-by: Parity Benchmarking Bot <[email protected]> Co-authored-by: Peter Goodspeed-Niklaus <[email protected]>
1 parent 2c52359 commit 4435b08

32 files changed

Lines changed: 1085 additions & 723 deletions

File tree

Cargo.lock

Lines changed: 357 additions & 353 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ members = [
7676
"frame/try-runtime",
7777
"frame/elections",
7878
"frame/election-provider-multi-phase",
79+
"frame/election-provider-support",
7980
"frame/example",
8081
"frame/example-offchain-worker",
8182
"frame/example-parallel",
@@ -145,7 +146,6 @@ members = [
145146
"primitives/database",
146147
"primitives/debug-derive",
147148
"primitives/externalities",
148-
"primitives/election-providers",
149149
"primitives/finality-grandpa",
150150
"primitives/inherents",
151151
"primitives/io",

frame/babe/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ pallet-offences = { version = "3.0.0", path = "../offences" }
3737
pallet-staking = { version = "3.0.0", path = "../staking" }
3838
pallet-staking-reward-curve = { version = "3.0.0", path = "../staking/reward-curve" }
3939
sp-core = { version = "3.0.0", path = "../../primitives/core" }
40-
sp-election-providers = { version = "3.0.0", path = "../../primitives/election-providers" }
40+
frame-election-provider-support = { version = "3.0.0", path = "../election-provider-support" }
4141

4242
[features]
4343
default = ["std"]

frame/babe/src/mock.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use sp_consensus_babe::{AuthorityId, AuthorityPair, Slot};
3737
use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof};
3838
use sp_staking::SessionIndex;
3939
use pallet_staking::EraIndex;
40-
use sp_election_providers::onchain;
40+
use frame_election_provider_support::onchain;
4141
use pallet_session::historical as pallet_session_historical;
4242

4343
type DummyValidatorId = u64;
@@ -187,6 +187,7 @@ parameter_types! {
187187
impl onchain::Config for Test {
188188
type AccountId = <Self as frame_system::Config>::AccountId;
189189
type BlockNumber = <Self as frame_system::Config>::BlockNumber;
190+
type BlockWeights = ();
190191
type Accuracy = Perbill;
191192
type DataProvider = Staking;
192193
}

frame/election-provider-multi-phase/Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ sp-std = { version = "3.0.0", default-features = false, path = "../../primitives
2626
sp-runtime = { version = "3.0.0", default-features = false, path = "../../primitives/runtime" }
2727
sp-npos-elections = { version = "3.0.0", default-features = false, path = "../../primitives/npos-elections" }
2828
sp-arithmetic = { version = "3.0.0", default-features = false, path = "../../primitives/arithmetic" }
29-
sp-election-providers = { version = "3.0.0", default-features = false, path = "../../primitives/election-providers" }
29+
frame-election-provider-support = { version = "3.0.0", default-features = false, path = "../election-provider-support" }
3030

3131
# Optional imports for benchmarking
3232
frame-benchmarking = { version = "3.1.0", default-features = false, path = "../benchmarking", optional = true }
@@ -41,9 +41,9 @@ substrate-test-utils = { version = "3.0.0", path = "../../test-utils" }
4141
sp-io = { version = "3.0.0", path = "../../primitives/io" }
4242
sp-core = { version = "3.0.0", path = "../../primitives/core" }
4343
sp-tracing = { version = "3.0.0", path = "../../primitives/tracing" }
44-
sp-election-providers = { version = "3.0.0", features = ["runtime-benchmarks"], path = "../../primitives/election-providers" }
44+
frame-election-provider-support = { version = "3.0.0", features = ["runtime-benchmarks"], path = "../election-provider-support" }
4545
pallet-balances = { version = "3.0.0", path = "../balances" }
46-
frame-benchmarking = { path = "../benchmarking" , version = "3.1.0"}
46+
frame-benchmarking = { version = "3.1.0", path = "../benchmarking" }
4747

4848
[features]
4949
default = ["std"]
@@ -60,7 +60,7 @@ std = [
6060
"sp-runtime/std",
6161
"sp-npos-elections/std",
6262
"sp-arithmetic/std",
63-
"sp-election-providers/std",
63+
"frame-election-provider-support/std",
6464
"log/std",
6565
]
6666
runtime-benchmarks = [

frame/election-provider-multi-phase/src/benchmarking.rs

Lines changed: 54 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// This file is part of Substrate.
22

3-
// Copyright (C) 2020 Parity Technologies (UK) Ltd.
3+
// Copyright (C) 2021 Parity Technologies (UK) Ltd.
44
// SPDX-License-Identifier: Apache-2.0
55

66
// Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,17 +19,16 @@
1919
2020
use super::*;
2121
use crate::Module as MultiPhase;
22-
23-
pub use frame_benchmarking::{account, benchmarks, whitelist_account, whitelisted_caller};
22+
use frame_benchmarking::impl_benchmark_test_suite;
2423
use frame_support::{assert_ok, traits::OnInitialize};
2524
use frame_system::RawOrigin;
2625
use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng};
27-
use sp_election_providers::Assignment;
26+
use frame_election_provider_support::Assignment;
2827
use sp_arithmetic::traits::One;
2928
use sp_runtime::InnerOf;
3029
use sp_std::convert::TryInto;
3130

32-
const SEED: u32 = 0;
31+
const SEED: u32 = 999;
3332

3433
/// Creates a **valid** solution with exactly the given size.
3534
///
@@ -55,9 +54,9 @@ fn solution_with_size<T: Config>(
5554

5655
// first generates random targets.
5756
let targets: Vec<T::AccountId> =
58-
(0..size.targets).map(|i| account("Targets", i, SEED)).collect();
57+
(0..size.targets).map(|i| frame_benchmarking::account("Targets", i, SEED)).collect();
5958

60-
let mut rng = SmallRng::seed_from_u64(999u64);
59+
let mut rng = SmallRng::seed_from_u64(SEED as u64);
6160

6261
// decide who are the winners.
6362
let winners = targets
@@ -75,7 +74,7 @@ fn solution_with_size<T: Config>(
7574
.choose_multiple(&mut rng, <CompactOf<T>>::LIMIT)
7675
.cloned()
7776
.collect::<Vec<_>>();
78-
let voter = account::<T::AccountId>("Voter", i, SEED);
77+
let voter = frame_benchmarking::account::<T::AccountId>("Voter", i, SEED);
7978
(voter, stake, winner_votes)
8079
})
8180
.collect::<Vec<_>>();
@@ -89,7 +88,7 @@ fn solution_with_size<T: Config>(
8988
.choose_multiple(&mut rng, <CompactOf<T>>::LIMIT)
9089
.cloned()
9190
.collect::<Vec<T::AccountId>>();
92-
let voter = account::<T::AccountId>("Voter", i, SEED);
91+
let voter = frame_benchmarking::account::<T::AccountId>("Voter", i, SEED);
9392
(voter, stake, votes)
9493
})
9594
.collect::<Vec<_>>();
@@ -109,8 +108,9 @@ fn solution_with_size<T: Config>(
109108
<DesiredTargets<T>>::put(desired_targets);
110109
<Snapshot<T>>::put(RoundSnapshot { voters: all_voters.clone(), targets: targets.clone() });
111110

112-
// write the snapshot to staking or whoever is the data provider.
113-
T::DataProvider::put_snapshot(all_voters.clone(), targets.clone());
111+
// write the snapshot to staking or whoever is the data provider, in case it is needed further
112+
// down the road.
113+
T::DataProvider::put_snapshot(all_voters.clone(), targets.clone(), Some(stake));
114114

115115
let cache = helpers::generate_voter_cache::<T>(&all_voters);
116116
let stake_of = helpers::stake_of_fn::<T>(&all_voters, &cache);
@@ -138,10 +138,12 @@ fn solution_with_size<T: Config>(
138138
<CompactOf<T>>::from_assignment(assignments, &voter_index, &target_index).unwrap();
139139
let score = compact.clone().score(&winners, stake_of, voter_at, target_at).unwrap();
140140
let round = <MultiPhase<T>>::round();
141+
142+
assert!(score[0] > 0, "score is zero, this probably means that the stakes are not set.");
141143
RawSolution { compact, score, round }
142144
}
143145

144-
benchmarks! {
146+
frame_benchmarking::benchmarks! {
145147
on_initialize_nothing {
146148
assert!(<MultiPhase<T>>::current_phase().is_off());
147149
}: {
@@ -157,7 +159,7 @@ benchmarks! {
157159
assert!(<MultiPhase<T>>::snapshot().is_none());
158160
assert!(<MultiPhase<T>>::current_phase().is_off());
159161
}: {
160-
<MultiPhase<T>>::on_initialize_open_signed();
162+
<MultiPhase<T>>::on_initialize_open_signed().unwrap();
161163
} verify {
162164
assert!(<MultiPhase<T>>::snapshot().is_some());
163165
assert!(<MultiPhase<T>>::current_phase().is_signed());
@@ -167,29 +169,59 @@ benchmarks! {
167169
assert!(<MultiPhase<T>>::snapshot().is_none());
168170
assert!(<MultiPhase<T>>::current_phase().is_off());
169171
}: {
170-
<MultiPhase<T>>::on_initialize_open_unsigned(true, true, 1u32.into());
172+
<MultiPhase<T>>::on_initialize_open_unsigned(true, true, 1u32.into()).unwrap();
171173
} verify {
172174
assert!(<MultiPhase<T>>::snapshot().is_some());
173175
assert!(<MultiPhase<T>>::current_phase().is_unsigned());
174176
}
175177

176178
on_initialize_open_unsigned_without_snapshot {
177179
// need to assume signed phase was open before
178-
<MultiPhase<T>>::on_initialize_open_signed();
180+
<MultiPhase<T>>::on_initialize_open_signed().unwrap();
179181
assert!(<MultiPhase<T>>::snapshot().is_some());
180182
assert!(<MultiPhase<T>>::current_phase().is_signed());
181183
}: {
182-
<MultiPhase<T>>::on_initialize_open_unsigned(false, true, 1u32.into());
184+
<MultiPhase<T>>::on_initialize_open_unsigned(false, true, 1u32.into()).unwrap();
183185
} verify {
184186
assert!(<MultiPhase<T>>::snapshot().is_some());
185187
assert!(<MultiPhase<T>>::current_phase().is_unsigned());
186188
}
187189

190+
// a call to `<Pallet as ElectionProvider>::elect` where we only return the queued solution.
191+
elect_queued {
192+
// assume largest values for the election status. These will merely affect the decoding.
193+
let v = T::BenchmarkingConfig::VOTERS[1];
194+
let t = T::BenchmarkingConfig::TARGETS[1];
195+
let a = T::BenchmarkingConfig::ACTIVE_VOTERS[1];
196+
let d = T::BenchmarkingConfig::DESIRED_TARGETS[1];
197+
198+
let witness = SolutionOrSnapshotSize { voters: v, targets: t };
199+
let raw_solution = solution_with_size::<T>(witness, a, d);
200+
let ready_solution =
201+
<MultiPhase<T>>::feasibility_check(raw_solution, ElectionCompute::Signed).unwrap();
202+
203+
// these are set by the `solution_with_size` function.
204+
assert!(<DesiredTargets<T>>::get().is_some());
205+
assert!(<Snapshot<T>>::get().is_some());
206+
assert!(<SnapshotMetadata<T>>::get().is_some());
207+
<CurrentPhase<T>>::put(Phase::Signed);
208+
// assume a queued solution is stored, regardless of where it comes from.
209+
<QueuedSolution<T>>::put(ready_solution);
210+
}: {
211+
let _ = <MultiPhase<T> as ElectionProvider<T::AccountId, T::BlockNumber>>::elect();
212+
} verify {
213+
assert!(<MultiPhase<T>>::queued_solution().is_none());
214+
assert!(<DesiredTargets<T>>::get().is_none());
215+
assert!(<Snapshot<T>>::get().is_none());
216+
assert!(<SnapshotMetadata<T>>::get().is_none());
217+
assert_eq!(<CurrentPhase<T>>::get(), <Phase<T::BlockNumber>>::Off);
218+
}
219+
188220
#[extra]
189221
create_snapshot {
190222
assert!(<MultiPhase<T>>::snapshot().is_none());
191223
}: {
192-
<MultiPhase::<T>>::create_snapshot()
224+
<MultiPhase::<T>>::create_snapshot().unwrap()
193225
} verify {
194226
assert!(<MultiPhase<T>>::snapshot().is_some());
195227
}
@@ -248,35 +280,8 @@ benchmarks! {
248280
}
249281
}
250282

251-
#[cfg(test)]
252-
mod test {
253-
use super::*;
254-
use crate::mock::*;
255-
256-
#[test]
257-
fn test_benchmarks() {
258-
ExtBuilder::default().build_and_execute(|| {
259-
assert_ok!(test_benchmark_feasibility_check::<Runtime>());
260-
});
261-
262-
ExtBuilder::default().build_and_execute(|| {
263-
assert_ok!(test_benchmark_submit_unsigned::<Runtime>());
264-
});
265-
266-
ExtBuilder::default().build_and_execute(|| {
267-
assert_ok!(test_benchmark_on_initialize_open_unsigned_with_snapshot::<Runtime>());
268-
});
269-
270-
ExtBuilder::default().build_and_execute(|| {
271-
assert_ok!(test_benchmark_on_initialize_open_unsigned_without_snapshot::<Runtime>());
272-
});
273-
274-
ExtBuilder::default().build_and_execute(|| {
275-
assert_ok!(test_benchmark_on_initialize_nothing::<Runtime>());
276-
});
277-
278-
ExtBuilder::default().build_and_execute(|| {
279-
assert_ok!(test_benchmark_create_snapshot::<Runtime>());
280-
});
281-
}
282-
}
283+
impl_benchmark_test_suite!(
284+
MultiPhase,
285+
crate::mock::ExtBuilder::default().build(),
286+
crate::mock::Runtime,
287+
);

frame/election-provider-multi-phase/src/helpers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ macro_rules! log {
2525
($level:tt, $pattern:expr $(, $values:expr)* $(,)?) => {
2626
log::$level!(
2727
target: $crate::LOG_TARGET,
28-
concat!("🗳 ", $pattern) $(, $values)*
28+
concat!("[#{:?}] 🗳 ", $pattern), <frame_system::Module<T>>::block_number() $(, $values)*
2929
)
3030
};
3131
}

0 commit comments

Comments
 (0)