Upgraded Q -> 2 from #154 [1676532286167]

[c4-judge]

Judge has assessed an item in Issue #154 as 2 risk. The relevant finding follows:

Quest.claim() can risk gas exhaustion on large receipt claims due to multiple mandatory loops function claim() public virtual onlyQuestActive { if (isPaused) revert QuestPaused();

uint256[] memory tokens = rabbitHoleReceiptContract.getOwnedTokenIdsOfQuest(questId, msg.sender);

if (tokens.length == 0) revert NoTokensToClaim(); 

uint256 redeemableTokenCount = 0;
for (uint256 i = 0; i < tokens.length; i++) {
    if (!isClaimed(tokens[i])) { 
        redeemableTokenCount++;
    }
}
if (redeemableTokenCount == 0) revert AlreadyClaimed(); 

uint256 totalRedeemableRewards = _calculateRewards(redeemableTokenCount);
_setClaimed(tokens);
_transferRewards(totalRedeemableRewards);
redeemedTokens += redeemableTokenCount;

emit Claimed(msg.sender, totalRedeemableRewards);

} The function contains 4 separate loops over arrays that the user cannot influence (such as only submitting a subset of claims at once). Two loops are in the call to rabbitHoleReceiptContract.getOwnedTokenIdsOfQuest(), the overt for loop in the middle, and the loop in _setClaimed.

This risk is small, but could be lessened somewhat by performing the boolean update contained in _setClaimed() inside the if branch in the existing for loop.

Otherwise, consider allowing partial claims of receipts via array arguments, or detecting expensive claims bundles and only partially iterating over them. Since the logic already filters out claimed receipts, this would make the later loops smaller.

[c4-judge]

kirk-baird marked the issue as duplicate of #552

[c4-judge]

kirk-baird marked the issue as satisfactory