refactor: enhance PDA validation #40
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
=====================================
Coverage 8.30% 8.30%
=====================================
Files 1 1
Lines 253 253
=====================================
Hits 21 21
Misses 232 232 ☔ View full report in Codecov by Sentry. |
brewmaster012
requested review from
lumtis,
fbac,
ws4charlie and
skosito
as code owners
| @@ -381,7 +382,6 @@ pub struct WithdrawSPLToken<'info> { | |||
| #[account(mut)] | |||
| pub pda_ata: Account<'info, TokenAccount>, // associated token address of PDA | |||
There was a problem hiding this comment.
just to double check if we can add some of these spl token constraints on these related accounts:
https://www.quicknode.com/guides/solana-development/anchor/how-to-use-constraints-in-anchor#spl-token-constraints
There was a problem hiding this comment.
added a few constraints. This seems to be redundant to the checks inside the withdrawSPLToken function:
as a result, there is no way to trigger the error SPLAtaAndMintAddressMismatch in the test.
For now let's leave both validation there as they do not harm.
let pda_ata = get_associated_token_address(&pda.key(), &ctx.accounts.mint_account.key());
require!(
pda_ata == ctx.accounts.pda_ata.to_account_info().key(),
Errors::SPLAtaAndMintAddressMismatch
);There was a problem hiding this comment.
yes i suspected we can achieve the same validation with anchor after checking link above
i am ok with leaving both validations, but also would be ok to remove ours if it cant be triggered anymore to keep programs simpler
There was a problem hiding this comment.
will keep both for now.
Co-authored-by: skosito <[email protected]>
Co-authored-by: skosito <[email protected]>
| @@ -422,6 +420,38 @@ describe("some tests", () => { | |||
| } | |||
| }); | |||
|
|
|||
| it("create an account owned by the gateway program", async () => { | |||
There was a problem hiding this comment.
nits: maybe lets remove console logs and comment if we need this test
There was a problem hiding this comment.
this test case is removed in the subsequent PR #42
Add seeds and bump to all references of PDA accounts.
This strengthens validation.