token 2022: make IncorrectMintAuthority a TokenError

[buffalojoec]

This PR makes IncorrectMintAuthority a TokenError, rather than a TokenMetadataError, since it will likely be used across multiple extension processors (such as SPL Token Group).

[buffalojoec]

Current dependencies on/for this PR:

• master
  • PR token 2022: move update authority check to shared file #5595
    • PR token metadata: replace IncorrectMintAuthority with TokenError::OwnerMismatch #5615
    • PR token 2022: make IncorrectMintAuthority a TokenError #5596 👈
      • PR token 2022: add InitializeGroup instruction from SPL Token Group interface #5601

This comment was auto-generated by Graphite.

[joncinque]

Can we instead switch to the current TokenError::OwnerMismatch instead of adding a new error?

[buffalojoec]

Can we instead switch to the current TokenError::OwnerMismatch instead of adding a new error?

Wouldn't that be confusing to some people since the mint authority is a separate thing from the owner?

[joncinque]

Wouldn't that be confusing to some people since the mint authority is a separate thing from the owner?

It's true that Owner is a very overloaded term in token, and especially in token-2022. Anytime we validate an owner or authority (freeze, transfer fee, confidential transfer, metadata, metadata pointer, transfer hook, etc etc etc), we call validate_owner, which can return OwnerMismatch.

If we want to maintain ABI compatibility in Token-2022, it must return the same errors as tokenkeg for things like mint, freeze, and owner. We can make a new error for the other authorities though, and potentially use that new error in instructions that aren't present in tokenkeg. That seems like it'll be needlessly brittle and prone to error though.

We can also decide to break ABI compatibility, but it seems like a lot of work just to fix error text. We could also change the name OwnerMismatch to something else, but that might cause other kinds of confusion.

So to me, since we don't have a great alternative, I prefer to stick with what we have in OwnerMismatch, even though it's imperfect. What do you think?

[buffalojoec]

We can also decide to break ABI compatibility, but it seems like a lot of work just to fix error text. We could also change the name OwnerMismatch to something else, but that might cause other kinds of confusion.

No, we definitely don't want to break ABI compatibility or rename the enum variant.

We can make a new error for the other authorities though, and potentially use that new error in instructions that aren't present in tokenkeg. That seems like it'll be needlessly brittle and prone to error though.

This was what I was going for - particularly for SPL Token Metadata. However, if on the Tokenkeg/Token2022 side of things we're throwing OwnerMismatch on mint authority checks, then it makes sense to just follow that pattern with SPL Token Metadata.

So to me, since we don't have a great alternative, I prefer to stick with what we have in OwnerMismatch, even though it's imperfect. What do you think?

Yeah, that sounds like the best move here. I'll update the changes.

[buffalojoec]

Going to submit a new PR for the above and close this one.