From 26b6eb3c5e53ddaf2dc06e1cda52d0c79cdb167a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 31 Oct 2025 23:07:56 +0100 Subject: [PATCH 1/8] frame-system: Ensure that `BlockNumber` is strictly increasing Otherwise a block should fail to build/import. --- substrate/frame/system/src/lib.rs | 3 +++ substrate/frame/system/src/tests.rs | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 483ef74ce3816..1d2514e008e4f 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1907,6 +1907,9 @@ impl Pallet { /// Start the execution of a particular block. pub fn initialize(number: &BlockNumberFor, parent_hash: &T::Hash, digest: &generic::Digest) { + let expected_block_number = Self::block_number() + One::one(); + assert_eq!(expected_block_number, *number, "Block number must be strictly increasing."); + // populate environment ExecutionPhase::::put(Phase::Initialization); storage::unhashed::put(well_known_keys::EXTRINSIC_INDEX, &0u32); diff --git a/substrate/frame/system/src/tests.rs b/substrate/frame/system/src/tests.rs index 59137f4bf9d46..287873588113a 100644 --- a/substrate/frame/system/src/tests.rs +++ b/substrate/frame/system/src/tests.rs @@ -964,3 +964,16 @@ fn reclaim_works() { assert_eq!(crate::ExtrinsicWeightReclaimed::::get(), Weight::zero()); }); } + +#[test] +#[should_panic(expected = "Block number must be strictly increasing.")] +fn initialize_block_number_must_be_sequential() { + new_test_ext().execute_with(|| { + // Initialize block 1 + System::initialize(&1, &[0u8; 32].into(), &Default::default()); + System::finalize(); + + // Try to initialize block 3, skipping block 2 - this should panic + System::initialize(&3, &[0u8; 32].into(), &Default::default()); + }); +} From 18c4a1ab2ed204b648a022f8fceac4a345befd97 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 31 Oct 2025 22:12:07 +0000 Subject: [PATCH 2/8] Update from github-actions[bot] running command 'prdoc --audience runtime_dev --bump patch' --- prdoc/pr_10180.prdoc | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 prdoc/pr_10180.prdoc diff --git a/prdoc/pr_10180.prdoc b/prdoc/pr_10180.prdoc new file mode 100644 index 0000000000000..155bd3f54c0df --- /dev/null +++ b/prdoc/pr_10180.prdoc @@ -0,0 +1,8 @@ +title: 'frame-system: Ensure that `BlockNumber` is strictly increasing' +doc: +- audience: Runtime Dev + description: |- + Otherwise a block should fail to build/import. +crates: +- name: frame-system + bump: patch From fb3d41d54479faf839db1256e38a6c022d6be3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 3 Nov 2025 10:07:01 +0100 Subject: [PATCH 3/8] Fix benchmarks --- polkadot/runtime/parachains/src/builder.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index dcbc337815602..6153f29fe1129 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -44,10 +44,10 @@ use polkadot_primitives::{ }; use sp_core::H256; use sp_runtime::{ - generic::Digest, traits::{Header as HeaderT, One, TrailingZeroInput, Zero}, RuntimeAppPublic, }; + fn mock_validation_code() -> ValidationCode { ValidationCode(vec![1, 2, 3]) } @@ -494,15 +494,11 @@ impl BenchBuilder { Self::run_to_block(block); } - let block_number = BlockNumberFor::::from(block); + let block_number = BlockNumberFor::::from(block + 1); let header = Self::header(block_number); frame_system::Pallet::::reset_events(); - frame_system::Pallet::::initialize( - &header.number(), - &header.hash(), - &Digest { logs: Vec::new() }, - ); + frame_system::Pallet::::initialize(&header.number(), &header.hash(), header.digest()); assert_eq!(shared::CurrentSessionIndex::::get(), target_session); From 0eac57288a5129517c5b293d56c2b078d396c5e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 4 Nov 2025 16:07:06 +0100 Subject: [PATCH 4/8] Fix tests --- cumulus/pallets/parachain-system/src/tests.rs | 80 +++++++++---------- substrate/frame/aura/src/tests.rs | 24 +++--- substrate/frame/babe/src/mock.rs | 31 ++++--- substrate/frame/sassafras/src/tests.rs | 7 +- substrate/frame/system/src/lib.rs | 5 ++ 5 files changed, 80 insertions(+), 67 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/tests.rs b/cumulus/pallets/parachain-system/src/tests.rs index 0b1ad28b4e82a..ca2767e1c87af 100755 --- a/cumulus/pallets/parachain-system/src/tests.rs +++ b/cumulus/pallets/parachain-system/src/tests.rs @@ -152,7 +152,7 @@ fn unincluded_segment_works() { BlockTests::new() .with_inclusion_delay(1) .add_with_post_test( - 123, + 1, || {}, || { let segment = >::get(); @@ -161,7 +161,7 @@ fn unincluded_segment_works() { }, ) .add_with_post_test( - 124, + 2, || {}, || { let segment = >::get(); @@ -169,11 +169,11 @@ fn unincluded_segment_works() { }, ) .add_with_post_test( - 125, + 3, || {}, || { let segment = >::get(); - // Block 123 was popped from the segment, the len is still 2. + // Block 1 was popped from the segment, the len is still 2. assert_eq!(segment.len(), 2); }, ); @@ -189,7 +189,7 @@ fn unincluded_segment_is_limited() { BlockTests::new() .with_inclusion_delay(2) .add_with_post_test( - 123, + 1, || {}, || { let segment = >::get(); @@ -197,7 +197,7 @@ fn unincluded_segment_is_limited() { assert!(>::get().is_some()); }, ) - .add(124, || {}); // The previous block wasn't included yet, should panic in `create_inherent`. + .add(2, || {}); // The previous block wasn't included yet, should panic in `create_inherent`. } #[test] @@ -209,15 +209,15 @@ fn unincluded_code_upgrade_handles_signal() { BlockTests::new() .with_inclusion_delay(1) .with_relay_sproof_builder(|_, block_number, builder| { - if block_number > 123 && block_number <= 125 { + if block_number > 1 && block_number <= 3 { builder.upgrade_go_ahead = Some(relay_chain::UpgradeGoAhead::GoAhead); } }) - .add(123, || { + .add(1, || { assert_ok!(System::set_code(RawOrigin::Root.into(), Default::default())); }) .add_with_post_test( - 124, + 2, || {}, || { assert!( @@ -227,7 +227,7 @@ fn unincluded_code_upgrade_handles_signal() { }, ) .add_with_post_test( - 125, + 3, || { // The signal is present in relay state proof and ignored. // Block that processed the signal is still not included. @@ -244,7 +244,7 @@ fn unincluded_code_upgrade_handles_signal() { }, ) .add_with_post_test( - 126, + 4, || {}, || { let aggregated_segment = @@ -264,15 +264,15 @@ fn unincluded_code_upgrade_scheduled_after_go_ahead() { BlockTests::new() .with_inclusion_delay(1) .with_relay_sproof_builder(|_, block_number, builder| { - if block_number > 123 && block_number <= 125 { + if block_number > 1 && block_number <= 3 { builder.upgrade_go_ahead = Some(relay_chain::UpgradeGoAhead::GoAhead); } }) - .add(123, || { + .add(1, || { assert_ok!(System::set_code(RawOrigin::Root.into(), Default::default())); }) .add_with_post_test( - 124, + 2, || {}, || { assert!( @@ -284,7 +284,7 @@ fn unincluded_code_upgrade_scheduled_after_go_ahead() { }, ) .add_with_post_test( - 125, + 3, || { // The signal is present in relay state proof and ignored. // Block that processed the signal is still not included. @@ -301,7 +301,7 @@ fn unincluded_code_upgrade_scheduled_after_go_ahead() { }, ) .add_with_post_test( - 126, + 4, || {}, || { assert!(>::exists(), "upgrade is pending"); @@ -684,10 +684,10 @@ fn hrmp_ingress_channels_are_checked() { let mut test = BlockTests::new() .with_inclusion_delay(1) - .with_relay_block_number(|block_number| 2.max(*block_number as RelayChainBlockNumber)) + .with_relay_block_number(|block_number| 1.max(*block_number as RelayChainBlockNumber)) .with_relay_sproof_builder(move |_, relay_block_num, sproof| match relay_block_num { // Let's open a channel only with parachain 1000. - 2 => { + 1 => { let mqc_head = sproof.upsert_inbound_channel(1000.into()).mqc_head.get_or_insert_default(); let mut mqc = MessageQueueChain::new(*mqc_head); @@ -697,14 +697,14 @@ fn hrmp_ingress_channels_are_checked() { _ => {}, }) .with_inherent_data(move |_, relay_block_num, data| match relay_block_num { - // Simulate receiving a message from parachain 1000 at block 2. This should work. - 2 => { + // Simulate receiving a message from parachain 1000 at block 1. This should work. + 1 => { let entry = data.horizontal_messages.entry(1000.into()).or_default(); entry.push(mk_hrmp(1, 100)) }, _ => {}, }) - .add(2, move || { + .add(1, move || { HANDLED_XCMP_MESSAGES.with(|m| { let m = m.borrow_mut(); assert_eq!(&*m, &vec![(1000.into(), 1, vec![1; 100])]); @@ -713,11 +713,11 @@ fn hrmp_ingress_channels_are_checked() { test.run(); let mut test = test - .with_relay_block_number(|block_number| 3.max(*block_number as RelayChainBlockNumber)) + .with_relay_block_number(|block_number| 2.max(*block_number as RelayChainBlockNumber)) .with_inherent_data(move |_, relay_block_num, data| match relay_block_num { - // Simulate receiving a message from parachain 2000 at block 3. This should lead to a + // Simulate receiving a message from parachain 2000 at block 2. This should lead to a // panic. - 3 => { + 2 => { let entry = data.horizontal_messages.entry(2000.into()).or_default(); entry.push(mk_hrmp(1, 100)) }, @@ -866,12 +866,12 @@ fn hrmp_outbound_respects_used_bandwidth() { fn runtime_upgrade_events() { BlockTests::new() .with_relay_sproof_builder(|_, block_number, builder| { - if block_number > 123 { + if block_number > 1 { builder.upgrade_go_ahead = Some(relay_chain::UpgradeGoAhead::GoAhead); } }) .add_with_post_test( - 123, + 1, || { assert_ok!(System::set_code(RawOrigin::Root.into(), Default::default())); }, @@ -884,7 +884,7 @@ fn runtime_upgrade_events() { }, ) .add_with_post_test( - 1234, + 2, || {}, || { let events = System::events(); @@ -894,7 +894,7 @@ fn runtime_upgrade_events() { assert_eq!( events[1].event, RuntimeEvent::ParachainSystem(crate::Event::ValidationFunctionApplied { - relay_chain_block_num: 1234 + relay_chain_block_num: 2 }) ); @@ -912,10 +912,10 @@ fn non_overlapping() { .with_relay_sproof_builder(|_, _, builder| { builder.host_config.validation_upgrade_delay = 1000; }) - .add(123, || { + .add(1, || { assert_ok!(System::set_code(RawOrigin::Root.into(), Default::default())); }) - .add(234, || { + .add(2, || { assert_eq!( System::set_code(RawOrigin::Root.into(), Default::default()), Err(Error::::OverlappingUpgrades.into()), @@ -927,11 +927,11 @@ fn non_overlapping() { fn manipulates_storage() { BlockTests::new() .with_relay_sproof_builder(|_, block_number, builder| { - if block_number > 123 { + if block_number > 1 { builder.upgrade_go_ahead = Some(relay_chain::UpgradeGoAhead::GoAhead); } }) - .add(123, || { + .add(1, || { assert!( !>::exists(), "validation function must not exist yet" @@ -940,7 +940,7 @@ fn manipulates_storage() { assert!(>::exists(), "validation function must now exist"); }) .add_with_post_test( - 1234, + 2, || {}, || { assert!( @@ -955,15 +955,15 @@ fn manipulates_storage() { fn aborted_upgrade() { BlockTests::new() .with_relay_sproof_builder(|_, block_number, builder| { - if block_number > 123 { + if block_number > 1 { builder.upgrade_go_ahead = Some(relay_chain::UpgradeGoAhead::Abort); } }) - .add(123, || { + .add(1, || { assert_ok!(System::set_code(RawOrigin::Root.into(), Default::default())); }) .add_with_post_test( - 1234, + 2, || {}, || { assert!( @@ -985,7 +985,7 @@ fn checks_code_size() { .with_relay_sproof_builder(|_, _, builder| { builder.host_config.max_code_size = 8; }) - .add(123, || { + .add(1, || { assert_eq!( System::set_code(RawOrigin::Root.into(), vec![0; 64]), Err(Error::::TooBig.into()), @@ -1161,7 +1161,7 @@ fn send_hrmp_message_buffer_channel_close() { 2, || {}, || { - // both channels are at capacity so we do not expect any messages. + // Both channels are at capacity so we do not expect any messages. let v = HrmpOutboundMessages::::get(); assert!(v.is_empty()); }, @@ -1381,7 +1381,7 @@ fn receive_hrmp() { data.horizontal_messages.insert( ParaId::from(300), vec![ - // can't be sent at the block 1 actually. However, we cheat here + // Can't be sent at the block 1 actually. However, we cheat here // because we want to test the case where there are multiple messages // but the harness at the moment doesn't support block skipping. mk_hrmp(2, 1).clone(), @@ -1594,7 +1594,7 @@ fn upgrade_version_checks_should_work() { #[test] fn deposits_relay_parent_storage_root() { BlockTests::new().add_with_post_test( - 123, + 1, || {}, || { let digest = System::digest(); diff --git a/substrate/frame/aura/src/tests.rs b/substrate/frame/aura/src/tests.rs index 5374105a2f3b2..89307af8ae8c9 100644 --- a/substrate/frame/aura/src/tests.rs +++ b/substrate/frame/aura/src/tests.rs @@ -47,13 +47,13 @@ fn disabled_validators_cannot_author_blocks() { Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] }; System::reset_events(); - System::initialize(&42, &System::parent_hash(), &pre_digest); + System::initialize(&1, &System::parent_hash(), &pre_digest); // let's disable the validator MockDisabledValidators::disable_validator(1); // and we should not be able to initialize the block - Aura::on_initialize(42); + Aura::on_initialize(1); }); } @@ -68,11 +68,11 @@ fn pallet_requires_slot_to_increase_unless_allowed() { Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] }; System::reset_events(); - System::initialize(&42, &System::parent_hash(), &pre_digest); + System::initialize(&1, &System::parent_hash(), &pre_digest); // and we should not be able to initialize the block with the same slot a second time. - Aura::on_initialize(42); - Aura::on_initialize(42); + Aura::on_initialize(1); + Aura::on_initialize(1); }); } @@ -84,13 +84,13 @@ fn pallet_can_allow_unchanged_slot() { Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] }; System::reset_events(); - System::initialize(&42, &System::parent_hash(), &pre_digest); + System::initialize(&1, &System::parent_hash(), &pre_digest); crate::mock::AllowMultipleBlocksPerSlot::set(true); // and we should be able to initialize the block with the same slot a second time. - Aura::on_initialize(42); - Aura::on_initialize(42); + Aura::on_initialize(1); + Aura::on_initialize(1); }); } @@ -103,17 +103,17 @@ fn pallet_always_rejects_decreasing_slot() { Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] }; System::reset_events(); - System::initialize(&42, &System::parent_hash(), &pre_digest); + System::initialize(&1, &System::parent_hash(), &pre_digest); crate::mock::AllowMultipleBlocksPerSlot::set(true); - Aura::on_initialize(42); + Aura::on_initialize(1); System::finalize(); let earlier_slot = Slot::from(1); let pre_digest = Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, earlier_slot.encode())] }; - System::initialize(&43, &System::parent_hash(), &pre_digest); - Aura::on_initialize(43); + System::initialize(&2, &System::parent_hash(), &pre_digest); + Aura::on_initialize(2); }); } diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index 961c3bb06c4cf..8e5a0ac1b8ba0 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -39,7 +39,7 @@ use sp_runtime::{ impl_opaque_keys, testing::{Digest, DigestItem, Header, TestXt}, traits::{Header as _, OpaqueKeys}, - BuildStorage, Perbill, + BuildStorage, DispatchError, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; @@ -372,16 +372,24 @@ pub fn generate_equivocation_proof( let current_slot = CurrentSlot::::get(); let make_header = || { - let parent_hash = System::parent_hash(); - let pre_digest = make_secondary_plain_pre_digest(offender_authority_index, slot); - System::reset_events(); - System::initialize(¤t_block, &parent_hash, &pre_digest); - System::set_block_number(current_block); - Timestamp::set_timestamp(*current_slot * Babe::slot_duration()); - System::finalize() + // We don't want to change any state, so we build the headers in a transaction and revert it + // afterward. + frame_support::storage::with_transaction(|| { + let parent_hash = System::parent_hash(); + let pre_digest = make_secondary_plain_pre_digest(offender_authority_index, slot); + System::reset_events(); + System::set_block_number(System::block_number() - 1); + System::initialize(¤t_block, &parent_hash, &pre_digest); + System::set_block_number(current_block); + Timestamp::set_timestamp(*current_slot * Babe::slot_duration()); + let header = System::finalize(); + + sp_runtime::TransactionOutcome::Rollback(Ok::<_, DispatchError>(header)) + }) + .unwrap() }; - // sign the header prehash and sign it, adding it to the block as the seal + // Sign the header prehash and sign it, adding it to the block as the seal // digest item let seal_header = |header: &mut Header| { let prehash = header.hash(); @@ -391,16 +399,13 @@ pub fn generate_equivocation_proof( header.digest_mut().push(seal); }; - // generate two headers at the current block + // Generate two headers at the current block let mut h1 = make_header(); let mut h2 = make_header(); seal_header(&mut h1); seal_header(&mut h2); - // restore previous runtime state - go_to_block(current_block, *current_slot); - sp_consensus_babe::EquivocationProof { slot, offender: offender_authority_pair.public(), diff --git a/substrate/frame/sassafras/src/tests.rs b/substrate/frame/sassafras/src/tests.rs index 193ec675a8d08..7236deeaa6518 100644 --- a/substrate/frame/sassafras/src/tests.rs +++ b/substrate/frame/sassafras/src/tests.rs @@ -65,6 +65,7 @@ fn post_genesis_randomness_initialization() { let (id1, _) = make_ticket_bodies(1, Some(pair))[0]; // Reset what is relevant + System::set_block_number(0); NextRandomness::::set([0; 32]); RandomnessAccumulator::::set([0; 32]); @@ -88,6 +89,7 @@ fn post_genesis_randomness_initialization() { assert_ne!(id1, id2); // Reset what is relevant + System::set_block_number(0); NextRandomness::::set([0; 32]); RandomnessAccumulator::::set([0; 32]); @@ -666,6 +668,7 @@ fn block_allowed_to_skip_epochs() { // We want to skip 3 epochs in this test. let offset = 4 * epoch_length; + System::set_block_number(start_block + offset - 1); go_to_block(start_block + offset, start_slot + offset, &pairs[0]); // Post-initialization status @@ -715,7 +718,7 @@ fn obsolete_tickets_are_removed_on_epoch_change() { }); // Advance one epoch to enact the tickets - go_to_block(start_block + epoch_length, start_slot + epoch_length, pair); + progress_to_block(start_block + epoch_length, pair).unwrap(); assert_eq!(TicketsMeta::::get().tickets_count, [0, 4]); // Persist some tickets for next epoch (N+1) @@ -736,7 +739,7 @@ fn obsolete_tickets_are_removed_on_epoch_change() { // Advance to epoch 2 and check for cleanup - go_to_block(start_block + 2 * epoch_length, start_slot + 2 * epoch_length, pair); + progress_to_block(start_block + 2 * epoch_length, pair).unwrap(); assert_eq!(TicketsMeta::::get().tickets_count, [6, 0]); (0..epoch1_tickets.len()).into_iter().for_each(|i| { diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 1d2514e008e4f..c0729d47a6efe 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1906,6 +1906,11 @@ impl Pallet { } /// Start the execution of a particular block. + /// + /// # Panics + /// + /// Panics when the given `number` is not `Self::block_number() + 1`. If you are using this in + /// tests, you can use [`Self::set_block_number`] to make the check succeed. pub fn initialize(number: &BlockNumberFor, parent_hash: &T::Hash, digest: &generic::Digest) { let expected_block_number = Self::block_number() + One::one(); assert_eq!(expected_block_number, *number, "Block number must be strictly increasing."); From 1fb35b137cf42a111393b42de528be9b95c8cca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 5 Nov 2025 12:49:57 +0100 Subject: [PATCH 5/8] More fixes --- Cargo.lock | 1 + polkadot/runtime/parachains/Cargo.toml | 2 +- polkadot/runtime/parachains/src/builder.rs | 10 +++++++++- .../runtime/parachains/src/paras_inherent/tests.rs | 12 ++++++------ substrate/frame/executive/src/tests.rs | 7 ++++--- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0ffbbdfec990..eb36d79562a54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16427,6 +16427,7 @@ dependencies = [ "polkadot-primitives", "polkadot-primitives-test-helpers", "polkadot-runtime-metrics", + "pretty_assertions", "rand 0.8.5", "rand_chacha 0.3.1", "rstest", diff --git a/polkadot/runtime/parachains/Cargo.toml b/polkadot/runtime/parachains/Cargo.toml index cddfb75d3e9f9..b3efd0d7c31df 100644 --- a/polkadot/runtime/parachains/Cargo.toml +++ b/polkadot/runtime/parachains/Cargo.toml @@ -63,7 +63,7 @@ static_assertions = { optional = true, workspace = true, default-features = true [dev-dependencies] polkadot-primitives = { workspace = true, features = ["test"] } - +pretty_assertions = { workspace = true } assert_matches = { workspace = true } frame-support-test = { workspace = true } hex-literal = { workspace = true, default-features = true } diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index 6153f29fe1129..a20867f0b600a 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -483,7 +483,7 @@ impl BenchBuilder { extra_cores: usize, ) -> Self { let mut block = 1; - for session in 0..=target_session { + for session in 0..target_session { initializer::Pallet::::test_trigger_on_new_session( false, session, @@ -494,11 +494,19 @@ impl BenchBuilder { Self::run_to_block(block); } + initializer::Pallet::::test_trigger_on_new_session( + false, + block - 1, + validators.iter().map(|(a, v)| (a, v.clone())), + None, + ); + initializer::Pallet::::on_finalize(block.into()); let block_number = BlockNumberFor::::from(block + 1); let header = Self::header(block_number); frame_system::Pallet::::reset_events(); frame_system::Pallet::::initialize(&header.number(), &header.hash(), header.digest()); + initializer::Pallet::::on_initialize(*header.number()); assert_eq!(shared::CurrentSessionIndex::::get(), target_session); diff --git a/polkadot/runtime/parachains/src/paras_inherent/tests.rs b/polkadot/runtime/parachains/src/paras_inherent/tests.rs index 521c308ac1d69..0a68fc049336d 100644 --- a/polkadot/runtime/parachains/src/paras_inherent/tests.rs +++ b/polkadot/runtime/parachains/src/paras_inherent/tests.rs @@ -44,10 +44,6 @@ fn default_config() -> MockGenesisConfig { #[cfg(not(feature = "runtime-benchmarks"))] mod enter { use super::{inclusion::tests::TestCandidateBuilder, *}; - use polkadot_primitives::{ApprovedPeerId, ClaimQueueOffset, CoreSelector, UMPSignal}; - use rstest::rstest; - use sp_core::ByteArray; - use crate::{ builder::{Bench, BenchBuilder, CandidateModifier}, disputes::clear_dispute_storage, @@ -62,10 +58,14 @@ mod enter { use frame_support::assert_ok; use frame_system::limits; use polkadot_primitives::{ - AvailabilityBitfield, CandidateDescriptorV2, CollatorId, CollatorSignature, - CommittedCandidateReceiptV2, InternalVersion, MutateDescriptorV2, UncheckedSigned, + ApprovedPeerId, AvailabilityBitfield, CandidateDescriptorV2, ClaimQueueOffset, CollatorId, + CollatorSignature, CommittedCandidateReceiptV2, CoreSelector, InternalVersion, + MutateDescriptorV2, UMPSignal, UncheckedSigned, }; use polkadot_primitives_test_helpers::CandidateDescriptor; + use pretty_assertions::assert_eq; + use rstest::rstest; + use sp_core::ByteArray; use sp_runtime::Perbill; struct TestConfig { diff --git a/substrate/frame/executive/src/tests.rs b/substrate/frame/executive/src/tests.rs index 20bc4c91de818..d8d0836384054 100644 --- a/substrate/frame/executive/src/tests.rs +++ b/substrate/frame/executive/src/tests.rs @@ -867,6 +867,10 @@ fn validate_unsigned() { let mut t = new_test_ext(1); t.execute_with(|| { + // Need to initialize the block before applying extrinsics for the `MockedSystemCallbacks` + // check. + Executive::initialize_block(&Header::new_from_number(1)); + assert_eq!( Executive::validate_transaction( TransactionSource::InBlock, @@ -883,9 +887,6 @@ fn validate_unsigned() { ), Err(TransactionValidityError::Unknown(UnknownTransaction::NoUnsignedValidator)), ); - // Need to initialize the block before applying extrinsics for the `MockedSystemCallbacks` - // check. - Executive::initialize_block(&Header::new_from_number(1)); assert_eq!(Executive::apply_extrinsic(valid), Ok(Err(DispatchError::BadOrigin))); assert_eq!( Executive::apply_extrinsic(invalid), From 019baf67c964d471d2438ae2176d2240ad9b9128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 5 Nov 2025 17:49:25 +0100 Subject: [PATCH 6/8] Fix benchmark --- substrate/frame/revive/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/src/benchmarking.rs b/substrate/frame/revive/src/benchmarking.rs index e5b19c5976e4d..de64c6824ae62 100644 --- a/substrate/frame/revive/src/benchmarking.rs +++ b/substrate/frame/revive/src/benchmarking.rs @@ -1090,7 +1090,7 @@ mod benchmarks { digest.push(DigestItem::Seal(AURA_ENGINE_ID, slot.encode())); frame_system::Pallet::::initialize( - &BlockNumberFor::::from(1u32), + &BlockNumberFor::::from(2u32), &Default::default(), &digest, ); From 9e5cfa9c853e6adc45c5f502fe0e4d8c488a6b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 6 Nov 2025 12:09:34 +0100 Subject: [PATCH 7/8] Fix it in a different way --- substrate/frame/revive/src/benchmarking.rs | 44 ++++++++++++---------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/substrate/frame/revive/src/benchmarking.rs b/substrate/frame/revive/src/benchmarking.rs index de64c6824ae62..f20b6432e9eda 100644 --- a/substrate/frame/revive/src/benchmarking.rs +++ b/substrate/frame/revive/src/benchmarking.rs @@ -71,10 +71,7 @@ use sp_consensus_babe::{ BABE_ENGINE_ID, }; use sp_consensus_slots::Slot; -use sp_runtime::{ - generic::{Digest, DigestItem}, - traits::Zero, -}; +use sp_runtime::{generic::DigestItem, traits::Zero}; /// How many runs we do per API benchmark. /// @@ -1061,16 +1058,20 @@ mod benchmarks { fn seal_block_author() { build_runtime!(runtime, memory: [[123u8; 20], ]); - let mut digest = Digest::default(); - // The pre-runtime digest log is unbounded; usually around 3 items but it can vary. // To get safe benchmark results despite that, populate it with a bunch of random logs to // ensure iteration over many items (we just overestimate the cost of the API). for i in 0..16 { - digest.push(DigestItem::PreRuntime([i, i, i, i], vec![i; 128])); - digest.push(DigestItem::Consensus([i, i, i, i], vec![i; 128])); - digest.push(DigestItem::Seal([i, i, i, i], vec![i; 128])); - digest.push(DigestItem::Other(vec![i; 128])); + frame_system::Pallet::::deposit_log(DigestItem::PreRuntime( + [i, i, i, i], + vec![i; 128], + )); + frame_system::Pallet::::deposit_log(DigestItem::Consensus( + [i, i, i, i], + vec![i; 128], + )); + frame_system::Pallet::::deposit_log(DigestItem::Seal([i, i, i, i], vec![i; 128])); + frame_system::Pallet::::deposit_log(DigestItem::Other(vec![i; 128])); } // The content of the pre-runtime digest log depends on the configured consensus. @@ -1081,19 +1082,22 @@ mod benchmarks { let primary_pre_digest = vec![0; ::max_encoded_len()]; let pre_digest = PreDigest::Primary(PrimaryPreDigest::decode(&mut &primary_pre_digest[..]).unwrap()); - digest.push(DigestItem::PreRuntime(BABE_ENGINE_ID, pre_digest.encode())); - digest.push(DigestItem::Seal(BABE_ENGINE_ID, pre_digest.encode())); + frame_system::Pallet::::deposit_log(DigestItem::PreRuntime( + BABE_ENGINE_ID, + pre_digest.encode(), + )); + frame_system::Pallet::::deposit_log(DigestItem::Seal( + BABE_ENGINE_ID, + pre_digest.encode(), + )); // Construct a `Digest` log fixture returning some value in AURA let slot = Slot::default(); - digest.push(DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())); - digest.push(DigestItem::Seal(AURA_ENGINE_ID, slot.encode())); - - frame_system::Pallet::::initialize( - &BlockNumberFor::::from(2u32), - &Default::default(), - &digest, - ); + frame_system::Pallet::::deposit_log(DigestItem::PreRuntime( + AURA_ENGINE_ID, + slot.encode(), + )); + frame_system::Pallet::::deposit_log(DigestItem::Seal(AURA_ENGINE_ID, slot.encode())); let result; #[block] From 657211537654d3e5ee532515330c9bfa49f77e22 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 6 Nov 2025 11:17:32 +0000 Subject: [PATCH 8/8] Update from github-actions[bot] running command 'fmt' --- polkadot/runtime/parachains/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/parachains/Cargo.toml b/polkadot/runtime/parachains/Cargo.toml index b3efd0d7c31df..15c47d723050e 100644 --- a/polkadot/runtime/parachains/Cargo.toml +++ b/polkadot/runtime/parachains/Cargo.toml @@ -62,12 +62,12 @@ rand_chacha = { workspace = true } static_assertions = { optional = true, workspace = true, default-features = true } [dev-dependencies] -polkadot-primitives = { workspace = true, features = ["test"] } -pretty_assertions = { workspace = true } assert_matches = { workspace = true } frame-support-test = { workspace = true } hex-literal = { workspace = true, default-features = true } +polkadot-primitives = { workspace = true, features = ["test"] } polkadot-primitives-test-helpers = { workspace = true } +pretty_assertions = { workspace = true } rstest = { workspace = true } sc-keystore = { workspace = true, default-features = true } serde_json = { workspace = true, default-features = true }