[CI] Add pipeline for checking benchmarks #197
Comments
3 tasks
github-merge-queue bot
pushed a commit
to paritytech/polkadot-sdk
that referenced
this issue
Feb 26, 2024
…ts) not relying on ED (#3464) ## Problem During the bumping of the `polkadot-fellows` repository to `polkadot-sdk@1.6.0`, I encountered a situation where the benchmarks `teleport_assets` and `reserve_transfer_assets` in AssetHubKusama started to fail. This issue arose due to a decreased ED balance for AssetHubs introduced [here](https://github.com/polkadot-fellows/runtimes/pull/158/files#diff-80668ff8e793b64f36a9a3ec512df5cbca4ad448c157a5d81abda1b15f35f1daR213), and also because of a [missing CI pipeline](polkadot-fellows/runtimes#197) to check the benchmarks, which went unnoticed. These benchmarks expect the `caller` to have enough: 1. balance to transfer (BTT) 2. balance for paying delivery (BFPD). So the initial balance was calculated as `ED * 100`, which seems reasonable: ``` const ED_MULTIPLIER: u32 = 100; let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());` ``` The problem arises when the price for delivery is 100 times higher than the existential deposit. In other words, when `ED * 100` does not cover `BTT` + `BFPD`. I check AHR/AHW/AHK/AHP and this problem has only AssetHubKusama ``` ED: 3333333 calculated price to parent delivery: 1031666634 (from xcm logs from the benchmark) --- 3333333 * 100 - BTT(3333333) - BFPD(1031666634) = −701666667 ``` which results in the error; ``` 2024-02-23 09:19:42 Unable to charge fee with error Module(ModuleError { index: 31, error: [17, 0, 0, 0], message: Some("FeesNotMet") }) Error: Input("Benchmark pallet_xcm::reserve_transfer_assets failed: FeesNotMet") ``` ## Solution The benchmarks `teleport_assets` and `reserve_transfer_assets` were fixed by removing `ED * 100` and replacing it with `DeliveryHelper` logic, which calculates the (almost real) price for delivery and sets it along with the existential deposit as the initial balance for the account used in the benchmark. ## TODO - [ ] patch for 1.6 - #3466 - [ ] patch for 1.7 - #3465 - [ ] patch for 1.8 - TODO: PR --------- Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
This was referenced Mar 25, 2024
3 tasks
fellowship-merge-bot bot
pushed a commit
that referenced
this issue
Jul 24, 2024
…pec-builder` stuff to `get_preset` (#379) This PR introduces a new CI pipeline for checking and running runtime benchmarks as part of the test pipelines. This means we will now know if any changes break the benchmarks. The pipeline is based on downloading `frame-omni-bencher` (building it in-place is too expensive for every matrix run) and running it with a minimal setup: `--steps 2 --repeat 1`. Another part of this PR splits the default genesis setups from `chain-spec-generator` and moves them to the `sp_genesis_builder::GenesisBuilder::get_preset` runtime API implementation for every runtime, which is required by `frame-omni-bencher`. Closes: #197 Relates to: #298 Relates to: #324 <!-- Remember that you can run `/merge` to enable auto-merge in the PR --> <!-- Remember to modify the changelog. If you don't need to modify it, you can check the following box. Instead, if you have already modified it, simply delete the following line. --> - [X] Does not require a CHANGELOG entry ## Future works When [this issue](paritytech/polkadot-sdk#5083) is fixed, I will rewrite [weight-generation.md](https://github.com/polkadot-fellows/runtimes/blob/main/docs/weight-generation.md). The new version will be significantly simplified, with instructions to simply download `frame-omni-bencher`, build the runtime WASM, and run it—no other steps will be necessary. ``` 2024-07-18 14:45:51 Using the chain spec instead of the runtime to generate the genesis state is deprecated. Please remove the `--chain`/`--dev`/`--local` argument, point `--runtime` to your runtime blob and set `--genesis-builder=runtime`. This warning may become a hard error any time after December 2024. ``` --------- Co-authored-by: Bastian Köcher <git@kchr.de>
|
I had the same problem as you. Are you a member of the org? I have a theory that, when you create a PR linked to an issue, if you are not a org member GitHub will not close the issue (for security reasons?) |
|
I would bet this is some kind of permission issue, because the pr was merged by the bot? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
polkadot-sdkrepository has a short-benchmarks pipeline that helps verify the correctness of benchmarks.Continuation of: #50
Depends on: paritytech/polkadot-sdk#3045
The text was updated successfully, but these errors were encountered: