Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
44 changes: 36 additions & 8 deletions contracts/src/CollectionsERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
pragma solidity 0.8.24;

// import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "./ERC721EthscriptionsUpgradeable.sol";
import "./ERC721EthscriptionsEnumerableUpgradeable.sol";
// import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
// import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "./Ethscriptions.sol";
import {LibString} from "solady/utils/LibString.sol";
import {Base64} from "solady/utils/Base64.sol";
import "./CollectionsProtocolHandler.sol";
import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import {IERC721Metadata} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";

/// @title CollectionsERC721
/// @notice ERC-721 contract for an Ethscription collection
/// @dev Maintains internal state but overrides ownerOf to delegate to Ethscriptions contract
contract CollectionsERC721 is ERC721EthscriptionsUpgradeable {
contract CollectionsERC721 is ERC721EthscriptionsEnumerableUpgradeable {
using LibString for *;

/// @notice The main Ethscriptions contract
Expand Down Expand Up @@ -118,7 +120,12 @@ contract CollectionsERC721 is ERC721EthscriptionsUpgradeable {
}

// Override ownerOf to delegate to Ethscriptions contract
function ownerOf(uint256 tokenId) public view override returns (address) {
function ownerOf(uint256 tokenId)
public
view
override(ERC721EthscriptionsUpgradeable, IERC721)
returns (address)
{
// Check if token exists in collection
if (!_tokenExists(tokenId)) {
revert("Token does not exist");
Expand All @@ -137,7 +144,12 @@ contract CollectionsERC721 is ERC721EthscriptionsUpgradeable {
}

// Override tokenURI to generate full metadata JSON
function tokenURI(uint256 tokenId) public view override returns (string memory) {
function tokenURI(uint256 tokenId)
public
view
override(ERC721EthscriptionsUpgradeable)
returns (string memory)
{
if (!_tokenExists(tokenId)) {
revert("Token does not exist");
}
Expand Down Expand Up @@ -221,20 +233,36 @@ contract CollectionsERC721 is ERC721EthscriptionsUpgradeable {
}

// Block external transfers - only internal _transfer is allowed for syncing
function transferFrom(address, address, uint256) public pure override {
function transferFrom(address, address, uint256)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransferNotAllowed();
}

function safeTransferFrom(address, address, uint256, bytes memory) public pure override {
function safeTransferFrom(address, address, uint256, bytes memory)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransferNotAllowed();
}

// Block approvals - not needed for non-transferable tokens
function approve(address, uint256) public pure override {
function approve(address, uint256)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransferNotAllowed();
}

function setApprovalForAll(address, bool) public pure override {
function setApprovalForAll(address, bool)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransferNotAllowed();
}
}
67 changes: 67 additions & 0 deletions contracts/src/ERC721EthscriptionsEnumerableUpgradeable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import "./ERC721EthscriptionsUpgradeable.sol";
import {IERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";

/**
* @dev Enumerable mixin for collections that require general token ID tracking and burns.
*/
abstract contract ERC721EthscriptionsEnumerableUpgradeable is ERC721EthscriptionsUpgradeable, IERC721Enumerable {
/// @inheritdoc ERC165Upgradeable
function supportsInterface(bytes4 interfaceId)
public
view
virtual
override(ERC721EthscriptionsUpgradeable, IERC165)
returns (bool)
{
return interfaceId == type(IERC721Enumerable).interfaceId || super.supportsInterface(interfaceId);
}

/// @inheritdoc IERC721Enumerable
function tokenOfOwnerByIndex(address owner, uint256 index)
public
view
virtual
override(IERC721Enumerable, ERC721EthscriptionsUpgradeable)
returns (uint256)
{
return super.tokenOfOwnerByIndex(owner, index);
}

/// @inheritdoc IERC721Enumerable
function totalSupply() public view virtual override returns (uint256) {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
return $._allTokens.length;
}

/// @inheritdoc IERC721Enumerable
function tokenByIndex(uint256 index) public view virtual override returns (uint256) {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
if (index >= $._allTokens.length) {
revert ERC721OutOfBoundsIndex(address(0), index);
}
return $._allTokens[index];
}

function _afterTokenMint(uint256 tokenId) internal virtual override {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
$._allTokensIndex[tokenId] = $._allTokens.length;
$._allTokens.push(tokenId);
}

function _beforeTokenRemoval(uint256 tokenId, address) internal virtual override {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
uint256 tokenIndex = $._allTokensIndex[tokenId];
uint256 lastTokenIndex = $._allTokens.length - 1;
uint256 lastTokenId = $._allTokens[lastTokenIndex];

$._allTokens[tokenIndex] = lastTokenId;
$._allTokensIndex[lastTokenId] = tokenIndex;

delete $._allTokensIndex[tokenId];
$._allTokens.pop();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import "./ERC721EthscriptionsUpgradeable.sol";
import {IERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";

/**
* @dev Enumerable mixin for Ethscriptions-style collections where token IDs are
* sequential, start at zero, and tokens are never burned.
*/
abstract contract ERC721EthscriptionsSequentialEnumerableUpgradeable is ERC721EthscriptionsUpgradeable, IERC721Enumerable {
/// @dev Raised when a mint attempts to skip or reuse a token ID.
error ERC721SequentialEnumerableInvalidTokenId(uint256 expected, uint256 actual);
/// @dev Raised if a contract attempts to remove a token from supply.
error ERC721SequentialEnumerableTokenRemoval(uint256 tokenId);

/// @custom:storage-location erc7201:ethscriptions.storage.ERC721SequentialEnumerable
struct ERC721SequentialEnumerableStorage {
uint256 _mintCount;
}

function _getERC721SequentialEnumerableStorage()
private
pure
returns (ERC721SequentialEnumerableStorage storage $)
{
bytes32 slot = keccak256(abi.encode("ethscriptions.storage.ERC721SequentialEnumerable"));
Comment thread
RogerPodacter marked this conversation as resolved.
Outdated
assembly {
$.slot := slot
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: ERC-7201 Storage Slot Mismatch

The _getERC721SequentialEnumerableStorage function calculates its storage slot using a simple keccak256 hash. This deviates from the ERC-7201 standard indicated by the @custom:storage-location comment, potentially causing storage slot collisions and inconsistencies with other ERC-7201 compliant implementations.

Fix in Cursor Fix in Web


/// @inheritdoc ERC165Upgradeable
function supportsInterface(bytes4 interfaceId)
public
view
virtual
override(ERC721EthscriptionsUpgradeable, IERC165)
returns (bool)
{
return interfaceId == type(IERC721Enumerable).interfaceId || super.supportsInterface(interfaceId);
}

/// @inheritdoc IERC721Enumerable
function tokenOfOwnerByIndex(address owner, uint256 index)
public
view
virtual
override(IERC721Enumerable, ERC721EthscriptionsUpgradeable)
returns (uint256)
{
return super.tokenOfOwnerByIndex(owner, index);
}

/// @inheritdoc IERC721Enumerable
function totalSupply() public view virtual override returns (uint256) {
ERC721SequentialEnumerableStorage storage $ = _getERC721SequentialEnumerableStorage();
return $._mintCount;
}

/// @inheritdoc IERC721Enumerable
function tokenByIndex(uint256 index) public view virtual override returns (uint256) {
if (index >= totalSupply()) {
revert ERC721OutOfBoundsIndex(address(0), index);
}
return index;
}

function _afterTokenMint(uint256 tokenId) internal virtual override {
ERC721SequentialEnumerableStorage storage $ = _getERC721SequentialEnumerableStorage();

uint256 expectedId = $._mintCount;
if (tokenId != expectedId) {
revert ERC721SequentialEnumerableInvalidTokenId(expectedId, tokenId);
}

unchecked {
$._mintCount = expectedId + 1;
}
}

function _beforeTokenRemoval(uint256 tokenId, address) internal virtual override {
revert ERC721SequentialEnumerableTokenRemoval(tokenId);
}
}
70 changes: 14 additions & 56 deletions contracts/src/ERC721EthscriptionsUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pragma solidity 0.8.24;

import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import {IERC721Metadata} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import {IERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";
import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import {ERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol";
Expand All @@ -22,7 +21,7 @@ import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Ini
* - No burn function (transfer to address(0) instead)
* - Keeps only core transfer and ownership logic
*/
abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgradeable, ERC165Upgradeable, IERC721, IERC721Metadata, IERC721Enumerable, IERC721Errors {
abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgradeable, ERC165Upgradeable, IERC721, IERC721Metadata, IERC721Errors {
// Errors for enumerable functionality
error ERC721OutOfBoundsIndex(address owner, uint256 index);
error ERC721EnumerableForbiddenBatchMint();
Expand Down Expand Up @@ -62,7 +61,7 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
// keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC721Enumerable")) - 1)) & ~bytes32(uint256(0xff))
bytes32 private constant ERC721EnumerableStorageLocation = 0x645e039705490088daad89bae25049a34f4a9072d398537b1ab2425f24cbed00;

function _getERC721EnumerableStorage() private pure returns (ERC721EnumerableStorage storage $) {
function _getERC721EnumerableStorage() internal pure returns (ERC721EnumerableStorage storage $) {
assembly {
$.slot := ERC721EnumerableStorageLocation
}
Expand All @@ -88,7 +87,6 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
return
interfaceId == type(IERC721).interfaceId ||
interfaceId == type(IERC721Metadata).interfaceId ||
interfaceId == type(IERC721Enumerable).interfaceId ||
super.supportsInterface(interfaceId);
}

Expand Down Expand Up @@ -132,13 +130,7 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
*/
function tokenURI(uint256 tokenId) public view virtual returns (string memory);

/// @inheritdoc IERC721Enumerable
function totalSupply() public view virtual returns (uint256) {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
return $._allTokens.length;
}

/// @inheritdoc IERC721Enumerable
/// @dev Return token owned by `owner` at `index`.
function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual returns (uint256) {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
if (index >= balanceOf(owner)) {
Expand All @@ -147,15 +139,6 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
return $._ownedTokens[owner][index];
}

/// @inheritdoc IERC721Enumerable
function tokenByIndex(uint256 index) public view virtual returns (uint256) {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
if (index >= totalSupply()) {
revert ERC721OutOfBoundsIndex(address(0), index);
}
return $._allTokens[index];
}

/**
* @dev Approval functions removed - not needed for Ethscriptions.
* These can be added back in child contracts if needed.
Expand Down Expand Up @@ -256,8 +239,8 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
// This is a mint
$._existsFlag[tokenId] = true;

// Add to all tokens enumeration
_addTokenToAllTokensEnumeration(tokenId);
// Allow derived contracts to adjust enumeration state for newly minted token
_afterTokenMint(tokenId);

// Add to owner enumeration (also increments balance)
_addTokenToOwnerEnumeration(to, tokenId);
Expand Down Expand Up @@ -335,9 +318,10 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
if (!exists && $._existsFlag[tokenId]) {
address owner = $._owners[tokenId];

_beforeTokenRemoval(tokenId, owner);

// Remove from enumerations (balance is decremented inside _removeTokenFromOwnerEnumeration)
_removeTokenFromOwnerEnumeration(owner, tokenId);
_removeTokenFromAllTokensEnumeration(tokenId);

// Clear owner storage for cleanliness
delete $._owners[tokenId];
Expand Down Expand Up @@ -378,16 +362,6 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
}
}

/**
* @dev Private function to add a token to this extension's token tracking data structures.
* @param tokenId uint256 ID of the token to be added to the tokens list
*/
function _addTokenToAllTokensEnumeration(uint256 tokenId) private {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
$._allTokensIndex[tokenId] = $._allTokens.length;
$._allTokens.push(tokenId);
}

/**
* @dev Private function to remove a token from this extension's ownership-tracking data structures.
* Also decrements the owner's balance.
Expand Down Expand Up @@ -429,28 +403,12 @@ abstract contract ERC721EthscriptionsUpgradeable is Initializable, ContextUpgrad
}

/**
* @dev Private function to remove a token from this extension's token tracking data structures.
* This has O(1) time complexity, but alters the order of the _allTokens array.
* @param tokenId uint256 ID of the token to be removed from the tokens list
* @dev Hook for derived contracts to react to token minting.
*/
function _removeTokenFromAllTokensEnumeration(uint256 tokenId) private {
ERC721EnumerableStorage storage $ = _getERC721EnumerableStorage();
// To prevent a gap in the tokens array, we store the last token in the index of the token to delete, and
// then delete the last slot (swap and pop).

uint256 lastTokenIndex = $._allTokens.length - 1;
uint256 tokenIndex = $._allTokensIndex[tokenId];

// When the token to delete is the last token, the swap operation is unnecessary. However, since this occurs so
// rarely (when the last minted token is burnt) that we still do the swap here to avoid the gas cost of adding
// an 'if' statement (like in _removeTokenFromOwnerEnumeration)
uint256 lastTokenId = $._allTokens[lastTokenIndex];
function _afterTokenMint(uint256) internal virtual {}

$._allTokens[tokenIndex] = lastTokenId; // Move the last token to the slot of the to-delete token
$._allTokensIndex[lastTokenId] = tokenIndex; // Update the moved token's index

// This also deletes the contents at the last position of the array
delete $._allTokensIndex[tokenId];
$._allTokens.pop();
}
}
/**
* @dev Hook for derived contracts to react to token removal.
*/
function _beforeTokenRemoval(uint256, address) internal virtual {}
}
Loading