Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(world-modules): properly concat baseURI and tokenURI in ERC721 module #2686

Merged
merged 4 commits into from
Apr 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/kind-fireants-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@latticexyz/world-modules": patch
---

Fixed ERC721 module to properly encode token ID as part of token URI.
Original file line number Diff line number Diff line change
@@ -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;
}

Original file line number Diff line number Diff line change
@@ -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)
}
}
}
16 changes: 15 additions & 1 deletion packages/world-modules/test/ERC721.t.sol
Original file line number Diff line number Diff line change
@@ -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));