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

Transition to dynamic PoV metering #992

Closed
pgherveou opened this issue Aug 24, 2023 · 3 comments
Closed

Transition to dynamic PoV metering #992

pgherveou opened this issue Aug 24, 2023 · 3 comments
Assignees

Comments

@pgherveou
Copy link
Contributor

This issue is a follow up from this forum conversation: Eliminating pre-dispatch weight.

Our objective is to transition from PoV pre-dispatch weight to dynamic PoV metering. This will eliminate the need of benchmarking the PoV weight component, bringing us closer to liberating developers from writing benchmarks (see on-going effort to move to dynamic ref_time metering here and mentioned as well in the post above)

Benefits

  • Enhanced Developer Productivity: Benchmarks are error-prone and take a toll on developer productivity.
  • Reduce PoV weight: Benchmarks can inflate gas fees since they over-count repeated key accesses across multiple transactions.

Approach

1. Client approach

Using this method, we'll design a host function that outputs the PoV size.
Fetching this data from the collator is straightforward since it owns a ProofRecorder that can be used for querying such information.

For validators, the mechanism isn't as straightforward. The db is not available, So we would need to come up with an implementation, that construct another recorder from the StorageProof to track read accesses. Such an implementation would also need to return the exact same values as the one recorded by the collator.

Another downside of this approach is that any breaking change could cause consensus issues, as two conflicting clients would disagree on the weight value.

2. Runtime approach

With a runtime approach, we would need to keep track of all storages read in a global Hashset, to not double count repeated access to the same key. This would work along a global counter that keep track of the current PoV value of the transaction.

Notes:

  • Since the runtime does not have access to the exact Trie footprint of a given read, it will approximate by summing key & value byte size.
  • A refined strategy might also adjust each transaction's PoV size using a pro-rated value derived from the block PoV size. This would ensure that a transaction's index (and hence the probability of accessing a previously accessed key) doesn't influence the transaction PoV size.
  • Until recently using global values was not a viable option, since these globals would outlive the block production on a native runtime. Now that new clients use the wasm runtime exclusively this should not be an issue anymore. See comments on this PR Add RuntimeCell and RuntimeRefCell
@pgherveou pgherveou self-assigned this Aug 24, 2023
@bkchr
Copy link
Member

bkchr commented Aug 24, 2023

And what happens if you use more pov size than there is left for the block?
There are reasons we account for the weight before executing an extrinsic, to ensure that transactions are fitting into block before trying them.

#209 there is also this issue that will improve the precision of the used pov size, but only after the transaction was applied.

@athei
Copy link
Member

athei commented Aug 24, 2023

And what happens if you use more pov size than there is left for the block?
There are reasons we account for the weight before executing an extrinsic, to ensure that transactions are fitting into block before trying them.

"systematic over-estimation": https://forum.polkadot.network/t/eliminating-pre-dispatch-weight/400/7?u=alex

#209 there is also this issue that will improve the precision of the used pov size, but only after the transaction was applied.

Yeah I wasn't able to find this issue. I think this is more a less a duplicate of that.

@pgherveou
Copy link
Contributor Author

pgherveou commented Aug 24, 2023

closing as duplicate then, will move the conversation to this issue

claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
* Upgrade to Solidity 0.8.22
* Update foundry to support Solidity 0.8.22
* Update bindings for Gateway.sol
* Smokestests now require serde
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants