Skip to content
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

fix: mint authority #573

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Conversation

yanCode
Copy link
Contributor

@yanCode yanCode commented Oct 18, 2024

Problem

in this page https://solana.com/developers/courses/onchain-development/anchor-cpi
if you run anchor test in the final example, you can see errors:

 "Program log: Instruction: MintTo",
  "Program log: Error: owner does not match",

Summary of Changes

the reason is in this line:

authority: ctx.accounts.mint.to_account_info(),

it passed an incorrect mint authority, because in InitializeMint, the authority passed with user, which is the signer. but when it call MintTo in CpiContext::new_with_signer, authority is incorrectly passed as mint. which is not the authority of itself, instead the signer, ctx.accounts.initializer.to_account_info(), should be passed in.

Fixes #

Also I see in this PR: https://github.com/solana-foundation/developer-content/pull/571/files. it uses the mint as its own authority. well, i am afriad I can't think this is very good based on two reasons:

  1. Security: If the mint were its own authority, anyone who could predict the PDA (which is techincally possible! ) could potentially mint tokens.

  2. Security: If the mint were its own authority, anyone who could predict the PDA could potentially mint tokens.

@yanCode yanCode requested a review from mikemaccana as a code owner October 18, 2024 09:30
@mikemaccana mikemaccana merged commit 767d78e into solana-foundation:main Oct 21, 2024
2 checks passed
@mikemaccana
Copy link
Collaborator

Yep 'initializer` seems right. Re: the other PR, please add your comments to the discussion in that PR - that way we can have it in one place!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants