PoV Reclaim Runtime Side

[skunert]

Runtime side for PoV Reclaim

Implementation Overview

• Hostfunction to fetch the storage proof size has been added to the PVF. It uses the size tracking recorder that was introduced in my previous PR.
• Mechanisms to use the reclaim HostFunction have been introduced.
  • 1. A SignedExtension that checks the node-reported proof size before and after application of an extrinsic. Then it reclaims the difference.
  • 2. A manual helper to make reclaiming easier when manual interaction is required, for example in on_idle or other hooks.
• In order to utilize the manual reclaiming, I modified WeightMeter to support the reduction of consumed weight, at least for storage proof size.

How to use

To enable the general functionality for a parachain:

1. Add the SignedExtension to your parachain runtime.
2. Provide the HostFunction to the node
3. Enable proof recording during block import

TODO

• PRDoc

[dmitry-markin]

Nit: is it on purpose that one of the variables called proof_weight, and another proof_size? I can see it as convention that weight refers to benchmarked "weight" and size refers to actually consumed proof size, but below in post_dispatch() there is a variable called consumed_weight.

[skunert]

I tried to use proof_size as the absolute number that is reported by the host function. Weight as the amount consumed or reported by the runtime facilities.

But I should double check if that makes sense and usage is consistent.

[dmitry-markin]

Looks good to me, but it's a good idea for somebody else to have a closer look at validate_block and that the host function is registered everywhere where needed, as I lack the needed knowledge to check this.

[davxy]

Mostly nitpicks

[davxy]

Dumb question: do we need access to the recorder in this closure as well?
Shouldn't we need access to the recorder only in the scope where we call execute_block only?
(i.e. in the call below)
Or we need it to check_inherents as well?

Another dumb question: why we need two calls to run_with_externalities_and_recorder?
Can't we unify the closures in a single one?

[bkchr]

Yeah, we should not need it.

[bkchr]

Let me phrase it better, we should NOT use the recorder.

[skunert]

I mean we need to supply the recorder everywhere where host functions are potentially being called. And since the CheckInherent type is user supplied, host functions could be called there.

The two closures are separate because we want to start with a fresh overlay and externalities. But this is a good point, I now reset the recorder too between these two code sections.

[davxy]

If we propose this host function in the template. Maybe we should also need to add the StorageWeigthReclaim signed extension to the runtime signed extensions tuple? (i.e. here)

Otherwise what is its usage?

IIUC the StorageWeightReclaimer alone can't work correctly if we don't also return the extra weight to frame_system::BlockWeight ( by means of this signed extension hooks right?
Or I'm missing some other means of doing this? E.g. the user directly altering frame_system::BlockWeight in some other hook

[skunert]

IIUC the StorageWeightReclaimer alone can't work correctly if we don't also return the extra weight to frame_system::BlockWeight ( by means of this signed extension hooks right?
Or I'm missing some other means of doing this? E.g. the user directly altering frame_system::BlockWeight in some other hook.

The StorageWeightReclaimer is a separate mechanism from the SignedExtension. It is designed to be used inside the on_idle hook, where weight management happens manually via WeightMeter. Users need to check whether the operation they plan fits into the remaining weight. In the end, on_idle will return the consumed weight and it will be accounted for after the hook finished here:

polkadot-sdk/substrate/frame/executive/src/lib.rs

Lines 607 to 614 in 150a360

	let used_weight = <AllPalletsWithSystem as OnIdle<BlockNumberFor<System>>>::on_idle(
	block_number,
	remaining_weight,
	);
	<frame_system::Pallet<System>>::register_extra_weight_unchecked(
	used_weight,
	DispatchClass::Mandatory,
	);

This means that the weightreclaimer should only modify the value in the weightmeter, not the storage value itself.

If we propose this host function in the template. Maybe we should also need to add the StorageWeigthReclaim signed extension to the runtime signed extensions tuple? (i.e. here)

Otherwise what is its usage?

This is a good point, I will add it.

[michalkucharczyk]

Code/concept looking good to me. Just a random thought: did you consider adding signed extensions to one of test runtimes (e.g. substrate-test-runtime or cumulus-test-runtime) to add some client-level testing involving more components?