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

Updated account-data-matching, bump-seed-canonicalization #407

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

Epistetechnician
Copy link

Summary of Changes: Bump Seed Canonicalization

Frontmatter

  • Added keywords and tags fields in the new version.
  • Removed objectives field in the new version.

Summary Section

  • Updated the summary to be more concise and focused on the key points.

Lesson Section

  • Expanded explanation of bump seeds and canonical bumps.
  • Added detailed examples of insecure and secure PDA derivation.
  • Included code examples for both insecure and secure implementations.
  • Added explanation of using Anchor's seeds and bump constraints.

Code Examples

  • Added code examples for:
    • Insecure PDA derivation using create_program_address.
    • Secure PDA derivation using find_program_address.
    • Using Anchor's seeds and bump constraints.

Test Section

  • Added a detailed test example to show how the secure implementation prevents multiple claims.

Challenge Section

  • Added a challenge section encouraging users to audit their own or others' programs for proper PDA derivation.

Callout Section

  • Added a callout section prompting users to push their code to GitHub and provide feedback.

Epistetechnician and others added 13 commits August 26, 2024 17:29
Updated lesson for account data matching
Up-to date compatibility and clean format
prettier --write "/Users/shaan.s.patel/Desktop/SF Bounty/account-data-matching-updated.md"
Checked formatting...
All matched files use Prettier code style!
Resolving comments made by @mikemaccana
# Changes Made to arbitrary-cpi.md

1. **Line Length**: Adjusted lines exceeding 80 characters for improved
   readability and adherence to contributing guidelines.

2. **Consistency**: Ensured consistent capitalization in headers and throughout
   the document.

3. **Code Snippets**: Updated Rust code snippets for compatibility with Anchor
   0.29.0:
   - Used `Result<()>` instead of `ProgramResult` for return types
   - Applied `to_account_info()` method instead of `clone()` for account info
   - Utilized `key()` method instead of `key` for public keys

4. **Explanations**: Enhanced clarity in explanations, particularly for
   arbitrary CPIs and program checks concepts.

5. **Lab Instructions**: Refined step-by-step instructions in the lab section
   for clarity and ease of following.

6. **Links**: Added and reviewed cross-references, ensuring inclusion of all
   necessary links (e.g., link to previous Anchor CPIs lesson).

7. **Formatting**: Verified proper Markdown formatting throughout, including
   correct use of code blocks with language specifications.

8. **Content Structure**: Confirmed overall lesson structure (Summary, Lesson,
   Lab, Challenge) adheres to contributing guidelines.

9. **Callouts**: Verified presence of final callout encouraging user feedback
   on the lesson.
ran yarn prettier --write "arbitrary.md"
Updated hyperlinks
Frontmatter:
Removed the 'objectives' field
Added 'keywords' and 'tags' fields
Simplified the 'description'
Content structure:
Removed the "Lesson" heading, integrating its content directly under the main sections
Kept the overall structure (Summary, main content, Lab, Challenge) intact
Code blocks:
Removed language specifiers from code blocks (e.g., rust)4. Styling and formatting: - Removed bold formatting from some terms and links - Slightly adjusted some headings for better flow5. Content updates: - Slightly reworded some explanations for clarity - Removed some repetitive information - Consolidated some information in the Summary section6. Lab and Challenge sections: - These sections remain largely unchanged, with only minor wording adjustments7. Callout: - Retained the Callout component at the end of the documentOverall, the changes aim to streamline the content and improve readability while maintaining the core structure and information of the original lesson. The most significant changes are in the frontmatter and the removal of language specifiers from code blocks. The main content, examples, and practical sections (Lab and Challenge) remain very similar to the original version.
@mikemaccana mikemaccana changed the title Updated-bump-seed-canonicalization-lesson Updated account-data-matching, arbitrary-cpi, and bump-seed-canonicalization-lesson Sep 5, 2024
Copy link
Collaborator

@mikemaccana mikemaccana left a comment

Choose a reason for hiding this comment

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

Some good stuff but also a few things that seem odd - please see the comments in this PR. Also note that that multiple people are competing for some (but not all) of these lessons. We'd still love to get all the changes in though!

content/courses/program-security/account-data-matching.md Outdated Show resolved Hide resolved
content/courses/program-security/account-data-matching.md Outdated Show resolved Hide resolved
content/courses/program-security/account-data-matching.md Outdated Show resolved Hide resolved
content/courses/program-security/account-data-matching.md Outdated Show resolved Hide resolved
docs/programs/examples.md Outdated Show resolved Hide resolved
@mikemaccana
Copy link
Collaborator

mikemaccana commented Sep 16, 2024

@Epistetechnician you have a bunch of references to unboxed repos still in this code:

https://github.com/Unboxed-Software/solana-account-data-matching is now https://github.com/solana-developers/solana-account-data-matching
https://github.com/Unboxed-Software/solana-arbitrary-cpi is now https://github.com/solana-developers/arbitrary-cpi

Also https://github.com/Unboxed-Software/solana-course/blob/main/content/anchor-cpi - make this a link to https://solana.com/developers/courses/onchain-development/anchor-cpi in whatever style the links section in CONTRIBUTING.md uses.

If you fix these, and handle the merge conflicts (arbitrary-cpi has now been updated), I'll mark you as the winner for account-data-matching and bump-seed-canonicalization lessons.

@Epistetechnician Epistetechnician changed the title Updated account-data-matching, arbitrary-cpi, and bump-seed-canonicalization-lesson Updated account-data-matching, bump-seed-canonicalization Sep 17, 2024
Copy link
Collaborator

@mikemaccana mikemaccana left a comment

Choose a reason for hiding this comment

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

Some minor fixes (see above) still to go then this can go in.

content/courses/program-security/arbitrary-cpi.md Outdated Show resolved Hide resolved
@@ -218,7 +221,7 @@ mints, distribution, and transfers, and a separate metadata program is used to
assign metadata to tokens. So the vulnerability we go through here could also be
applied to real tokens.

#### 1. Setup
### 1. Setup

We'll start with the `starter` branch of
[this repository](https://github.com/Unboxed-Software/solana-arbitrary-cpi/tree/starter).
Copy link
Collaborator

Choose a reason for hiding this comment

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

You haven't fixed the link! Please don't resolve things unless the concerns are addressed.

content/courses/program-security/arbitrary-cpi.md Outdated Show resolved Hide resolved
@mikemaccana
Copy link
Collaborator

mikemaccana commented Sep 30, 2024

@Epistetechnician you have a bunch of references to unboxed repos still in this code:
If you fix these I'll mark you as the winner for account-data-matching and bump-seed-canonicalization lessons.

You didn't update the old unboxed repos as requested (as mentioned in the Superteam Bounty), just made links to a new repo that didn't exist. As the bounty has ended, and another user did these changes we will give the prize to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants