FestivalPass::getUserMemorabiliaDetailed
.Description: The FestivalPass::getUserMemorabiliaDetailed
function loops from cId = 1
to cId < nextCollectionId
even though the contract initializes FestivalPass::nextCollectionId = 100
. This results in 99 unnecessary iterations over uninitialized FestivalPass::collectections[cId]
entries. Since each iteration involves a nested loop and multiple balanceOf and FestivalPass::encodeTokenId
calls, this adds significant overhead to what is expected to be a lightweight view function.
Impact: While this does not affect the correctness of the function or user balances, it:
Wastes gas in view simulations.
Slows down frontend responsiveness.
May lead to out-of-gas errors in off-chain tools or future on-chain calls if scaled further.
Proof of Concept: Add this into the FestivalPass.t.sol :
This test measures how much gas is used when calling FestivalPass::getUserMemorabiliaDetailed(user2)
by checking the gas before and after the call, then logging the difference. It's useful to see how expensive the function is.
Gas usage for a sample view call reached ~286,000 even without a valid collection.
Recommended Mitigation: Update the loop to start from the actual collection start ID:
Alternatively, refactor the initial value of FestivalPass::nextCollectionId
:
Additionally, refactor FestivalPass::getUserMemorabiliaDetailed
to eliminate the two-pass array allocation pattern and avoid redundant memory copying. A single-pass solution using inline assembly can significantly reduce memory operations and improve execution efficiency. This approach minimizes memory overhead, eliminates unnecessary condition checks, and scales better with user holdings especially in read-heavy environments like off-chain calls or dashboards.
Note: Gas fees went from ~300,000 to ~40,000 after testing with this code.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.