-
Notifications
You must be signed in to change notification settings - Fork 287
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 and improve pda-sharing course #529
Conversation
….InitSpace.html) to calculate space needed for accounts. change "token account" in the "Starter" section to "associated token account" change "token account" in the "Add `initialize_pool_secure" section to "associated token account"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some good changes but please see the notes above re: CONTRIBUTING.md
|
||
In the example below, the `authority` of the `vault` account is a PDA derived | ||
using the `mint` address stored on the `pool` account. This PDA is passed into | ||
the instruction handler as the `authority` account to sign for the transfer of | ||
tokens from the `vault` to the `withdraw_destination`. | ||
the instruction as the `authority` account to sign for the transfer tokens from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See CONTRIBUTING.md re instruction handlers.
@@ -63,31 +63,29 @@ pub struct WithdrawTokens<'info> { | |||
pool: Account<'info, TokenPool>, | |||
vault: Account<'info, TokenAccount>, | |||
withdraw_destination: Account<'info, TokenAccount>, | |||
/// CHECK: This is the PDA that signs for the transfer | |||
/// CHECK: PDA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// CHECK
is supposed to be a sentence explaining why you're not checking the account.
@@ -96,7 +94,7 @@ pub struct TokenPool { | |||
One approach to create an account specific PDA is to use the | |||
`withdraw_destination` as a seed to derive the PDA used as the authority of the | |||
`vault` token account. This ensures the PDA signing for the CPI in the | |||
`withdraw_tokens` instruction handler is derived using the intended | |||
`withdraw_tokens` instruction is derived using the intended |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See CONTRIBUTING.md again.
@@ -127,57 +125,54 @@ pub struct WithdrawTokens<'info> { | |||
pool: Account<'info, TokenPool>, | |||
vault: Account<'info, TokenAccount>, | |||
withdraw_destination: Account<'info, TokenAccount>, | |||
/// CHECK: This is the PDA that signs for the transfer | |||
/// CHECK: PDA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above re: /// CHECK
has_one = withdraw_destination, | ||
seeds = [withdraw_destination.key().as_ref()], | ||
bump = pool.bump, | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check your editor settings, pretty sure you're using tabs. Rustfmt uses 4 spaces.
This pull request has been automatically marked as stale because it has not had recent activity. Remove stale label or comment or this will be closed in 7 days. |
Problem
Magic number found on space calculation.
"token account" in the" Starter" section should refer to "associated token account"
"token account" in the "Add
initialize_pool_secure
instruction" section should refer to "associated token account"Some typescript code is not synced to the project.
Summary of Changes
Use InitSpace to calculate space needed for accounts.
change "token account" in the "Starter" section to "associated token account"
change "token account" in the "Add `initialize_pool_secure" section to "associated token account"
Also, I made a PR for solana-pda-sharing starter branch and a PR for solana-pda-sharing solution branch
which must be synced with this PR.