-
Notifications
You must be signed in to change notification settings - Fork 36
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
Benchmarking RMRK-core #219
Benchmarking RMRK-core #219
Conversation
aac5225
to
8fb369e
Compare
f7e026e
to
200cd5d
Compare
The |
5a8b33b
to
7abb495
Compare
0185f7c
to
14563ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to remove the commented out code. I'll run this in my environment. Do the integration tests pass, as well?
Another thought is on downstream implementations. These chains will need to be notified so they do the appropriate storage migrations for the changes to Storage.
Other than these items, it LGTM. Nice work Maario |
87bfba7
to
9c68ab9
Compare
rebase typo merge conflict resolved use T::CollectionId instead of u32 create collection with the given id updates to traits update market and equip update benchmark tests generated and used benchmaked weights fixing tests cleanup before review more cleanup
removing commented-out code
9c68ab9
to
9411414
Compare
…mrk-substrate into feature/pallet-benchmarks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you.
Was able to re-generate weights and approach is more clear now.
Will proceed with the merge and create separate PR for extra calls.
This PR introduces benchmark test for
pallet-rmrk-core
The major change in the most of the files is that following construct could not be used in the
benchmark!
macropallet-uniques
associated typeCollectionId
does not supportFrom<u32>
and it is therefore used asT::CollectionId
replacing the use of locally definedCollectionId
.pallet-uniques
associated typeItemId
does not supportFrom<u32>
and it is therefore used asT::ItemId
replacing the use of locally definedNftId
.ExtBuilder::default()
in order to accommodate benchmark unit tests.Another big change is the removal of
CollectionIndex
storage item and allowing caller to give the collection_id as an argument. It is not likely that rmrk pallets will be sole user of the uniques ids. Second reason for removingCollectionIndex
is due to change ofcollection_id
type fromu32
to Unique's associated typeT::CollectionId,
After this change try_mutate will not work over CollectionIndexCollectionIndex
last_collection_idx()
see aboveNext steps