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

Consider tracking candidate enactment weight in a weight only function #896

Open
emostov opened this issue Dec 11, 2021 · 0 comments
Open

Comments

@emostov
Copy link

emostov commented Dec 11, 2021

Right now the path the weight gets bubbled up is enact_candidate -> update_pending_availability_and_get_freed_cores -> process_bitfields. In enact_candidate we go through and track the actual weights consumed (opposed to the worst case estimates we use prior).

With the current function having a separate weight only function would be tricky and require some duplicate logic. Basically we would need to duplicate some of the logic that executes here: https://github.com/paritytech/polkadot/blob/888162259348c7fa3e9ad7454e8056235565efa6/runtime/parachains/src/inclusion/mod.rs#L370-L402 . At the moment we are actually modifying storage right there, so we essentially would need to first get the weight of enacting candidates without side effects and then call update_pending_availability_and_get_freed_cores, which would duplicate the filtering logic but actually enact the candidate and write to storage

In an enact_candidate_weight function we would need to take the weight calculations done within these calls https://github.com/paritytech/polkadot/blob/888162259348c7fa3e9ad7454e8056235565efa6/runtime/parachains/src/inclusion/mod.rs#L748-L772 and abstract them to their own weight only functions

While I see the general design merit of having the weight tracked by weight only functions - in this particular case there is a lot of branching logic to determine how much weight actually gets used and duplicating much of that logic in the weight only functions since harder to keep track of then tracking the weight as the actual execution takes place.

Originally posted by @emostov in paritytech/polkadot#4172 (comment)

@Sophia-Gold Sophia-Gold transferred this issue from paritytech/polkadot Aug 24, 2023
claravanstaden added a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
* Improve contracts

* More improvements to contracts

* Fix parachain build in proxy-contracts branch (paritytech#889)

* Upgrade ssz_rs crate. (paritytech#880)

* Upgrade ssz_rs crate.

* Upgrade ssz_rs crate.

---------

Co-authored-by: claravanstaden <Cats 4 life!>

* Bump nixpkgs to use its geth package (paritytech#885)

* Bump nixpkgs to use its geth package

* Use source config in PolkadotListener

* Match field order in struct def

* Move info log to relay creation

* Remove unused channel

* Whitespace

* Add context to errors

* Add logs

* Bump node & pnpm in workflow

* Rename locals

* Remove unused variable

* Add Troubleshooting README section

* Fix up .envrc-example files

* Add note about pure shells

* Update cumulus submodule (paritytech#886)

* Inbound queue benchmarks (paritytech#876)

* Start with inbound channel benchmarks.

* Add method to set execution header storage for benchmark test.

* Working on benchmarks

* Basic working version

* Cleanup

* Removes cleanup.

* Adds some comments for Alistair.

* Adds branch name.

* Makes note

* Test transactions

* Cleaning up beacon client deps.

* Clean up comments.

* Tests cleanup.

* Fixes non-benchmark test runs.

* Cleanup.

* Update fixtures and generates benchmarks.

* Revert relayer logs.

* Cleanup BenchmarkHelper impl and inbound queue dependencies.

* fmt

* Cleanup imports.

* Cleanup imports.

* Touch

* Adds weights in inbound queue pallet.

* Fix tests.

* Update cumulus.

---------

Co-authored-by: claravanstaden <Cats 4 life!>

* Fix parachain build

* Move BalanceOf outside of pallet

* remove benchmark for non existing method

* downgrade cargo.lock to match cumulus

* fix benchmarks

---------

Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Co-authored-by: Alistair Singh <alistair.singh7@gmail.com>

* forge install: openzeppelin-contracts

v4.9.2

* sno-472 rebased (paritytech#890)

* Halting & resuming bridge pallets

* Ignore .env

* Remove .env

* Some polish

* Set owner of bridge pallets

* Upgrade ssz_rs crate. (paritytech#880)

* Upgrade ssz_rs crate.

* Upgrade ssz_rs crate.

---------

Co-authored-by: claravanstaden <Cats 4 life!>

* Update cumulus

* Bump nixpkgs to use its geth package (paritytech#885)

* Bump nixpkgs to use its geth package

* Use source config in PolkadotListener

* Match field order in struct def

* Move info log to relay creation

* Remove unused channel

* Whitespace

* Add context to errors

* Add logs

* Bump node & pnpm in workflow

* Rename locals

* Remove unused variable

* Add Troubleshooting README section

* Fix up .envrc-example files

* Add note about pure shells

* Relax RANDAO_COMMIT_DELAY for local setup

* Update cumulus

* Update cumulus submodule (paritytech#886)

* Update cumulus

* Inbound queue benchmarks (paritytech#876)

* Start with inbound channel benchmarks.

* Add method to set execution header storage for benchmark test.

* Working on benchmarks

* Basic working version

* Cleanup

* Removes cleanup.

* Adds some comments for Alistair.

* Adds branch name.

* Makes note

* Test transactions

* Cleaning up beacon client deps.

* Clean up comments.

* Tests cleanup.

* Fixes non-benchmark test runs.

* Cleanup.

* Update fixtures and generates benchmarks.

* Revert relayer logs.

* Cleanup BenchmarkHelper impl and inbound queue dependencies.

* fmt

* Cleanup imports.

* Cleanup imports.

* Touch

* Adds weights in inbound queue pallet.

* Fix tests.

* Update cumulus.

---------

Co-authored-by: claravanstaden <Cats 4 life!>

* Fix test

* Fix parachain build

* Move BalanceOf outside of pallet

* remove benchmark for non existing method

* downgrade cargo.lock to match cumulus

* fix benchmarks

* Clara/sno 552 (paritytech#887)

* Spacing

* Spacing

* Undo typo.

* Minor updates.

* Adds comment about IrrelevantUpdate.

* One more comment.

* Update error name.

---------

Co-authored-by: claravanstaden <Cats 4 life!>

* Halting & resuming bridge pallets (paritytech#883)

* Halting & resuming bridge pallets

* Ignore .env

* Remove .env

* Some polish

* Set owner of bridge pallets

* Update cumulus

* Relax RANDAO_COMMIT_DELAY for local setup

* Update cumulus

* Update cumulus

* Fix test

* Fix Warnings

* Fix test

* Fix build & format

* Fix benchmark test

* Check for duplicate versions of substrate and polkadot (paritytech#891)

* modified pre-commit

* fixes

* testing

* testing

* testing

* fixed tests

* Format

* Some fix

* Update cumulus

* Update cumulus

---------

Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>
Co-authored-by: Alistair Singh <alistair.singh7@gmail.com>

* Proxy contracts Tests fixes (paritytech#892)

* fixed tests

* warnings and imports

* rustfmt

* updated cumulus

* Revert rustfmt (paritytech#896)

* Revert "rustfmt"

This reverts commit b83cec7929cdcc6ac972a2b1cad3a0e1fde81870.

* reverted xcm-builder

* Create agent (paritytech#895)

* base

* removed location conversion

* completed implementation

* remove xcm-builder

* update cumulus

* update cumulus

* use contains_key

* Fix openzeppelin-contracts submodule

---------

Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>

* improve API for sending tokens

* Messy working version.

* Cleanup.

* More cleanup.

* Rollback unnecessary changes.

* Cleanup whitespace.

* Fix tests.

* Fuzz the submit method.

* Revert rebase oopsies.

* More fuzzing.

* More fuzzing.

* Last extrinsic.

* Remove unnecessary dependency.

* Revert readme.

* Fix tests and feature issues.

* Cleanup types.

* More cleanup.

* Call extrinsics directly. Adds readme.

* Adds CI.

* cd to correct dir

* Update CI.

* Correct nightly param.

* Remove runs.

* Own impl for SyncCommittee.

* Remove rng.
Cleans up workflow.

* Last cleanup.

* Update parachain/pallets/ethereum-beacon-client/fuzz/Cargo.toml

Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>

* Adds cargo-fuzz to init script.

* PR comment changes.

* Update parachain/pallets/ethereum-beacon-client/fuzz/src/impls.rs

Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>

* Update rust-toolchain.toml

Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>

* Update .github/workflows/parachain.yml

Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>

* Less runs for shorter Github actions.

---------

Co-authored-by: Vincent Geddes <vincent.geddes@hey.com>
Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>
Co-authored-by: Alistair Singh <alistair.singh7@gmail.com>
Co-authored-by: Ron <yrong1997@gmail.com>
Co-authored-by: claravanstaden <Cats 4 life!>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
bkchr pushed a commit that referenced this issue Apr 10, 2024
* Support moar validators.

* Update weights script.

* Use correct type for benchmarking accounts

* Use package name instead of manifest path

Co-authored-by: Hernando Castano <hernando@hcastano.com>
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

1 participant