Platform: Code4rena
Start Date: 09/02/2024
Pot Size: $60,500 USDC
Total HM: 17
Participants: 283
Period: 12 days
Judge:
Id: 328
League: ETH
Rank: 22/283
Findings: 2
Award: $262.57
๐ Selected for report: 0
๐ Solo Findings: 0
๐ Selected for report: 0xDetermination
Also found by: 0x11singh99, 0xAnah, 0xRiO, JcFichtner, K42, MatricksDeCoder, McToady, PetarTolev, Raihan, SAQ, SM3_SS, SY_S, Timenov, ahmedaghadi, al88nsk, dharma09, donkicha, emrekocak, favelanky, hunter_w3b, judeabara, kiqo, lrivo, lsaudit, merlinboii, mikesans, offside0011, oualidpro, peter, rekxor, shamsulhaq123, unique, yashgoel72, yovchev_yoan, ziyou-
242.4952 USDC - $242.50
Possible Optimization =
Here is the optimized code snippet:
// Before Optimization mapping(uint256 => mapping(string => uint8[])) public attributeProbabilities; // After Optimization mapping(bytes32 => uint8[]) public attributeProbabilitiesOptimized; function _generateCompositeKey(uint256 generation, string memory attribute) private pure returns (bytes32) { return keccak256(abi.encodePacked(generation, attribute)); } function addAttributeProbabilitiesOptimized(uint256 generation, uint8[][] memory probabilities) public { require(msg.sender == _ownerAddress, "Only owner can add probabilities"); require(probabilities.length == attributes.length, "Invalid probabilities length"); for (uint8 i = 0; i < attributes.length; i++) { bytes32 key = _generateCompositeKey(generation, attributes[i]); attributeProbabilitiesOptimized[key] = probabilities[i]; } }
Possible Optimization 1 =
Here is the optimized code snippet:
// Before optimization function updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType) external { require(msg.sender == ownerOf(tokenId)); fighters[tokenId].modelHash = modelHash; fighters[tokenId].modelType = modelType; numTrained[tokenId] += 1; totalNumTrained += 1; } // After optimization function updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType) external { require(msg.sender == ownerOf(tokenId)); fighters[tokenId].modelHash = modelHash; fighters[tokenId].modelType = modelType; emit ModelUpdated(tokenId, modelHash, modelType); // Use an event instead of modifying state }
Possible Optimization 2 =
nftsClaimed
after the loop to reduce the gas cost associated with state changes.Here is the optimized code:
function claimFighters( uint8[2] calldata numToMint, bytes calldata signature, string[] calldata modelHashes, string[] calldata modelTypes ) external { // Existing validation logic remains unchanged uint16 totalToMint = uint16(numToMint[0] + numToMint[1]); // Perform minting operations first without updating nftsClaimed for (uint16 i = 0; i < totalToMint; i++) { _createNewFighter( msg.sender, uint256(keccak256(abi.encode(msg.sender, fighters.length))), modelHashes[i], modelTypes[i], i < numToMint[0] ? 0 : 1, 0, [uint256(100), uint256(100)] ); } // Batch update nftsClaimed after all minting operations nftsClaimed[msg.sender][0] += numToMint[0]; nftsClaimed[msg.sender][1] += numToMint[1]; }
Possible Optimization 1 =
dailyAllowanceReplenishTime
and allowanceRemaining
for each mint operation, which can be optimized.Here is the optimized code:
function mint(uint256 tokenId, uint256 quantity) external { // Check and replenish daily allowance at the start if (dailyAllowanceReplenishTime[msg.sender][tokenId] <= block.timestamp) { _replenishDailyAllowance(tokenId); } // Continue with existing mint logic... }
Possible Optimization 2 =
allowanceRemaining
and dailyAllowanceReplenishTime
separately for each token ID.Here is the optimized code snippet:
function _replenishDailyAllowances(uint256[] memory tokenIds) private { for (uint i = 0; i < tokenIds.length; i++) { uint256 tokenId = tokenIds[i]; allowanceRemaining[msg.sender][tokenId] = allGameItemAttributes[tokenId].dailyAllowance; dailyAllowanceReplenishTime[msg.sender][tokenId] = uint32(block.timestamp + 1 days); } }
Possible Optimization 3 =
Here is the optimized code:
function setTokenURIs(uint256[] calldata tokenIds, string[] calldata uris) external { require(isAdmin[msg.sender], "Only admin can set URIs"); require(tokenIds.length == uris.length, "TokenIds and URIs length mismatch"); for (uint256 i = 0; i < tokenIds.length; i++) { _tokenURIs[tokenIds[i]] = uris[i]; } }
Possible Optimization 1 =
fighterPoints
and totalPoints
for each token ID individually, which could be optimized for scenarios where multiple points need to be added across different token IDs.Here is the optimized code snippet:
function addPointsBatch(uint256[] calldata tokenIds, uint256[] calldata pointsToAdd) external { require(msg.sender == _rankedBattleAddress, "Only Ranked Battle can add points"); require(tokenIds.length == pointsToAdd.length, "Array length mismatch"); for (uint256 i = 0; i < tokenIds.length; i++) { fighterPoints[tokenIds[i]] += pointsToAdd[i]; totalPoints += pointsToAdd[i]; emit PointsAdded(tokenIds[i], pointsToAdd[i]); } }
Possible Optimization 2 =
Here is the optimized code:
function claimRewardsBatch( uint32[] calldata roundIndices, string[] calldata modelURIs, string[] calldata modelTypes, uint256[2][] calldata customAttributes ) external { require(roundIndices.length > 0, "No rounds specified"); uint32 claimCount = 0; for (uint32 i = 0; i < roundIndices.length; i++) { uint32 round = roundIndices[i]; require(round < roundId, "Round does not exist"); require(isWinner(msg.sender, round), "Not a winner for round"); _fighterFarmInstance.mintFromMergingPool( msg.sender, modelURIs[claimCount], modelTypes[claimCount], customAttributes[claimCount] ); claimCount++; numRoundsClaimed[msg.sender]++; } if (claimCount > 0) { emit Claimed(msg.sender, claimCount); } }
Possible Optimization 1 =
Here is the optimized code snippet:
function setupAirdropBatch(address[] calldata recipients, uint256[] calldata amounts) external { require(isAdmin[msg.sender], "Only admin can setup airdrops"); require(recipients.length == amounts.length, "Mismatch between recipients and amounts"); for (uint256 i = 0; i < recipients.length; i++) { _approve(treasuryAddress, recipients[i], amounts[i]); } }
Possible Optimization 2 =
Here is the optimized code:
function mintOptimized(address to, uint256 amount) public { require(hasRole(MINTER_ROLE, msg.sender), "Must have minter role"); uint256 remainingSupply = MAX_SUPPLY - totalSupply(); require(amount <= remainingSupply, "Exceeds max supply"); _mint(to, amount); }
totalSupply()
function involves non-trivial computation or state access. Savings are minor per transaction but could add up over many transactions.Possible Optimization 1 =
Here is the optimized code snippet:
function updateBattleRecordsBatch(uint256[] calldata tokenIds, uint8[] calldata battleResults) external { require(isAdmin[msg.sender], "Only admin can update records"); require(tokenIds.length == battleResults.length, "Mismatch between token IDs and results"); for (uint256 i = 0; i < tokenIds.length; i++) { _updateRecord(tokenIds[i], battleResults[i]); } }
tokenIds
and battleResults
arrays are of equal length and contain valid data) is crucial. The require(isAdmin[msg.sender], "Only admin can update records");
check is essential to prevent unauthorized access to this function.Possible Optimization 2 =
NRN
rewards can lead to higher transaction costs for users.Here is the optimized code:
function claimNRNMultiround() external { uint256 totalClaimableNRN = 0; for (uint32 round = numRoundsClaimed[msg.sender]; round < roundId; round++) { uint256 nrnDistribution = rankedNrnDistribution[round]; uint256 points = accumulatedPointsPerAddress[msg.sender][round]; totalClaimableNRN += (points * nrnDistribution) / totalAccumulatedPoints[round]; numRoundsClaimed[msg.sender] = round + 1; } if (totalClaimableNRN > 0) { _neuronInstance.mint(msg.sender, totalClaimableNRN); emit Claimed(msg.sender, totalClaimableNRN); } }
totalClaimableNRN
is accurate and cannot be manipulated. The loop should correctly iterate over the rounds the user has participated in, and the calculations must accurately reflect the user's earned rewards without allowing for double-claiming or skipping rounds. The contract must also handle the potential for overflow in totalClaimableNRN
and ensure that the numRoundsClaimed[msg.sender]
is updated atomically to reflect the new state post-claim.Possible Optimization 1 =
NRN
tokens. This would allow multiple reclaims to be processed in a single transaction, reducing the gas cost per reclaim.Here is the optimized code snippet:
function reclaimNRNBatch(uint256[] calldata nrnToReclaimAmounts, uint256[] calldata fighterIds, address[] calldata fighterOwners) external { require(msg.sender == _rankedBattleAddress, "Call must be from RankedBattle contract"); require(nrnToReclaimAmounts.length == fighterIds.length && fighterIds.length == fighterOwners.length, "Array length mismatch"); for (uint256 i = 0; i < fighterIds.length; i++) { uint256 fighterId = fighterIds[i]; uint256 nrnToReclaim = nrnToReclaimAmounts[i]; address fighterOwner = fighterOwners[i]; require(stakeAtRisk[roundId][fighterId] >= nrnToReclaim, "Insufficient stake at risk"); stakeAtRisk[roundId][fighterId] -= nrnToReclaim; totalStakeAtRisk[roundId] -= nrnToReclaim; amountLost[fighterOwner] -= nrnToReclaim; emit ReclaimedStake(fighterId, nrnToReclaim); } bool success = _neuronInstance.transfer(_rankedBattleAddress, sum(nrnToReclaimAmounts)); require(success, "NRN transfer failed"); }
Possible Optimization 2 =
Here is the optimized code:
function updateAtRiskRecordsBatch(uint256[] calldata nrnToPlaceAtRiskAmounts, uint256[] calldata fighterIds, address[] calldata fighterOwners) external { require(msg.sender == _rankedBattleAddress, "Call must be from RankedBattle contract"); require(nrnToPlaceAtRiskAmounts.length == fighterIds.length && fighterIds.length == fighterOwners.length, "Array length mismatch"); uint256 totalRiskIncrease = 0; for (uint256 i = 0; i < fighterIds.length; i++) { uint256 fighterId = fighterIds[i]; uint256 nrnToPlaceAtRisk = nrnToPlaceAtRiskAmounts[i]; address fighterOwner = fighterOwners[i]; stakeAtRisk[roundId][fighterId] += nrnToPlaceAtRisk; totalRiskIncrease += nrnToPlaceAtRisk; amountLost[fighterOwner] += nrnToPlaceAtRisk; emit IncreasedStakeAtRisk(fighterId, nrnToPlaceAtRisk); } totalStakeAtRisk[roundId] += totalRiskIncrease; }
Possible Optimization =
Here is the optimized code snippet:
function replenishVoltageBatch(address[] calldata owners) external { require(isAdmin[msg.sender], "Only admin can replenish voltage"); for (uint i = 0; i < owners.length; i++) { if (playerData[owners[i]].voltageReplenishTime <= block.timestamp) { playerData[owners[i]].voltage = 100; playerData[owners[i]].voltageReplenishTime = uint32(block.timestamp + 1 days); emit VoltageRemaining(owners[i], 100); } } }
#0 - raymondfam
2024-02-25T22:32:02Z
28 non-generic optimized code refactoring
#1 - c4-pre-sort
2024-02-25T22:32:22Z
raymondfam marked the issue as high quality report
#2 - c4-sponsor
2024-03-04T01:31:14Z
brandinho (sponsor) acknowledged
#3 - brandinho
2024-03-04T01:33:50Z
We might implement some of these optimizations, but most of the batch ones are not applicable for how we want to structure transactions with our project.
#4 - c4-judge
2024-03-19T07:45:10Z
HickupHH3 marked the issue as grade-a
๐ Selected for report: 0xSmartContract
Also found by: 0xAsen, 0xDetermination, 0xStriker, 0xblack_bird, 0xbrett8571, 0xepley, 0xweb3boy, 14si2o_Flint, Bauchibred, DarkTower, JayShreeRAM, JcFichtner, K42, McToady, Myd, PoeAudits, Rolezn, SAQ, ZanyBonzy, aariiif, albahaca, ansa-zanjbeel, cheatc0d3, clara, cudo, dimulski, fouzantanveer, foxb868, hassanshakeel13, hunter_w3b, kaveyjoe, klau5, peanuts, pipidu83, popeye, rspadi, scokaf, shaflow2, tala7985, wahedtalash77, yongskiws
20.0744 USDC - $20.07
NFT
game that leverages the ERC-721
and ERC-1155
standards for a game involving NFT
characters and in-game items.ERC721
and ERC1155
tokens for characters and items, respectively, and ERC20
tokens for in-game currency. It is supported by a range of solidity smart contracts that manage the game's logic, including battles, NFT
staking, and NFT
trading.1. Key Data Structures and Libraries in all contracts
OpenZeppelin
libraries for ERC
standards, AccessControl
, and security measures is a positive sign. As well as the use of solmate
for gas efficent fixed point math usage, that is awesome. However, the game's data and the in-game logic are tightly coupled across the contracts, which could be a risk for upgradability and long term maintainability.2. Use of Modifiers and Access Control in all contracts
onlyOwner
and isAdmin
access controls is a concern, but at first glance well implemented. The game's power is too centralized however as a result, and the use of a single account to manage the game's control can be a single point of failure.3. Use of State Variables in all contracts
public
and private
state variables across the ecosystem can be a major risk, if creative attack vectors are used. It is always a good idea to review the scoping of these variables to ensure they are only accessible to the right entities.4. Use of Events and Logging in all contracts
transferOwnership(address newOwnerAddress)
addAttributeDivisor(uint8[] memory attributeDivisors)
createPhysicalAttributes(uint256 dna, uint8 generation, uint8 iconsType, bool dendroidBool)
addAttributeProbabilities(uint256 generation, uint8[][] memory probabilities)
deleteAttributeProbabilities(uint8 generation)
getAttributeProbabilities(uint256 generation, string memory attribute)
dnaToIndex(uint256 generation, uint256 rarityRank, string memory attribute)
transferOwnership(address newOwnerAddress)
incrementGeneration(uint8 fighterType)
addStaker(address newStaker)
instantiateAIArenaHelperContract(address aiArenaHelperAddress)
claimFighters(...)
redeemMintPass(...)
updateFighterStaking(uint256 tokenId, bool stakingStatus)
setTokenURI(uint256 tokenId, string calldata newTokenURI)
updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType)
fighterCreatedEmitter(uint256 id, uint256 weight, uint256 element, uint8 generation)
getFighterAttributes(Fighter storage self)
viewFighterInfo(Fighter storage self, address owner)
owner
address.owner
parameter and the data returned by this method are in compliance with the intended game dynamics and privacy considerations. Evaluate the necessity of all the returned data and potentially restrict access to sensitive data.transferOwnership(address newOwnerAddress)
adjustAdminAccess(address adminAddress, bool access)
adjustTransferability(uint256 tokenId, bool transferable)
mint(uint256 tokenId, uint256 quantity)
setAllowedBurningAddresses(address newBurningAddress)
setTokenURI(uint256 tokenId, string memory _tokenURI)
createGameItem(...)
burn(address account, uint256 tokenId, uint256 amount)
mint(uint256 tokenId, uint256 quantity)
safeTransferFrom(...)
transferOwnership(address newOwnerAddress)
adjustAdminAccess(address adminAddress, bool access)
updateWinnersPerPeriod(uint256 newWinnersPerPeriodAmount)
pickWinner(uint256[] calldata winners)
claimRewards(...)
addPoints(uint256 tokenId, uint256 points)
getFighterPoints(uint256 maxId)
transferOwnership(address newOwnerAddress)
addMinter(address newMinterAddress)
addStaker(address newStakerAddress)
addSpender(address newSpenderAddress)
adjustAdminAccess(address adminAddress, bool access)
setupAirdrop(address[] calldata recipients, uint256[] calldata amounts)
claim(uint256 amount)
mint(address to, uint256 amount)
burn(uint256 amount)
approveSpender(address account, uint256 amount)
burnFrom(address account, uint256 amount)
transferOwnership(address newOwnerAddress)
adjustAdminAccess(address adminAddress, bool access)
setGameServerAddress(address gameServerAddress)
stakeNRN(uint256 amount, uint256 tokenId)
unstakeNRN(uint256 amount, uint256 tokenId)
claimNRN()
updateBattleRecord(...)
setRankedNrnDistribution(...)
setNewRound(uint256 roundId_)
reclaimNRN(uint256 nrnToReclaim, uint256 fighterId, address fighterOwner)
updateAtRiskRecords(uint256 nrnToPlaceAtRisk, uint256 fighterId, address fighterOwner)
transferOwnership(address newOwnerAddress)
adjustAdminAccess(address adminAddress, bool access)
NFT
gaming. By leveraging the ERC-721
and ERC-1155
standards, it has created a game that is not only unique in its play-to-earn (P2E)
dynamics but also in the deep integration of NFTs
and in-game micro-economies for a more immersive and self-sustaining game world.28 hours
#0 - c4-pre-sort
2024-02-25T20:35:40Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2024-03-19T08:11:18Z
HickupHH3 marked the issue as grade-b