This repository was archived by the owner on Jan 24, 2025. It is now read-only.
fix and improve pda-sharing course #529
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….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"
mikemaccana
suggested changes
Oct 17, 2024
| 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 |
| vault: Account<'info, TokenAccount>, | ||
| withdraw_destination: Account<'info, TokenAccount>, | ||
| /// CHECK: This is the PDA that signs for the transfer | ||
| /// CHECK: PDA |
Contributor
There was a problem hiding this comment.
/// CHECK is supposed to be a sentence explaining why you're not checking the account.
| `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 |
Contributor
There was a problem hiding this comment.
See CONTRIBUTING.md again.
| vault: Account<'info, TokenAccount>, | ||
| withdraw_destination: Account<'info, TokenAccount>, | ||
| /// CHECK: This is the PDA that signs for the transfer | ||
| /// CHECK: PDA |
Contributor
There was a problem hiding this comment.
Same as above re: /// CHECK
| has_one = withdraw_destination, | ||
| seeds = [withdraw_destination.key().as_ref()], | ||
| bump = pool.bump, | ||
| )] |
Contributor
There was a problem hiding this comment.
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_secureinstruction" 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.