From 764b87f4d83fc803b19f346536e175d58d451370 Mon Sep 17 00:00:00 2001 From: 0xth0mas <35287002+0xth0mas@users.noreply.github.com> Date: Fri, 31 May 2024 18:29:01 -0500 Subject: [PATCH] change function selectors Some services that inspect bytecode to determine the token type have falsely identified the base contract as an ERC721 due to the existence of certain function selectors in the fallback function. This change updates the function selectors for communication between the mirror contract and the base contract to avoid any overlap with ERC721 function selectors. It has been tested in a production environment and the services that were previously picking up the base ERC20 tokens as ERC721 no longer identify them as such. --- src/DN404.sol | 24 ++++++++++++------------ src/DN404Mirror.sol | 14 +++++++------- test/DN404.t.sol | 4 +++- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/DN404.sol b/src/DN404.sol index 8086822..c6fd5e2 100644 --- a/src/DN404.sol +++ b/src/DN404.sol @@ -1200,8 +1200,8 @@ abstract contract DN404 { ); _return(1); } - // `setApprovalForAll(address,bool,address)`. - if (fnSelector == 0x813500fc) { + // `setApprovalForAllNFT(address,bool,address)`. + if (fnSelector == 0xf6916ddd) { if (msg.sender != $.mirrorERC721) revert SenderNotMirror(); _setApprovalForAll( address(uint160(_calldataload(0x04))), // `spender`. @@ -1210,20 +1210,20 @@ abstract contract DN404 { ); _return(1); } - // `isApprovedForAll(address,address)`. - if (fnSelector == 0xe985e9c5) { + // `isApprovedForAllNFT(address,address)`. + if (fnSelector == 0x62fb246d) { bool result = _isApprovedForAll( address(uint160(_calldataload(0x04))), // `owner`. address(uint160(_calldataload(0x24))) // `operator`. ); _return(_toUint(result)); } - // `ownerOf(uint256)`. - if (fnSelector == 0x6352211e) { + // `ownerOfNFT(uint256)`. + if (fnSelector == 0x2d8a746e) { _return(uint160(_ownerOf(_calldataload(0x04)))); } - // `ownerAt(uint256)`. - if (fnSelector == 0x24359879) { + // `ownerAtNFT(uint256)`. + if (fnSelector == 0xc016aa52) { _return(uint160(_ownerAt(_calldataload(0x04)))); } // `approveNFT(address,uint256,address)`. @@ -1236,8 +1236,8 @@ abstract contract DN404 { ); _return(uint160(owner)); } - // `getApproved(uint256)`. - if (fnSelector == 0x081812fc) { + // `getApprovedNFT(uint256)`. + if (fnSelector == 0x27ef5495) { _return(uint160(_getApproved(_calldataload(0x04)))); } // `balanceOfNFT(address)`. @@ -1248,8 +1248,8 @@ abstract contract DN404 { if (fnSelector == 0xe2c79281) { _return(_totalNFTSupply()); } - // `tokenURI(uint256)`. - if (fnSelector == 0xc87b56dd) { + // `tokenURINFT(uint256)`. + if (fnSelector == 0xcb30b460) { /// @solidity memory-safe-assembly assembly { mstore(0x40, add(mload(0x40), 0x20)) diff --git a/src/DN404Mirror.sol b/src/DN404Mirror.sol index 20a2373..499f0ae 100644 --- a/src/DN404Mirror.sol +++ b/src/DN404Mirror.sol @@ -125,7 +125,7 @@ contract DN404Mirror { ownerOf(id); // `ownerOf` reverts if the token does not exist. // We'll leave if optional for `_tokenURI` to revert for non-existent token // on the ERC20 side, since this is only recommended by the ERC721 standard. - return _readString(0xc87b56dd, id); // `tokenURI(uint256)`. + return _readString(0xcb30b460, id); // `tokenURINFT(uint256)`. } /// @dev Returns the total NFT supply from the base DN404 contract. @@ -146,13 +146,13 @@ contract DN404Mirror { /// Requirements: /// - Token `id` must exist. function ownerOf(uint256 id) public view virtual returns (address) { - return address(uint160(_readWord(0x6352211e, id, 0))); // `ownerOf(uint256)`. + return address(uint160(_readWord(0x2d8a746e, id, 0))); // `ownerOfNFT(uint256)`. } /// @dev Returns the owner of token `id` from the base DN404 contract. /// Returns `address(0)` instead of reverting if the token does not exist. function ownerAt(uint256 id) public view virtual returns (address) { - return address(uint160(_readWord(0x24359879, id, 0))); // `ownerAt(uint256)`. + return address(uint160(_readWord(0xc016aa52, id, 0))); // `ownerAtNFT(uint256)`. } /// @dev Sets `spender` as the approved account to manage token `id` in @@ -196,7 +196,7 @@ contract DN404Mirror { /// Requirements: /// - Token `id` must exist. function getApproved(uint256 id) public view virtual returns (address) { - return address(uint160(_readWord(0x081812fc, id, 0))); // `getApproved(uint256)`. + return address(uint160(_readWord(0x27ef5495, id, 0))); // `getApprovedNFT(uint256)`. } /// @dev Sets whether `operator` is approved to manage the tokens of the caller in @@ -209,7 +209,7 @@ contract DN404Mirror { assembly { operator := shr(96, shl(96, operator)) let m := mload(0x40) - mstore(0x00, 0x813500fc) // `setApprovalForAll(address,bool,address)`. + mstore(0x00, 0xf6916ddd) // `setApprovalForAllNFT(address,bool,address)`. mstore(0x20, operator) mstore(0x40, iszero(iszero(approved))) mstore(0x60, caller()) @@ -238,8 +238,8 @@ contract DN404Mirror { virtual returns (bool) { - // `isApprovedForAll(address,address)`. - return _readWord(0xe985e9c5, uint160(nftOwner), uint160(operator)) != 0; + // `isApprovedForAllNFT(address,address)`. + return _readWord(0x62fb246d, uint160(nftOwner), uint160(operator)) != 0; } /// @dev Transfers token `id` from `from` to `to`. diff --git a/test/DN404.t.sol b/test/DN404.t.sol index bcf834e..0a47a32 100644 --- a/test/DN404.t.sol +++ b/test/DN404.t.sol @@ -38,10 +38,12 @@ contract DN404Test is SoladyTest { } function testTokenURI(string memory baseURI, uint256 id) public { + id = _bound(id, 1, 1000); dn.initializeDN404(1000 * _WAD, address(this), address(mirror)); + dn.transfer(address(0xbeef), 1000 * _WAD); dn.setBaseURI(baseURI); string memory expected = string(abi.encodePacked(baseURI, id)); - assertEq(DN404Mirror(payable(address(dn))).tokenURI(id), expected); + assertEq(DN404Mirror(payable(address(mirror))).tokenURI(id), expected); } function testRegisterAndResolveAlias(address a0, address a1) public {