Skip to content

Commit

Permalink
💥 Full ERC721 spec parity
Browse files Browse the repository at this point in the history
  • Loading branch information
@transmissions11
transmissions11 committed Apr 9, 2022
1 parent 0c25a9b commit 921a9ad
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 68 deletions.
60 changes: 31 additions & 29 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,42 +86,44 @@ ERC4626Test:testMintZero() (gas: 54598)
ERC4626Test:testMultipleMintDepositRedeemWithdraw() (gas: 1283937)
ERC4626Test:testVaultInteractionsForSomeoneElse() (gas: 1124924)
ERC4626Test:testWithdrawZero() (gas: 52465)
ERC721Test:testApprove() (gas: 78404)
ERC721Test:testApproveAll() (gas: 31085)
ERC721Test:testApproveBurn() (gas: 63216)
ERC721Test:testBurn() (gas: 43772)
ERC721Test:testFailApproveUnAuthorized() (gas: 55554)
ERC721Test:testFailApproveUnMinted() (gas: 10172)
ERC721Test:testFailBurnUnMinted() (gas: 7879)
ERC721Test:testFailDoubleBurn() (gas: 58965)
ERC721Test:testFailDoubleMint() (gas: 53308)
ERC721Test:testApprove() (gas: 78427)
ERC721Test:testApproveAll() (gas: 31063)
ERC721Test:testApproveBurn() (gas: 65550)
ERC721Test:testBurn() (gas: 46107)
ERC721Test:testFailApproveUnAuthorized() (gas: 55598)
ERC721Test:testFailApproveUnMinted() (gas: 10236)
ERC721Test:testFailBalanceOfZeroAddress() (gas: 5555)
ERC721Test:testFailBurnUnMinted() (gas: 7857)
ERC721Test:testFailDoubleBurn() (gas: 58943)
ERC721Test:testFailDoubleMint() (gas: 53286)
ERC721Test:testFailMintToZero() (gas: 5753)
ERC721Test:testFailSafeMintToERC721RecipientWithWrongReturnData() (gas: 159032)
ERC721Test:testFailSafeMintToERC721RecipientWithWrongReturnDataWithData() (gas: 159853)
ERC721Test:testFailSafeMintToNonERC721Recipient() (gas: 89187)
ERC721Test:testFailSafeMintToNonERC721RecipientWithData() (gas: 89994)
ERC721Test:testFailSafeMintToRevertingERC721Recipient() (gas: 204765)
ERC721Test:testFailOwnerOfUnminted() (gas: 7609)
ERC721Test:testFailSafeMintToERC721RecipientWithWrongReturnData() (gas: 159076)
ERC721Test:testFailSafeMintToERC721RecipientWithWrongReturnDataWithData() (gas: 159831)
ERC721Test:testFailSafeMintToNonERC721Recipient() (gas: 89210)
ERC721Test:testFailSafeMintToNonERC721RecipientWithData() (gas: 89995)
ERC721Test:testFailSafeMintToRevertingERC721Recipient() (gas: 204743)
ERC721Test:testFailSafeMintToRevertingERC721RecipientWithData() (gas: 205517)
ERC721Test:testFailSafeTransferFromToERC721RecipientWithWrongReturnData() (gas: 187276)
ERC721Test:testFailSafeTransferFromToERC721RecipientWithWrongReturnDataWithData() (gas: 187685)
ERC721Test:testFailSafeTransferFromToNonERC721Recipient() (gas: 117435)
ERC721Test:testFailSafeTransferFromToERC721RecipientWithWrongReturnDataWithData() (gas: 187728)
ERC721Test:testFailSafeTransferFromToNonERC721Recipient() (gas: 117413)
ERC721Test:testFailSafeTransferFromToNonERC721RecipientWithData() (gas: 117872)
ERC721Test:testFailSafeTransferFromToRevertingERC721Recipient() (gas: 233009)
ERC721Test:testFailSafeTransferFromToRevertingERC721RecipientWithData() (gas: 233395)
ERC721Test:testFailSafeTransferFromToRevertingERC721RecipientWithData() (gas: 233396)
ERC721Test:testFailTransferFromNotOwner() (gas: 57872)
ERC721Test:testFailTransferFromToZero() (gas: 53403)
ERC721Test:testFailTransferFromToZero() (gas: 53381)
ERC721Test:testFailTransferFromUnOwned() (gas: 8000)
ERC721Test:testFailTransferFromWrongFrom() (gas: 53338)
ERC721Test:testMint() (gas: 54278)
ERC721Test:testSafeMintToEOA() (gas: 56913)
ERC721Test:testSafeMintToERC721Recipient() (gas: 381679)
ERC721Test:testSafeMintToERC721RecipientWithData() (gas: 402801)
ERC721Test:testSafeTransferFromToEOA() (gas: 509430)
ERC721Test:testSafeTransferFromToERC721Recipient() (gas: 853875)
ERC721Test:testSafeTransferFromToERC721RecipientWithData() (gas: 874696)
ERC721Test:testTransferFrom() (gas: 487111)
ERC721Test:testTransferFromApproveAll() (gas: 506662)
ERC721Test:testTransferFromSelf() (gas: 64650)
ERC721Test:testFailTransferFromWrongFrom() (gas: 53361)
ERC721Test:testMint() (gas: 54336)
ERC721Test:testSafeMintToEOA() (gas: 56993)
ERC721Test:testSafeMintToERC721Recipient() (gas: 381737)
ERC721Test:testSafeMintToERC721RecipientWithData() (gas: 402881)
ERC721Test:testSafeTransferFromToEOA() (gas: 509556)
ERC721Test:testSafeTransferFromToERC721Recipient() (gas: 854024)
ERC721Test:testSafeTransferFromToERC721RecipientWithData() (gas: 874822)
ERC721Test:testTransferFrom() (gas: 487215)
ERC721Test:testTransferFromApproveAll() (gas: 506788)
ERC721Test:testTransferFromSelf() (gas: 64776)
FixedPointMathLibTest:testDivWadDown() (gas: 864)
FixedPointMathLibTest:testDivWadDownEdgeCases() (gas: 423)
FixedPointMathLibTest:testDivWadUp() (gas: 981)
Expand Down
20 changes: 0 additions & 20 deletions src/test/ERC1155.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1758,24 +1758,4 @@ contract ERC1155Test is DSTestPlus, ERC1155TokenReceiver {

token.balanceOfBatch(tos, ids);
}

function onERC1155Received(
address,
address,
uint256,
uint256,
bytes calldata
) public pure override returns (bytes4) {
return ERC1155TokenReceiver.onERC1155Received.selector;
}

function onERC1155BatchReceived(
address,
address,
uint256[] calldata,
uint256[] calldata,
bytes calldata
) external pure override returns (bytes4) {
return ERC1155TokenReceiver.onERC1155BatchReceived.selector;
}
}
28 changes: 24 additions & 4 deletions src/test/ERC721.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ contract ERC721Test is DSTestPlus {
token.burn(1337);

assertEq(token.balanceOf(address(0xBEEF)), 0);
assertEq(token.ownerOf(1337), address(0));

hevm.expectRevert("NOT_MINTED");
token.ownerOf(1337);
}

function testApprove() public {
Expand All @@ -97,8 +99,10 @@ contract ERC721Test is DSTestPlus {
token.burn(1337);

assertEq(token.balanceOf(address(this)), 0);
assertEq(token.ownerOf(1337), address(0));
assertEq(token.getApproved(1337), address(0));

hevm.expectRevert("NOT_MINTED");
token.ownerOf(1337);
}

function testApproveAll() public {
Expand Down Expand Up @@ -352,6 +356,14 @@ contract ERC721Test is DSTestPlus {
token.safeMint(address(new WrongReturnDataERC721Recipient()), 1337, "testing 123");
}

function testFailBalanceOfZeroAddress() public view {
token.balanceOf(address(0));
}

function testFailOwnerOfUnminted() public view {
token.ownerOf(1337);
}

function testMetadata(string memory name, string memory symbol) public {
MockERC721 tkn = new MockERC721(name, symbol);

Expand All @@ -375,7 +387,9 @@ contract ERC721Test is DSTestPlus {
token.burn(id);

assertEq(token.balanceOf(to), 0);
assertEq(token.ownerOf(id), address(0));

hevm.expectRevert("NOT_MINTED");
token.ownerOf(id);
}

function testApprove(address to, uint256 id) public {
Expand All @@ -396,8 +410,10 @@ contract ERC721Test is DSTestPlus {
token.burn(id);

assertEq(token.balanceOf(address(this)), 0);
assertEq(token.ownerOf(id), address(0));
assertEq(token.getApproved(id), address(0));

hevm.expectRevert("NOT_MINTED");
token.ownerOf(id);
}

function testApproveAll(address to, bool approved) public {
Expand Down Expand Up @@ -694,4 +710,8 @@ contract ERC721Test is DSTestPlus {
function testFailSafeMintToERC721RecipientWithWrongReturnDataWithData(uint256 id, bytes calldata data) public {
token.safeMint(address(new WrongReturnDataERC721Recipient()), id, data);
}

function testFailOwnerOfUnminted(uint256 id) public view {
token.ownerOf(id);
}
}
43 changes: 28 additions & 15 deletions src/tokens/ERC721.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pragma solidity >=0.8.0;

/// @notice Modern, minimalist, and gas efficient ERC-721 implementation.
/// @author Solmate (https://github.com/Rari-Capital/solmate/blob/main/src/tokens/ERC721.sol)
/// @dev Note that balanceOf does not revert if passed the zero address, in defiance of the ERC.
abstract contract ERC721 {
/*//////////////////////////////////////////////////////////////
EVENTS
Expand All @@ -26,12 +25,26 @@ abstract contract ERC721 {
function tokenURI(uint256 id) public view virtual returns (string memory);

/*//////////////////////////////////////////////////////////////
ERC721 STORAGE
ERC721 BALANCE/OWNER STORAGE
//////////////////////////////////////////////////////////////*/

mapping(address => uint256) public balanceOf;
mapping(address => uint256) internal _balanceOf;

mapping(uint256 => address) public ownerOf;
mapping(uint256 => address) internal _ownerOf;

function balanceOf(address owner) public view returns (uint256) {
require(owner != address(0), "ZERO_ADDRESS");

return _balanceOf[owner];
}

function ownerOf(uint256 id) public view returns (address owner) {
require((owner = _ownerOf[id]) != address(0), "NOT_MINTED");
}

/*//////////////////////////////////////////////////////////////
ERC721 APPROVAL STORAGE
//////////////////////////////////////////////////////////////*/

mapping(uint256 => address) public getApproved;

Expand All @@ -51,7 +64,7 @@ abstract contract ERC721 {
//////////////////////////////////////////////////////////////*/

function approve(address spender, uint256 id) public virtual {
address owner = ownerOf[id];
address owner = _ownerOf[id];

require(msg.sender == owner || isApprovedForAll[owner][msg.sender], "NOT_AUTHORIZED");

Expand All @@ -71,7 +84,7 @@ abstract contract ERC721 {
address to,
uint256 id
) public virtual {
require(from == ownerOf[id], "WRONG_FROM");
require(from == _ownerOf[id], "WRONG_FROM");

require(to != address(0), "INVALID_RECIPIENT");

Expand All @@ -83,12 +96,12 @@ abstract contract ERC721 {
// Underflow of the sender's balance is impossible because we check for
// ownership above and the recipient's balance can't realistically overflow.
unchecked {
balanceOf[from]--;
_balanceOf[from]--;

balanceOf[to]++;
_balanceOf[to]++;
}

ownerOf[id] = to;
_ownerOf[id] = to;

delete getApproved[id];

Expand Down Expand Up @@ -144,29 +157,29 @@ abstract contract ERC721 {
function _mint(address to, uint256 id) internal virtual {
require(to != address(0), "INVALID_RECIPIENT");

require(ownerOf[id] == address(0), "ALREADY_MINTED");
require(_ownerOf[id] == address(0), "ALREADY_MINTED");

// Counter overflow is incredibly unrealistic.
unchecked {
balanceOf[to]++;
_balanceOf[to]++;
}

ownerOf[id] = to;
_ownerOf[id] = to;

emit Transfer(address(0), to, id);
}

function _burn(uint256 id) internal virtual {
address owner = ownerOf[id];
address owner = _ownerOf[id];

require(owner != address(0), "NOT_MINTED");

// Ownership check above ensures no underflow.
unchecked {
balanceOf[owner]--;
_balanceOf[owner]--;
}

delete ownerOf[id];
delete _ownerOf[id];

delete getApproved[id];

Expand Down

0 comments on commit 921a9ad

Please sign in to comment.