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

Missing check_number_of_instruction_accounts() in StakeInstruction::Authorize #23672

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Mar 15, 2022

Problem

#23375 was missing a check_number_of_instruction_accounts() in StakeInstruction::Authorize for:

let _current_authority = keyed_account_at_index(keyed_accounts, first_instruction_account + 2)?;

Summary of Changes

Adds the missing check_number_of_instruction_accounts().

Fixes #

@Lichtso Lichtso merged commit e9040d2 into solana-labs:master Mar 15, 2022
@Lichtso Lichtso deleted the fix/stake_authorize_missing_check_number_of_instruction_accounts branch March 15, 2022 14:53
@mvines
Copy link
Member

mvines commented Mar 15, 2022

@Lichtso - hey doesn't this potentially break consensus? #23375 too

The current version of StakeInstruction::Authorize on mainnet will accept more than 3 accounts, but with this change this is no longer the case.

@Lichtso
Copy link
Contributor Author

Lichtso commented Mar 15, 2022

@mvines #23375 had a consensus issue, this PR is the fix.
Both are reverted in #23649.

@mvines
Copy link
Member

mvines commented Mar 15, 2022

hmm, the v1.9 version of StakeInstruction::Authorize will not fail if 4 accounts are provided but now master requires 3 accounts. Am I misreading this patch?

@Lichtso
Copy link
Contributor Author

Lichtso commented Mar 15, 2022

At least 3, not exactly 3. So 4 is ok as well.

@mvines
Copy link
Member

mvines commented Mar 15, 2022

ah, I missed the <. Thank you!

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