Beatland Festival
AI First Flight #4
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Gas Inefficient Double Iteration in `getUserMemorabiliaDetailed()`

muzammilmohammed678

Root + Impact

Description

  • The `getUserMemorabiliaDetailed()` function performs two complete iterations over all collections and items - first to count owned items, then again to populate the result arrays. This O(n²) approach becomes extremely gas-expensive as the number of collections and items grows, potentially causing the function to exceed gas limits or become economically unfeasible to call.

    The normal behavior should efficiently retrieve user-owned memorabilia in a single pass. However, the current implementation uses a two-pass approach that doubles gas costs.

    ```solidity

    function getUserMemorabiliaDetailed(address user) external view returns (...) {

    // @> First iteration to count

    uint256 count = 0;

    for (uint256 cId = 1; cId < nextCollectionId; cId++) {

    for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {

    uint256 tokenId = encodeTokenId(cId, iId);

    if (balanceOf(user, tokenId) > 0) {

    count++;

    }

    }

    }

    // @> Second iteration to populate

    tokenIds = new uint256[](count);

    // ... populate arrays with another nested loop

    }

    ```


Risk

Likelihood:

  • * Function is called whenever user wants to view their memorabilia

    * Gas cost increases quadratically with number of collections and items

    * With many collections/items, function will become unusable

    * No upper bound on collections or items per collection

Impact:

  • * Extremely high gas costs for users with many memorabilia items

    * Potential denial of service if gas limit is exceeded

    * Function becomes economically unfeasible to call

    * Poor user experience

Proof of Concept

```solidity
// Create 10 collections with 100 items each = 1000 total items
for (uint i = 0; i < 10; i++) {
organizer.createMemorabiliaCollection(...);
// Mint 100 items per collection
}
​
// User owns items across all collections
// getUserMemorabiliaDetailed() must iterate:
// - First pass: 10 collections × 100 items = 1000 iterations
// - Second pass: 10 collections × 100 items = 1000 iterations
// Total: 2000 iterations, extremely gas expensive
```

Recommended Mitigation

```diff
function getUserMemorabiliaDetailed(address user) external view returns (...) {
- // First, count how many memorabilia they own
- uint256 count = 0;
- for (uint256 cId = 1; cId < nextCollectionId; cId++) {
- for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
- uint256 tokenId = encodeTokenId(cId, iId);
- if (balanceOf(user, tokenId) > 0) {
- count++;
- }
- }
- }
-
- // Then populate arrays
- tokenIds = new uint256[](count);
- collectionIds = new uint256[](count);
- itemIds = new uint256[](count);
-
- uint256 index = 0;
+ // Use dynamic arrays and resize as needed
+ uint256[] memory tempTokenIds = new uint256[](100); // Initial size
+ uint256[] memory tempCollectionIds = new uint256[](100);
+ uint256[] memory tempItemIds = new uint256[](100);
+ uint256 count = 0;
+
for (uint256 cId = 1; cId < nextCollectionId; cId++) {
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
+ if (count >= tempTokenIds.length) {
+ // Resize arrays if needed
+ }
- tokenIds[index] = tokenId;
- collectionIds[index] = cId;
- itemIds[index] = iId;
- index++;
+ tempTokenIds[count] = tokenId;
+ tempCollectionIds[count] = cId;
+ tempItemIds[count] = iId;
+ count++;
}
}
}
+
+ // Copy to correctly sized arrays
+ // ... return arrays
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!