Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Code duplication

tm2

Summary

The trickOrTreat function contains duplicated code segments that replicate the functionality of the mintTreat internal function. This duplication not only violates best coding practices but also introduces potential risks related to maintainability and consistency.

Vulnerability Details

Upon inspecting the SpookySwap contract, it becomes evident that both the trickOrTreat and mintTreat functions perform similar operations related to NFT minting. Specifically, within the trickOrTreat function, there is a segment of code responsible for minting the NFT directly to the user when sufficient ETH is provided:

if (msg.value >= requiredCost) {
// User sent enough ETH
mintTreat(msg.sender, treat);
} else {
// User didn't send enough ETH
// Mint NFT to contract and store pending purchase
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
​
pendingNFTs[tokenId] = msg.sender;
pendingNFTsAmountPaid[tokenId] = msg.value;
tokenIdToTreatName[tokenId] = _treatName;
​
emit Swapped(msg.sender, _treatName, tokenId);
​
// User needs to call resolveTrick() to finish the transaction
}

Conversely, the mintTreat function encapsulates similar minting logic:

function mintTreat(address recipient, Treat memory treat) internal {
uint256 tokenId = nextTokenId;
_mint(recipient, tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
​
emit Swapped(recipient, treat.name, tokenId);
}

The duplication arises from the trickOrTreat function manually minting the NFT to the contract's address and setting the token URI, actions that are already abstracted within the mintTreat function. This redundancy not only makes the codebase less efficient but also increases the risk of discrepancies between the two minting processes.

Impact

  • If future modifications are required in the minting process (e.g., adding new features or fixing bugs), developers must remember to update both the trickOrTreat and mintTreat functions. Failure to do so can result in inconsistent behaviors between the two minting pathways.

  • Duplicated operations can lead to unnecessary gas usage, making transactions more expensive for users. Optimizing code by reusing functions helps in reducing gas costs.

  • The "Don't Repeat Yourself" (DRY) principle is a fundamental software engineering guideline aimed at reducing repetition. Violating this principle can lead to a bloated and error-prone codebase, complicating both audits and future developments.

Tools Used

Manual review

Recommendations

Refactor trickOrTreat: Ensure that all NFT minting operations within the trickOrTreat function utilize the mintTreat internal function exclusively. This eliminates redundant code and centralizes the minting logic.

Updates

Appeal created

bube Lead Judge 9 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.