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

Feat: Update AccountsClose to be safe to call manually #2209

Merged
merged 12 commits into from
Oct 21, 2022

Conversation

stegaBOB
Copy link
Contributor

@stegaBOB stegaBOB commented Oct 6, 2022

Updates the AccountsClose trait to ensure its safe to call manually.

Also removed the warnings and added more tests to make sure its safe. Also added AccountsClose to the prelude.

@vercel
Copy link

vercel bot commented Oct 6, 2022

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

A member of the Team first needs to authorize it.

@armaniferrante armaniferrante requested a review from Henry-E October 6, 2022 20:50
@Henry-E
Copy link

Henry-E commented Oct 6, 2022

@armaniferrante The core part of this change is switching
if &T::owner() == program_id {
to
if self.info.owner == program_id {
If that's a safe change to make then we should totally do this. But I don't feel qualified yet to comment on using the owner trait vs. the self.info.owner value.

@stegaBOB
Copy link
Contributor Author

stegaBOB commented Oct 6, 2022

This may actually cause issues if assign is called manually. I'm not sure if the serialization would normally fail in that case. I'm going to do an additional test here first and I'll report back.

@stegaBOB
Copy link
Contributor Author

stegaBOB commented Oct 7, 2022

@Henry-E @armaniferrante I updated the AccountsExit logic and it should prevent any false positives from being triggered on accounts with reassigned owners.

@stegaBOB
Copy link
Contributor Author

stegaBOB commented Oct 7, 2022

This PR should be ready to merge in. The failed workflow runs fails were GitHub network errors.

lang/src/accounts/account_loader.rs Outdated Show resolved Hide resolved
lang/src/accounts/account.rs Show resolved Hide resolved
@Henry-E Henry-E merged commit fa12498 into coral-xyz:master Oct 21, 2022
Henry-E pushed a commit to Henry-E/anchor that referenced this pull request Dec 6, 2022
* fix other lints to make the test pass

(cherry picked from commit d6e43c1)

* update close to make it safe to call manually

* fix test script

* re-add safety warnings for deprecated account types

* update close checking logic

* readd logic for deprecated methods

* add additional checks to account_loader in exit
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.

3 participants