Inefficient Loops in getUserMemorabiliaDetailed

The getUserMemorabiliaDetailed function iterates over all collections and all items in each collection to determine which memorabilia NFTs a user owns. This design is highly inefficient and can fail for large collections or users holding many items.

Description

The function attempts to return detailed ownership data for a given user:

The function loops twice:

1. Counting tokens to allocate memory arrays.

2. Populating arrays with token IDs, collection IDs, and item IDs.

Issue: Both loops are O(n × m), where n = number of collections and m = items per collection. This complexity grows rapidly as collections or items increase, leading to potential performance and gas issues.

Risk

Likelihood:

• High for platforms with many collections or users owning multiple NFTs.

• Particularly relevant for long-running contracts where the number of minted items increases over time.

Impact:

• High gas cost for on-chain calls, potentially exceeding block gas limits.

• Function may revert for users with large collections, preventing retrieval of their NFTs.

• Frontends relying on this function may fail to display user-owned items correctly.

Proof Of Concept

• Suppose there are 1,000 collections, each with 100 items (currentItemId = 101).

• Number of iterations: 1,000 × 100 = 100,000 iterations × 2 loops = 200,000 calls to balanceOf.

• Likely exceeds Ethereum gas limit if called on-chain.

• A single user with many tokens forces all iterations to run, further increasing gas and potentially causing a revert.

Mitigation

1. Use a mapping for ownership tracking: