r0gue-io / pop-node

Pop Network makes it easy for smart contract developers to use the Power of Polkadot.
The Unlicense
18 stars 4 forks source link

fix: include proof size when charging weight #208

Closed Daanvdplas closed 13 hours ago

Daanvdplas commented 4 weeks ago

The weight function in the extension is incorrect and should include the proof size https://github.com/r0gue-io/pop-node/pull/132#discussion_r1717507035.

Daanvdplas commented 3 weeks ago

Not sure how to do this atm. Created a question on SE.

EDIT: answer above:

The Proof size weight of a storage read is the maximal size of the storage item being read. We have the MaxEncodedLen trait to track this. So you need contextual information about the type that is being read and then use MaxEncodedLen::max_encoded_len() on it. Depending on what the chain extension does, it may or may not be easily possible to know what key is being accessed. Smart contracts are normally uses an optimistic approach and revert on error, whereas FRAME requires to know the worst case effort in advance. These are just fundamentally different computational models. If you can: write a benchmark. The only way to currently write FRAME benchmarks it to use a pallet; for example the minimal pallet or some mocked types just to make the benchmark compile. If you want to know how to charge it exactly, that would be env.charge_weight(Weight::from_parts(0, proof_size)).

Seems that #127 is the way to go then.

Daanvdplas commented 13 hours ago

Closed by #284