diff --git a/.changeset/kind-fireants-battle.md b/.changeset/kind-fireants-battle.md new file mode 100644 index 0000000000..08df8f5e30 --- /dev/null +++ b/.changeset/kind-fireants-battle.md @@ -0,0 +1,5 @@ +--- +"@latticexyz/world-modules": patch +--- + +Fixed ERC721 module to properly encode token ID as part of token URI. diff --git a/packages/world-modules/src/modules/erc721-puppet/ERC721System.sol b/packages/world-modules/src/modules/erc721-puppet/ERC721System.sol index ab79d31566..bb8c58f16a 100644 --- a/packages/world-modules/src/modules/erc721-puppet/ERC721System.sol +++ b/packages/world-modules/src/modules/erc721-puppet/ERC721System.sol @@ -21,9 +21,11 @@ import { TokenApproval } from "./tables/TokenApproval.sol"; import { TokenURI } from "./tables/TokenURI.sol"; import { _balancesTableId, _metadataTableId, _tokenUriTableId, _operatorApprovalTableId, _ownersTableId, _tokenApprovalTableId } from "./utils.sol"; +import { LibString } from "./libraries/LibString.sol"; contract ERC721System is IERC721Mintable, System, PuppetMaster { using WorldResourceIdInstance for ResourceId; + using LibString for uint256; /** * @dev See {IERC721-balanceOf}. @@ -64,7 +66,7 @@ contract ERC721System is IERC721Mintable, System, PuppetMaster { string memory baseURI = _baseURI(); string memory _tokenURI = TokenURI.get(_tokenUriTableId(_namespace()), tokenId); - _tokenURI = bytes(_tokenURI).length > 0 ? _tokenURI : string(abi.encodePacked(tokenId)); + _tokenURI = bytes(_tokenURI).length > 0 ? _tokenURI : tokenId.toString(); return bytes(baseURI).length > 0 ? string.concat(baseURI, _tokenURI) : _tokenURI; } diff --git a/packages/world-modules/src/modules/erc721-puppet/libraries/LibString.sol b/packages/world-modules/src/modules/erc721-puppet/libraries/LibString.sol new file mode 100644 index 0000000000..e7319c512d --- /dev/null +++ b/packages/world-modules/src/modules/erc721-puppet/libraries/LibString.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.8.0; + +/// @notice Efficient library for creating string representations of integers. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/utils/LibString.sol) +/// @author Modified from Solady (https://github.com/Vectorized/solady/blob/main/src/utils/LibString.sol) +library LibString { + function toString(int256 value) internal pure returns (string memory str) { + if (value >= 0) return toString(uint256(value)); + + unchecked { + str = toString(uint256(-value)); + + /// @solidity memory-safe-assembly + assembly { + // Note: This is only safe because we over-allocate memory + // and write the string from right to left in toString(uint256), + // and thus can be sure that sub(str, 1) is an unused memory location. + + let length := mload(str) // Load the string length. + // Put the - character at the start of the string contents. + mstore(str, 45) // 45 is the ASCII code for the - character. + str := sub(str, 1) // Move back the string pointer by a byte. + mstore(str, add(length, 1)) // Update the string length. + } + } + } + + function toString(uint256 value) internal pure returns (string memory str) { + /// @solidity memory-safe-assembly + assembly { + // The maximum value of a uint256 contains 78 digits (1 byte per digit), but we allocate 160 bytes + // to keep the free memory pointer word aligned. We'll need 1 word for the length, 1 word for the + // trailing zeros padding, and 3 other words for a max of 78 digits. In total: 5 * 32 = 160 bytes. + let newFreeMemoryPointer := add(mload(0x40), 160) + + // Update the free memory pointer to avoid overriding our string. + mstore(0x40, newFreeMemoryPointer) + + // Assign str to the end of the zone of newly allocated memory. + str := sub(newFreeMemoryPointer, 32) + + // Clean the last word of memory it may not be overwritten. + mstore(str, 0) + + // Cache the end of the memory to calculate the length later. + let end := str + + // We write the string from rightmost digit to leftmost digit. + // The following is essentially a do-while loop that also handles the zero case. + // prettier-ignore + for { let temp := value } 1 {} { + // Move the pointer 1 byte to the left. + str := sub(str, 1) + + // Write the character to the pointer. + // The ASCII index of the '0' character is 48. + mstore8(str, add(48, mod(temp, 10))) + + // Keep dividing temp until zero. + temp := div(temp, 10) + + // prettier-ignore + if iszero(temp) { break } + } + + // Compute and cache the final total length of the string. + let length := sub(end, str) + + // Move the pointer 32 bytes leftwards to make room for the length. + str := sub(str, 32) + + // Store the string's length at the start of memory allocated for our string. + mstore(str, length) + } + } +} diff --git a/packages/world-modules/test/ERC721.t.sol b/packages/world-modules/test/ERC721.t.sol index 80899fffb0..92d7ad73ac 100644 --- a/packages/world-modules/test/ERC721.t.sol +++ b/packages/world-modules/test/ERC721.t.sol @@ -18,6 +18,7 @@ import { PuppetModule } from "../src/modules/puppet/PuppetModule.sol"; import { ERC721Module } from "../src/modules/erc721-puppet/ERC721Module.sol"; import { ERC721MetadataData } from "../src/modules/erc721-puppet/tables/ERC721Metadata.sol"; import { IERC721Mintable } from "../src/modules/erc721-puppet/IERC721Mintable.sol"; +import { IERC721Metadata } from "../src/modules/erc721-puppet/IERC721Metadata.sol"; import { registerERC721 } from "../src/modules/erc721-puppet/registerERC721.sol"; import { IERC721Errors } from "../src/modules/erc721-puppet/IERC721Errors.sol"; import { IERC721Events } from "../src/modules/erc721-puppet/IERC721Events.sol"; @@ -77,7 +78,11 @@ contract ERC721Test is Test, GasReporter, IERC721Events, IERC721Errors { StoreSwitch.setStoreAddress(address(world)); // Register a new ERC721 token - token = registerERC721(world, "myERC721", ERC721MetadataData({ name: "Token", symbol: "TKN", baseURI: "" })); + token = registerERC721( + world, + "myERC721", + ERC721MetadataData({ name: "Token", symbol: "TKN", baseURI: "base-uri/" }) + ); } function _expectAccessDenied(address caller) internal { @@ -164,6 +169,15 @@ contract ERC721Test is Test, GasReporter, IERC721Events, IERC721Errors { assertEq(token.ownerOf(id), owner); } + function testTokenURI(address owner) public { + vm.assume(owner != address(0)); + + token.mint(owner, 1); + + IERC721Metadata tokenMetadata = IERC721Metadata(address(token)); + assertEq(tokenMetadata.tokenURI(1), "base-uri/1"); + } + function testMintRevertAccessDenied(uint256 id, address owner, address operator) public { _assumeDifferentNonZero(owner, operator, address(this));