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

Commit 324a18e

Browse files
kianenigmaggwpezdvdplm
authored
Runtime State Test + Integration with try-runtime (#10174)
* add missing version to dependencies * Huh * add features more * more fixing * last touches * it all finally works * remove some feature gates * remove unused * fix old macro * make it work again * fmt * remove unused import * ".git/.scripts/fmt.sh" 1 * Cleanup more * fix and rename everything * a few clippy fixes * Add try-runtime feature Signed-off-by: Oliver Tale-Yazdi <[email protected]> * small fixes * fmt * Update bin/node-template/runtime/src/lib.rs * fix build * Update utils/frame/try-runtime/cli/src/lib.rs Co-authored-by: David <[email protected]> * Update utils/frame/try-runtime/cli/src/commands/execute_block.rs Co-authored-by: David <[email protected]> * address all review comments * fix typos * revert spec change * last touches * update docs * fmt * remove some debug_assertions * fmt Signed-off-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: command-bot <> Co-authored-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: David <[email protected]>
1 parent 4f8207d commit 324a18e

File tree

39 files changed

+621
-179
lines changed

39 files changed

+621
-179
lines changed

‎Cargo.lock‎

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

‎bin/node-template/runtime/Cargo.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ std = [
6363
"frame-support/std",
6464
"frame-system-rpc-runtime-api/std",
6565
"frame-system/std",
66+
"frame-try-runtime/std",
6667
"pallet-aura/std",
6768
"pallet-balances/std",
6869
"pallet-grandpa/std",
@@ -97,9 +98,10 @@ runtime-benchmarks = [
9798
"sp-runtime/runtime-benchmarks",
9899
]
99100
try-runtime = [
100-
"frame-executive/try-runtime",
101101
"frame-try-runtime",
102+
"frame-executive/try-runtime",
102103
"frame-system/try-runtime",
104+
"frame-support/try-runtime",
103105
"pallet-aura/try-runtime",
104106
"pallet-balances/try-runtime",
105107
"pallet-grandpa/try-runtime",

‎bin/node-template/runtime/src/lib.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,14 @@ impl_runtime_apis! {
545545
(weight, BlockWeights::get().max_block)
546546
}
547547

548-
fn execute_block_no_check(block: Block) -> Weight {
549-
Executive::execute_block_no_check(block)
548+
fn execute_block(
549+
block: Block,
550+
state_root_check: bool,
551+
select: frame_try_runtime::TryStateSelect
552+
) -> Weight {
553+
// NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to
554+
// have a backtrace here.
555+
Executive::try_execute_block(block, state_root_check, select).expect("execute-block failed")
550556
}
551557
}
552558
}

‎bin/node/runtime/Cargo.toml‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,16 @@ runtime-benchmarks = [
245245
"hex-literal",
246246
]
247247
try-runtime = [
248-
"frame-executive/try-runtime",
249248
"frame-try-runtime",
249+
"frame-executive/try-runtime",
250250
"frame-system/try-runtime",
251+
"frame-support/try-runtime",
251252
"pallet-alliance/try-runtime",
252253
"pallet-assets/try-runtime",
253254
"pallet-authority-discovery/try-runtime",
254255
"pallet-authorship/try-runtime",
255256
"pallet-babe/try-runtime",
257+
"pallet-bags-list/try-runtime",
256258
"pallet-balances/try-runtime",
257259
"pallet-bounties/try-runtime",
258260
"pallet-child-bounties/try-runtime",
@@ -264,32 +266,36 @@ try-runtime = [
264266
"pallet-elections-phragmen/try-runtime",
265267
"pallet-gilt/try-runtime",
266268
"pallet-grandpa/try-runtime",
267-
"pallet-identity/try-runtime",
268269
"pallet-im-online/try-runtime",
269270
"pallet-indices/try-runtime",
271+
"pallet-identity/try-runtime",
270272
"pallet-lottery/try-runtime",
271273
"pallet-membership/try-runtime",
272274
"pallet-mmr/try-runtime",
273275
"pallet-multisig/try-runtime",
276+
"pallet-nomination-pools/try-runtime",
274277
"pallet-offences/try-runtime",
275278
"pallet-preimage/try-runtime",
276279
"pallet-proxy/try-runtime",
277-
"pallet-ranked-collective/try-runtime",
278280
"pallet-randomness-collective-flip/try-runtime",
281+
"pallet-ranked-collective/try-runtime",
279282
"pallet-recovery/try-runtime",
280283
"pallet-referenda/try-runtime",
281-
"pallet-scheduler/try-runtime",
284+
"pallet-remark/try-runtime",
282285
"pallet-session/try-runtime",
283-
"pallet-society/try-runtime",
284286
"pallet-staking/try-runtime",
285287
"pallet-state-trie-migration/try-runtime",
288+
"pallet-scheduler/try-runtime",
289+
"pallet-society/try-runtime",
286290
"pallet-sudo/try-runtime",
287291
"pallet-timestamp/try-runtime",
288292
"pallet-tips/try-runtime",
289-
"pallet-transaction-payment/try-runtime",
290293
"pallet-treasury/try-runtime",
291-
"pallet-uniques/try-runtime",
292294
"pallet-utility/try-runtime",
295+
"pallet-transaction-payment/try-runtime",
296+
"pallet-asset-tx-payment/try-runtime",
297+
"pallet-transaction-storage/try-runtime",
298+
"pallet-uniques/try-runtime",
293299
"pallet-vesting/try-runtime",
294300
"pallet-whitelist/try-runtime",
295301
]

‎bin/node/runtime/src/lib.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,8 +2079,21 @@ impl_runtime_apis! {
20792079
(weight, RuntimeBlockWeights::get().max_block)
20802080
}
20812081

2082-
fn execute_block_no_check(block: Block) -> Weight {
2083-
Executive::execute_block_no_check(block)
2082+
fn execute_block(
2083+
block: Block,
2084+
state_root_check: bool,
2085+
select: frame_try_runtime::TryStateSelect
2086+
) -> Weight {
2087+
log::info!(
2088+
target: "node-runtime",
2089+
"try-runtime: executing block {:?} / root checks: {:?} / try-state-select: {:?}",
2090+
block.header.hash(),
2091+
state_root_check,
2092+
select,
2093+
);
2094+
// NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to
2095+
// have a backtrace here.
2096+
Executive::try_execute_block(block, state_root_check, select).unwrap()
20842097
}
20852098
}
20862099

‎frame/bags-list/fuzzer/src/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ fn main() {
8888
},
8989
}
9090

91-
assert!(BagsList::sanity_check().is_ok());
91+
assert!(BagsList::try_state().is_ok());
9292
})
9393
});
9494
}

‎frame/bags-list/remote-tests/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ use sp_std::prelude::*;
2424
pub const LOG_TARGET: &str = "runtime::bags-list::remote-tests";
2525

2626
pub mod migration;
27-
pub mod sanity_check;
2827
pub mod snapshot;
28+
pub mod try_state;
2929

3030
/// A wrapper for a runtime that the functions of this crate expect.
3131
///

frame/bags-list/remote-tests/src/sanity_check.rs renamed to frame/bags-list/remote-tests/src/try_state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ pub async fn execute<Runtime: crate::RuntimeT, Block: BlockT + DeserializeOwned>
4444

4545
ext.execute_with(|| {
4646
sp_core::crypto::set_default_ss58_version(Runtime::SS58Prefix::get().try_into().unwrap());
47-
pallet_bags_list::Pallet::<Runtime>::sanity_check().unwrap();
47+
pallet_bags_list::Pallet::<Runtime>::try_state().unwrap();
4848
log::info!(target: crate::LOG_TARGET, "executed bags-list sanity check with no errors.");
4949

5050
crate::display_and_check_bags::<Runtime>(currency_unit, currency_name);

‎frame/bags-list/src/lib.rs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,11 @@ pub mod pallet {
263263
"thresholds must strictly increase, and have no duplicates",
264264
);
265265
}
266+
267+
#[cfg(feature = "try-runtime")]
268+
fn try_state(_: BlockNumberFor<T>) -> Result<(), &'static str> {
269+
<Self as SortedListProvider<T::AccountId>>::try_state()
270+
}
266271
}
267272
}
268273

@@ -340,14 +345,8 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
340345
List::<T, I>::unsafe_regenerate(all, score_of)
341346
}
342347

343-
#[cfg(feature = "std")]
344-
fn sanity_check() -> Result<(), &'static str> {
345-
List::<T, I>::sanity_check()
346-
}
347-
348-
#[cfg(not(feature = "std"))]
349-
fn sanity_check() -> Result<(), &'static str> {
350-
Ok(())
348+
fn try_state() -> Result<(), &'static str> {
349+
List::<T, I>::try_state()
351350
}
352351

353352
fn unsafe_clear() {

‎frame/bags-list/src/list/mod.rs‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use crate::Config;
2828
use codec::{Decode, Encode, MaxEncodedLen};
2929
use frame_election_provider_support::ScoreProvider;
3030
use frame_support::{
31-
ensure,
32-
traits::{Defensive, Get},
31+
defensive, ensure,
32+
traits::{Defensive, DefensiveOption, Get},
3333
DefaultNoBound, PalletError,
3434
};
3535
use scale_info::TypeInfo;
@@ -220,7 +220,8 @@ impl<T: Config<I>, I: 'static> List<T, I> {
220220
crate::ListBags::<T, I>::remove(removed_bag);
221221
}
222222

223-
debug_assert_eq!(Self::sanity_check(), Ok(()));
223+
#[cfg(feature = "std")]
224+
debug_assert_eq!(Self::try_state(), Ok(()));
224225

225226
num_affected
226227
}
@@ -325,8 +326,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
325326

326327
crate::log!(
327328
debug,
328-
"inserted {:?} with score {:?
329-
} into bag {:?}, new count is {}",
329+
"inserted {:?} with score {:?} into bag {:?}, new count is {}",
330330
id,
331331
score,
332332
bag_score,
@@ -457,11 +457,8 @@ impl<T: Config<I>, I: 'static> List<T, I> {
457457

458458
// re-fetch `lighter_node` from storage since it may have been updated when `heavier_node`
459459
// was removed.
460-
let lighter_node = Node::<T, I>::get(lighter_id).ok_or_else(|| {
461-
debug_assert!(false, "id that should exist cannot be found");
462-
crate::log!(warn, "id that should exist cannot be found");
463-
ListError::NodeNotFound
464-
})?;
460+
let lighter_node =
461+
Node::<T, I>::get(lighter_id).defensive_ok_or_else(|| ListError::NodeNotFound)?;
465462

466463
// insert `heavier_node` directly in front of `lighter_node`. This will update both nodes
467464
// in storage and update the node counter.
@@ -508,7 +505,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
508505
node.put();
509506
}
510507

511-
/// Sanity check the list.
508+
/// Check the internal state of the list.
512509
///
513510
/// This should be called from the call-site, whenever one of the mutating apis (e.g. `insert`)
514511
/// is being used, after all other staking data (such as counter) has been updated. It checks:
@@ -517,8 +514,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
517514
/// * length of this list is in sync with `ListNodes::count()`,
518515
/// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure
519516
/// all bags and nodes are checked per *any* update to `List`.
520-
#[cfg(feature = "std")]
521-
pub(crate) fn sanity_check() -> Result<(), &'static str> {
517+
pub(crate) fn try_state() -> Result<(), &'static str> {
522518
let mut seen_in_list = BTreeSet::new();
523519
ensure!(
524520
Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)),
@@ -546,7 +542,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
546542
thresholds.into_iter().filter_map(|t| Bag::<T, I>::get(t))
547543
};
548544

549-
let _ = active_bags.clone().try_for_each(|b| b.sanity_check())?;
545+
let _ = active_bags.clone().try_for_each(|b| b.try_state())?;
550546

551547
let nodes_in_bags_count =
552548
active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32);
@@ -557,17 +553,12 @@ impl<T: Config<I>, I: 'static> List<T, I> {
557553
// check that all nodes are sane. We check the `ListNodes` storage item directly in case we
558554
// have some "stale" nodes that are not in a bag.
559555
for (_id, node) in crate::ListNodes::<T, I>::iter() {
560-
node.sanity_check()?
556+
node.try_state()?
561557
}
562558

563559
Ok(())
564560
}
565561

566-
#[cfg(not(feature = "std"))]
567-
pub(crate) fn sanity_check() -> Result<(), &'static str> {
568-
Ok(())
569-
}
570-
571562
/// Returns the nodes of all non-empty bags. For testing and benchmarks.
572563
#[cfg(any(feature = "std", feature = "runtime-benchmarks"))]
573564
#[allow(dead_code)]
@@ -701,8 +692,7 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
701692
if *tail == node.id {
702693
// this should never happen, but this check prevents one path to a worst case
703694
// infinite loop.
704-
debug_assert!(false, "system logic error: inserting a node who has the id of tail");
705-
crate::log!(warn, "system logic error: inserting a node who has the id of tail");
695+
defensive!("system logic error: inserting a node who has the id of tail");
706696
return
707697
};
708698
}
@@ -753,16 +743,15 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
753743
}
754744
}
755745

756-
/// Sanity check this bag.
746+
/// Check the internal state of the bag.
757747
///
758748
/// Should be called by the call-site, after any mutating operation on a bag. The call site of
759749
/// this struct is always `List`.
760750
///
761751
/// * Ensures head has no prev.
762752
/// * Ensures tail has no next.
763753
/// * Ensures there are no loops, traversal from head to tail is correct.
764-
#[cfg(feature = "std")]
765-
fn sanity_check(&self) -> Result<(), &'static str> {
754+
fn try_state(&self) -> Result<(), &'static str> {
766755
frame_support::ensure!(
767756
self.head()
768757
.map(|head| head.prev().is_none())
@@ -801,7 +790,6 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
801790
}
802791

803792
/// Check if the bag contains a node with `id`.
804-
#[cfg(feature = "std")]
805793
fn contains(&self, id: &T::AccountId) -> bool {
806794
self.iter().any(|n| n.id() == id)
807795
}
@@ -906,8 +894,7 @@ impl<T: Config<I>, I: 'static> Node<T, I> {
906894
self.bag_upper
907895
}
908896

909-
#[cfg(feature = "std")]
910-
fn sanity_check(&self) -> Result<(), &'static str> {
897+
fn try_state(&self) -> Result<(), &'static str> {
911898
let expected_bag = Bag::<T, I>::get(self.bag_upper).ok_or("bag not found for node")?;
912899

913900
let id = self.id();

0 commit comments

Comments
 (0)