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

lang: Require IdlCreateAccounts instruction authority to be the program authority #3423

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

acheroncrypto
Copy link
Collaborator

@acheroncrypto acheroncrypto commented Dec 11, 2024

Problem

As mentioned in #444 (comment), IdlCreateAccounts instruction is permissionless, meaning anyone can claim the IDL authority of a program if it hasn't yet been claimed:

// Payer of the transaction.
#[account(signer)]
pub from: AccountInfo<'info>,

Summary of changes

Require from account of IdlCreateAccounts to be the program authority.

Note: This only changes the initial IDL creation instruction authority, meaning any existing IDL authority won't be affected by this change. Ideally, the IDL authority and the program deploy authority should be the same, but given that could break existing programs, it makes more sense to only let the IDL account be initialized by the program deploy authority.

Resolves #444

Copy link

vercel bot commented Dec 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
anchor-docs ⬜️ Ignored (Inspect) Visit Preview Dec 11, 2024 7:02am

Copy link

vercel bot commented Dec 11, 2024

@acheroncrypto is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@acheroncrypto acheroncrypto added lang breaking idl related to the IDL, either program or client side fix Bug fix PR labels Dec 11, 2024
| bench | 1,041,928 | 🔴 **+250,920 (31.72%)** |
| bench | 1,106,136 | 🔴 **+315,128 (39.84%)** |
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's crazy that this simple change adds ~60 KB to the program binary. I guess we could optimize it by not using the existing types such as ProgramData (which imports bincode) and interpreting the account data ourselves, but given we're planning to get rid of these instructions, it's probably fine to keep it as is and not complicate things further.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool to see that you're tracking this. 60kb does sound way too much, but I agree with you here

@robre
Copy link

robre commented Dec 11, 2024

looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking fix Bug fix PR idl related to the IDL, either program or client side lang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lang: IDL authority should use the program upgrade authority
2 participants