Skip to content

Refactor ERC404NullOwnerCappedUpgradeable contract by removing unused… #150

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.

Sign up for GitHub

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

Merged
RogerPodacter merged 1 commit into from
Nov 25, 2025
Merged

Refactor ERC404NullOwnerCappedUpgradeable contract by removing unused… #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
160 changes: 34 additions & 126 deletions contracts/src/ERC404NullOwnerCappedUpgradeable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@ pragma solidity 0.8.24;

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
import {IERC20Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
import "./interfaces/IERC404.sol";
import "./lib/DoubleEndedQueue.sol";

/// @title ERC404NullOwnerCappedUpgradeable
/// @notice Hybrid ERC20/ERC721 implementation with null owner support, supply cap, and upgradeability
Expand All @@ -20,11 +17,8 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
IERC165,
IERC20,
IERC20Metadata,
IERC20Errors,
IERC404
IERC20Errors
{
using DoubleEndedQueue for DoubleEndedQueue.Uint256Deque;

struct TokenData {
address owner; // current owner (can be address(0) for null-owner)
uint88 index; // position in owned[owner] array
Expand All @@ -43,44 +37,26 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
uint256 totalSupply;
uint256 cap;

// === ERC404 NFT State ===
DoubleEndedQueue.Uint256Deque storedERC721Ids;
mapping(address => uint256[]) owned;
mapping(uint256 => TokenData) tokens;
mapping(uint256 => address) getApproved;
mapping(address => mapping(address => bool)) isApprovedForAll;
mapping(address => bool) erc721TransferExempt;
uint256 minted; // Number of NFTs minted
uint256 units; // Units for NFT minting (e.g., 1000 * 10^18)
uint256 initialChainId;
bytes32 initialDomainSeparator;
mapping(address => uint256) nonces;

// === Metadata ===
string name;
string symbol;
}

// =============================================================
// CONSTANTS
// =============================================================

/// @dev Unique storage slot for EIP-7201 namespaced storage
/// keccak256(abi.encode(uint256(keccak256("ethscriptions.storage.ERC404NullOwnerCapped")) - 1)) & ~bytes32(uint256(0xff))
bytes32 private constant STORAGE_LOCATION = 0x8a0c9d8e5f7b3a2c1d4e6f8a9b7c5d3e2f1a4b6c8d9e7f5a3b2c1d4e6f8a9b00;


Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Trailing whitespace detected. The line contains only spaces after the closing brace, which violates code formatting standards.

Suggested change

Copilot uses AI. Check for mistakes.
// =============================================================
// EVENTS
// =============================================================

// ERC20 Events are inherited from IERC20 (Transfer, Approval)

// ERC721 Events (using different names to avoid conflicts with ERC20)
// event Transfer(address indexed from, address indexed to, uint256 value);
event ERC20Transfer(address indexed from, address indexed to, uint256 value);
event ERC721Transfer(address indexed from, address indexed to, uint256 indexed id);
event ERC721Approval(address indexed owner, address indexed spender, uint256 indexed id);
event ApprovalForAll(address indexed owner, address indexed operator, bool approved);

// =============================================================
// CUSTOM ERRORS
Expand All @@ -91,14 +67,22 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
error ERC20InvalidCap(uint256 cap);
error InvalidUnits(uint256 units);
error NotImplemented();
error NotFound();
error InvalidTokenId();
error AlreadyExists();
error InvalidRecipient();
error Unauthorized();
error OwnedIndexOverflow();

// =============================================================
// STORAGE ACCESSOR
// =============================================================

function _getS() internal pure returns (TokenStorage storage $) {
bytes32 slot = keccak256("ethscriptions.storage.ERC404NullOwnerCapped");
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The storage slot calculation has been simplified from a constant to runtime computation, but this changes the actual storage slot location. The old implementation used a constant STORAGE_LOCATION = 0x8a0c9d8e5f7b3a2c1d4e6f8a9b7c5d3e2f1a4b6c8d9e7f5a3b2c1d4e6f8a9b00 (supposedly following EIP-7201's formula), while the new code directly hashes the namespace string.

Critical Issue: If this contract has already been deployed, this change will cause the contract to read from/write to a completely different storage slot, resulting in:

  1. Loss of access to existing state (balances, tokens, etc.)
  2. Potential corruption of unrelated storage slots
  3. Breaking existing deployed proxies

If this is a new deployment: Verify that the new slot calculation matches EIP-7201 specification: keccak256(abi.encode(uint256(keccak256("namespace")) - 1)) & ~bytes32(uint256(0xff)). The current implementation keccak256("namespace") does not follow this formula.

Recommend either:

  • Reverting to the constant if contracts are already deployed
  • Using the proper EIP-7201 formula if this is intentional
  • Documenting if EIP-7201 compliance is not required
Suggested change
bytes32 slot = keccak256("ethscriptions.storage.ERC404NullOwnerCapped");
bytes32 slot = keccak256(abi.encode(uint256(keccak256("ethscriptions.storage.ERC404NullOwnerCapped")) - 1)) & ~bytes32(uint256(0xff));

Copilot uses AI. Check for mistakes.

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Trailing whitespace detected. The line contains only spaces, which violates code formatting standards.

Suggested change

Copilot uses AI. Check for mistakes.
assembly {
$.slot := STORAGE_LOCATION
$.slot := slot
Copy link

@cursor cursor bot Nov 25, 2025

Choose a reason for hiding this comment

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

Bug: Storage slot calculation breaks EIP-7201 compatibility for upgrades

The _getS() function now computes the storage slot using simple keccak256("ethscriptions.storage.ERC404NullOwnerCapped"), but the original code used the proper EIP-7201 formula: keccak256(abi.encode(uint256(keccak256(id)) - 1)) & ~bytes32(uint256(0xff)) which produced 0x8a0c9d8e5f7b3a2c1d4e6f8a9b7c5d3e2f1a4b6c8d9e7f5a3b2c1d4e6f8a9b00. These produce completely different storage slots. For this upgradeable contract, any upgrade would cause the contract to read/write to a different storage location, making all existing state data (balances, tokens, ownership) completely inaccessible.

Fix in Cursor Fix in Web

}
}

Expand Down Expand Up @@ -132,38 +116,36 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
$.symbol = symbol_;
$.cap = cap_;
$.units = units_;
$.initialChainId = block.chainid;
$.initialDomainSeparator = _computeDomainSeparator();
}

// =============================================================
// ERC20 METADATA VIEWS
// =============================================================

function name() public view virtual override(IERC404, IERC20Metadata) returns (string memory) {
function name() public view virtual override(IERC20Metadata) returns (string memory) {
TokenStorage storage $ = _getS();
return $.name;
}

function symbol() public view virtual override(IERC404, IERC20Metadata) returns (string memory) {
function symbol() public view virtual override(IERC20Metadata) returns (string memory) {
TokenStorage storage $ = _getS();
return $.symbol;
}

function decimals() public pure override(IERC404, IERC20Metadata) returns (uint8) {
function decimals() public pure override(IERC20Metadata) returns (uint8) {
return 18;
}

// =============================================================
// ERC20 VIEWS
// =============================================================

function totalSupply() public view virtual override(IERC404, IERC20) returns (uint256) {
function totalSupply() public view virtual override returns (uint256) {
TokenStorage storage $ = _getS();
return $.totalSupply;
}

function balanceOf(address account) public view virtual override(IERC404, IERC20) returns (uint256) {
function balanceOf(address account) public view virtual override returns (uint256) {
TokenStorage storage $ = _getS();
return $.balances[account];
}
Expand All @@ -179,7 +161,7 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
return t.owner == owner_ ? 1 : 0;
}

function allowance(address owner, address spender) public view virtual override(IERC404, IERC20) returns (uint256) {
function allowance(address owner, address spender) public view virtual override returns (uint256) {
TokenStorage storage $ = _getS();
return $.allowances[owner][spender];
}
Expand All @@ -196,17 +178,17 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
// ERC721 VIEWS
// =============================================================

function erc721TotalSupply() public view virtual override(IERC404) returns (uint256) {
function erc721TotalSupply() public view virtual returns (uint256) {
TokenStorage storage $ = _getS();
return $.minted;
}

function erc721BalanceOf(address owner_) public view virtual override(IERC404) returns (uint256) {
function erc721BalanceOf(address owner_) public view virtual returns (uint256) {
TokenStorage storage $ = _getS();
return $.owned[owner_].length;
}

function ownerOf(uint256 id_) public view virtual override(IERC404) returns (address) {
function ownerOf(uint256 id_) public view virtual returns (address) {
_validateTokenId(id_);
TokenStorage storage $ = _getS();
TokenData storage t = $.tokens[id_];
Expand All @@ -216,7 +198,7 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
return t.owner;
}

function owned(address owner_) public view virtual override(IERC404) returns (uint256[] memory) {
function owned(address owner_) public view virtual returns (uint256[] memory) {
TokenStorage storage $ = _getS();
return $.owned[owner_];
}
Expand All @@ -229,42 +211,11 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
return $.getApproved[id_];
}

function isApprovedForAll(address owner_, address operator_) public view virtual override(IERC404) returns (bool) {
function isApprovedForAll(address owner_, address operator_) public view virtual returns (bool) {
TokenStorage storage $ = _getS();
return $.isApprovedForAll[owner_][operator_];
}

function erc721TransferExempt(address account_) public view virtual override returns (bool) {
TokenStorage storage $ = _getS();
return $.erc721TransferExempt[account_];
}

// =============================================================
// QUEUE VIEWS
// =============================================================

function getERC721QueueLength() public view virtual override returns (uint256) {
TokenStorage storage $ = _getS();
return $.storedERC721Ids.length();
}

function getERC721TokensInQueue(
uint256 start_,
uint256 count_
) public view virtual override returns (uint256[] memory) {
TokenStorage storage $ = _getS();
uint256[] memory tokensInQueue = new uint256[](count_);

for (uint256 i = start_; i < start_ + count_;) {
tokensInQueue[i - start_] = $.storedERC721Ids.at(i);
unchecked {
++i;
}
}

return tokensInQueue;
}

// =============================================================
// OTHER VIEWS
// =============================================================
Expand All @@ -285,57 +236,53 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
}

/// @notice tokenURI must be implemented by child contract
function tokenURI(uint256 id_) public view virtual override(IERC404) returns (string memory);
function tokenURI(uint256 id_) public view virtual returns (string memory);

// =============================================================
// ERC20 OPERATIONS
// =============================================================

function transfer(address, uint256) public pure virtual override(IERC404, IERC20) returns (bool) {
function transfer(address, uint256) public pure virtual override returns (bool) {
revert NotImplemented();
}

function approve(address, uint256) public pure virtual override(IERC404, IERC20) returns (bool) {
function approve(address, uint256) public pure virtual override returns (bool) {
revert NotImplemented();
}

function transferFrom(address, address, uint256) public pure virtual override(IERC404, IERC20) returns (bool) {
function transferFrom(address, address, uint256) public pure virtual override returns (bool) {
revert NotImplemented();
}

function erc20Approve(address, uint256) public pure virtual override returns (bool) {
function erc20Approve(address, uint256) public pure virtual returns (bool) {
revert NotImplemented();
}

function erc20TransferFrom(address, address, uint256) public pure virtual override returns (bool) {
function erc20TransferFrom(address, address, uint256) public pure virtual returns (bool) {
revert NotImplemented();
}

// =============================================================
// ERC721 OPERATIONS
// =============================================================

function erc721Approve(address, uint256) public pure virtual override {
function erc721Approve(address, uint256) public pure virtual {
revert NotImplemented();
}

function erc721TransferFrom(address, address, uint256) public pure virtual override {
function erc721TransferFrom(address, address, uint256) public pure virtual {
revert NotImplemented();
}

function setApprovalForAll(address, bool) public pure virtual override {
function setApprovalForAll(address, bool) public pure virtual {
revert NotImplemented();
}

function safeTransferFrom(address, address, uint256) public pure virtual override {
function safeTransferFrom(address, address, uint256) public pure virtual {
revert NotImplemented();
}

function safeTransferFrom(address, address, uint256, bytes memory) public pure virtual override {
revert NotImplemented();
}

function setSelfERC721TransferExempt(bool) public pure virtual override {
function safeTransferFrom(address, address, uint256, bytes memory) public pure virtual {
revert NotImplemented();
}

Expand Down Expand Up @@ -367,7 +314,6 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
}

emit Transfer(from_, to_, value_);
// emit ERC20Transfer(from_, to_, value_);
}

/// @notice Transfer an ERC721 token
Expand Down Expand Up @@ -454,44 +400,6 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
return
interfaceId == type(IERC165).interfaceId ||
interfaceId == type(IERC20).interfaceId ||
interfaceId == type(IERC20Metadata).interfaceId ||
interfaceId == type(IERC404).interfaceId;
}

/// @notice Internal function to compute domain separator for EIP-2612 permits
function _computeDomainSeparator() internal view virtual returns (bytes32) {
return
keccak256(
abi.encode(
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes(name())),
keccak256("1"),
block.chainid,
address(this)
)
);
}

function permit(
address owner_,
address spender_,
uint256 value_,
uint256 deadline_,
uint8 v_,
bytes32 r_,
bytes32 s_
) public virtual {
revert NotImplemented();
}

/// @notice EIP-2612 domain separator
function DOMAIN_SEPARATOR() public view virtual returns (bytes32) {
TokenStorage storage $ = _getS();
return
block.chainid == $.initialChainId
? $.initialDomainSeparator
: _computeDomainSeparator();
interfaceId == type(IERC20Metadata).interfaceId;
}
}
Loading
Loading