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

Rebenchmark pallet-mantapay #1150

Merged
merged 30 commits into from Jun 28, 2023
Merged

Rebenchmark pallet-mantapay #1150

merged 30 commits into from Jun 28, 2023

Conversation

Dengjianping
Copy link
Contributor

@Dengjianping Dengjianping commented Jun 12, 2023

Description

Closes #1146


Before we can approve this PR for merge, please make sure that all the following items have been checked off:

  • Connected to an issue with discussion and accepted design using zenhub "Connect issue" button below
  • Added one label out of the L- group to this PR
  • Added one or more labels from the A- and C- groups to this PR
  • Explicitly labelled A-calamari and/or A-manta if your changes are meant for/impact either of these (CI depends on it)
  • Re-reviewed Files changed in the Github PR explorer.

Situational Notes:

  • If adding functionality, write unit tests!
  • If importing a new pallet, choose a proper module index for it, and allow it in BaseFilter. Ensure every extrinsic works from front-end. If there's corresponding tool, ensure both work for each other.
  • If needed, update our Javascript/Typescript APIs. These APIs are officially used by exchanges or community developers.
  • If modifying existing runtime storage items, make sure to implement storage migrations for the runtime and test them with try-runtime. This includes migrations inherited from upstream changes, and you can search the diffs for modifications of #[pallet::storage] items to check for any.

Signed-off-by: Dengjianping <djptux@gmail.com>
@Dengjianping Dengjianping added A-weights Area: Issues and PRs related to Substrate Weights A-calamari Area: Issues and PRs related to the Calamari Runtime A-automation Area: Issues and PRs related to Automation A-ci Area: Continuous Integration A-development Area: Development Resources and Upgrades A-manta-pay Area: Issues and PRs related to the MantaPay Pallet L-changed Log: Issues and PRs related to changes labels Jun 12, 2023
@Dengjianping Dengjianping self-assigned this Jun 12, 2023
@Dengjianping Dengjianping added the A-manta Area: Issues and PRs related to the Manta Runtime label Jun 12, 2023
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Copy link
Contributor

@ghzlatarev ghzlatarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, let's run it and see if we get different results. i have a feeling we'll need more coins.

pallets/manta-pay/src/benchmark/mod.rs Outdated Show resolved Hide resolved
pallets/manta-pay/src/benchmark/mod.rs Outdated Show resolved Hide resolved
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Copy link
Contributor

@ferrell-code ferrell-code left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI error in benchmarks, Looks reasonable to me curious what new benchmark result will be.

We also need to do the same for MantaSbt, but that can be different PR

Signed-off-by: Dengjianping <djptux@gmail.com>
@Dengjianping
Copy link
Contributor Author

CI error in benchmarks, Looks reasonable to me curious what new benchmark result will be.

We also need to do the same for MantaSbt, but that can be different PR

#1151

Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Copy link
Contributor

@ghzlatarev ghzlatarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually breaks the mantapay benchmarks units tests

Not sure if there's a way to just disable the unit tests for MantaPay benchmark ?

@Dengjianping
Copy link
Contributor Author

Dengjianping commented Jun 26, 2023

Because rebenchmarking mantapay uses a concrete chain spec, which contains some storage that will affect benchmark code, like there're some asset ids registered already, so we cannot register them in benchmark code.

@ghzlatarev
Copy link
Contributor

Because rebenchmarking mantapay uses a concrete chain spec, which contains some storage that will affect benchmark code, like there're some asset id registered already, so we cannot register in benchmark code.

right can we just exclude MantaPay from our benchmark test in check-tests workflow

Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
ghzlatarev
ghzlatarev previously approved these changes Jun 26, 2023
@github-actions
Copy link

github-actions bot commented Jun 26, 2023

⚠️ Congestion test: 1-day congestion cost (manta-runtime) is NOT above target_daily_congestion_cost_kma

@github-actions
Copy link

github-actions bot commented Jun 26, 2023

⚠️ Congestion test: 1-day congestion cost (calamari-runtime) is NOT above target_daily_congestion_cost_kma

ghzlatarev
ghzlatarev previously approved these changes Jun 28, 2023
@ghzlatarev ghzlatarev self-requested a review June 28, 2023 06:18
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
Signed-off-by: Dengjianping <djptux@gmail.com>
@ghzlatarev ghzlatarev merged commit f0ed595 into manta Jun 28, 2023
34 of 35 checks passed
@ghzlatarev ghzlatarev deleted the rebenchmark-pallet-mantapay branch June 28, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-automation Area: Issues and PRs related to Automation A-calamari Area: Issues and PRs related to the Calamari Runtime A-ci Area: Continuous Integration A-development Area: Development Resources and Upgrades A-manta Area: Issues and PRs related to the Manta Runtime A-manta-pay Area: Issues and PRs related to the MantaPay Pallet A-weights Area: Issues and PRs related to Substrate Weights L-changed Log: Issues and PRs related to changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rebenchmark pallet manta pay
4 participants