Skip to content

Commit

Permalink
[EPM pallet]: remove number of signed submissions (paritytech#10945)
Browse files Browse the repository at this point in the history 
* [EPM pallet]: remove `number of signed submissions`

Closing paritytech#9229

* fix tests

* remove needless assert

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

* cargo fmt

Signed-off-by: Niklas <niklasadolfsson1@gmail.com>

* fix grumbles

* cargo run --quiet --profile=production  --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

* drop `num_signed_submissions` in WeightInfo too

* fix build

Co-authored-by: Parity Bot <admin@parity.io>
  • Loading branch information
@niklasad1 @grishasobol
2 people authored and grishasobol committed Mar 28, 2022
1 parent 6ee7334 commit 1acb138
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 156 deletions.
10 changes: 6 additions & 4 deletions frame/election-provider-multi-phase/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ frame_benchmarking::benchmarks! {
}

submit {
let c in 1 .. (T::SignedMaxSubmissions::get() - 1);

// the solution will be worse than all of them meaning the score need to be checked against
// ~ log2(c)
Expand All @@ -324,7 +323,10 @@ frame_benchmarking::benchmarks! {
<Round<T>>::put(1);

let mut signed_submissions = SignedSubmissions::<T>::get();
for i in 0..c {

// Insert `max - 1` submissions because the call to `submit` will insert another
// submission and the score is worse then the previous scores.
for i in 0..(T::SignedMaxSubmissions::get() - 1) {
let raw_solution = RawSolution {
score: ElectionScore { minimal_stake: 10_000_000u128 + (i as u128), ..Default::default() },
..Default::default()
Expand All @@ -342,9 +344,9 @@ frame_benchmarking::benchmarks! {
let caller = frame_benchmarking::whitelisted_caller();
T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() * 10u32.into());

}: _(RawOrigin::Signed(caller), Box::new(solution), c)
}: _(RawOrigin::Signed(caller), Box::new(solution))
verify {
assert!(<MultiPhase<T>>::signed_submissions().len() as u32 == c + 1);
assert!(<MultiPhase<T>>::signed_submissions().len() as u32 == T::SignedMaxSubmissions::get());
}

submit_unsigned {
Expand Down
23 changes: 3 additions & 20 deletions frame/election-provider-multi-phase/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,25 +963,13 @@ pub mod pallet {
///
/// A deposit is reserved and recorded for the solution. Based on the outcome, the solution
/// might be rewarded, slashed, or get all or a part of the deposit back.
///
/// # <weight>
/// Queue size must be provided as witness data.
/// # </weight>
#[pallet::weight(T::WeightInfo::submit(*num_signed_submissions))]
#[pallet::weight(T::WeightInfo::submit())]
pub fn submit(
origin: OriginFor<T>,
raw_solution: Box<RawSolution<SolutionOf<T>>>,
num_signed_submissions: u32,
) -> DispatchResult {
let who = ensure_signed(origin)?;

// ensure witness data is correct.
ensure!(
num_signed_submissions >=
<SignedSubmissions<T>>::decode_len().unwrap_or_default() as u32,
Error::<T>::SignedInvalidWitness,
);

// ensure solution is timely.
ensure!(Self::current_phase().is_signed(), Error::<T>::PreDispatchEarlySubmission);

Expand All @@ -1000,8 +988,7 @@ pub mod pallet {
// create the submission
let deposit = Self::deposit_for(&raw_solution, size);
let reward = {
let call =
Call::submit { raw_solution: raw_solution.clone(), num_signed_submissions };
let call = Call::submit { raw_solution: raw_solution.clone() };
let call_fee = T::EstimateCallFee::estimate_call_fee(&call, None.into());
T::SignedRewardBase::get().saturating_add(call_fee)
};
Expand Down Expand Up @@ -1970,11 +1957,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(MultiPhase::submit(
crate::mock::Origin::signed(99),
Box::new(solution),
MultiPhase::signed_submissions().len() as u32
));
assert_ok!(MultiPhase::submit(crate::mock::Origin::signed(99), Box::new(solution)));
}

// an unexpected call to elect.
Expand Down
4 changes: 2 additions & 2 deletions frame/election-provider-multi-phase/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,11 @@ impl multi_phase::weights::WeightInfo for DualMockWeightInfo {
<() as multi_phase::weights::WeightInfo>::finalize_signed_phase_reject_solution()
}
}
fn submit(c: u32) -> Weight {
fn submit() -> Weight {
if MockWeightInfo::get() {
Zero::zero()
} else {
<() as multi_phase::weights::WeightInfo>::submit(c)
<() as multi_phase::weights::WeightInfo>::submit()
}
}
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32) -> Weight {
Expand Down
84 changes: 27 additions & 57 deletions frame/election-provider-multi-phase/src/signed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,18 +500,7 @@ mod tests {
},
Error, Phase,
};
use frame_support::{assert_noop, assert_ok, assert_storage_noop, dispatch::DispatchResult};

fn submit_with_witness(
origin: Origin,
solution: RawSolution<SolutionOf<Runtime>>,
) -> DispatchResult {
MultiPhase::submit(
origin,
Box::new(solution),
MultiPhase::signed_submissions().len() as u32,
)
}
use frame_support::{assert_noop, assert_ok, assert_storage_noop};

#[test]
fn cannot_submit_too_early() {
Expand All @@ -524,31 +513,12 @@ mod tests {
let solution = raw_solution();

assert_noop!(
submit_with_witness(Origin::signed(10), solution),
MultiPhase::submit(Origin::signed(10), Box::new(solution)),
Error::<Runtime>::PreDispatchEarlySubmission,
);
})
}

#[test]
fn wrong_witness_fails() {
ExtBuilder::default().build_and_execute(|| {
roll_to(15);
assert!(MultiPhase::current_phase().is_signed());

let solution = raw_solution();
// submit this once correctly
assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
assert_eq!(MultiPhase::signed_submissions().len(), 1);

// now try and cheat by passing a lower queue length
assert_noop!(
MultiPhase::submit(Origin::signed(99), Box::new(solution), 0),
Error::<Runtime>::SignedInvalidWitness,
);
})
}

#[test]
fn should_pay_deposit() {
ExtBuilder::default().build_and_execute(|| {
Expand All @@ -558,7 +528,7 @@ mod tests {
let solution = raw_solution();
assert_eq!(balances(&99), (100, 0));

assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));

assert_eq!(balances(&99), (95, 5));
assert_eq!(MultiPhase::signed_submissions().iter().next().unwrap().deposit, 5);
Expand All @@ -574,7 +544,7 @@ mod tests {
let solution = raw_solution();
assert_eq!(balances(&99), (100, 0));

assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
assert_eq!(balances(&99), (95, 5));

assert!(MultiPhase::finalize_signed_phase());
Expand All @@ -594,7 +564,7 @@ mod tests {
// make the solution invalid.
solution.score.minimal_stake += 1;

assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
assert_eq!(balances(&99), (95, 5));

// no good solution was stored.
Expand All @@ -615,11 +585,11 @@ mod tests {
assert_eq!(balances(&999), (100, 0));

// submit as correct.
assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution.clone())));

// make the solution invalid and weaker.
solution.score.minimal_stake -= 1;
assert_ok!(submit_with_witness(Origin::signed(999), solution));
assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution)));
assert_eq!(balances(&99), (95, 5));
assert_eq!(balances(&999), (95, 5));

Expand All @@ -645,7 +615,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}

// weaker.
Expand All @@ -655,7 +625,7 @@ mod tests {
};

assert_noop!(
submit_with_witness(Origin::signed(99), solution),
MultiPhase::submit(Origin::signed(99), Box::new(solution)),
Error::<Runtime>::SignedQueueFull,
);
})
Expand All @@ -673,7 +643,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}

assert_eq!(
Expand All @@ -689,7 +659,7 @@ mod tests {
score: ElectionScore { minimal_stake: 20, ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));

// the one with score 5 was rejected, the new one inserted.
assert_eq!(
Expand All @@ -714,14 +684,14 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}

let solution = RawSolution {
score: ElectionScore { minimal_stake: 4, ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));

assert_eq!(
MultiPhase::signed_submissions()
Expand All @@ -736,7 +706,7 @@ mod tests {
score: ElectionScore { minimal_stake: 5, ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));

// the one with score 5 was rejected, the new one inserted.
assert_eq!(
Expand All @@ -761,7 +731,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}

assert_eq!(balances(&99).1, 2 * 5);
Expand All @@ -772,7 +742,7 @@ mod tests {
score: ElectionScore { minimal_stake: 20, ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(999), solution));
assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution)));

// got one bond back.
assert_eq!(balances(&99).1, 2 * 4);
Expand All @@ -791,7 +761,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + i).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}
assert_eq!(
MultiPhase::signed_submissions()
Expand All @@ -807,7 +777,7 @@ mod tests {
..Default::default()
};
assert_noop!(
submit_with_witness(Origin::signed(99), solution),
MultiPhase::submit(Origin::signed(99), Box::new(solution)),
Error::<Runtime>::SignedQueueFull,
);
})
Expand All @@ -829,18 +799,18 @@ mod tests {
let solution = raw_solution();

// submit a correct one.
assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution.clone())));

// make the solution invalidly better and submit. This ought to be slashed.
let mut solution_999 = solution.clone();
solution_999.score.minimal_stake += 1;
assert_ok!(submit_with_witness(Origin::signed(999), solution_999));
assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution_999)));

// make the solution invalidly worse and submit. This ought to be suppressed and
// returned.
let mut solution_9999 = solution.clone();
solution_9999.score.minimal_stake -= 1;
assert_ok!(submit_with_witness(Origin::signed(9999), solution_9999));
assert_ok!(MultiPhase::submit(Origin::signed(9999), Box::new(solution_9999)));

assert_eq!(
MultiPhase::signed_submissions().iter().map(|x| x.who).collect::<Vec<_>>(),
Expand Down Expand Up @@ -881,14 +851,14 @@ mod tests {
assert_eq!(raw.solution.voter_count(), 5);
assert_eq!(<Runtime as Config>::SignedMaxWeight::get(), 40);

assert_ok!(submit_with_witness(Origin::signed(99), raw.clone()));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(raw.clone())));

<SignedMaxWeight>::set(30);

// note: resubmitting the same solution is technically okay as long as the queue has
// space.
assert_noop!(
submit_with_witness(Origin::signed(99), raw),
MultiPhase::submit(Origin::signed(99), Box::new(raw)),
Error::<Runtime>::SignedTooMuchWeight,
);
})
Expand All @@ -904,7 +874,7 @@ mod tests {

assert_eq!(balances(&123), (0, 0));
assert_noop!(
submit_with_witness(Origin::signed(123), solution),
MultiPhase::submit(Origin::signed(123), Box::new(solution)),
Error::<Runtime>::SignedCannotPayDeposit,
);

Expand All @@ -926,7 +896,7 @@ mod tests {
score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
..Default::default()
};
assert_ok!(submit_with_witness(Origin::signed(99), solution));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
}

// this solution has a higher score than any in the queue
Expand All @@ -940,7 +910,7 @@ mod tests {

assert_eq!(balances(&123), (0, 0));
assert_noop!(
submit_with_witness(Origin::signed(123), solution),
MultiPhase::submit(Origin::signed(123), Box::new(solution)),
Error::<Runtime>::SignedCannotPayDeposit,
);

Expand Down Expand Up @@ -969,7 +939,7 @@ mod tests {
let solution = raw_solution();

// submit a correct one.
assert_ok!(submit_with_witness(Origin::signed(99), solution.clone()));
assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));

// _some_ good solution was stored.
assert!(MultiPhase::finalize_signed_phase());
Expand Down
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/unsigned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ mod max_weight {
fn finalize_signed_phase_reject_solution() -> Weight {
unreachable!()
}
fn submit(c: u32) -> Weight {
fn submit() -> Weight {
unreachable!()
}
fn submit_unsigned(v: u32, t: u32, a: u32, d: u32) -> Weight {
Expand Down
Loading

0 comments on commit 1acb138

Please sign in to comment.