-
Notifications
You must be signed in to change notification settings - Fork 305
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes and here we want - thanks! There's also a few we don't - please see the comments above and update accordingly!
// Check if the withdraw destination is different from the vault's token account | ||
if ctx.accounts.token_account.key() == ctx.accounts.withdraw_destination.key() { | ||
return Err(ProgramError::InvalidArgument.into()); | ||
} | ||
let seeds = &[b"vault".as_ref(), &[*ctx.bumps.get("vault").unwrap()]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can ensure the code is properly formatted (4 spaces per CONTRIBUTING.md)? Looks like you're in one level too far left.
@@ -424,7 +441,7 @@ describe("signer-authorization", () => { | |||
}) | |||
``` | |||
|
|||
Run `anchor test` to see that both transactions will complete successfully. | |||
Run `anchor test` to see that both transactions will be completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is clearer than the old language.
@@ -7,7 +7,7 @@ objectives: | |||
- Implement signer checks using Anchor’s `Signer` type | |||
- Implement signer checks using Anchor’s `#[account(signer)]` constraint | |||
description: | |||
"Ensure instructions are only ran by authorized accounts by implmementing | |||
"Ensure instructions are only run by authorized accounts by implementing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to double check the grammar here but you're correct. 👍
@@ -199,7 +199,7 @@ performed. | |||
#### Use Anchor’s `#[account(signer)]` constraint | |||
|
|||
While in most cases, the `Signer` account type will suffice to ensure an account | |||
has signed a transaction, the fact that no other ownership or type checks are | |||
has signed a transaction, the fact that no other ownership or type of checks are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. We want to say that there are no other checks regarding ownership or types.
If you like, you can re-word this sentence to make it clearer.
@@ -553,7 +570,7 @@ the remainder of the lessons on security vulnerabilities, the Challenge for each | |||
lesson will be to audit your own code for the security vulnerability discussed | |||
in the lesson. | |||
|
|||
Alternatively, you can find open source programs to audit. There are plenty of | |||
Alternatively, you can find open-source programs to audit. There are plenty of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No dash needed, see https://opensource.org/
a. The withdraw amount is not zero. | ||
b. There are sufficient funds in the account. | ||
c. The withdraw destination is not the same as the vault's token account. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're trying to focus on just signer checks in this chapter. I'm not totally sure adding other checks helps.
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. |
Problem
Summary of Changes
Add
a. The withdraw amount is not zero.
b. There are sufficient funds in the account.
c. The withdraw destination is not the same as the vault's token account.
By implementing these checks, the program becomes more secure and resistant to potential exploits or user errors.
Fixes #