From 16ebab6abfc516deb9c6a0f4fb25f43e996ac277 Mon Sep 17 00:00:00 2001 From: manas Date: Fri, 30 Jun 2023 23:13:57 -0700 Subject: [PATCH 1/7] Update ERC721.t.sol --- test/tokens/ERC721.t.sol | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/test/tokens/ERC721.t.sol b/test/tokens/ERC721.t.sol index d8686529..16449ac4 100644 --- a/test/tokens/ERC721.t.sol +++ b/test/tokens/ERC721.t.sol @@ -118,7 +118,10 @@ contract ERC721Test is Test { // We can't burn a token we don't have assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - assertEq(token.ownerOf(1337), address(0)); + + // Commenting out as this will revert due to not having ownership + // assertEq(token.ownerOf(1337), address(0)); + vm.expectRevert(bytes("NOT_MINTED")); token.burn(1337); @@ -136,7 +139,9 @@ contract ERC721Test is Test { token.burn(1337); assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - assertEq(token.ownerOf(1337), address(0)); + + // Commenting out as this will revert due to not having ownership + // assertEq(token.ownerOf(1337), address(0)); vm.expectRevert(bytes("NOT_MINTED")); token.burn(1337); @@ -459,8 +464,8 @@ contract ERC721Test is Test { } function testBalanceOfZeroAddress() public { + vm.expectRevert(bytes("ZERO_ADDRESS")); uint256 bal = token.balanceOf(address(0)); - assertEq(0, bal); } // function testFailOwnerOfUnminted() public view { @@ -484,12 +489,8 @@ contract ERC721Test is Test { assertEq(token.balanceOf(to), 0); - // vm.expectRevert("NOT_MINTED"); - // token.ownerOf(id); - - // vm.expectRevert("NOT_MINTED"); - address owner = token.ownerOf(id); - assertEq(owner, address(0)); + vm.expectRevert(bytes("NOT_MINTED")); + token.ownerOf(id); } function testApprove(address to, uint256 id) public { @@ -512,7 +513,7 @@ contract ERC721Test is Test { assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(id), address(0)); - // vm.expectRevert("NOT_MINTED"); + vm.expectRevert(bytes("NOT_MINTED")); address owner = token.ownerOf(id); assertEq(owner, address(0)); } @@ -819,7 +820,7 @@ contract ERC721Test is Test { } function testOwnerOfUnminted(uint256 id) public { + vm.expectRevert(bytes("NOT_MINTED")); address owner = token.ownerOf(id); - assertEq(owner, address(0)); } } From 890e91e707520886376b006dd554e2286f28ae21 Mon Sep 17 00:00:00 2001 From: manas Date: Fri, 30 Jun 2023 23:14:01 -0700 Subject: [PATCH 2/7] fix: added reverts on view functions --- src/tokens/ERC721.huff | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tokens/ERC721.huff b/src/tokens/ERC721.huff index cde479d8..9cf2ab26 100644 --- a/src/tokens/ERC721.huff +++ b/src/tokens/ERC721.huff @@ -104,6 +104,10 @@ #define macro BALANCE_OF() = takes (0) returns (0) { NON_PAYABLE() // [] 0x04 calldataload // [account] + // revert if account is zero address + dup1 continue jumpi + ZERO_ADDRESS(0x00) + continue: [BALANCE_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [balance] 0x00 mstore // [] 0x20 0x00 return // [] @@ -114,6 +118,10 @@ #define macro OWNER_OF() = takes (0) returns (0) { 0x04 calldataload // [tokenId] [OWNER_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [owner] + // revert if owner is zero address/not minted + dup1 continue jumpi + NOT_MINTED(0x00) + continue: 0x00 mstore // [] 0x20 0x00 return // [] } From c6380f7028ed951b9d2b8c1aec0ada0654453928 Mon Sep 17 00:00:00 2001 From: manas! Date: Fri, 30 Jun 2023 23:21:59 -0700 Subject: [PATCH 3/7] changed continue -> cont --- src/tokens/ERC721.huff | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tokens/ERC721.huff b/src/tokens/ERC721.huff index 9cf2ab26..09d73c7d 100644 --- a/src/tokens/ERC721.huff +++ b/src/tokens/ERC721.huff @@ -105,9 +105,9 @@ NON_PAYABLE() // [] 0x04 calldataload // [account] // revert if account is zero address - dup1 continue jumpi + dup1 cont jumpi ZERO_ADDRESS(0x00) - continue: + cont: [BALANCE_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [balance] 0x00 mstore // [] 0x20 0x00 return // [] @@ -119,9 +119,9 @@ 0x04 calldataload // [tokenId] [OWNER_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [owner] // revert if owner is zero address/not minted - dup1 continue jumpi + dup1 cont jumpi NOT_MINTED(0x00) - continue: + cont: 0x00 mstore // [] 0x20 0x00 return // [] } @@ -505,9 +505,9 @@ cont: // If to === address(0) revert with "INVALID_RECIPIENT" - dup2 iszero iszero continue jumpi // [from, to, tokenId] + dup2 iszero iszero cont jumpi // [from, to, tokenId] INVALID_RECIPIENT(0x00) - continue: + cont: // Check if msg.sender == from dup1 caller eq // [msg.sender == from, from, to, tokenId] From b301c30cd51250d68639da521cb903e4d1de2670 Mon Sep 17 00:00:00 2001 From: manas Date: Sat, 1 Jul 2023 15:45:57 -0700 Subject: [PATCH 4/7] Revert "changed continue -> cont" This reverts commit c6380f7028ed951b9d2b8c1aec0ada0654453928. --- src/tokens/ERC721.huff | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tokens/ERC721.huff b/src/tokens/ERC721.huff index 09d73c7d..9cf2ab26 100644 --- a/src/tokens/ERC721.huff +++ b/src/tokens/ERC721.huff @@ -105,9 +105,9 @@ NON_PAYABLE() // [] 0x04 calldataload // [account] // revert if account is zero address - dup1 cont jumpi + dup1 continue jumpi ZERO_ADDRESS(0x00) - cont: + continue: [BALANCE_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [balance] 0x00 mstore // [] 0x20 0x00 return // [] @@ -119,9 +119,9 @@ 0x04 calldataload // [tokenId] [OWNER_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [owner] // revert if owner is zero address/not minted - dup1 cont jumpi + dup1 continue jumpi NOT_MINTED(0x00) - cont: + continue: 0x00 mstore // [] 0x20 0x00 return // [] } @@ -505,9 +505,9 @@ cont: // If to === address(0) revert with "INVALID_RECIPIENT" - dup2 iszero iszero cont jumpi // [from, to, tokenId] + dup2 iszero iszero continue jumpi // [from, to, tokenId] INVALID_RECIPIENT(0x00) - cont: + continue: // Check if msg.sender == from dup1 caller eq // [msg.sender == from, from, to, tokenId] From 2e83a1c15eeb003095bd797adedc2d9f3af1f469 Mon Sep 17 00:00:00 2001 From: manas! Date: Wed, 5 Jul 2023 09:04:50 -1000 Subject: [PATCH 5/7] Update test/tokens/ERC721.t.sol Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com> --- test/tokens/ERC721.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tokens/ERC721.t.sol b/test/tokens/ERC721.t.sol index 16449ac4..41a0ef7e 100644 --- a/test/tokens/ERC721.t.sol +++ b/test/tokens/ERC721.t.sol @@ -119,8 +119,8 @@ contract ERC721Test is Test { assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - // Commenting out as this will revert due to not having ownership - // assertEq(token.ownerOf(1337), address(0)); + vm.expectRevert(bytes("ZERO_ADDRESS")); + assertEq(token.ownerOf(1337), address(0)); vm.expectRevert(bytes("NOT_MINTED")); token.burn(1337); From 306440650ba94c54647373ec005bb7d3280e5b0f Mon Sep 17 00:00:00 2001 From: manas! Date: Wed, 5 Jul 2023 09:05:10 -1000 Subject: [PATCH 6/7] Update test/tokens/ERC721.t.sol Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com> --- test/tokens/ERC721.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tokens/ERC721.t.sol b/test/tokens/ERC721.t.sol index 41a0ef7e..5605f367 100644 --- a/test/tokens/ERC721.t.sol +++ b/test/tokens/ERC721.t.sol @@ -140,8 +140,8 @@ contract ERC721Test is Test { assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - // Commenting out as this will revert due to not having ownership - // assertEq(token.ownerOf(1337), address(0)); + vm.expectRevert(bytes("ZERO_ADDRESS")); + assertEq(token.ownerOf(1337), address(0)); vm.expectRevert(bytes("NOT_MINTED")); token.burn(1337); From 55eec6e4c60b7c0dd8357e85ca2e8035dcca527e Mon Sep 17 00:00:00 2001 From: manas Date: Thu, 6 Jul 2023 14:13:49 -1000 Subject: [PATCH 7/7] fixed tests --- test/tokens/ERC721.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tokens/ERC721.t.sol b/test/tokens/ERC721.t.sol index 5605f367..7e855cfe 100644 --- a/test/tokens/ERC721.t.sol +++ b/test/tokens/ERC721.t.sol @@ -119,7 +119,7 @@ contract ERC721Test is Test { assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - vm.expectRevert(bytes("ZERO_ADDRESS")); + vm.expectRevert(bytes("NOT_MINTED")); assertEq(token.ownerOf(1337), address(0)); vm.expectRevert(bytes("NOT_MINTED")); @@ -140,7 +140,7 @@ contract ERC721Test is Test { assertEq(token.balanceOf(address(this)), 0); assertEq(token.getApproved(1337), address(0)); - vm.expectRevert(bytes("ZERO_ADDRESS")); + vm.expectRevert(bytes("NOT_MINTED")); assertEq(token.ownerOf(1337), address(0)); vm.expectRevert(bytes("NOT_MINTED"));