Tracking issue for adding try_state hook to all pallets.

[kianenigma]

This hook was added in paritytech/substrate#10174. The idea is as follows:

/// Execute some checks to ensure the internal state of a pallet is consistent.
///
/// Usually, these checks should check all of the invariants that are expected to be held on all of
/// the storage items of your pallet.
///
/// This hook should not alter any storage.
pub trait TryState<BlockNumber> {
	/// Execute the state checks.
	fn try_state(_: BlockNumber) -> Result<(), &'static str>;
}

(source)

In other words, the goal is to focus on invariants that must always be held in a pallet. A naive, yet important example of this is that in pallet-balances, iterating over all users should yield the correct total issuance.

If in doubt, see a number of other pallets that already have this hook to get further inspiration.

Implementations should provide a clear and concise documentation as to why the given invariant must always hold.

Next, we want all pallets to call their try_state after each test. This can easily be achieved with a helper function like:

polkadot-sdk/substrate/frame/nomination-pools/src/mock.rs

Lines 401 to 407 in 53f615d

	pub fn build_and_execute(self, test: impl FnOnce()) {
	self.build().execute_with(|| {
	test();
	Pools::do_try_state(CheckLevel::get()).unwrap();
	})
	}
	}

Implementing this hook will allow these checks to be called in try-runtime-cli, and other testing tools that compile a runtime with feature = try-runtime.

Note that for those who want to tackle a particular pallet, you must for sure deeply understand a pallet in order to correctly identify its list of invariants. Moreover, you might do your future generation a favor by documenting the invariants as good as you can.

TLDR Checklist

• Read the pallet code carefully, and find invariants that must always hold. This is usually in #[pallet::storage] types, but sometimes also in #[pallet::config] trait.
• Implement the invariants with respect to details mentioned on the trait: everything must be #[cfg(any(feature = "try-runtime"), test)], and no storage must be altered.
• tweak the test setup to call try_state after each test, as per the example above.

Pallet list

• alliance
• assets
• atomic-swap
• aura Add try-state hook to pallet aura  substrate#14363
• authority-discovery
• authorship
• babe
• bags-list
• balances
• beefy Try State Hook for Beefy #3246
• beefy-mmr
• bounties
• child-bounties
• collective Try-state for Collective pallet substrate#13645
• contracts
• conviction-voting
• democracy
• election-provider-multi-phase Implements try_state hook in elections and EPM pallets substrate#13979
• elections-phragmen Implements try_state hook in elections and EPM pallets substrate#13979
• fast-unstake
• gilt
• grandpa
• identity
• im-online
• indices Refactor indices pallet #1789
• lottery
• membership
• merkle-mountain-range
• message-queue Add TryState hook for MessageQueue substrate#13115
• multisig
• nicks
• node-authorization
• nomination-pools
• offences
• preimage
• proxy
• randomness-collective-flip
• ranked-collective Try State Hook for Ranked Collective #3007
• recovery
• referenda @Szegoo Try-state for Referenda pallet substrate#13949
• remark
• root-offences noop
• root-testing noop
• scheduler
• session
• society
• staking
• state-trie-migration
• sudo
• system
• timestamp
• tips @Doordashcon Adding try_state hook(tips & vesting) substrate#13515
• transaction-payment
• transaction-storage
• treasury Adding try_state hook for Treasury pallet #1820
• uniques
• utility
• vesting @Doordashcon Adding try_state hook(tips & vesting) substrate#13515
• whitelist

[vjgaur]

I want to take this up. Can you please assign this to me ?

[kianenigma]

I want to take this up. Can you please assign this to me ?

This is a tracking issue, you can go ahead and work on any item, no need to assign 👍

[Doordashcon]

@kianenigma The idea of the try_state hook is to reduce testing code?

So removing duplicate tests after try_state inclusion is mandatory?

[ggwpez]

The idea of the try_state hook is to reduce testing code?

There are actually two advantages that try_state has. Once for invariant assertion in unit tests, but also in try-runtime migration testing. This is very nice since currently we dont know if a pallet's state is valid after a migration ran.
So the goal is to increase test coverage by always checking the invariants. We would probably end up with more testing code, not less 😆
There is some more explanation here paritytech/substrate#13115 (comment)

[Doordashcon]

Hello @ggwpez could I list out pallets I think won't require try_state ? For now pallet-timestamp which uses the #[pallet::inherent] and pallet-utility which is stateless.

Hoping this might speed things up for other contributors :)

[ggwpez]

Yes sure @Doordashcon. Marked them together with the nicks pallet.

[Szegoo]

I will start working on the try-state for the referenda pallet if no one else picked that already.

[Polkadot-Forum]

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/discussion-shaping-the-future-of-try-runtime-and-try-runtime-cli/2657/4

[pmikolajczyk41]

aura implemented here: paritytech/substrate#14363