-
Notifications
You must be signed in to change notification settings - Fork 4
Move protocol fee management to treasury admin #102
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
Conversation
allow treasury admin to change it on a specific pool
|
Actually, you don't need to do the linked list trick; Because fields in a struct are already a linked list, and I'm 90% sure that Aiken scripts will just ignore extra fields when deserializing (we should double check this). So we can just make settings a struct, and keep adding to it and old scripts will continue to work. If that doesn't work, we should make it a For example, 0 would be stablecoin settings, 1 would be permissioned pool settings, etc. |
|
I tried your first suggestion, will push the test case so you can verify I tested it correctly, it crashed on it. |
|
Example of the suggested method: Gives the following result: |
|
A list would require that we always have each setting in the list, which maybe makes sense 🤔 |
Worst case if we want to "delete" a setting we could replace it with a Void, I did already implement the Pairs version though. Maybe in the end Pairs gives more flexibility for some future usecase. I think we can go with Pairs |
| /// The name of the token that authenticates the settings UTXO | ||
| pub const settings_nft_name: AssetName = "settings" | ||
|
|
||
| pub fn find_protocol_fee_extension( |
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.
since the first thing we do is fail when this isn't set (via expect Some(...)), we could probably just name this must_find_protocol_fee_extension and simplify the code.
but it's not a major deal.
We use the extension functionality of the settings datum to define default protocol fee basis points and allow the treasury admin to change it on a specific pool
The intended usage of the extensions field was not fully clear so I now made a setup with each extension pointing to a potential future extension, in essence creating a linked list, but perhaps there is a more simple setup I overlooked.
Also in this PR: The initial liquidity is now the sum invariant divided by the precision rather than just the sum invariant to avoid very large amounts potentially causing issues in poorly written off chain code.