-
Notifications
You must be signed in to change notification settings - Fork 66
cli: make set-mint-authority a top-level command with chain parameter support #657
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
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.
Currently the set-mint-authority
function only applies to SVM so I don't think it should be made top-level.
I'm not super familiar with Fogo tooling but seeing as the function itself does not have any major changes, I'll assume it has similar tooling. Is there a reason we want to move the set-mint-authority
command up rather just rename the solana
command to svm
? Do some solana
subcommands not apply to Fogo?
I think the main benefit of doing this is so we can support other runtimes down the line other than SVM under the same command. It would be neat if we could set up a token on a new chain in a platform agnostic way (as far as the CLI interface is concerned). That said, this command should error for now if the platform is not SVM |
59b6e78 here's a branch I started which has some overlap, but should show the general direction |
I added the svm error. I would like to keep this PR separate from the branch you created @kcsongor, just to have less code to review. |
Co-authored-by: Sriram Nurani Viswanathan <[email protected]>
Co-authored-by: Sriram Nurani Viswanathan <[email protected]>
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 believe as a consequence, the original helper function would also need to be renamed to checkSvmValidSplMultisig
along with all other references of it in this file.
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.
LGTM!
in preparation for other SVM chains being supported by the CLI. So far the default was Solana