-
Notifications
You must be signed in to change notification settings - Fork 7
Sync to token-2022 CLI helper #278
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.
Take it or leave it on the Metaplex shorthand arg!
e5ba9a0
to
0ddc7c7
Compare
f10e537
to
04f83d0
Compare
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.
Looks great! Just a couple of nits
/// Optional owner program for the source metadata account, when owned by a | ||
/// third-party program | ||
#[clap(long, value_parser = parse_pubkey)] | ||
pub program_id: Option<Pubkey>, |
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.
nit: Typically the --program-id
arg is to specify the address of the program to target, ie the token-wrap program in this case.
I'd go with something like --metadata-program-id
so that we can eventually specify --program-id
globally in the CLI
/// Specify that the source metadata is from a `Metaplex` Token Metadata | ||
/// account. The CLI will derive the PDA automatically. | ||
#[clap(long)] | ||
pub metaplex: bool, |
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.
Rather than forcing people to specify this, it would be better to figure it out dynamically.
- If
metadata_account
is specified, use that. - If the unwrapped mint is SPL-Token, then use metaplex.
- If the unwrapped mint is SPL-Token-2022, then check for the metadata pointer, and fall back to metaplex.
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.
If the unwrapped mint is SPL-Token-2022, then check for the metadata pointer, and fall back to metaplex.
Oh, wait a sec. Are you saying that having a Metaplex PDA for a token-2022 is authoritative if it doesn't have a metadata pointer? If so the program also needs to be updated to handle this case. At the moment, it's treating the pointer as the only authoritative source.
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.
That's up to us to decide! It's probably the best fallback -- if someone didn't specify anything during their token-2022 mint creation, and later created metaplex metadata, it would be nice to use that.
I don't know how common this is though -- it's possible that no mints like this exist. What do you think?
Introduces new CLI helper that corresponds to new instruction: #229