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

Improper mapping structure and inefficient addTreat Function

hawks

Summary

The addTreat function, originally designed to map treat data by a string name, is inefficient and lacks user-specific treat management. The current mapping(string => Treat) approach leads to higher gas costs and does not support associating treats with specific users. The updated mapping structure, mapping(address => Treat[]), resolves these issues by organizing treats per user, eliminating the need for the treatNames array.

Vulnerability Details

The use of mapping(string => Treat) creates inefficiencies because string is more expensive to use than simpler types like address. Additionally, it prevents effective user-specific tracking, as all treats are stored globally by name, rather than being tied to individual users. The addTreat function also pushes treat names into an unnecessary treatNames array, further complicating the contract and increasing storage costs.

Impact

Gas Inefficiency: Using string as the mapping key results in higher gas costs for storage and lookups.

Lack of User-Specific Tracking: The current design doesn’t allow treats to be tied to specific users, limiting flexibility.

Unnecessary Data Storage: The treatNames array is redundant with treats now being tracked per user.

Original Code Before Updates:

// Mapping treats by a string name
mapping(string => Treat) public treatList;
string[] public treatNames;
​
struct Treat {
string name;
uint256 cost; // Cost in ETH (in wei) to get one treat
string metadataURI; // URI for the NFT metadata
}
​
// Function to add a treat with the string name as the key
function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
treatList[_name] = Treat(_name, _rate, _metadataURI); // Storing treat by its name (string)
treatNames.push(_name); // Keeping a list of all treat names
emit TreatAdded(_name, _rate, _metadataURI);
}
​
// Event to signal the addition of a new treat
event TreatAdded(string name, uint256 cost, string metadataURI);
}

Tools Used

Manual Review

Recommendations

1: Switch Mapping to address: Replace mapping(string => Treat) with mapping(address => Treat[]) to store treats per user, improving gas efficiency and enabling user-specific treat management.

2: Remove treatNames: Eliminate the redundant treatNames array to reduce storage and simplify the contract.

3: Update addTreat: Modify the addTreat function to use the new mapping(address => Treat[]) structure, adding treats directly to a user’s array.

Updated Code Example:

​
mapping(address => Treat[]) public userTreats;
​
struct Treat {
string name;
uint256 cost;
string metadataURI;
}
​
function addTreat(address _user, string memory _name, uint256 _cost, string memory _metadataURI) public onlyOwner {
Treat memory newTreat = Treat(_name, _cost, _metadataURI);
userTreats[_user].push(newTreat);
emit TreatAdded(_user, _name, _cost, _metadataURI);
}
​
event TreatAdded(address indexed user, string name, uint256 cost, string metadataURI);
}

These changes ensure the contract is more efficient, user-friendly, and removes redundant data handling.

Updates

Appeal created

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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