try-runtime::fast-forward

[pmikolajczyk41]

We introduce a new subcommand for try-runtime tool: fast-forward.

Functionality description

1. We build RemoteExternalities from either a live chain or a local snapshot (try-runtime folklore)
2. In a loop (either for n blocks or until Ctrl+C) we produce an empty block and execute it atop of the local state

By empty block we mean a block with only pre-runtime digest (like slot for aura) and inherent extrinsics (like timestamp::set). Notice, that we don't respect the standard blocktime / slot-duration, but instead we are proceeding immediately with subsequent block.

Also, observe that effectively we are simulating (testing) two processes separately:

• block production ("Core_initialize_block", "BlockBuilder_inherent_extrinsics", "BlockBuilder_apply_extrinsic", "BlockBuilder_finalize_block")
• block execution ("TryRuntime_execute_block")

Usage

As pre-runtime digest and inherents are chain-specific and are crucial for block correctness, every chain must provide its own way of producing these. Please consult node-template for the example implementation.

Example output

piomiko:~/Desktop/substrate$ ./target/release/node-template try-runtime --runtime existing fast-forward --n-blocks 2 live --uri ws://127.0.0.1:9944
2022-12-09 21:48:11 Connection established to target: Target { sockaddrs: [], host: "127.0.0.1", host_header: "127.0.0.1:9944", _mode: Plain, path_and_query: "/" }
2022-12-09 21:48:11 since no at is provided, setting it to latest finalized head, 0xc9f96f3d2fd40cf4b7de01c768cd4349cff7df65a9a425ec557fcc0896651c2a    
2022-12-09 21:48:11 since no prefix is filtered, the data for all pallets will be downloaded    
2022-12-09 21:48:11 scraping key-pairs from remote at block height 0xc9f96f3d2fd40cf4b7de01c768cd4349cff7df65a9a425ec557fcc0896651c2a    
2022-12-09 21:48:11 Querying a total of 365 keys from prefix , splitting among 20 threads, 19 keys per thread    
2022-12-09 21:48:11 adding data for hashed prefix: , took 0s    
2022-12-09 21:48:11 adding data for hashed key: 3a636f6465    
2022-12-09 21:48:11 adding data for hashed key: 26aa394eea5630e07c48ae0c9558cef7f9cce9c888469bb1a0dceaa129672ef8    
2022-12-09 21:48:11 adding data for hashed key: 26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac    
2022-12-09 21:48:11 Custom("[backend]: frontend dropped; terminate client")
2022-12-09 21:48:11 initialized state externalities with storage root 0x9122dc8e4a24afbe80612625cba6685ddd00f4f27fabb57a7c64ccdb2c518e48 and state_version V1    
2022-12-09 21:48:11 Connection established to target: Target { sockaddrs: [], host: "127.0.0.1", host_header: "127.0.0.1:9944", _mode: Plain, path_and_query: "/" }
2022-12-09 21:48:11 Custom("[backend]: frontend dropped; terminate client")
2022-12-09 21:48:11 Producing new empty block at height 332    
2022-12-09 21:48:11 Produced a new block: Header { parent_hash: 0xc9f96f3d2fd40cf4b7de01c768cd4349cff7df65a9a425ec557fcc0896651c2a, number: 332, state_root: 0xd2c79d79468618b57ea330c5c5328b9f9862b3bc695040f25cb4828e26f470de, extrinsics_root: 0xcbcbbd0025d0ff19ce642b0a7a3bd3f48a63cb5140dfa58e169b77ac979ce9dd, digest: Digest { logs: [DigestItem::PreRuntime([97, 117, 114, 97], [129, 154, 152, 16, 0, 0, 0, 0])] } }    
2022-12-09 21:48:11 Executed the new block    
2022-12-09 21:48:11 Producing new empty block at height 333    
2022-12-09 21:48:12 Produced a new block: Header { parent_hash: 0x7a8bbca9d11633bd05897b7b0d75c4d82a62b7e09b4f8e3dd46236e7d03a1e8f, number: 333, state_root: 0xd228a9d592794c4f0718c458d40eb83d4ec4c1466cfe5093d98d67cd98cb69b4, extrinsics_root: 0xd943453d4f8c4f364fa680aded640ad29dab8c2b4283879795cc9f64969f3877, digest: Digest { logs: [DigestItem::PreRuntime([97, 117, 114, 97], [130, 154, 152, 16, 0, 0, 0, 0])] } }    
2022-12-09 21:48:12 Executed the new block   

cc: @kianenigma

polkadot companion: paritytech/polkadot#6567
cumulus companion: paritytech/cumulus#2100

Polkadot address: 15fdtPm7jNN9VZk5LokKcmBVfmYZkCXdjPpaPVL2w7WgCgRY

[kianenigma]

Once revived, I should remember to delete #12537 :)

[kianenigma]

Notice, that we don't respect the standard blocktime / slot-duration, but instead we are proceeding immediately with subsequent block.

Won't the runtime panic if we don't set the timestamp correctly?

[pmikolajczyk41]

Notice, that we don't respect the standard blocktime / slot-duration, but instead we are proceeding immediately with subsequent block.

Won't the runtime panic if we don't set the timestamp correctly?

Pallet timestamp only requires that subsequent timestamps are strictly increasing. However, pallet aura has a problematic hook:

	fn on_timestamp_set(moment: T::Moment) {
		let slot_duration = Self::slot_duration();
		assert!(!slot_duration.is_zero(), "Aura slot duration cannot be zero.");

		let timestamp_slot = moment / slot_duration;
		let timestamp_slot = Slot::from(timestamp_slot.saturated_into::<u64>());

		assert!(
			CurrentSlot::<T>::get() == timestamp_slot,
			"Timestamp slot must match `CurrentSlot`"
		);
	}

which will fail if the timestamp is inconsistent with slot duration. Therefore, in fast-forward we cannot use actual timestamps.

[ggwpez]

Therefore, in fast-forward we cannot use actual timestamps

I dont quite understand the problem. Why can you not multiple the block number with the block time? Like 6s normally?
The timestamp will obviously be in the future, but that is correct since the block is also in the future.

[pmikolajczyk41]

I dont quite understand the problem. Why can you not multiple the block number with the block time? Like 6s normally?
The timestamp will obviously be in the future, but that is correct since the block is also in the future.

Effectively I'm doing this here, right?:

let timestamp_idp = match maybe_prev_info {
    Some((inherent_data, _)) => sp_timestamp::InherentDataProvider::new(
	inherent_data.timestamp_inherent_data().unwrap().unwrap() +
	    BLOCKTIME_MILLIS,
    ),
    None => sp_timestamp::InherentDataProvider::from_system_time(),
};

And this must be done (calculated) on the client side (i.e. outside try-runtime) - my intention was to make try-runtime agnostic to inherents and pre-runtime digests. Thus it's client, that must provide this data via such callback.

Another thing is that I have BLOCKTIME_MILLIS hardcoded (as const BLOCKTIME_MILLIS: u64 = 2 * 3_000;), which must be aligned to the chain. I cannot read anything from runtime configuration at this point (since runtime resolution is done on the try-runtime site). But imho this is acceptable.

[kianenigma]

Slightly unfortunate that chains implementing try-runtime have to add this snippet. Also, this kinda breaks #12975, any alternatives?

[pmikolajczyk41]

I definitely agree that this complicates things, but I'm afraid that as long as we want to remain chain-agnostic, similar approach must be taken.

Firstly, please notice that all other subcommands of try-runtime were oblivious to the chain they were working with (as long it was exposing the proper runtime API calls). However, when it comes to block production (like in fast-forward), we are facing the process that is often unique to every chain (here: inherents and digests). I suppose that majority of the chains in the ecosystem will have exactly the same configuration here (timestamp + aura), but this won't be true everywhere.

It can be also anticipated, that sooner or later we might enrich try-runtime with new functionalities that will have some chain-dependent components. This is why in [12975](#12975 I proposed splitting try-runtime tool into a part that contains pure (chain-agnostic) logic and a part where customization takes place. In this sense, I don't see using user snippet as breaking.

It seems like this is a misunderstanding. Everything in the try-runtime-cli crate is also "client side code", and has full access to system time in order to build the inherent and all. Only things within state_machine_call are actually runtime calls.

Yes, the misunderstanding comes from my poor wording - by 'client side code' I was referring to the CLI part, i.e. the code that handles user's input to the try-runtime tool.

[kianenigma]

Okay, two thoughts for now:

1. it might help both of us find an alternative solution if you actually create a companion PR for this PR to Aleph-node, Polkadot, or some other client that actually requires a different setup. This will prove your point that we NEED this customization, but might shed some light into how else it could be done.
2. If not useful, then I am happy to merge this and we shall find a better solution, even if it exists, in try-runtime-cli: path to being an independent CLI #12975. We should just document this label properly so that other parachain teams can easily follow-along and update their client integration.

[pmikolajczyk41]

To find a client with different setup, I had to just complete this PR for node-cli (i.e. substrate node) - there we have Babe instead of Aura, uncles and storage proof inherents. Apart from this, you can check:

• Polkadot with its polkadot_node_core_parachains_inherent (https://github.com/paritytech/polkadot/blob/master/node/service/src/lib.rs#L1155), or
• Subspace with its sp_consensus_subspace::inherents (https://github.com/subspace/subspace/blob/main/crates/subspace-service/src/lib.rs#L282)

I don't want to push my approach, so if you see any alternative, I'm eager to implement it. Otherwise, if we stick to this way of customization (at least for now), I will create companion PR for Polkadot.

[bkchr]

I did not really check the code in here, but most of the stuff you are doing here is already provided for manual-seal:

substrate/client/consensus/manual-seal/src/consensus/aura.rs

Lines 75 to 90 in 4e38342

	fn create_digest(
	&self,
	_parent: &B::Header,
	inherents: &InherentData,
	) -> Result<Digest, Error> {
	let timestamp =
	inherents.timestamp_inherent_data()?.expect("Timestamp is always present; qed");
	// we always calculate the new slot number based on the current time-stamp and the slot
	// duration.
	let digest_item = <DigestItem as CompatibleDigestItem<AuthoritySignature>>::aura_pre_digest(
	Slot::from_timestamp(timestamp, self.slot_duration),
	);
	Ok(Digest { logs: vec![digest_item] })
	}

In general, manual-seal already provides functionality to "fast-forward" as well.

[pmikolajczyk41]

while semantically create_digest is perfectly applicable in this PR, it requires a type that supports AuxStore + ProvideRuntimeApi<B> + UsageProvider<B> for Aura or AuxStore + HeaderBackend<B> + HeaderMetadata<B, Error = sp_blockchain::Error> + UsageProvider<B> + ProvideRuntimeApi<B> for Babe, which goes far out of scope what we use in either try-runtime itself or even commands modules

[bkchr]

I mean the logic should be shared. I also need to rewrite sp-api that you can use it here instead of calling functions by their name.

[pmikolajczyk41]

Okay, apologies for misunderstanding. Do you think that I should make some steps towards this logic sharing in this PR? If so, then what would be the good place to put common things in?

[bkchr]

Maybe for now you just copy the relevant code to try-build? And make it reusable for people? So that they don't always need to implement the logic?

[pmikolajczyk41]

I couldn't find any try-build, so I assumed that you meant try-runtime... I moved both trait and two most common (I guess) impementations to a new module within try-runtime. It cost a few sp-* dependencies, but indeed, ease of usage is way higher. Please let me know if this is what you suggested.

[kianenigma]

Most of the code looks very good, just worried about https://github.com/paritytech/substrate/pull/12896/files#r1063305503.

[kianenigma]

And this must be done (calculated) on the client side (i.e. outside try-runtime) - my intention was to make try-runtime agnostic to inherents and pre-runtime digests. Thus it's client, that must provide this data via such callback.

It seems like this is a misunderstanding. Everything in the try-runtime-cli crate is also "client side code", and has full access to system time in order to build the inherent and all. Only things within state_machine_call are actually runtime calls.

[paritytech-processbot]

Error: "Check reviews" status is not passing for paritytech/cumulus#2100

[kianenigma]

bot merge

[Polkadot-Forum]

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

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-39/2277/1