Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Refactor pallet recovery to fungibles #1807
Refactor pallet recovery to fungibles #1807
Changes from 8 commits
1060325
880f041
3ba2634
f693be3
964c2cf
b9717be
6d02f14
ef5995a
7fed5fb
798a69b
7f3898d
3a00835
72707cf
0f22f2d
658971e
c95a0c9
729781d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
We need to
set_balance
in benchmarking, soMutate
is neededThere 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.
It is possible to set custom where bounds for benchmarks, like this.
So it should be possible to add a
<T as Config>::Currency: Mutate
there.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.
I am thinking about how to do a lazy migration here. Maybe you can require the
Currency
config trait to beCurrency + MutateHold
so that it requires currency and fungibles.Then in this function you can first try to use the new
transfer_on_hold
function, but if that does not work then fallback to the oldrepatriate_reserved
. Do you think this could work?Otherwise the MBMs are still in review and will need audit, so its a bit out.
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.
I am not a big fan of lazy migrations here, cause that will generate some "garbage" code that will live there forever. It seems more like a patch to me than a real solution. It's been a while already since we wanted the traits to be migrated that I would rather wait for the MBM to be ready and apply a proper solution