Benchmark Overhead lacks support for pre-runtime digests

[notlesh]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Description of bug

Benchmark Overhead supports inherents but not pre-runtime digests. Here is some additional context, copied from my substate stack echange Q:

Our runtime uses an inherent which itself requires a pre-runtime digest to be present. I don't believe this is supported; from what I can tell, the benchmarking only handles the inherents provided by the implementation:

https://github.com/paritytech/substrate/blob/master/utils/frame/benchmarking-cli/src/extrinsic/bench.rs#L130

Specifically, we use author-inherent from Nimbus: https://github.com/PureStake/nimbus/blob/0e5702105941baba0e08b343ceaa217192191b73/pallets/author-inherent/src/lib.rs

Which follows this workflow:

• on_initialize() must be called with a certain pre_runtime_digest present. This is copied into a storage item.
• The kickoff_authorship_validation inherent must then be called, which then expects this storage item to be present.
• Without the ability to inject this pre-runtime digest, benchmark overhead is triggering a failure in this inherent because on_initialize failed to find the injected digest into pallet storage.

Steps to reproduce

No response

[shawntabrizi]

cc @ggwpez

[ggwpez]

Okay then why is that pre-runtime digest not in there? Should the Nimbus consensus not add it?
Or is it normally added after building the block?

We can just push it onto the list of digests for sure, but I dont know if that is the right way of doing it:

// utils/frame/benchmarking-cli/src/extrinsic/bench.rs
pub fn bench_block(&self) -> Result<Stats> {
	let (block, _) = self.build_block(None)?;

	// NEW
	let (mut head, exts) = block.deconstruct();
 	head.digest_mut().logs.push(<YOUR DIGEST>);
	let block = Block::new(head, exts);	

	let record = self.measure_block(&block)?;
	Stats::new(&record)
}

[notlesh]

Nimbus injects the digest in its produce_candidate(). Specifically, seal_header() produces and returns a DigestItem:

https://github.com/PureStake/nimbus/blob/ec08bbeebfc07c790013434bb437376789107d35/nimbus-consensus/src/lib.rs#L268

which is injected into BlockImportParams inside produce_candidate():

https://github.com/PureStake/nimbus/blob/ec08bbeebfc07c790013434bb437376789107d35/nimbus-consensus/src/lib.rs#L417

[notlesh]

I can give your suggestion a shot to see if that is a viable solution. A good starting point would be to hack up utils/frame/benchmarking-cli/src/extrinsic/bench.rs to inject a hard-coded digest and see if it helps.

If this works, would it make sense to add an additional field to OverheadCmd::run() to provide digests?

[bkchr]

@notlesh but the seal digest is a post runtime digest. Why do you need this for execute_block?

[bkchr]

With paritytech/polkadot-sdk#63 I would get that we need the seal :D

[notlesh]

A potential (and quite trivial) fix: #12159.

Note that I tested a similar concept against moonbeam and substrate v0.9.26 but there has been significant refactor of these parts of benchmarking since. I do believe this will work equivalently.

Also note (and maybe this is relevant to your question, @bkchr ) that I could not push the digest items after build_block() as @ggwpez suggested because it is when the inherents are push()ed that the original problem occurs.