Add storage size component to weights

[KiChjang]

This PR adds the long-awaited storage size weight component to the Weight struct in sp-weights.

Part of #12176.

[KiChjang]

What I just realized is that we don't need a CompactAs implementation when the fields of the Weight struct are compact by default. In that case, when Weight appears in a parameter of an extrinsic, no #[pallet::compact] is necessary at all.

[KiChjang]

Okay so now CI is failing on runtime integrity tests, because the extrinsic weights have a proof size of 0, hence they are not strictly greater than 0, which is what we require:

// Make sure that if total is set it's greater than base_block &&
// base_for_class
error_assert!(
	(max_for_class.all_gt(self.base_block) && max_for_class.all_gt(base_for_class))
	|| max_for_class == Weight::zero(),
	&mut error,
	"[{:?}] {:?} (total) has to be greater than {:?} (base block) & {:?} (base extrinsic)",
	class, max_for_class, self.base_block, base_for_class,
);
// Max extrinsic can't be greater than max_for_class.
error_assert!(
	weights
		.max_extrinsic
		.unwrap_or(Weight::zero())
		.all_lte(max_for_class.saturating_sub(base_for_class)),
	&mut error,
	"[{:?}] {:?} (max_extrinsic) can't be greater than {:?} (max for class)",
	class,
	weights.max_extrinsic,
	max_for_class.saturating_sub(base_for_class),
);
// Max extrinsic should not be 0
error_assert!(
	weights.max_extrinsic.unwrap_or_else(Weight::max_value).all_gt(Weight::zero()),
	&mut error,
	"[{:?}] {:?} (max_extrinsic) must not be 0. Check base cost and average initialization cost.",
	class, weights.max_extrinsic,
);
// Make sure that if reserved is set it's greater than base_for_class.
error_assert!(
	reserved.all_gt(base_for_class) || reserved == Weight::zero(),
	&mut error,
	"[{:?}] {:?} (reserved) has to be greater than {:?} (base extrinsic) if set",
	class,
	reserved,
	base_for_class,
);
// Make sure max block is greater than max_total if it's set.
error_assert!(
	self.max_block.all_gte(weights.max_total.unwrap_or(Weight::zero())),
	&mut error,
	"[{:?}] {:?} (max block) has to be greater than {:?} (max for class)",
	class,
	self.max_block,
	weights.max_total,
);
// Make sure we can fit at least one extrinsic.
error_assert!(
	self.max_block.all_gt(base_for_class + self.base_block),
	&mut error,
	"[{:?}] {:?} (max block) must fit at least one extrinsic {:?} (base weight)",
	class,
	self.max_block,
	base_for_class + self.base_block,
);

Should these all be any_* instead of all_*? Or should we actually start filling up the proof size weight for each extrinsic?

[ggwpez]

Or should we actually start filling up the proof size weight for each extrinsic?

I think we should, https://github.com/paritytech/polkadot-sdk/issues/384. But it could legitimately be that the base_extrinsic.pov() is zero, since we only read/write overlayed storage. So for now just short-circuit them with any_gt and add it as TODO to the issue?

[KiChjang]

There's a unit test that I keep failing right now, and it seems to be related to how the storage root has changed after executing a test extrinsic. Here's the code for the test:

fn block_import_works() {
	block_import_works_inner(
		new_test_ext_v0(1),
		hex!("1039e1a4bd0cf5deefe65f313577e70169c41c7773d6acf31ca8d671397559f5").into(),
	);
	block_import_works_inner(
		new_test_ext(1),
		hex!("75e7d8f360d375bbe91bcf8019c01ab6362448b4a89e3b329717eb9d910340e5").into(),
	);
}
fn block_import_works_inner(mut ext: sp_io::TestExternalities, state_root: H256) {
	ext.execute_with(|| {
		Executive::execute_block(Block {
			header: Header {
				parent_hash: [69u8; 32].into(),
				number: 1,
				state_root,
				extrinsics_root: hex!(
					"03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314"
				)
				.into(),
				digest: Digest { logs: vec![] },
			},
			extrinsics: vec![],
		});
	});
}

Am I correct in thinking that this is actually functioning as expected due to the addition of a new field in Weight, and we should instead change the expected state root for this test?

[cheme]

Am I correct in thinking that this is actually functioning as expected due to the addition of a new field in Weight, and we should instead change the expected state root for this test?

Yes looks like weights are stored in state on initialization. Changing the roots should be fine (

	#[test]
	fn block_import_works() {
		block_import_works_inner(
			new_test_ext_v0(1),
			hex!("0d786e24c1f9e6ce237806a22c005bbbc7dee4edd6692b6c5442843d164392de").into(),
		);
		block_import_works_inner(
			new_test_ext(1),
			hex!("348485a4ab856467b440167e45f99b491385e8528e09b0e51f85f814a3021c93").into(),
		);
	}
``` I think)

[KiChjang]

bot merge

[nazar-pc]

Hm... this is really annoying for chains that don't have PoV, now I need to learn the concept which is not applicable to the chain whatsoever. Is there a chance to make it optional somehow?

[KiChjang]

You can do so by setting the max proof size to u64::MAX. The docs somewhere should mention this, but unsure where exactly.

[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/1026/2