Skip to content

refactor: remove all_tokens enumeration from EnumerableComponent #91

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
starknetdev merged 1 commit into from
Apr 6, 2026
Merged

refactor: remove all_tokens enumeration from EnumerableComponent #91

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
49 changes: 9 additions & 40 deletions packages/embeddable_game_standard/src/token/extensions/enumerable/enumerable.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,31 @@
/// # EnumerableComponent
///
/// Drop-in replacement for OZ's ERC721EnumerableComponent that uses felt252
/// for internal storage instead of u256, saving ~2x gas per storage operation.
/// The external interface remains IERC721Enumerable (u256) for full compatibility
/// with existing consumers and dispatchers.
///
/// Burn is not supported — `all_tokens_index` has been removed to save one
/// storage write per mint.
/// Tracks per-owner token enumeration using felt252 for internal storage
/// instead of u256, saving ~2x gas per storage operation. Only owner-based
/// enumeration is supported — global `all_tokens` enumeration has been
/// removed to save gas on every mint.
///
/// WARNING: The `before_update` function must be called after every transfer
/// or mint operation via the ERC721HooksTrait::before_update hook.
#[starknet::component]
pub mod EnumerableComponent {
use core::num::traits::Zero;
use openzeppelin_interfaces::erc721 as oz_interface;
use openzeppelin_introspection::src5::SRC5Component;
use openzeppelin_introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait;
use openzeppelin_token::erc721::ERC721Component;
use openzeppelin_token::erc721::ERC721Component::{
ERC721Impl, InternalImpl as ERC721InternalImpl,
};
use starknet::ContractAddress;
use starknet::storage::{
Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess,
StoragePointerWriteAccess,
};
use crate::token::extensions::enumerable::interface::IERC721_ENUMERABLE_ID;
use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess};
use crate::token::extensions::enumerable::interface::IENUMERABLE_OWNER_ID;

// Internal storage uses felt252 for single-slot efficiency.
// External interface converts u256 <-> felt252 at the boundary.
#[storage]
pub struct Storage {
pub Enumerable_owned_tokens: Map<(ContractAddress, felt252), felt252>,
pub Enumerable_owned_tokens_index: Map<felt252, felt252>,
pub Enumerable_all_tokens_len: felt252,
pub Enumerable_all_tokens: Map<felt252, felt252>,
}

pub mod Errors {
Expand All @@ -50,19 +41,7 @@ pub mod EnumerableComponent {
+ERC721Component::ERC721HooksTrait<TContractState>,
+SRC5Component::HasComponent<TContractState>,
+Drop<TContractState>,
> of oz_interface::IERC721Enumerable<ComponentState<TContractState>> {
fn total_supply(self: @ComponentState<TContractState>) -> u256 {
let len: felt252 = self.Enumerable_all_tokens_len.read();
len.into()
}

fn token_by_index(self: @ComponentState<TContractState>, index: u256) -> u256 {
assert(index < self.total_supply(), Errors::OUT_OF_BOUNDS_INDEX);
let index_felt: felt252 = index.try_into().unwrap();
let token_id: felt252 = self.Enumerable_all_tokens.read(index_felt);
token_id.into()
}

> of super::super::interface::IEnumerableOwner<ComponentState<TContractState>> {
fn token_of_owner_by_index(
self: @ComponentState<TContractState>, owner: ContractAddress, index: u256,
) -> u256 {
Expand All @@ -85,7 +64,7 @@ pub mod EnumerableComponent {
> of InternalTrait<TContractState> {
fn initializer(ref self: ComponentState<TContractState>) {
let mut src5_component = get_dep_component_mut!(ref self, SRC5);
src5_component.register_interface(IERC721_ENUMERABLE_ID);
src5_component.register_interface(IENUMERABLE_OWNER_ID);
}

fn before_update(
Expand All @@ -96,9 +75,7 @@ pub mod EnumerableComponent {
let previous_owner = erc721_component._owner_of(token_id);
let token_id_felt: felt252 = token_id.try_into().unwrap();

if previous_owner.is_zero() {
self._add_token_to_all_tokens_enumeration(token_id_felt);
} else if previous_owner != to {
if !previous_owner.is_zero() && previous_owner != to {
self._remove_token_from_owner_enumeration(previous_owner, token_id_felt);
}

Expand Down Expand Up @@ -130,14 +107,6 @@ pub mod EnumerableComponent {
self.Enumerable_owned_tokens_index.write(token_id, len);
}

fn _add_token_to_all_tokens_enumeration(
ref self: ComponentState<TContractState>, token_id: felt252,
) {
let supply = self.Enumerable_all_tokens_len.read();
self.Enumerable_all_tokens.write(supply, token_id);
self.Enumerable_all_tokens_len.write(supply + 1);
}

fn _remove_token_from_owner_enumeration(
ref self: ComponentState<TContractState>, from: ContractAddress, token_id: felt252,
) {
Expand Down
16 changes: 10 additions & 6 deletions packages/embeddable_game_standard/src/token/extensions/enumerable/interface.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Re-export OZ IERC721Enumerable interface.
// The EnumerableComponent implements this interface with felt252 storage internally.
pub use openzeppelin_interfaces::erc721::{
IERC721Enumerable, IERC721EnumerableDispatcher, IERC721EnumerableDispatcherTrait,
IERC721_ENUMERABLE_ID,
};
use starknet::ContractAddress;

/// EFS: token_of_owner_by_index(ContractAddress,(u128,u128))->(u128,u128)
pub const IENUMERABLE_OWNER_ID: felt252 =
0x312c74a3a4f7aaf9aa3e80ddea171f958139ef0c3dbea524e0763682b7d57dd;
Comment on lines +4 to +5
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 6, 2026

Choose a reason for hiding this comment

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

medium

The interface ID 0x312c74a3a4f7aaf9aa3e80ddea171f958139ef0c3dbea524e0763682b7d57dd does not follow the SRC5 (and ERC165) standard, which defines the interface ID as the XOR of all function selectors in the trait. For a single-function trait like IEnumerableOwner, the ID should be exactly the selector of token_of_owner_by_index. The current value seems to be the hash of the trait name IEnumerableOwner, which will break standard-compliant interface discovery.

pub const IENUMERABLE_OWNER_ID: felt252 =
    0x02e30a6aa3d393d8d117333e03280b639194efc4480276dad4d23d53f2f9131;


#[starknet::interface]
pub trait IEnumerableOwner<TState> {
fn token_of_owner_by_index(self: @TState, owner: ContractAddress, index: u256) -> u256;
}
99 changes: 4 additions & 95 deletions packages/embeddable_game_standard/src/token/tests/test_enumerable.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use EnumerableComponent::{EnumerableImpl, InternalImpl};
use game_components_embeddable_game_standard::token::extensions::enumerable::enumerable::EnumerableComponent;
use game_components_embeddable_game_standard::token::extensions::enumerable::interface::IERC721_ENUMERABLE_ID;
use game_components_embeddable_game_standard::token::extensions::enumerable::interface::IENUMERABLE_OWNER_ID;
use game_components_test_common::mocks::mock_enumerable::EnumerableMock;
use openzeppelin_interfaces::introspection::ISRC5_ID;
use openzeppelin_introspection::src5::SRC5Component::SRC5Impl;
Expand Down Expand Up @@ -65,55 +65,10 @@ fn test_initializer() {

state.initializer();

assert!(mock_state.supports_interface(IERC721_ENUMERABLE_ID));
assert!(mock_state.supports_interface(IENUMERABLE_OWNER_ID));
assert!(mock_state.supports_interface(ISRC5_ID));
}

// ================================================================================================
// total_supply
// ================================================================================================

#[test]
fn test_total_supply() {
let mut state = COMPONENT_STATE();
let mut contract_state = CONTRACT_STATE();

assert!(state.total_supply() == 0, "initial supply should be 0");

contract_state.erc721.mint(OWNER(), TOKEN_1);
assert!(state.total_supply() == 1, "supply should be 1 after mint");

contract_state.erc721.mint(OWNER(), TOKEN_2);
assert!(state.total_supply() == 2, "supply should be 2 after second mint");
}

// ================================================================================================
// token_by_index
// ================================================================================================

#[test]
fn test_token_by_index() {
let (_, token_list) = setup();
assert_token_by_index(token_list);
}

#[test]
#[should_panic(expected: 'ERC721Enum: out of bounds index')]
fn test_token_by_index_equal_to_supply() {
let (state, token_list) = setup();
let supply: u256 = token_list.len().into();
state.token_by_index(supply);
}

#[test]
#[should_panic(expected: 'ERC721Enum: out of bounds index')]
fn test_token_by_index_greater_than_supply() {
let (state, token_list) = setup();
let supply_plus_one: u256 = token_list.len().into() + 1;
state.token_by_index(supply_plus_one);
}


// ================================================================================================
// token_of_owner_by_index
// ================================================================================================
Expand Down Expand Up @@ -211,11 +166,9 @@ fn test_before_update_when_mint() {

state.before_update(OWNER(), new_token);

assert!(state.total_supply() == 4, "supply should be 4");
assert_owned_tokens_list_after_update(
OWNER(), array![TOKEN_1, TOKEN_2, TOKEN_3, new_token].span(),
);
assert_token_by_index(array![TOKEN_1, TOKEN_2, TOKEN_3, new_token].span());
}

#[test]
Expand All @@ -224,10 +177,8 @@ fn test_before_update_when_transfer_last_token() {

state.before_update(RECIPIENT(), TOKEN_3);

assert!(state.total_supply() == 3, "supply should stay 3");
assert_owned_tokens_list_after_update(OWNER(), array![TOKEN_1, TOKEN_2].span());
assert_owned_tokens_list_after_update(RECIPIENT(), array![TOKEN_3].span());
assert_token_by_index(array![TOKEN_1, TOKEN_2, TOKEN_3].span());
}

#[test]
Expand All @@ -236,10 +187,8 @@ fn test_before_update_when_transfer_first_token() {

state.before_update(RECIPIENT(), TOKEN_1);

assert!(state.total_supply() == 3, "supply should stay 3");
assert_owned_tokens_list_after_update(OWNER(), array![TOKEN_3, TOKEN_2].span());
assert_owned_tokens_list_after_update(RECIPIENT(), array![TOKEN_1].span());
assert_token_by_index(array![TOKEN_1, TOKEN_2, TOKEN_3].span());
}

// ================================================================================================
Expand All @@ -261,32 +210,14 @@ fn test__add_token_to_owner_enumeration() {
assert_owner_tokens_id_to_index(new_token_id, new_token_index);
}

// ================================================================================================
// _add_token_to_all_tokens_enumeration
// ================================================================================================

#[test]
fn test__add_token_to_all_tokens_enumeration() {
let (mut state, _) = setup();
let new_token_id: felt252 = 'TOKEN_4';
let initial_supply_felt: felt252 = state.total_supply().try_into().unwrap();

assert_all_tokens_index_to_id(initial_supply_felt, 0);

state._add_token_to_all_tokens_enumeration(new_token_id);

assert_all_tokens_index_to_id(initial_supply_felt, new_token_id);
assert!(state.total_supply() == 4, "supply should be 4");
}

// ================================================================================================
// _remove_token_from_owner_enumeration
// ================================================================================================

#[test]
fn test__remove_token_from_owner_enumeration_with_last_token() {
let (mut state, _) = setup();
let last_token_index: felt252 = (state.total_supply() - 1).try_into().unwrap();
let (mut state, tokens_list) = setup();
let last_token_index: felt252 = (tokens_list.len() - 1).into();
let last_token_id: felt252 = TOKEN_3.try_into().unwrap();

assert_owner_tokens_index_to_id(OWNER(), last_token_index, last_token_id);
Expand Down Expand Up @@ -367,22 +298,6 @@ fn assert_token_of_owner_by_index(owner: ContractAddress, expected_token_list: S
}
}

fn assert_token_by_index(expected_token_list: Span<u256>) {
let state = @COMPONENT_STATE();

let total_supply: u256 = state.total_supply();
let expected_list_len: u256 = expected_token_list.len().into();
assert!(total_supply == expected_list_len, "total_supply mismatch");

let mut i: u32 = 0;
while i != expected_token_list.len() {
let index: u256 = i.into();
let token = state.token_by_index(index);
assert!(token == *expected_token_list.at(i), "token_by_index mismatch");
i += 1;
}
}

/// Reads from storage directly, bypassing the out of bounds check.
/// The `before_update` function does not update the ERC721 state.
fn assert_owned_tokens_list_after_update(owner: ContractAddress, expected_list: Span<u256>) {
Expand All @@ -398,12 +313,6 @@ fn assert_owned_tokens_list_after_update(owner: ContractAddress, expected_list:
}
}

fn assert_all_tokens_index_to_id(index: felt252, exp_token_id: felt252) {
let state = @COMPONENT_STATE();
let index_to_id = state.Enumerable_all_tokens.read(index);
assert!(index_to_id == exp_token_id, "all_tokens index->id mismatch");
}

fn assert_owner_tokens_index_to_id(owner: ContractAddress, index: felt252, exp_token_id: felt252) {
let state = @COMPONENT_STATE();
let index_to_id = state.Enumerable_owned_tokens.read((owner, index));
Expand Down
9 changes: 0 additions & 9 deletions packages/interfaces/src/token/enumerable.cairo
View file
Open in desktop

This file was deleted.

Loading