BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

MEDIUM-01: setCountry Uses memory Instead of calldata

mostafapahlevani93

Root + Impact

The setCountry() function uses memory parameter type which copies the entire 48-element array into memory, wasting significant gas when a calldata reference would be sufficient.

Description

Normal behavior for external functions that don't modify parameters expects using calldata to avoid unnecessary memory allocation and copying operations.

The current implementation copies the entire string array from calldata to memory even though the function only reads the values to copy them to storage. This wastes gas on the copy operation.

// @> 'memory' copies entire array to memory - expensive
function setCountry(string[48] memory countries) public onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Risk

Likelihood:

  • Every call to setCountry() incurs this cost

  • Function is called at least once per tournament setup

  • May be called multiple times if teams need updating

Impact:

  • Wastes approximately 20,000-50,000 gas per call

  • Unnecessary cost for contract owner during setup

  • Makes contract deployment and configuration more expensive

  • Compounds with other gas inefficiencies

Proof of Concept

// Current implementation - memory
function setCountry(string[48] memory countries) public onlyOwner {
// Calldata -> Memory copy: ~30,000 gas
// Then Memory -> Storage copy: normal cost
}
​
// Gas cost: ~30,000 extra gas for the memory copy
​
// Optimized implementation - calldata
function setCountry(string[48] calldata countries) external onlyOwner {
// Calldata -> Storage copy directly: no extra cost
}
​
// Gas saved: ~20,000-50,000 gas per call

Recommended Mitigation

-function setCountry(string[48] memory countries) public onlyOwner {
+function setCountry(string[48] calldata countries) external onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Additional optimization: Change public to external since this function is never called internally, saving additional gas on function selector matching.

Estimated Gas Savings: ~20,000-50,000 gas per setCountry() call

Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Gas optimizations

Gas optimizations are invalid according to the CodeHawks documentation.

Support

FAQs

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

Give us feedback!