-
Notifications
You must be signed in to change notification settings - Fork 14
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
Implement pallet that extends our current treasury pallet with buyout feature #396
Implement pallet that extends our current treasury pallet with buyout feature #396
Conversation
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 left some comments.
Overall I think we should never use '+,-,*,/' operators directly and rather use defensive operations like 'saturating_..' or 'checked_..'.
And I think we can remove the tight coupling to the oracle pallet.
Besides that we also need the logic for the signed extension but I know you are aware of this and it's still a WIP PR after all.
… to blocks, generate weights
…extends-our-current-treasury-pallet-with-buyout-feature
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.
Nice changes 👍 I think it would have been fine to just make the extrinsic such that the list is totally replaced, ie. implementing a setter and ignoring the last state. But it's fine either way 🤷
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 checked the changes since last review and everything looks ready! I just left a comment but I don't think it is a blocker.
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 think it's good (better now with the simpler extrinsic). Just left a comment on the benchmark which is more an FYI, I don't mind if you don't change it @bogdanS98 and a question.
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 think it's ready for merge now 👍
Closes #395.
Overview
This pallet allows users which hold other assets than our native asset to exchange them against the treasury in return for our native. This is limited by the assets supported, minimum and maximum buyout amounts and buyout period (after which the total buyout amount for a user is reset).
Buyout limit can only be changed by root. If set to None, then there is no amount limit for buyouts.
This pallet was only added for Foucoco runtime in this PR.
Assets supported
Amplitude
Pendulum
Notes:
SellFee
MinAmountToBuyout
BuyoutPeriod