Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix refund benchmark for pallet_assets #14561

Merged
merged 5 commits into from
Jul 12, 2023
Merged

Fix refund benchmark for pallet_assets #14561

merged 5 commits into from
Jul 12, 2023

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Jul 12, 2023

No description provided.

@bkontur bkontur added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 12, 2023
@bkontur bkontur requested review from a team July 12, 2023 11:39
@bkontur
Copy link
Contributor Author

bkontur commented Jul 12, 2023

bot bench $ pallet dev pallet-assets

@command-bot
Copy link

command-bot bot commented Jul 12, 2023 •
edited
Loading

@bkontur https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3171809 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-assets. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 4-8c395116-c243-4f28-b2ad-d37113d466e6 to cancel this command or bot cancel to cancel all commands in this pull request.

joepetrowski
joepetrowski approved these changes Jul 12, 2023
View reviewed changes
frame/assets/src/benchmarking.rs Outdated Show resolved Hide resolved
@bkontur @joepetrowski
Update frame/assets/src/benchmarking.rs
c0f98d5 
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
@bkontur
Copy link
Contributor Author

bkontur commented Jul 12, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

bkchr
bkchr reviewed Jul 12, 2023
View reviewed changes
frame/assets/src/benchmarking.rs
@@ -514,8 +514,7 @@ benchmarks_instance_pallet! {
SystemOrigin::Signed(new_account.clone()).into(),
asset_id
).is_ok());
// `touch` should reserve some balance of the caller...
assert!(!T::Currency::reserved_balance(&new_account).is_zero());
assert_eq!(T::Currency::reserved_balance(&new_account), T::AssetAccountDeposit::get());
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can re-add the comment? Basically this pr doesn't "fix" anything, it just ensures that the right amount is reserved or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment re-added,
yeah, PR fixes just benchmark and right amount comparison, which didnt work for AssetAccountDeposit = 0

@command-bot
Copy link

command-bot bot commented Jul 12, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet-assets has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3171809 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3171809/artifacts/download.

@bkontur bkontur added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Jul 12, 2023
@bkontur
Copy link
Contributor Author

bkontur commented Jul 12, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 7b5a4d2 into master Jul 12, 2023
4 checks passed
@paritytech-processbot paritytech-processbot bot deleted the bko-assets-benchmarks branch July 12, 2023 13:41
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@bkontur @joepetrowski @nathanwhit
Fix refund benchmark for pallet_assets (paritytech#14561)
da1b8d6 
* Fix refund benchmark for pallet_assets

* Update frame/assets/src/benchmarking.rs

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Re-added docs

* Another "fix"

---------

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Ank4n pushed a commit that referenced this pull request Jul 22, 2023
@bkontur @joepetrowski @Ank4n
Fix refund benchmark for pallet_assets (#14561)
e3efcc3 
* Fix refund benchmark for pallet_assets

* Update frame/assets/src/benchmarking.rs

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Re-added docs

* Another "fix"

---------

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr left review comments

@ggwpez ggwpez ggwpez approved these changes

@joepetrowski joepetrowski joepetrowski approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bkontur @bkchr @ggwpez @joepetrowski