Skip to content

Z24028002 HW Review#2

Open
alex0207s wants to merge 44 commits intomainfrom
Z24028002
Open

Z24028002 HW Review#2
alex0207s wants to merge 44 commits intomainfrom
Z24028002

Conversation

@alex0207s
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

@alex0207s alex0207s left a comment

Choose a reason for hiding this comment

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

  • DO NOT merge this branch to the main branch.
  • Adding more descriptive details to the commit message would be beneficial rather than just stating "update".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DO NOT push this file on Github


function test_TransferAcrossMultipleAccounts() public {
vm.startPrank(user1);
token.mint(user1, 10 * 10 ** 18);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you can replace ether with 10**18.

}

function mint(address to, uint256 amount) public {
require(balanceOf(msg.sender) + amount <= 10 * 10 ** 18, "You can only mint less than 10");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. DON'T rely solely on the balanceOf to determine the number of tokens a user can mint. Depending solely on this could cause a vulnerability, especially when a user transfers out the minted tokens and continues to mint more tokens.
  2. This require statement will restrict the contract owner to mint more than 10 tokens. However we allow the contract owner mint more than 10 tokens.
  3. use custom errors are more gas efficient than using require with a string


contract Token is ERC20 {
constructor() ERC20("Token", "TK") {
_mint(msg.sender, 100 * (10 ** uint256(decimals())));
Copy link
Collaborator Author

@alex0207s alex0207s Mar 6, 2024

Choose a reason for hiding this comment

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

If you don't override the decimals() function, you can use the ether keyword directly.

// console2.logBytes(abi.encodeWithSelector(selector, address(0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69), 1, 6));
// vm.expectRevert(abi.encodeWithSelector(selector, 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 1, 6));
vm.expectRevert(abi.encodeWithSelector(selector, user2, token.allowance(user1, user2), token.balanceOf(user1)));
token.transferFrom(user1, user2, token.balanceOf(user1));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

user1 approves user2 to spend only 3 tokens. You cannot transfer user1's total balance to user2.

1. user2 將 user1 的 2 個 token 轉移到 user3 身上,並將剩餘的 token 轉移給自己
2. 因 allowance 不足,會導致 ERC20InsufficientAllowance revert
- 將 user1 的 balance 分別用 Ether、Gwei 為單位表示
1. cast call 0xaFfB198193e8681f58DB28DBBaf72be91699Fb73 "balanceOf(address)(uint256)" --rpc-url="https://eth-sepolia.g.alchemy.com/v2/your_alchemy_api_key" 0x0278137e8E2C38111297c9991815507eB16eaf25 得到 6000000000000000000
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I recommend using .md instead of .txt and enclosing command lines within backticks for easier readability (e.g. forge test)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not push this file to GitHub.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants