Beatland Festival
First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: high
Invalid

Repeated Storage Reads in getUserMemorabiliaDetailed Cause Excessive Gas Usage

r4y4n3

Root + Impact

Description

  • The getUserMemorabiliaDetailed function should efficiently return all memorabilia owned by a user, minimizing gas costs.

    Specific issue:
    The function repeatedly reads from storage mappings (collections, balanceOf) inside nested loops, causing excessive gas usage as the number of collections and items grows.

// Root cause in the codebase with @> marks to highlight the relevant section
function getUserMemorabiliaDetailed(address user) external view returns (
uint256[] memory tokenIds,
uint256[] memory collectionIds,
uint256[] memory itemIds
) {
// ...existing code...
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++;
}
}
}
// ...existing code...
}

Risk

Likelihood:

  • The bug will occur for any user with a large number of memorabilia tokens or as the number of collections/items increases.

Impact:

  • Users may experience high gas costs or even run out of gas when calling this function, especially as the contract grows in usage.

  • This can make the function impractical for users with many tokens.

Proof of Concept

The following Foundry test demonstrates the gas usage for a user with 5 memorabilia tokens. The measured gas for getUserMemorabiliaDetailed is 304,379 gas, run with forge test --gas-report to see the gas usage:

function testGas_getUserMemorabiliaDetailed() public {
// Measure gas for the current implementation
festival.getUserMemorabiliaDetailed(user);
}

Recommended Mitigation

Cache storage reads in local variables to reduce redundant access, especially in nested loops:

- 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++;
- }
- }
- }
+ for (uint256 cId = 1; cId < nextCollectionId; cId++) {
+ uint256 currentItemId = collections[cId].currentItemId; // cache storage read
+ for (uint256 iId = 1; iId < currentItemId; iId++) {
+ uint256 tokenId = encodeTokenId(cId, iId);
+ if (balanceOf(user, tokenId) > 0) {
+ count++;
+ }
+ }
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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