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

Redundant discriminator checks in AccountLoader::load* methods #3172

Open
acheroncrypto opened this issue Aug 16, 2024 · 0 comments
Open

Redundant discriminator checks in AccountLoader::load* methods #3172

acheroncrypto opened this issue Aug 16, 2024 · 0 comments
Labels
lang performance Performance related issues/PRs

Comments

@acheroncrypto
Copy link
Collaborator

Problem

load* methods check the discriminator of the account each time they're called. This results in unnecessary performance penalty, especially when used in account constraints.

The new custom discriminator support also added a bit of additional overhead when comparing the discriminators.

It looks like these checks are all redundant because the only way to create an AccountLoader instance is to use AccountLoader::try_from or AccountLoader::try_from_unchecked.

try_from:

  • Used when the account already exists

  • Checks both the owner and the discriminator:

    if acc_info.owner != &T::owner() {
    return Err(Error::from(ErrorCode::AccountOwnedByWrongProgram)
    .with_pubkeys((*acc_info.owner, T::owner())));
    }
    let data = &acc_info.try_borrow_data()?;
    let disc = T::DISCRIMINATOR;
    if data.len() < disc.len() {
    return Err(ErrorCode::AccountDiscriminatorNotFound.into());
    }
    let given_disc = &data[..disc.len()];
    if given_disc != disc {
    return Err(ErrorCode::AccountDiscriminatorMismatch.into());
    }

try_from_unchecked:

Given this, the discriminator is already checked when it should be (using try_from), and if it shouldn't get checked (like when using init), it's not checked.

This also means load_init method is fully redundant.

Solution

If the above assumptions are correct, it should be safe to remove them:

  • Remove the discriminator checks from the load and load_mut methods
  • Remove the load_init method completely

But before doing so, we must be certain that those methods are indeed redundant, and this change would not affect the security of programs in any way.

@acheroncrypto acheroncrypto added lang performance Performance related issues/PRs labels Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang performance Performance related issues/PRs
Projects
None yet
Development

No branches or pull requests

1 participant