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

Double for-loop iteration in `getUserMemorabiliaDetailed` caused inefficiency and high gas overhead for view call, potentially revert due to block gas limit when collections grow too large.

soupy

Summary

The double for-loop iteration found in getUserMemorabiliaDetailed function is inefficient and causes high gas overhead for this view call function. The nextCollectionId is initialized at 100, but cId starts at 1, resulting unused iteration of all collection id below 100, this unnecessary memory reads adds on to the expensive view call.

Description

Inefficient double for-loop was found in getUserMemorabiliaDetailed view function in addition to unused 100 iteration of collection id cId since nextCollectionId is initialized at 100. These will cause the view function becomes slow over time along with expesive high gas overhead and could even result this view function to revert when the collections grow too large.

function getUserMemorabiliaDetailed(address user) external view returns (
uint256[] memory tokenIds,
uint256[] memory collectionIds,
uint256[] memory itemIds
) {
// 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;
<@@>! 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) {
tokenIds[index] = tokenId;
collectionIds[index] = cId;
itemIds[index] = iId;
index++;
}
}
}
return (tokenIds, collectionIds, itemIds);
}

Risk

Likelihood:

  • Whenever this view function is called, inefficiency and relatively high gas cost imposed due to the double for-loop iteration. It will get slower over time when the collections grow larger and larger

Impact:

  • Inefficient and expensive view function call that could get slow over time and potentially revert when collections grow large that hits the block gas limit

Recommended Mitigation

To refactor the code and implement a mapping function to correlate user address to list of owned memorabilias such as
mapping(address => uint256[]) public userOwnMemorabilias
Keep this mapping updated during redeemMemorabilia to directly capture the owned memorabilias.

function redeemMemorabilia(uint256 collectionId) external {
MemorabiliaCollection storage collection = collections[collectionId];
require(collection.priceInBeat > 0, "Collection does not exist");
require(collection.isActive, "Collection not active");
require(collection.currentItemId < collection.maxSupply, "Collection sold out");
// Burn BEAT tokens
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
// Generate unique token ID
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
// Store edition number
tokenIdToEdition[tokenId] = itemId;
// Mint the unique NFT
_mint(msg.sender, tokenId, 1, "");
​
+ userOwnMemorabilias[msg.sender].push(tokenId);
emit MemorabiliaRedeemed(msg.sender, tokenId, collectionId, itemId);
}
​

Then in the getUserMemorabiliaDetailed function, simplify the codes by tapping on the userOwnMemorabilias mapping as suggested below:

function getUserMemorabiliaDetailed(address user) external view returns (
uint256[] memory tokenIds,
uint256[] memory collectionIds,
uint256[] memory itemIds
) {
tokenIds = userOwnMemorabilias[user];
uint256 count = tokenIds.length;
collectionIds = new uint256[](count);
itemIds = new uint256[](count);
for (uint256 i = 0; i < count; ++i) {
(collectionIds[i], itemIds[i]) = decodeTokenId(tokenIds[i]);
}
​
return (tokenIds, collectionIds, itemIds);
}
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.