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

Program security - Updated type cosplay lesson #411

Conversation

0xCipherCoder
Copy link
Contributor

Problem

Summary of Changes

  • Updated code snippets with the latest anchor version
  • Fixed content,
  • Fixed grammar and styling
  • Fixed as per guidelines

Fixes #
Unboxed PRs -
Starter - Unboxed-Software/solana-type-cosplay#1
Solution - Unboxed-Software/solana-type-cosplay#2

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.

This is a good start, but I have a few large-ish questions to resolve before getting it merged.

content/courses/program-security/type-cosplay.md Outdated Show resolved Hide resolved
To solve this, you can add a discriminant field for each account type and set
the discriminant when initializing an account.
To resolve this, add a discriminant field for each account type and set the
discriminant when initializing an account.
Copy link
Collaborator

Choose a reason for hiding this comment

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

More of complaint with the old lesson, but is a discriminant the same thing as a discriminator? This is confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's a bit confusing to me as well initially but discriminant manages enum variants internally in the Rust and discriminator is an explicit identifier used to distinguish different account types within Solana programs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One of the aims of our docs is to avoid confusion - we should call it out explicitly. Add an info ContentBox with:

While they sound similar, a Rust discriminant isn't the same thing as an Anchor discriminator! Rust itself has the concept of discriminants which are used to manage enums.

Or something similar - you're an excellent writer and can use whatever words you feel best illuminates the difference.

content/courses/program-security/type-cosplay.md Outdated Show resolved Hide resolved
@0xCipherCoder
Copy link
Contributor Author

0xCipherCoder commented Sep 5, 2024

This is a good start, but I have a few large-ish questions to resolve before getting it merged.

@mikemaccana Answered the questions and resolved the comments. Let me know if there is any additional feedback.

@mikemaccana
Copy link
Collaborator

@0xCipherCoder just one small fix but happy to mark this as winner for this lesson assuming you do it.

@0xCipherCoder
Copy link
Contributor Author

@0xCipherCoder just one small fix but happy to mark this as winner for this lesson assuming you do it.

Thanks! @mikemaccana I have updated the content to make it easier to understand with callout. Please review and let me know if any additional changes are required. Also linting failed due to package not downloaded in git checks.

@mikemaccana mikemaccana merged commit bedcc4b into solana-foundation:main Sep 24, 2024
2 checks passed
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