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

fix and improve Signer-auth course #527

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

Conversation

wuuer
Copy link
Contributor

@wuuer wuuer commented Sep 27, 2024

Problem

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.

Summary of Changes

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.

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.

I started reviewing this but there were a immediately a bunch of small issues that can be fixed by reading https://github.com/solana-foundation/developer-content/blob/main/CONTRIBUTING.md - see the comments above.

description:
"Ensure instructions are only executed by authorized accounts by implementing
signer checks."
"Ensure instructions are only ran by authorized accounts by implmementing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo.

```rust
if !ctx.accounts.authority.is_signer {
return Err(ProgramError::MissingRequiredSignature.into());
return Err(ProgramError::MissingRequiredSignature.into());
Copy link
Collaborator

Choose a reason for hiding this comment

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

4 spaces per CONTRIBUTING

/// CHECK: This account will not be checked by Anchor
pub authority: UncheckedAccount<'info>,
pub new_authority: AccountInfo<'info>,
pub authority: AccountInfo<'info>,
Copy link
Collaborator

Choose a reason for hiding this comment

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


```rust
```typescript
Copy link
Collaborator

Choose a reason for hiding this comment

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

No. This is Rust.

})
describe("signer-authorization", () => {
...
it("Insecure withdraw should be successful", async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See CONTRUBUTING.md re: 'BDD'

Copy link

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 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