developer-content icon indicating copy to clipboard operation
developer-content copied to clipboard

fix and improve signer-auth.md & reinitialization-attacks.md & pda-sharing.md & owner-checks.md & closing-accounts.md & arbitrary-cpi.md

Open wuuer opened this issue 1 year ago • 5 comments

Problem

signer-auth.md

The way to get the seed "*ctx.bumps.get("vault").unwrap()" is outdated for the latest anchor version. Unnecessary parameters in "accounts()" in the test typescript.

reinitialization-attacks.md

Magic number found on space calculation “@project-serum/anchor” is outdated. The two recommendedInitialization test cases do not use the same method to send transactions.

pda-sharing.md

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.

owner-checks.md

Magic number found on space calculation. Unnecessary parameters are found in the test typescript. Two Different ways to send transactions are found.

duplicate-mutable-accounts.md

Magic number found on space calculation. "BorshDeserialize" and "BorshSerialize" are outdated. Test descriptions are not clear enough.

closing-accounts.md

Use InitSpace to calculate space needed for accounts. Delete "force_defund" because “CLOSED_ACCOUNT_DISCRIMINATOR” was removed in the latest version of anchor-lang Add the new secure instruction for account closing in the "Secure account closing" section . Delete Unnecessary parameters found in the test typescript. Change the Logic of account closing "Sets the account discriminator to the CLOSED_ACCOUNT_DISCRIMINATOR variant" to "Assigning the owner of the account to the System Program and rellocating the size of the account's data with 0 bytes."

arbitrary-cpi.md

The anchor cpi lesson link is invalid. Magic number found on space calculation.

Summary of Changes

signer-auth.md

Update the latest way to get the seed. replace "init" with "init_if_needed" for vault field in the struct InitializeVault<'info>. delete unnecessary parameters "accounts()" in the test typescript. Update the project repo links.

reinitialization-attacks.md

Use InitSpace to calculate space needed for accounts. Use "@coral-xyz/anchor" instead of "@project-serum/anchor" for the test typescript. Consistently use "rpc()" to send transactions in the test typescript. Update BDD style for the test typescript.

Also, I made a PR for solana-reinitialization-attacks starter branch and a PR for solana-reinitialization-attacks solution branch which must be sync with this PR.

pda-sharing.md

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.

owner-checks.md

Use InitSpace to calculate space needed for accounts. Delete Unnecessary parameters found in the test typescript. Consistently use "rpc()" as sending transactions in the test typescript.

Also, I made a PR for solana-owner-checks starter branch and a PR for solana-owner-checks solution branch which must be synced with this PR.

duplicate-mutable-accounts.md

Use InitSpace to calculate space needed for accounts. Replace "BorshDeserialize" and "BorshSerialize" with "AnchorDeserialize" and "AnchorSerialize". Make test descriptions more clear.

Also, I made a PR for solana-duplicate-mutable-accounts starter branch and a PR for solana-duplicate-mutable-accounts solution branch which must be synced with this PR.

closing-accounts.md

Use InitSpace to calculate space needed for accounts. Delete "force_defund"  because “CLOSED_ACCOUNT_DISCRIMINATOR” was removed in the latest version of anchor-lang. Add the new secure instruction for account closing in the "Secure account closing" section. Change the Logic of closing account "Sets the account discriminator to the CLOSED_ACCOUNT_DISCRIMINATOR variant" to "Assigning the owner to the System Program".

Also, I made a PR for solana-closing-accounts starter branch and a PR for solana-closing-accounts solution branch which must be synced with this PR.

arbitrary-cpi.md

Fix the anchor cpi lesson link. Use InitSpace to calculate space needed for accounts.

Also, I made a PR for solana-arbitrary-cpi starter branch and a PR for solana-arbitrary-cpi solution branch which must be synced with this PR.

wuuer avatar Aug 28 '24 16:08 wuuer

I've reviewed the PR on the other repo, but also forked it (and all branches to https://github.com/solana-developers/solana-signer-auth/ and invited you.

mikemaccana avatar Aug 28 '24 18:08 mikemaccana

I've reviewed the PR on the other repo, but also forked it (and all branches to https://github.com/solana-developers/solana-signer-auth/ and invited you. @mikemaccana I committed changes to this repo and changed the links in the course to refer to this repo.

Please do the same things:

  1. Fork solana-reinitialization-attacks including all the branches.
  2. Fork solana-pda-sharing including all the branches.
  3. Fork solana-owner-checks including all the branches.
  4. Fork solana-duplicate-mutable-accounts including all the branches.
  5. Fork solana-closing-accounts including all the branches.

wuuer avatar Aug 30 '24 14:08 wuuer

great work @wuuer!

  1. Great work! Let's jump into telegram or X and DM for faster chat - I'm mikemaccana on either.

  2. You have the same code in this PR and #423 - which should I review? There's no need to open two PRs for the same work.

  3. New repos made:

https://github.com/solana-developers/reinitialization-attacks/ https://github.com/solana-developers/pda-sharing https://github.com/solana-developers/closing-accounts

https://github.com/Unboxed-Software/solana-duplicate-mutable-accounts PRs need rebasing, as someone else already submitted updates.

I could not do https://github.com/Unboxed-Software/solana-owner-checks, see https://github.com/Unboxed-Software/solana-owner-checks/pull/2

mikemaccana avatar Sep 04 '24 20:09 mikemaccana

great work @wuuer!

  1. You have the same code in this PR and fix and improve program-configuration.md #423 - which should I review? There's no need to open two PRs for the same work.

@mikemaccana
This PR(https://github.com/solana-foundation/developer-content/pull/342) is for tokens-and-nfts course content. This PR(https://github.com/solana-foundation/developer-content/pull/372) is for program-security course content. This PR(https://github.com/solana-foundation/developer-content/pull/423) is for program-optimization course content. Do you agree with me doing this? If not, I will merge these three PRs.

wuuer avatar Sep 05 '24 01:09 wuuer

@wuuer sorry about the wait here - we had a 24-person in-room class with new content in NY, then Rustconf then Breakpoint.

Re: your recent comment, some of your PRs are shortlisted for some lessons but yes, it's much easier to do one PR per lesson as others have done. There's no guarantee that someone that wins one lesson will win another one so we want to be able to merge and review them independently.

After you split them out, link them here with the lesson they are for.

@mikemaccana

token and nfts

token-program token-program-advance

token extensions

close-mint

program security

signer-auth reinitialization-attacks pad-sharing owner-checks duplicate-mutable-accounts closing-account arbitrary-cpi

program optimization

program-configuration program-architecture

wuuer avatar Sep 27 '24 01:09 wuuer

This is some solid work, if you can address the small issues below you'll win at least another one of the bounties.

@mikemaccana . As 0xCipherCoder won the lesson program-configuration, I will not update the lesson.

Please let me know if there are other PRs of mine left to win the Superteam earn 😉

wuuer avatar Oct 08 '24 03:10 wuuer

@wuuer I sent you an email with details on your 3 official wins!

mikemaccana avatar Oct 09 '24 03:10 mikemaccana

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.

github-actions[bot] avatar Dec 02 '24 00:12 github-actions[bot]