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
[CU-37t8q47] Added RFC 0013 - Depracate Currency Factory #2489
[CU-37t8q47] Added RFC 0013 - Depracate Currency Factory #2489
Changes from 2 commits
40ffd87
3e0b229
90e0ad6
2da51a2
b2ace78
23092b3
de163c9
89a5b01
7511ace
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.
Are we missing any migration requirement from the existing solution to any new solution?
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.
so in theory we can ditch currency factory and move all to our assets in a way compable on binary level with substrate assets.
also we can throw away our governance-registry imho
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'll either need to request another back-port or upgrade our runtime to enjoy their newer additions
Like you concluded later, I think maintaining a fork would be the easiest way to get all the benefits of Parity's pallet-assets
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.
Is our old substrate version holding us back from using newer features? And if so, what is keeping us on an older substrate version?
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.
In order to get stuff done without mixing all topics together we need to constrain each topic to solving key concerns. There are two key concerns about general asset management AFAIK in our code base.
So to constrain and get stuff done, 2 above is not the subject of this RFC. That is a separate concern.
I also see that frame/pallet-assets does handle both but it does not matter as we can easily plug that in to place as long as we separate concerns.
PS: I also think we need to upgrade substrate ASAP, longer we delay the more complicated it becomes. So possible things to consider for release 3.
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.
This can be done withing each name space as proposed above :)
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.
This is a separate concern as proposed before, we can focus on concern one for release 2. But not a problem to document the solution. I agree in concept though we need to assess how well it supports our requirements such as asset locking at the time of implementation(release 3 or after)
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.
very important - very nice and clear and good documents are here. writing pallet is ony of work.
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 do not need. we just runtime config assets to be premissioned. when we will have permisionless assets they may already have reservation (any way making fork with reservation is easy later)
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.
so really we do not care lack of reservation system
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.
now
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.
+1
I've included this in the solution section
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 can use assets-registry here. it can do next
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.
Is there any particular reason that hold and lock/freeze are implemented differently?
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 should not use assets-manager imho. i used moonbeam just as example of good use of pallet-assets (it is proves assets are good for ETH bridges )
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.
pallet-asset-manager - idea we will have our own
assets-registy
which will be tune for our needs.it is important to check that
pallet-assets
can be wrappedThere 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.
Agree for not using Moonbeam stuff but we can learn from their impl :)