Skip to content

Commit 80ff238

Browse files
kianenigmaacatangiugithub-actions[bot]sigurpol
authored andcommitted
[AHM] Staking async fixes for XCM and election planning (#8422)
This PR brings a few small fixes related to the XCM messages of staking-async, among other small fixes: * [x] Allows `xcm::validate` to check the message size, and we actually now act upon it in the `staking-async-rc/parachain-runtime`s. The code is a bit duplicate now, and there is a TOOD about how to better refactor it later. * [x] Part of this work is backported separately as #8409 * [x] It brings a default `EraElectionPlannerOf` which should be the right tool to use to ensure elections always happen in time, with an educated guess based on `ElectionProvider::duration` rather than a random number. * [x] It adds a few unit tests about the above * [x] It silences some logs that were needlessly `INFO`, and makes the printing of some types a bit more CLI friendly. * [x] Fix the issue with duplicate voters in solution type: see #8585 * [x] Move `PagedRawSolution` to be unbounded, and therefore decode-able without PoV * [x] Undo a rename from `ClaimedRewards` to `ErasClaimedRewards` * [ ] Needs fixing in westend --------- Co-authored-by: Adrian Catangiu <adrian@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Paolo La Camera <paolo@parity.io>
1 parent f6bb878 commit 80ff238

28 files changed

Lines changed: 707 additions & 251 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

prdoc/pr_8422.prdoc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
title: '[AHM] Staking async fixes for XCM and election planning'
2+
doc:
3+
- audience: Runtime Dev
4+
description: |-
5+
This PR brings a few small fixes related to the XCM messages of stkaing-async, among other small fixes:
6+
7+
8+
* [x] Allows `xcm::validate` to check the message size, and we actually now act upon it in the `staking-async-rc/parachain-runtime`s. The code is a bit duplicate now, and there is a TOOD about how to better refactor it later.
9+
* [x] Part of this work is backported separately as https://github.com/paritytech/polkadot-sdk/pull/8409
10+
* [x] It brings a default `EraElectionPlannerOf` which should be the right tool to use to ensure elections always happen in time, with an educated guess based on `ElectionProvider::duration` rather than a random number.
11+
* [x] It adds a few unit tests about the above
12+
* [x] It silences some logs that were needlessly `INFO`, and makes the printing of some types a bit more CLI friendly.
13+
* [x] Renames `type SessionDuration` in `staking-async` to `type RelaySessionDuration` for better clarity.
14+
crates:
15+
- name: pallet-staking-async-ah-client
16+
bump: patch
17+
- name: pallet-staking-async-rc-client
18+
bump: minor
19+
- name: pallet-staking-async-parachain-runtime
20+
bump: minor
21+
- name: pallet-staking-async-rc-runtime
22+
bump: major
23+
- name: pallet-staking-async
24+
bump: major
25+
- name: pallet-election-provider-multi-block
26+
bump: major

substrate/frame/election-provider-multi-block/src/mock/signed.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ use crate::{
2525
};
2626
use frame_election_provider_support::PageIndex;
2727
use frame_support::{
28-
assert_ok, dispatch::PostDispatchInfo, parameter_types, traits::EstimateCallFee, BoundedVec,
28+
assert_ok, dispatch::PostDispatchInfo, parameter_types, traits::EstimateCallFee,
2929
};
3030
use sp_npos_elections::ElectionScore;
3131
use sp_runtime::{traits::Zero, Perbill};
3232

3333
parameter_types! {
34-
pub static MockSignedNextSolution: Option<BoundedVec<SolutionOf<Runtime>, Pages>> = None;
34+
pub static MockSignedNextSolution: Option<Vec<SolutionOf<Runtime>>> = None;
3535
pub static MockSignedNextScore: Option<ElectionScore> = Default::default();
3636
pub static MockSignedResults: Vec<VerificationResult> = Default::default();
3737
}

substrate/frame/election-provider-multi-block/src/types.rs

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,13 @@ pub type AssignmentOf<T> =
8282
CloneNoBound,
8383
EqNoBound,
8484
PartialEqNoBound,
85-
MaxEncodedLen,
8685
DefaultNoBound,
8786
)]
8887
#[codec(mel_bound(T: crate::Config))]
8988
#[scale_info(skip_type_params(T))]
9089
pub struct PagedRawSolution<T: MinerConfig> {
9190
/// The individual pages.
92-
pub solution_pages: BoundedVec<SolutionOf<T>, <T as MinerConfig>::Pages>,
91+
pub solution_pages: Vec<SolutionOf<T>>,
9392
/// The final claimed score post feasibility and concatenation of all pages.
9493
pub score: ElectionScore,
9594
/// The designated round.
@@ -165,6 +164,23 @@ pub trait PadSolutionPages: Sized {
165164
fn pad_solution_pages(self, desired_pages: PageIndex) -> Self;
166165
}
167166

167+
impl<T: Default + Clone + Debug> PadSolutionPages for Vec<T> {
168+
fn pad_solution_pages(self, desired_pages: PageIndex) -> Self {
169+
let desired_pages_usize = desired_pages as usize;
170+
debug_assert!(self.len() <= desired_pages_usize);
171+
if self.len() == desired_pages_usize {
172+
return self
173+
}
174+
175+
// we basically need to prepend the list with this many items.
176+
let empty_slots = desired_pages_usize.saturating_sub(self.len());
177+
sp_std::iter::repeat(Default::default())
178+
.take(empty_slots)
179+
.chain(self.into_iter())
180+
.collect::<Vec<_>>()
181+
}
182+
}
183+
168184
impl<T: Default + Clone + Debug, Bound: frame_support::traits::Get<u32>> PadSolutionPages
169185
for BoundedVec<T, Bound>
170186
{
@@ -391,8 +407,6 @@ impl<T: crate::Config> Phase<T> {
391407
#[cfg(test)]
392408
mod pagify {
393409
use super::{PadSolutionPages, Pagify};
394-
use frame_support::{traits::ConstU32, BoundedVec};
395-
use sp_core::bounded_vec;
396410

397411
#[test]
398412
fn pagify_works() {
@@ -410,15 +424,11 @@ mod pagify {
410424
#[test]
411425
fn pad_solution_pages_works() {
412426
// noop if the solution is complete, as with pagify.
413-
let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![1u32, 2, 3];
414-
assert_eq!(solution.pad_solution_pages(3).into_inner(), vec![1, 2, 3]);
427+
let solution = vec![1u32, 2, 3];
428+
assert_eq!(solution.pad_solution_pages(3), vec![1, 2, 3]);
415429

416430
// pads the solution with default if partial..
417-
let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![2, 3];
418-
assert_eq!(solution.pad_solution_pages(3).into_inner(), vec![0, 2, 3]);
419-
420-
// behaves the same as `pad_solution_pages(3)`.
421-
let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![2, 3];
422-
assert_eq!(solution.pad_solution_pages(4).into_inner(), vec![0, 2, 3]);
431+
let solution = vec![2, 3];
432+
assert_eq!(solution.pad_solution_pages(3), vec![0, 2, 3]);
423433
}
424434
}

substrate/frame/election-provider-multi-block/src/unsigned/miner.rs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ impl<T: MinerConfig> BaseMiner<T> {
371371
}
372372

373373
// convert each page to a compact struct -- no more change allowed.
374-
let solution_pages: BoundedVec<SolutionOf<T>, T::Pages> = paged_assignments
374+
let mut solution_pages: Vec<SolutionOf<T>> = paged_assignments
375375
.into_iter()
376376
.enumerate()
377377
.map(|(page_index, assignment_page)| {
@@ -382,12 +382,11 @@ impl<T: MinerConfig> BaseMiner<T> {
382382
.ok_or(MinerError::SnapshotUnAvailable(SnapshotType::Voters(page)))?;
383383

384384
// one last trimming -- `MaxBackersPerWinner`, the per-page variant.
385-
let trimmed_assignment_page =
386-
Self::trim_supports_max_backers_per_winner_per_page(
387-
assignment_page,
388-
voter_snapshot_page,
389-
page_index as u32,
390-
)?;
385+
let trimmed_assignment_page = Self::trim_supports_max_backers_per_winner_per_page(
386+
assignment_page,
387+
voter_snapshot_page,
388+
page_index as u32,
389+
)?;
391390

392391
let voter_index_fn = {
393392
let cache = helpers::generate_voter_cache::<T, _>(&voter_snapshot_page);
@@ -401,17 +400,11 @@ impl<T: MinerConfig> BaseMiner<T> {
401400
)
402401
.map_err::<MinerError<T>, _>(Into::into)
403402
})
404-
.collect::<Result<Vec<_>, _>>()?
405-
.try_into()
406-
.expect("`paged_assignments` is bound by `T::Pages`; length cannot change in iter chain; qed");
403+
.collect::<Result<Vec<_>, _>>()?;
407404

408405
// now do the length trim.
409-
let mut solution_pages_unbounded = solution_pages.into_inner();
410406
let _trim_length_weight =
411-
Self::maybe_trim_weight_and_len(&mut solution_pages_unbounded, &voter_pages)?;
412-
let solution_pages = solution_pages_unbounded
413-
.try_into()
414-
.expect("maybe_trim_weight_and_len cannot increase the length of its input; qed.");
407+
Self::maybe_trim_weight_and_len(&mut solution_pages, &voter_pages)?;
415408
miner_log!(debug, "trimmed {} voters due to length restriction.", _trim_length_weight);
416409

417410
// finally, wrap everything up. Assign a fake score here, since we might need to re-compute

substrate/frame/election-provider-multi-block/src/unsigned/mod.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ mod pallet {
165165
// we select the most significant pages, based on `T::MinerPages`.
166166
let page_indices = crate::Pallet::<T>::msp_range_for(T::MinerPages::get() as usize);
167167
<T::Verifier as Verifier>::verify_synchronous_multi(
168-
paged_solution.solution_pages.into_inner(),
168+
paged_solution.solution_pages,
169169
page_indices,
170170
claimed_score,
171171
)
@@ -235,7 +235,12 @@ mod pallet {
235235
assert!(
236236
UnsignedWeightsOf::<T>::submit_unsigned().all_lte(T::BlockWeights::get().max_block),
237237
"weight of `submit_unsigned` is too high"
238-
)
238+
);
239+
assert!(
240+
<T as Config>::MinerPages::get() as usize <=
241+
<T as crate::Config>::Pages::get() as usize,
242+
"number of pages in the unsigned phase is too high"
243+
);
239244
}
240245

241246
#[cfg(feature = "try-runtime")]
@@ -333,6 +338,10 @@ mod pallet {
333338
paged_solution.solution_pages.len() == T::MinerPages::get() as usize,
334339
CommonError::WrongPageCount
335340
);
341+
ensure!(
342+
paged_solution.solution_pages.len() <= <T as crate::Config>::Pages::get() as usize,
343+
CommonError::WrongPageCount
344+
);
336345

337346
Ok(())
338347
}

substrate/frame/election-provider-multi-block/src/verifier/tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ mod multi_page_sync_verification {
927927
assert_eq!(<VerifierPallet as Verifier>::queued_score(), None);
928928

929929
let _ = <VerifierPallet as Verifier>::verify_synchronous_multi(
930-
paged.solution_pages.clone().into_inner(),
930+
paged.solution_pages.clone(),
931931
MultiBlock::msp_range_for(2),
932932
paged.score,
933933
)
@@ -955,7 +955,7 @@ mod multi_page_sync_verification {
955955
assert_eq!(<VerifierPallet as Verifier>::queued_score(), None);
956956

957957
let _ = <VerifierPallet as Verifier>::verify_synchronous_multi(
958-
paged.solution_pages.clone().into_inner(),
958+
paged.solution_pages.clone(),
959959
MultiBlock::msp_range_for(3),
960960
paged.score,
961961
)
@@ -987,7 +987,7 @@ mod multi_page_sync_verification {
987987

988988
assert_eq!(
989989
<VerifierPallet as Verifier>::verify_synchronous_multi(
990-
paged.solution_pages.clone().into_inner(),
990+
paged.solution_pages.clone(),
991991
MultiBlock::msp_range_for(2),
992992
paged.score,
993993
)
@@ -1021,7 +1021,7 @@ mod multi_page_sync_verification {
10211021

10221022
assert_eq!(
10231023
<VerifierPallet as Verifier>::verify_synchronous_multi(
1024-
paged.solution_pages.clone().into_inner(),
1024+
paged.solution_pages.clone(),
10251025
MultiBlock::msp_range_for(2),
10261026
paged.score,
10271027
)
@@ -1055,7 +1055,7 @@ mod multi_page_sync_verification {
10551055

10561056
hypothetically!({
10571057
assert_ok!(<VerifierPallet as Verifier>::verify_synchronous_multi(
1058-
paged.solution_pages.clone().into_inner(),
1058+
paged.solution_pages.clone(),
10591059
MultiBlock::msp_range_for(2),
10601060
paged.score,
10611061
));
@@ -1098,7 +1098,7 @@ mod multi_page_sync_verification {
10981098

10991099
assert_eq!(
11001100
<VerifierPallet as Verifier>::verify_synchronous_multi(
1101-
paged.solution_pages.clone().into_inner(),
1101+
paged.solution_pages.clone(),
11021102
MultiBlock::msp_range_for(2),
11031103
paged.score,
11041104
)

substrate/frame/staking-async/ah-client/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ pub mod pallet {
430430
report: rc_client::ValidatorSetReport<T::AccountId>,
431431
) -> DispatchResult {
432432
// Ensure the origin is one of Root or whatever is representing AssetHub.
433-
log!(info, "Received new validator set report {:?}", report);
433+
log!(debug, "Received new validator set report {}", report);
434434
T::AssetHubOrigin::ensure_origin_or_root(origin)?;
435435

436436
// Check the operating mode.

substrate/frame/staking-async/ahm-test/src/ah/mock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ parameter_types! {
317317
pub static BondingDuration: u32 = 3;
318318
pub static SlashDeferredDuration: u32 = 2;
319319
pub static SessionsPerEra: u32 = 6;
320-
pub static PlanningEraOffset: u32 = 1;
320+
pub static PlanningEraOffset: u32 = 2;
321321
}
322322

323323
impl pallet_staking_async::Config for Runtime {

substrate/frame/staking-async/rc-client/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ scale-info = { workspace = true, features = ["derive"] }
1717
sp-core = { workspace = true }
1818
sp-runtime = { features = ["serde"], workspace = true }
1919
sp-staking = { features = ["serde"], workspace = true }
20+
xcm = { workspace = true }
2021

2122
[features]
2223
default = ["std"]
@@ -29,12 +30,14 @@ std = [
2930
"sp-core/std",
3031
"sp-runtime/std",
3132
"sp-staking/std",
33+
"xcm/std",
3234
]
3335
runtime-benchmarks = [
3436
"frame-support/runtime-benchmarks",
3537
"frame-system/runtime-benchmarks",
3638
"sp-runtime/runtime-benchmarks",
3739
"sp-staking/runtime-benchmarks",
40+
"xcm/runtime-benchmarks",
3841
]
3942
try-runtime = [
4043
"frame-support/try-runtime",

0 commit comments

Comments
 (0)