Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: reading state charging weight #127

Closed
Daanvdplas opened this issue Jul 25, 2024 · 3 comments
Closed

refactor: reading state charging weight #127

Daanvdplas opened this issue Jul 25, 2024 · 3 comments

Comments

@Daanvdplas
Copy link
Collaborator

Daanvdplas commented Jul 25, 2024

Read state weight implementation with benchmarks and test coverage (#132 (comment)).

Including proof size (SE).

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)).

@evilrobot-01
Copy link
Collaborator

evilrobot-01 commented Jul 26, 2024

(Outdated)

#122 (comment)

@Daanvdplas Daanvdplas added the question Further information is requested label Jul 26, 2024
@evilrobot-01
Copy link
Collaborator

evilrobot-01 commented Jul 27, 2024

(Outdated)

https://github.com/r0gue-io/pop-cli/blob/b4874220ca58ebb38453c7ab83670f7e0711c674/crates/pop-parachains/src/templates.rs#L168 shows an example of attaching a custom property to an enum using strum, with https://github.com/r0gue-io/pop-cli/blob/b4874220ca58ebb38453c7ab83670f7e0711c674/crates/pop-parachains/src/templates.rs#L194 showing how it can then be obtained. This could be reimplemented as a custom enum property which is statically defined on the enum, and can be used to charge the defined amount of reads/weight BEFORE execution of the read. Your suggestion of attaching it to the output of read_state wont work as charging after a read opens an attack vector.

Yes, a match may be required, but so what. The important thing is accurate weight charging before a read. This can be defined within an additional read_weight function, which could even potentially be defined as a trait with a default implementation. Further improvements could refactor any match out completely.

The point is making it easy to specify the weight/number of reads per query by decorating the enum variant so it is defined in a single place. Returning the value should then be straightforward.

@Daanvdplas Daanvdplas added high priority runtime pallet and removed question Further information is requested high priority labels Jul 30, 2024
@Daanvdplas Daanvdplas changed the title refactor: reading state charging weight fix: reading state charging weight Aug 16, 2024
@Daanvdplas Daanvdplas changed the title fix: reading state charging weight refactor: reading state charging weight Aug 16, 2024
@Daanvdplas Daanvdplas reopened this Sep 11, 2024
@Daanvdplas
Copy link
Collaborator Author

Closed by #284

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants