CodeHawks Finding Submission Report

Title: getUserMemorabiliaDetailed() unnecessarily iterates over invalid collection IDs (1–99), wasting gas

Severity: Gas Optimization (Low / QA)

Contest: [Insert Contest Name / ID – e.g., BeatDrop Festival Pass Audit] Date: December 27, 2025 Submitted by: [Your Handle / Team Name]

Description

The view function getUserMemorabiliaDetailed() returns all memorabilia NFTs owned by a user by iterating over all possible collection IDs starting from 1:

solidity

However, in the contract design:

• Pass types (General, VIP, Backstage) use fixed IDs 1, 2, 3

• Memorabilia collections are created dynamically starting from 100 (nextCollectionId = 100 initially, incremented on creation)

This means collection IDs 1 to 99 will never contain valid memorabilia NFTs — they are either reserved passes (1–3) or non-existent (4–99).

The function performs two full nested loops over these invalid ranges:

• One to count owned items

• One to populate the arrays

Each iteration involves:

• Encoding a token ID

• Calling balanceOf(user, tokenId) (an external SLOAD-heavy call in ERC1155)

This results in significant unnecessary gas usage on every call, especially as nextCollectionId grows.

Impact

• Low severity / Gas optimization: Pure gas waste, no security or functional incorrectness.

• Gas cost: For every call, up to 99 useless outer loop iterations (each with inner loops and balanceOf checks).

• Scalability issue: As more memorabilia collections are created (nextCollectionId increases), the wasted iterations remain fixed at ~99, but the proportional waste decreases — still, it's permanent overhead.

• User experience: Higher gas costs for users/frontends querying owned memorabilia.

Classified as Low / Gas / QA per CodeHawks guidelines.

Proof of Concept / Gas Analysis

Assume nextCollectionId = 105 (5 memorabilia collections created):

• Current code loops cId from 1 to 104

• Only cId >= 100 can possibly have memorabilia

• → 99 wasted outer iterations (cId 1–99)

• Each wasted iteration runs an inner loop up to currentItemId - 1 (could be 0, but still executes encoding + balanceOf call)

Even if inner loop is short, each balanceOf call costs ~2,100–5,000 gas (warm/cold access), leading to tens of thousands of wasted gas per query.

Recommended Mitigation

Change the loop start from 100 instead of 1:

solidity

Alternative (more flexible): Introduce a constant:

solidity

Then use it in both loops.

Benefits

• Eliminates ~99 wasted iterations permanently

• Reduces gas cost significantly for this frequently called view function

• No change in functionality — returns identical results

• Improves frontend performance and user experience