-
Notifications
You must be signed in to change notification settings - Fork 132
Permissioned burn extension #818
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
base: main
Are you sure you want to change the base?
Conversation
|
@joncinque Would really appreciate a quick review, just to confirm that I’m moving in the right direction before adding the tests. |
joncinque
left a comment
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.
Thanks for your contribution! This is definitely on the right track, but I think we'll need a bit more functionality. Let me know what you think!
|
@joncinque I would need a bit of guidance on adding tests for the permissioned burn. I see tests in rust-legacy and js-legacy, but since they’re marked legacy I’m unsure of the current convention. Where should new tests go? |
joncinque
left a comment
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 see tests in rust-legacy and js-legacy, but since they’re marked legacy I’m unsure of the current convention. Where should new tests go?
Thankfully, legacy doesn't mean deprecated, so please add the tests to the rust-legacy client 😄
This is on the right track, keep it up! Let me know if you need anything else on my side, apologies for the slowness on the re-review
|
@joncinque Everything should be resolved now :) |
joncinque
left a comment
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 looks really good! Mostly nits, and a question about what to do if the authority is set to None.
We'll eventually need to add the ability to specify a burn authority in the CLI and rust-legacy clients, but that can happen later
|
I kicked off CI, and there are a few little things to fix:
Let me know if you need any help with these pieces 😄 |
This PR implements a permissioned-burn extension as described in #772
Summary
Introduces the
PermissionedBurnmint extension, enabling bothburnandburn_checkedoperations that require approval from a designated authority configured during mint initialization.When this extension is enabled, standard
burnandburn_checkedinstructions are disallowed for the mint. This PR does not modify the existing behavior of regular burns for mints that do not use the extension.TODO:
Closes #772