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 CpiGuard typo, but actually DRY processor methods #3791

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

CriesofCarrots
Copy link
Contributor

I was reading #3756 to understand what the new extension does, and stumbled on a typo.
I also noticed an opportunity to DRY the processor code, and then realized I introduced the verbose pattern myself in RequiredMemoTransfers. Wdyt about this tidying?

@2501babe
Copy link
Member

2501babe commented Nov 2, 2022

100% in favor, i copied the existing structure just because i thought it was done that way on purpose for style reasons

}
CpiGuardInstruction::Disable => {
msg!("CpiGuardInstruction::Disable");
process_diasble_cpi_guard(program_id, accounts)
Copy link
Member

Choose a reason for hiding this comment

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

also thank you for catching this!

@CriesofCarrots
Copy link
Contributor Author

i thought it was done that way on purpose for style reasons

I figured! Originally, only enable initialized the extension, so it seemed clearer for the methods to be separate. When I updated the code to also initialize on disable, I neglected to clean up after myself (to wit: that incorrect code comment that you caught and fixed)

@CriesofCarrots CriesofCarrots merged commit 574fe73 into solana-labs:master Nov 2, 2022
HaoranYi pushed a commit to HaoranYi/solana-program-library that referenced this pull request Jul 19, 2023
* Fix typo

* DRY CpiGuard

* DRY RequiredMemoTransfers
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