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

pallet-offences-benchmarking: Box events in verify #13151

Merged
bkchr merged 3 commits into from
Jan 16, 2023
Merged

pallet-offences-benchmarking: Box events in verify #13151

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90b5f39
pallet-offences-benchmarking: Box events in verify
bkchr Jan 16, 2023
f9e0497
FMT
bkchr Jan 16, 2023
c359ca8
Disable checking again
bkchr Jan 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
pallet-offences-benchmarking: Box events in verify 
Events in frame are represented by an enum in the pallet and the runtime. The size of an enum in
Rust depends on the size of biggest variant. This means we always need to allocate memory for the
biggest variant when allocating memory for an event. The offences benchmarking is verifying the
benchmarking results by checking the events. To check the events it is generating all the expected
events. With the recent changes in Polkadot the events are too big and lead to issues when running
this verify functions. The solution is to box each event, as the vector holding all the events will
then only need to hold fat pointers * expected events, instead of size_of(event) * expected events.
This issue isn't a problem in production, as we never read the events on chain. When we are reading
the events, it is done in an offchain context and they are only decoded one by one.

Besides that this also enables the benchmarking verification for everyone running these benchmarks.
  • Loading branch information
@bkchr
bkchr committed Jan 16, 2023
commit 90b5f39d8ebfcf492f1577255dc188bbaff2103a
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions frame/offences/benchmarking/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pallet-staking = { version = "4.0.0-dev", default-features = false, path = "../.
sp-runtime = { version = "7.0.0", default-features = false, path = "../../../primitives/runtime" }
sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/staking" }
sp-std = { version = "5.0.0", default-features = false, path = "../../../primitives/std" }
log = { version = "0.4.17", default-features = false }

[dev-dependencies]
pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../../staking/reward-curve" }
Expand All @@ -55,6 +56,7 @@ std = [
"sp-runtime/std",
"sp-staking/std",
"sp-std/std",
"log/std",
]

runtime-benchmarks = [
Expand Down
177 changes: 90 additions & 87 deletions frame/offences/benchmarking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,56 +216,63 @@ fn make_offenders_im_online<T: Config>(
Ok((id_tuples, offenders))
}

#[cfg(test)]
fn check_events<T: Config, I: Iterator<Item = <T as SystemConfig>::RuntimeEvent>>(expected: I) {
fn check_events<
T: Config,
I: Iterator<Item = Item>,
Item: sp_std::borrow::Borrow<<T as SystemConfig>::RuntimeEvent> + sp_std::fmt::Debug,
>(
expected: I,
) {
let events = System::<T>::events()
.into_iter()
.map(|frame_system::EventRecord { event, .. }| event)
.collect::<Vec<_>>();
let expected = expected.collect::<Vec<_>>();

fn pretty<D: std::fmt::Debug>(header: &str, ev: &[D], offset: usize) {
println!("{}", header);
fn pretty<D: sp_std::fmt::Debug>(header: &str, ev: &[D], offset: usize) {
log::info!("{}", header);
for (idx, ev) in ev.iter().enumerate() {
println!("\t[{:04}] {:?}", idx + offset, ev);
log::info!("\t[{:04}] {:?}", idx + offset, ev);
}
}
fn print_events<D: std::fmt::Debug>(idx: usize, events: &[D], expected: &[D]) {
fn print_events<D: sp_std::fmt::Debug, E: sp_std::fmt::Debug>(
idx: usize,
events: &[D],
expected: &[E],
) {
let window = 10;
let start = idx.saturating_sub(window / 2);
let end_got = (idx + window / 2).min(events.len());
pretty("Got(window):", &events[start..end_got], start);
let end_expected = (idx + window / 2).min(expected.len());
pretty("Expected(window):", &expected[start..end_expected], start);
println!("---------------");
log::info!("---------------");
let start_got = events.len().saturating_sub(window);
pretty("Got(end):", &events[start_got..], start_got);
let start_expected = expected.len().saturating_sub(window);
pretty("Expected(end):", &expected[start_expected..], start_expected);
}
let events_copy = events.clone();
let expected_copy = expected.clone();

for (idx, (a, b)) in events.into_iter().zip(expected).enumerate() {
if a != b {
print_events(idx, &events_copy, &expected_copy);
println!("Mismatch at: {}", idx);
println!(" Got: {:?}", b);
println!("Expected: {:?}", a);
if events_copy.len() != expected_copy.len() {
println!(

for (idx, (a, b)) in events.iter().zip(expected.iter()).enumerate() {
Copy link
Member

@ggwpez ggwpez Jan 16, 2023

Choose a reason for hiding this comment

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

Could also have been done with the new StorageStreamIter, or?

Copy link
Member Author

@bkchr bkchr Jan 16, 2023

Choose a reason for hiding this comment

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

Yes good idea!

if a != sp_std::borrow::Borrow::borrow(b) {
print_events(idx, &events, &expected);
log::info!("Mismatch at: {}", idx);
log::info!(" Got: {:?}", b);
log::info!("Expected: {:?}", a);
if events.len() != expected.len() {
log::info!(
"Mismatching lengths. Got: {}, Expected: {}",
events_copy.len(),
expected_copy.len()
events.len(),
expected.len()
)
}
panic!("Mismatching events.");
}
}

if events_copy.len() != expected_copy.len() {
print_events(0, &events_copy, &expected_copy);
panic!("Mismatching lengths. Got: {}, Expected: {}", events_copy.len(), expected_copy.len())
if events.len() != expected.len() {
print_events(0, &events, &expected);
panic!("Mismatching lengths. Got: {}, Expected: {}", events.len(), expected.len(),)
}
}

Expand Down Expand Up @@ -304,81 +311,77 @@ benchmarks! {
);
}
verify {
let bond_amount: u32 = UniqueSaturatedInto::<u32>::unique_saturated_into(bond_amount::<T>());
let slash_amount = slash_fraction * bond_amount;
let reward_amount = slash_amount.saturating_mul(1 + n) / 2;
let reward = reward_amount / r;
let slash_report = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::SlashReported{ validator: id, fraction: slash_fraction, slash_era: 0})
);
let slash = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::Slashed{ staker: id, amount: BalanceOf::<T>::from(slash_amount) })
);
let balance_slash = |id| core::iter::once(
<T as BalancesConfig>::RuntimeEvent::from(pallet_balances::Event::<T>::Slashed{ who: id, amount: slash_amount.into() })
);
let chill = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::Chilled{ stash: id })
);
let balance_deposit = |id, amount: u32|
let bond_amount: u32 = UniqueSaturatedInto::<u32>::unique_saturated_into(bond_amount::<T>());
let slash_amount = slash_fraction * bond_amount;
let reward_amount = slash_amount.saturating_mul(1 + n) / 2;
let reward = reward_amount / r;
let slash_report = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::SlashReported{ validator: id, fraction: slash_fraction, slash_era: 0})
);
let slash = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::Slashed{ staker: id, amount: BalanceOf::<T>::from(slash_amount) })
);
let balance_slash = |id| core::iter::once(
<T as BalancesConfig>::RuntimeEvent::from(pallet_balances::Event::<T>::Slashed{ who: id, amount: slash_amount.into() })
);
let chill = |id| core::iter::once(
<T as StakingConfig>::RuntimeEvent::from(StakingEvent::<T>::Chilled{ stash: id })
);
let balance_deposit = |id, amount: u32|
<T as BalancesConfig>::RuntimeEvent::from(pallet_balances::Event::<T>::Deposit{ who: id, amount: amount.into() });
let mut first = true;
let slash_events = raw_offenders.into_iter()
.flat_map(|offender| {
let nom_slashes = offender.nominator_stashes.into_iter().flat_map(|nom| {
balance_slash(nom.clone()).map(Into::into)
.chain(slash(nom).map(Into::into))
});

let mut events = chill(offender.stash.clone()).map(Into::into)
.chain(slash_report(offender.stash.clone()).map(Into::into))
.chain(balance_slash(offender.stash.clone()).map(Into::into))
.chain(slash(offender.stash).map(Into::into))
.chain(nom_slashes)
.collect::<Vec<_>>();

// the first deposit creates endowed events, see `endowed_reward_events`
if first {
first = false;
let mut reward_events = reporters.clone().into_iter()
.flat_map(|reporter| vec![
balance_deposit(reporter.clone(), reward).into(),
frame_system::Event::<T>::NewAccount { account: reporter.clone() }.into(),
<T as BalancesConfig>::RuntimeEvent::from(
pallet_balances::Event::<T>::Endowed{account: reporter, free_balance: reward.into()}
).into(),
])
.collect::<Vec<_>>();
events.append(&mut reward_events);
events.into_iter()
} else {
let mut reward_events = reporters.clone().into_iter()
.map(|reporter| balance_deposit(reporter, reward).into())
let mut first = true;

// We need to box all events to prevent running into too big allocations in wasm.
// The event in FRAME is represented as an enum and the size of the enum depends on the biggest variant.
// So, instead of requiring `size_of<Event>() * expected_events` we only need to
// allocate `size_of<Box<Event>>() * expected_events`.
let slash_events = raw_offenders.into_iter()
.flat_map(|offender| {
let nom_slashes = offender.nominator_stashes.into_iter().flat_map(|nom| {
balance_slash(nom.clone()).map(Into::into).chain(slash(nom).map(Into::into)).map(Box::new)
});

let events = chill(offender.stash.clone()).map(Into::into).map(Box::new)
.chain(slash_report(offender.stash.clone()).map(Into::into).map(Box::new))
.chain(balance_slash(offender.stash.clone()).map(Into::into).map(Box::new))
.chain(slash(offender.stash).map(Into::into).map(Box::new))
.chain(nom_slashes)
.collect::<Vec<_>>();
events.append(&mut reward_events);
events.into_iter()
}
})
.collect::<Vec<_>>();


// the first deposit creates endowed events, see `endowed_reward_events`
if first {
first = false;
let reward_events = reporters.iter()
.flat_map(|reporter| vec![
Box::new(balance_deposit(reporter.clone(), reward).into()),
Box::new(frame_system::Event::<T>::NewAccount { account: reporter.clone() }.into()),
Box::new(<T as BalancesConfig>::RuntimeEvent::from(
pallet_balances::Event::<T>::Endowed{ account: reporter.clone(), free_balance: reward.into() }
).into()),
])
.collect::<Vec<_>>();
events.into_iter().chain(reward_events)
} else {
let reward_events = reporters.iter()
.map(|reporter| Box::new(balance_deposit(reporter.clone(), reward).into()))
.collect::<Vec<_>>();
events.into_iter().chain(reward_events)
}
});

#[cfg(test)]
{
// In case of error it's useful to see the inputs
println!("Inputs: r: {}, o: {}, n: {}", r, o, n);
log::info!("Inputs: r: {}, o: {}, n: {}", r, o, n);
// make sure that all slashes have been applied
check_events::<T, _>(
std::iter::empty()
.chain(slash_events.into_iter().map(Into::into))
.chain(std::iter::once(<T as OffencesConfig>::RuntimeEvent::from(
check_events::<T, _, _>(
sp_std::iter::empty()
.chain(slash_events)
.chain(sp_std::iter::once(Box::new(<T as OffencesConfig>::RuntimeEvent::from(
pallet_offences::Event::Offence{
kind: UnresponsivenessOffence::<T>::ID,
timeslot: 0_u32.to_le_bytes().to_vec(),
}
).into()))
).into())))
);
}
}

report_offence_grandpa {
Expand Down