| Number | Issues | Instances |
|---|---|---|
| [M-01] | Approve zero first | 1 |
| [M-02] | The surplus ether is not returned | 1 |
| [M-03] | Use _safeMint instead of _mint for ERC721 |
1 |
| [M-04] | Use .call() instead of .send() or .transfer() |
2 |
| [M-05] | Contracts are vulnerable to fee-on-transfer token related accounting issues | 17 |
| [M-06] | The owner is a single point of failure and a centralization risk | 24 |
| [M-07] | Unsafe ERC20 operations | 2 |
| [M-08] | ERC20 return values not checked | 2 |
Total: 50 over 8 instances.
| Number | Issues | Instances |
|---|---|---|
| [L-01] | Project has vulnarable NPM dependency version | 2 |
| [L-02] | Draft OpenZeppelin Dependencies | 1 |
| [L-03] | Some ERC20 revert on zero value transfer | 5 |
| [L-04] | tokenURI should throw an error if _tokenId is not a valid NFT |
1 |
| [L-05] | NFT doesn't handle hardforks | 1 |
| [L-06] | Condition will not revert when block.timestamp is == to the compared variable |
3 |
| [L-07] | Emitting storage values instead of the memory one. | 1 |
| [L-08] | Loss of precision | 1 |
| [L-09] | External calls in an un-bounded for-loop may result in a DOS |
1 |
| [L-10] | Empty receive()/payable fallback() function does not authorize requests |
1 |
| [L-11] | Use Ownable2Step's transfer function rather than Ownable's for transfers of ownership |
2 |
| [L-12] | Using named function calls is a much safer | 1 |
| [L-13] | Use safeApprove instead of approve |
1 |
| [L-14] | onlyOwner functions not accessible if owner renounces ownership |
8 |
| [L-15] | Gas griefing/theft is possible on unsafe external call |
1 |
| [L-16] | Constructor missing zero address check | 3 |
| [L-17] | Zero address check is missing | 2 |
| [L-18] | Division by zero not prevented | 1 |
| [L-19] | Unbounded for loops in public or external functions |
3 |
| [L-20] | Unsafe casting may overflow | 15 |
| [L-21] | Missing contract existence checks before low-level call | 1 |
| [L-22] | Constructor missing variable check | 6 |
Total: 55 over 21 instances.
| Number | Issues | Instances |
|---|---|---|
| [N-01] | NatSpec @return argument is missing |
24 |
| [N-02] | NatSpec @param is missing |
4 |
| [N-03] | Function must not be longer than 50 lines | 1 |
| [N-04] | It is standard for all external and public functions to be overridden from an interface | 35 |
| [N-05] | Using named parameters in mapping is best practice | 15 |
| [N-06] | Constant name must be in capitalized SNAKE_CASE | 6 |
| [N-07] | Use multiple require() and if statements instead of && |
5 |
| [N-08] | Change public to external for functions that are not called internally |
7 |
| [N-09] | internal functions not called by the contract should be removed |
2 |
| [N-10] | Expressions for constant values such as a call to keccak256(), should use immutable rather than constant |
6 |
| [N-11] | Events may be emitted out of order due to reentrancy | 17 |
| [N-12] | Setters should have initial value check | 4 |
| [N-13] | Incorrect formatting of braces in contracts, libraries, functions or structs | 6 |
| [N-14] | Unused modifier | 1 |
| [N-15] | Import only specific files | 46 |
| [N-16] | Empty function body | 4 |
| [N-17] | Incorrect formatting of braces in contracts, libraries, functions or structs | 20 |
| [N-18] | Change uint to uint256 |
1 |
| [N-19] | Events that mark critical parameter changes should contain both the old and the new value | 7 |
| [N-20] | Use uppercase for constants | 6 |
| [N-21] | Unbounded loop may run out of gas | 3 |
| [N-22] | Try to avoid double type casting | 5 |
| [N-23] | Missing event for critical parameters change | 1 |
| [N-24] | Event missing msg.sender information |
2 |
| [N-25] | Constants in comparisons should appear on the left side | 78 |
| [N-26] | Duplicated require/if checks should be refactored to a modifier or function |
4 |
| [N-27] | constants should be defined rather than using magic numbers |
3 |
| [N-28] | Do not use underscore at the end of variable name | 4 |
| [N-29] | Event is missing indexed field |
2 |
| [N-30] | Functions that alter state should emit events | 2 |
| [N-31] | Unused event definition | 1 |
| [N-32] | Use uppercase for immutables | 2 |
| [N-33] | Unsafe conversion from unsigned to signed values | 5 |
| [N-34] | State variable declaration should include comments | 3 |
| [N-35] | Empty bytes check is missing | 2 |
| [N-36] | Return values of approve() not checked |
1 |
| [N-37] | Consider disabling renounceOwnership() |
2 |
| [N-38] | Use bytes.concat() instead of abi.encodePacked(<bytes>, <bytes>) |
1 |
| [N-39] | Prefer double quotes for string quoting | 2 |
| [N-40] | Function ordering does not follow the Solidity style guide | 4 |
| [N-41] | Contract does not follow the Solidity style guide's suggested layout ordering | 3 |
| [N-42] | Private and internal variables should start with an underscore | 4 |
| [N-43] | Imports could be organized more systematically | 12 |
| [N-44] | Constant redefined elsewhere | 2 |
| [N-45] | Use solidity version 0.8.19 to gain some gas boost | 12 |
| [N-46] | Use a more recent version of Solidity | 12 |
| [N-47] | Function can be declared as pure |
8 |
| [N-48] | Consider activating via-ir for deploying |
1 |
| [N-49] | Contracts should have full test coverage | 1 |
| [N-50] | Large or complicated code bases should implement invariant tests | 1 |
| [N-51] | NatSpec documentation for function is missing | 3 |
Total: 397 over 47 instances.
| Number | Issues | Instances | Total Gas Saved |
|---|---|---|---|
| [G-01] | Use assembly to check for address(0) | 46 | - |
| [G-02] | Internal functions only called once can be inlined to save gas | 4 | 80 |
| [G-03] | Pre-increments and pre-decrements are cheaper than post-increments and post-decrements | 15 | 75 |
| [G-04] | selfbalance() is cheaper than address(this).balance |
1 | 100 |
| [G-05] | Usage of uint/int smaller than 32 bytes (256 bits) incurs overhead |
41 | - |
| [G-06] | Use unchecked keyword for loop counter |
3 | 180 |
| [G-07] | Functions guaranteed to revert when called by normal users can be marked payable |
10 | 130 |
| [G-08] | Setting the constructor to payable |
10 | 130 |
| [G-09] | >= costs less gas than > |
1 | 3 |
| [G-10] | Multiple mappings can be replaced with a single struct mapping | 6 | 252 |
| [G-11] | Avoid updating storage variables with the same value | 4 | 3200 |
| [G-12] | Use assembly to calculate hashes | 1 | 80 |
| [G-13] | Modifiers are redundant if used only once or not used at all | 2 | 80 |
| [G-14] | Use assembly to emit events | 28 | - |
| [G-15] | Use calldata instead of memory for function parameters | 6 | 300 |
| [G-16] | <array>.length should not be looked up in every loop of a for-loop |
1 | 3 |
| [G-17] | Using private rather than public for constants and immutables, saves gas |
21 | 73500 |
| [G-18] | State variables should be cached in stack variables rather than re-reading them from storage | 1 | 100 |
| [G-19] | Non efficient zero initialization | 3 | 15 |
| [G-20] | Use assembly when getting a contract’s balance of ETH | 1 | 28 |
| [G-21] | Use assembly to write address storage values | 9 | 900 |
| [G-22] | Using bools for storage incurs overhead |
6 | 102600 |
| [G-23] | Optimize names to save gas | 83 | 1826 |
Total Gas Saved: 183582
Total: 303 over 24 instances.
Total number of issues: 805
Some ERC20 tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. For example Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
384: function _approve(address token, address spender, uint256 amount, uint256 limit) internal {
385: // check that after processing this we will not have spent more than the block limit
386: uint256 spentThisBlock = blockExpenditure[block.number];
387: if (amount + spentThisBlock > limit) revert T_BlockSpendLimit();
388: blockExpenditure[block.number] = amount + spentThisBlock;
389:
390: // approve tokens
391: IERC20(token).approve(spender, amount);
392:
393: emit TreasuryApproval(token, spender, amount);
394: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L384-L394
The function is marked as payable, but the surplus ether is not returned. This may lead to the loss of ether.
The condition should be changed to check for equality, or the code should refund the excess.
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
98: function mint(
99: address recipient,
100: uint256 tokenId,
101: uint256 amount,
102: uint256 totalClaimable,
103: bytes32[] calldata merkleProof
104: ) external payable {
105: uint256 mintPrice = mintPrices[tokenId] * amount;
106: uint48 claimExpiration = claimExpirations[tokenId];
107:
108: if (block.timestamp > claimExpiration) revert RB_ClaimingExpired(claimExpiration, uint48(block.timestamp));
109: if (msg.value < mintPrice) revert RB_InvalidMintFee(mintPrice, msg.value);
110: if (!_verifyClaim(recipient, tokenId, totalClaimable, merkleProof)) revert RB_InvalidMerkleProof();
111: if (amountClaimed[recipient][tokenId] + amount > totalClaimable) {
112: revert RB_InvalidClaimAmount(amount, totalClaimable);
113: }
114:
115: // increment amount claimed
116: amountClaimed[recipient][tokenId] += amount;
117:
118: // mint to recipient
119: _mint(recipient, tokenId, amount, "");
120: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L98-L120
_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function.
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
119: _mint(recipient, tokenId, amount, "");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L119
The .send() function intends to transfer an ETH amount with a fixed amount of 2300 gas. This function is not equipped to handle changes in the underlying .send() and .transfer() functions which may supply different amounts of gas in the future. Additionally, if the recipient implements a fallback function containing some sort of logic, this may inevitably revert, meaning the vault and owner of the contract will never be able to call certain sensitive functions.
Consider using .call() instead with the checks-effects-interactions pattern implemented correctly. Careful consideration needs to be made to prevent reentrancy.
There are 2 instances of this issue:
File: contracts/ArcadeTreasury.sol
367: payable(destination).transfer(amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L367
File: contracts/nft/ReputationBadge.sol
171: payable(recipient).transfer(balance);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L171
Without measuring the balance before and after the transfer, there's no way to ensure that enough tokens were transferred, in the cases where the token has a fee-on-transfer mechanic. If there are latent funds in the contract, subsequent transfers will succeed.
There are 17 instances of this issue:
File: contracts/ARCDVestingVault.sol
167: token.safeTransfer(who, withdrawable);
172: token.safeTransfer(msg.sender, remaining);
201: token.transferFrom(msg.sender, address(this), amount);
217: token.safeTransfer(recipient, amount);
252: token.safeTransfer(msg.sender, withdrawable);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L167
File: contracts/ArcadeTreasury.sol
369: IERC20(token).safeTransfer(destination, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L369
File: contracts/NFTBoostVault.sol
258: token.safeTransfer(msg.sender, amount);
556: IERC1155(registration.tokenAddress).safeTransferFrom(
557: address(this),
558: msg.sender,
559: registration.tokenId,
560: 1,
561: bytes("")
562: );
657: token.transferFrom(from, address(this), amount);
674: IERC1155(tokenAddress).safeTransferFrom(from, address(this), tokenId, nftAmount, bytes(""));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L258
File: contracts/token/ArcadeAirdrop.sol
67: token.safeTransfer(destination, unclaimed);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L67
File: contracts/token/ArcadeTokenDistributor.sol
79: arcadeToken.safeTransfer(_treasury, treasuryAmount);
95: arcadeToken.safeTransfer(_devPartner, devPartnerAmount);
111: arcadeToken.safeTransfer(_communityRewards, communityRewardsAmount);
127: arcadeToken.safeTransfer(_communityAirdrop, communityAirdropAmount);
144: arcadeToken.safeTransfer(_vestingTeam, vestingTeamAmount);
161: arcadeToken.safeTransfer(_vestingPartner, vestingPartnerAmount);
Having a single EOA as the only owner of contracts is a large centralization risk and a single point of failure. A single private key may be taken in a hack, or the sole holder of the key may become unable to retrieve the key when necessary. Consider changing to a multi-signature setup, or having a role-based authorization model.
There are 24 instances of this issue:
File: contracts/ArcadeTreasury.sol
108: function gscSpend(
130: function smallSpend(
149: function mediumSpend(
168: function largeSpend(
189: function gscApprove(
211: function approveSmallSpend(
230: function approveMediumSpend(
249: function approveLargeSpend(
269: function setThreshold(address token, SpendThreshold memory thresholds) external onlyRole(ADMIN_ROLE) {
303: function setGSCAllowance(address token, uint256 newAllowance) external onlyRole(ADMIN_ROLE) {
333: function batchCalls(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L108
File: contracts/nft/BadgeDescriptor.sol
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57
File: contracts/nft/ReputationBadge.sol
140: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
163: function withdrawFees(address recipient) external onlyRole(BADGE_MANAGER_ROLE) {
184: function setDescriptor(address _descriptor) external onlyRole(RESOURCE_MANAGER_ROLE) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L140
File: contracts/token/ArcadeAirdrop.sol
62: function reclaim(address destination) external onlyOwner {
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L62
File: contracts/token/ArcadeTokenDistributor.sol
73: function toTreasury(address _treasury) external onlyOwner {
89: function toDevPartner(address _devPartner) external onlyOwner {
105: function toCommunityRewards(address _communityRewards) external onlyOwner {
121: function toCommunityAirdrop(address _communityAirdrop) external onlyOwner {
138: function toTeamVesting(address _vestingTeam) external onlyOwner {
155: function toPartnerVesting(address _vestingPartner) external onlyOwner {
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
Certain tokens may not adhere to the ERC20 standard completely, yet they are generally accepted by most code designed for ERC20 tokens. An instance of this is Tether (USDT), where the transfer() and transferFrom() functions on L1 do not conform to the specification's requirement of returning booleans; instead, they have no return value. Consequently, when such tokens are cast to IERC20, their function signatures do not align, leading to reverted calls (refer to this link for a test case). To mitigate this issue, it is recommended to utilize OpenZeppelin's SafeERC20's safeTransfer() and safeTransferFrom() functions instead.
There are 2 instances of this issue:
File: contracts/ARCDVestingVault.sol
201: token.transferFrom(msg.sender, address(this), amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L201
File: contracts/NFTBoostVault.sol
657: token.transferFrom(from, address(this), amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L657
Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a successful boolean. Calling these functions with the correct EIP20 function signatures will always revert.
There are 2 instances of this issue:
File: contracts/ARCDVestingVault.sol
201: token.transferFrom(msg.sender, address(this), amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L201
File: contracts/NFTBoostVault.sol
657: token.transferFrom(from, address(this), amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L657
Upgrade Openzeppelin versi,on to version 4.9.2 or higher. Check https://security.snyk.io/package/npm/@openzeppelin%2Fcontracts and https://security.snyk.io/package/npm/@openzeppelin%2Fcontracts-upgradeable for more information.
There are 2 instances of this issue:
File: package.json
103: "@openzeppelin/contracts": "4.3.2",
104: "@openzeppelin/contracts-upgradeable": "^4.7.2",
https://github.com/code-423n4/2023-07-arcade/blob/main/package.json#L103
OpenZeppelin contracts may be considered draft contracts if they have not received adequate security auditing or are liable to change with future development.
There are 1 instances of this issue:
File: contracts/token/ArcadeToken.sol
7: import "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L7
Consider checking that the sended value is not zero. Example: https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers.
There are 5 instances of this issue:
File: contracts/ARCDVestingVault.sol
201: token.transferFrom(msg.sender, address(this), amount);
217: token.safeTransfer(recipient, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L201
File: contracts/ArcadeTreasury.sol
369: IERC20(token).safeTransfer(destination, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L369
File: contracts/NFTBoostVault.sol
657: token.transferFrom(from, address(this), amount);
674: IERC1155(tokenAddress).safeTransferFrom(from, address(this), tokenId, nftAmount, bytes(""));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L657
According to EIP-721 and specifically, the metadata extension, the tokenURI function should throw an error if _tokenId is not a valid NFT. Contrary, the current implementation returns an empty string.
There are 1 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
48: function tokenURI(uint256 tokenId) external view override returns (string memory) {
49: return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
50: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L48-L50
Consider adding require(1 == chain.chainId), or the chain ID of whichever chain you prefer, to the functions below, or at least include the chain ID in the URI, so that there is no confusion about which chain is the owner of the NFT.
There are 1 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
48: function tokenURI(uint256 tokenId) external view override returns (string memory) {
49: return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
50: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L48-L50
The condition remains unaltered even if block.timestamp is equal to the compared > or < variable. For example, if there is a condition block.timestamp > proposerSignature.expirationTimestamp, it is necessary to also account for cases where block.timestamp is == to proposerSignature.expirationTimestamp.
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
308: if (uint48(block.timestamp) < lastAllowanceSet[token] + SET_ALLOWANCE_COOL_DOWN) {
309: revert T_CoolDownPeriod(block.timestamp, lastAllowanceSet[token] + SET_ALLOWANCE_COOL_DOWN);
310: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L308-L310
File: contracts/nft/ReputationBadge.sol
108: if (block.timestamp > claimExpiration) revert RB_ClaimingExpired(claimExpiration, uint48(block.timestamp));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L108
File: contracts/token/ArcadeToken.sol
146: if (block.timestamp < mintingAllowedAfter) revert AT_MintingNotStarted(mintingAllowedAfter, block.timestamp);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L146
Emitted values should not be read from storage again. Instead, the existing values from memory should be used.
There are 1 instances of this issue:
File: contracts/token/ArcadeToken.sol
136: emit MinterUpdated(minter);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L136
Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator
There are 1 instances of this issue:
File: contracts/token/ArcadeToken.sol
154: uint256 mintCapAmount = (totalSupply() * MINT_CAP) / PERCENT_DENOMINATOR;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L154
Consider limiting the number of iterations in for-loops that make external calls
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
341: (bool success, ) = targets[i].call(calldatas[i]);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L341
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))).
Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds. If the concern is having to spend a small amount of gas to check the sender against an immutable address, the code should at least have a function to rescue unused Ether.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
397: receive() external payable {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L397
Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.
There are 2 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
17: contract BadgeDescriptor is IBadgeDescriptor, Ownable {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L17
File: contracts/token/ArcadeTokenDistributor.sol
23: contract ArcadeTokenDistributor is Ownable {
Code base has an extensive use of named function calls, but it somehow missed one instance where this would be appropriate. It should use named function calls on function call, as such:
token.exampleFunction{value: value}(data);There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
341: (bool success, ) = targets[i].call(calldatas[i]);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L341
The approve function in ERC20 tokens is not safe to use. It is possible for an attacker to front-run the transaction and steal the tokens. Use safeApprove instead.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
391: IERC20(token).approve(spender, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L391
The owner is able to perform certain privileged activities, but it's possible to set the owner to address(0). This can represent a certain risk if the ownership is renounced for any other reason than by design.
Renouncing ownership will leave the contract without an owner, therefore limiting any functionality that needs authority.
There are 8 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57
File: contracts/token/ArcadeTokenDistributor.sol
73: function toTreasury(address _treasury) external onlyOwner {
89: function toDevPartner(address _devPartner) external onlyOwner {
105: function toCommunityRewards(address _communityRewards) external onlyOwner {
121: function toCommunityAirdrop(address _communityAirdrop) external onlyOwner {
138: function toTeamVesting(address _vestingTeam) external onlyOwner {
155: function toPartnerVesting(address _vestingPartner) external onlyOwner {
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
341: (bool success, ) = targets[i].call(calldatas[i]);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L341
It is important to ensure that the constructor does not allow zero address to be set. This is a common mistake that can lead to loss of funds or redeployment of the contract.
There are 3 instances of this issue:
File: contracts/ArcadeGSCCoreVoting.sol
26: constructor(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L26
File: contracts/ArcadeGSCVault.sol
35: constructor(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L35
File: contracts/ImmutableVestingVault.sol
28: constructor(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L28
Zero-address checks are a best-practise for input validation of critical address parameters. While the codebase applies this to most addresses in setters, there are many places where this is missing in constructors and setters.
There are 2 instances of this issue:
File: contracts/NFTBoostVault.sol
363: function setMultiplier(address tokenAddress, uint128 tokenId, uint128 multiplierValue) public override onlyManager {
392: function setAirdropContract(address newAirdropContract) external override onlyManager {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L363
The divisions below take an input parameter which does not have any zero-value checks, which may lead to the functions reverting when zero is passed.
There are 1 instances of this issue:
File: contracts/ARCDVestingVault.sol
330: uint256 unlocked = grant.cliffAmount + (postCliffAmount * blocksElapsedSinceCliff) / totalBlocksPostCliff;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L330
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
333: function batchCalls(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L333
File: contracts/NFTBoostVault.sol
342: function updateVotingPower(address[] calldata userAddresses) public override {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L342
File: contracts/nft/ReputationBadge.sol
140: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L140
SafeMath and Solidity 0.8.* handles overflows for basic math operations but not for casting. Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows.
There are 15 instances of this issue:
File: contracts/ARCDVestingVault.sol
148: emit VoteChange(grant.delegatee, who, int256(uint256(newVotingPower)));
166: grant.withdrawn += uint128(withdrawable);
171: grant.withdrawn += uint128(remaining);
242: grant.withdrawn += uint128(withdrawable);
244: grant.withdrawn += uint128(amount);
269: emit VoteChange(grant.delegatee, msg.sender, -1 * int256(grant.latestVotingPower));
281: emit VoteChange(to, msg.sender, int256(uint256(grant.latestVotingPower)));
350: int256 change = int256(newVotingPower) - int256(grant.latestVotingPower);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L148
File: contracts/ArcadeTreasury.sol
308: if (uint48(block.timestamp) < lastAllowanceSet[token] + SET_ALLOWANCE_COOL_DOWN) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L308
File: contracts/NFTBoostVault.sol
195: emit VoteChange(msg.sender, registration.delegatee, -1 * int256(uint256(registration.latestVotingPower)));
208: registration.latestVotingPower = uint128(addedVotingPower);
211: emit VoteChange(msg.sender, to, int256(addedVotingPower));
509: emit VoteChange(user, _delegatee, int256(uint256(newVotingPower)));
585: int256 change = int256(newVotingPower) - int256(uint256(registration.latestVotingPower));
596: registration.latestVotingPower = uint128(newVotingPower);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L195
Low-level calls return success if there is no code present at the specified address.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
341: (bool success, ) = targets[i].call(calldatas[i]);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L341
Variable checks in the constructor are important to ensure that the contract is initialized correctly.
There are 6 instances of this issue:
File: contracts/ARCDVestingVault.sol
66: constructor(IERC20 _token, uint256 _stale, address manager_, address timelock_) BaseVotingVault(_token, _stale) {
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
68: if (timelock_ == address(0)) revert AVV_ZeroAddress("timelock");
69:
70: Storage.set(Storage.addressPtr("manager"), manager_);
71: Storage.set(Storage.addressPtr("timelock"), timelock_);
72: Storage.set(Storage.uint256Ptr("entered"), 1);
73: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L66-L73
File: contracts/ArcadeGSCCoreVoting.sol
26: constructor(
27: address timelock,
28: uint256 baseQuorum,
29: uint256 minProposalPower,
30: address gsc,
31: address[] memory votingVaults
32: ) CoreVoting(timelock, baseQuorum, minProposalPower, gsc, votingVaults) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L26-L32
File: contracts/ArcadeGSCVault.sol
35: constructor(
36: ICoreVoting coreVoting,
37: uint256 votingPowerBound,
38: address owner
39: ) GSCVault(coreVoting, votingPowerBound, owner) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L35-L39
File: contracts/ImmutableVestingVault.sol
28: constructor(
29: IERC20 _token,
30: uint256 _stale,
31: address manager_,
32: address timelock_
33: ) ARCDVestingVault(_token, _stale, manager_, timelock_) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L28-L33
File: contracts/NFTBoostVault.sol
82: constructor(
83: IERC20 token,
84: uint256 staleBlockLag,
85: address timelock,
86: address manager
87: ) BaseVotingVault(token, staleBlockLag) {
88: if (timelock == address(0)) revert NBV_ZeroAddress("timelock");
89: if (manager == address(0)) revert NBV_ZeroAddress("manager");
90:
91: Storage.set(Storage.uint256Ptr("initialized"), 1);
92: Storage.set(Storage.addressPtr("timelock"), timelock);
93: Storage.set(Storage.addressPtr("manager"), manager);
94: Storage.set(Storage.uint256Ptr("entered"), 1);
95: Storage.set(Storage.uint256Ptr("locked"), 1);
96: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L82-L96
File: contracts/token/ArcadeAirdrop.sol
42: constructor(
43: address _governance,
44: bytes32 _merkleRoot,
45: IERC20 _token,
46: uint256 _expiration,
47: INFTBoostVault _votingVault
48: ) ArcadeMerkleRewards(_merkleRoot, _token, _expiration, _votingVault) {
49: if (_governance == address(0)) revert AA_ZeroAddress("governance");
50:
51: owner = _governance;
52: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L42-L52
There are 24 instances of this issue:
File: contracts/ARCDVestingVault.sol
65: // @audit @return is missing
66: function
constructor(IERC20 _token, uint256 _stale, address manager_, address timelock_) BaseVotingVault(_token, _stale)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L65-L66
File: contracts/ArcadeGSCCoreVoting.sol
25: // @audit @return is missing
26: function
constructor(
address timelock,
uint256 baseQuorum,
uint256 minProposalPower,
address gsc,
address[] memory votingVaults
) CoreVoting(timelock, baseQuorum, minProposalPower, gsc, votingVaults)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L25-L26
File: contracts/ArcadeGSCVault.sol
34: // @audit @return is missing
35: function
constructor(
ICoreVoting coreVoting,
uint256 votingPowerBound,
address owner
) GSCVault(coreVoting, votingPowerBound, owner)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L34-L35
File: contracts/ArcadeTreasury.sol
107: // @audit @return is missing
108: function gscSpend(
address token,
uint256 amount,
address destination
) external onlyRole(GSC_CORE_VOTING_ROLE) nonReentrant
129: // @audit @return is missing
130: function smallSpend(
address token,
uint256 amount,
address destination
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
148: // @audit @return is missing
149: function mediumSpend(
address token,
uint256 amount,
address destination
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
167: // @audit @return is missing
168: function largeSpend(
address token,
uint256 amount,
address destination
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
188: // @audit @return is missing
189: function gscApprove(
address token,
address spender,
uint256 amount
) external onlyRole(GSC_CORE_VOTING_ROLE) nonReentrant
210: // @audit @return is missing
211: function approveSmallSpend(
address token,
address spender,
uint256 amount
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
229: // @audit @return is missing
230: function approveMediumSpend(
address token,
address spender,
uint256 amount
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
248: // @audit @return is missing
249: function approveLargeSpend(
address token,
address spender,
uint256 amount
) external onlyRole(CORE_VOTING_ROLE) nonReentrant
268: // @audit @return is missing
269: function setThreshold(address token, SpendThreshold memory thresholds) external onlyRole(ADMIN_ROLE)
302: // @audit @return is missing
303: function setGSCAllowance(address token, uint256 newAllowance) external onlyRole(ADMIN_ROLE)
332: // @audit @return is missing
333: function batchCalls(
address[] memory targets,
bytes[] calldata calldatas
) external onlyRole(ADMIN_ROLE) nonReentrant
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L107-L108
File: contracts/NFTBoostVault.sol
81: // @audit @return is missing
82: function
constructor(
IERC20 token,
uint256 staleBlockLag,
address timelock,
address manager
) BaseVotingVault(token, staleBlockLag)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L81-L82
File: contracts/ImmutableVestingVault.sol
27: // @audit @return is missing
28: function
constructor(
IERC20 _token,
uint256 _stale,
address manager_,
address timelock_
) ARCDVestingVault(_token, _stale, manager_, timelock_)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L27-L28
File: contracts/token/ArcadeAirdrop.sol
41: // @audit @return is missing
42: function
constructor(
address _governance,
bytes32 _merkleRoot,
IERC20 _token,
uint256 _expiration,
INFTBoostVault _votingVault
) ArcadeMerkleRewards(_merkleRoot, _token, _expiration, _votingVault)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L41-L42
File: contracts/token/ArcadeToken.sol
112: // @audit @return is missing
113: function
constructor(address _minter, address _initialDistribution) ERC20("Arcade", "ARCD") ERC20Permit("Arcade")
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L112-L113
File: contracts/nft/ReputationBadge.sol
74: // @audit @return is missing
75: function
constructor(address _owner, address _descriptor) ERC1155("")
128: // @audit @return is missing
129: function uri(uint256 tokenId) public view override(ERC1155, IReputationBadge) returns (string memory)
139: // @audit @return is missing
140: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE)
162: // @audit @return is missing
163: function withdrawFees(address recipient) external onlyRole(BADGE_MANAGER_ROLE)
183: // @audit @return is missing
184: function setDescriptor(address _descriptor) external onlyRole(RESOURCE_MANAGER_ROLE)
216: // @audit @return is missing
217: function supportsInterface(
bytes4 interfaceId
) public view override(ERC1155, AccessControl, IERC165) returns (bool)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L74-L75
There are 4 instances of this issue:
File: contracts/NFTBoostVault.sol
696: // @audit missing @param: memory, uint256, address
697: function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L696-L697
File: contracts/BaseVotingVault.sol
95: // @audit missing @param: calldata
96: function queryVotePower(address user, uint256 blockNumber, bytes calldata) external override returns (uint256)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L95-L96
File: contracts/ImmutableVestingVault.sol
39: // @audit missing @param: address
40: function revokeGrant(address) public pure override
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L39-L40
File: contracts/nft/ReputationBadge.sol
216: // @audit missing @param: interfaceId
217: function supportsInterface(
bytes4 interfaceId
) public view override(ERC1155, AccessControl, IERC165) returns (bool)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L216-L217
There are 1 instances of this issue:
File: contracts/ARCDVestingVault.sol
91: function addGrantAndDelegate(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L91
Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface.
There are 35 instances of this issue:
File: contracts/ARCDVestingVault.sol
91: function addGrantAndDelegate(
157: function revokeGrant(address who) external virtual onlyManager {
197: function deposit(uint256 amount) external onlyManager {
260: function delegate(address to) external {
293: function claimable(address who) external view returns (uint256) {
304: function getGrant(address who) external view returns (ARCDVestingVaultStorage.Grant memory) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L91
File: contracts/ArcadeTreasury.sol
108: function gscSpend(
130: function smallSpend(
149: function mediumSpend(
168: function largeSpend(
189: function gscApprove(
211: function approveSmallSpend(
230: function approveMediumSpend(
249: function approveLargeSpend(
269: function setThreshold(address token, SpendThreshold memory thresholds) external onlyRole(ADMIN_ROLE) {
303: function setGSCAllowance(address token, uint256 newAllowance) external onlyRole(ADMIN_ROLE) {
333: function batchCalls(
397: receive() external payable {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L108
File: contracts/NFTBoostVault.sol
697: function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L697
File: contracts/token/ArcadeAirdrop.sol
62: function reclaim(address destination) external onlyOwner {
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L62
File: contracts/token/ArcadeToken.sol
132: function setMinter(address _newMinter) external onlyMinter {
145: function mint(address _to, uint256 _amount) external onlyMinter {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L132
File: contracts/token/ArcadeTokenDistributor.sol
73: function toTreasury(address _treasury) external onlyOwner {
89: function toDevPartner(address _devPartner) external onlyOwner {
105: function toCommunityRewards(address _communityRewards) external onlyOwner {
121: function toCommunityAirdrop(address _communityAirdrop) external onlyOwner {
138: function toTeamVesting(address _vestingTeam) external onlyOwner {
155: function toPartnerVesting(address _vestingPartner) external onlyOwner {
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
File: contracts/nft/BadgeDescriptor.sol
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57
File: contracts/nft/ReputationBadge.sol
98: function mint(
140: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
163: function withdrawFees(address recipient) external onlyRole(BADGE_MANAGER_ROLE) {
184: function setDescriptor(address _descriptor) external onlyRole(RESOURCE_MANAGER_ROLE) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L98
There are 15 instances of this issue:
File: contracts/ArcadeTreasury.sol
59: mapping(address => uint48) public lastAllowanceSet;
59: mapping(address => uint48) public lastAllowanceSet;
62: mapping(address => SpendThreshold) public spendThresholds;
62: mapping(address => SpendThreshold) public spendThresholds;
65: mapping(address => uint256) public gscAllowance;
65: mapping(address => uint256) public gscAllowance;
68: mapping(uint256 => uint256) public blockExpenditure;
68: mapping(uint256 => uint256) public blockExpenditure;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L59
File: contracts/nft/ReputationBadge.sol
49: mapping(address => mapping(uint256 => uint256)) public amountClaimed;
52: mapping(uint256 => bytes32) public claimRoots;
52: mapping(uint256 => bytes32) public claimRoots;
55: mapping(uint256 => uint48) public claimExpirations;
55: mapping(uint256 => uint48) public claimExpirations;
58: mapping(uint256 => uint256) public mintPrices;
58: mapping(uint256 => uint256) public mintPrices;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L49
There are 6 instances of this issue:
File: contracts/token/ArcadeTokenDistributor.sol
32: uint256 public constant treasuryAmount = 25_500_000 ether;
37: uint256 public constant devPartnerAmount = 600_000 ether;
42: uint256 public constant communityRewardsAmount = 15_000_000 ether;
47: uint256 public constant communityAirdropAmount = 10_000_000 ether;
52: uint256 public constant vestingTeamAmount = 16_200_000 ether;
57: uint256 public constant vestingPartnerAmount = 32_700_000 ether;
Using multiple require() and if improves code readability and makes it easier to debug.
There are 5 instances of this issue:
File: contracts/NFTBoostVault.sol
247: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
317: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
472: if (_tokenAddress != address(0) && _tokenId != 0) {
632: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
659: if (tokenAddress != address(0) && tokenId != 0) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L247
Contracts are allowed to override their parents' functions and change the visibility from external to public.
There are 7 instances of this issue:
File: contracts/BaseVotingVault.sol
126: function timelock() public view returns (address) {
137: function manager() public view returns (address) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L126
File: contracts/ImmutableVestingVault.sol
40: function revokeGrant(address) public pure override {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L40
File: contracts/NFTBoostVault.sol
342: function updateVotingPower(address[] calldata userAddresses) public override {
363: function setMultiplier(address tokenAddress, uint128 tokenId, uint128 multiplierValue) public override onlyManager {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L342
File: contracts/nft/ReputationBadge.sol
129: function uri(uint256 tokenId) public view override(ERC1155, IReputationBadge) returns (string memory) {
217: function supportsInterface(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L129
There are 2 instances of this issue:
File: contracts/BaseVotingVault.sol
148: function _balance() internal pure returns (Storage.Uint256 storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L148
File: contracts/nft/ReputationBadge.sol
204: function _verifyClaim(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L204
[N-10] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant
Although the compiler may automatically handle this mistake without saving any gas, it is still preferable to utilize the appropriate tool for each task. It is important to distinguish between constant variables and immutable variables and use them accordingly. Constants should be employed for literal values explicitly written in the code, whereas immutable variables are better suited for expressions or values calculated within or passed into the constructor.
There are 6 instances of this issue:
File: contracts/ArcadeTreasury.sol
48: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
49: bytes32 public constant GSC_CORE_VOTING_ROLE = keccak256("GSC_CORE_VOTING");
50: bytes32 public constant CORE_VOTING_ROLE = keccak256("CORE_VOTING");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L48
File: contracts/nft/ReputationBadge.sol
44: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
45: bytes32 public constant BADGE_MANAGER_ROLE = keccak256("BADGE_MANAGER");
46: bytes32 public constant RESOURCE_MANAGER_ROLE = keccak256("RESOURCE_MANAGER");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L44
Ensure that events follow the best practice of check-effects-interaction, and are emitted before external calls
There are 17 instances of this issue:
File: contracts/ARCDVestingVault.sol
148: emit VoteChange(grant.delegatee, who, int256(uint256(newVotingPower)));
269: emit VoteChange(grant.delegatee, msg.sender, -1 * int256(grant.latestVotingPower));
356: emit VoteChange(grant.delegatee, who, change);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L148
File: contracts/ArcadeTreasury.sol
372: emit TreasuryTransfer(token, destination, amount);
393: emit TreasuryApproval(token, spender, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L372
File: contracts/NFTBoostVault.sol
195: emit VoteChange(msg.sender, registration.delegatee, -1 * int256(uint256(registration.latestVotingPower)));
382: emit WithdrawalsUnlocked();
395: emit AirdropContractUpdated(newAirdropContract);
509: emit VoteChange(user, _delegatee, int256(uint256(newVotingPower)));
598: emit VoteChange(who, registration.delegatee, change);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L195
File: contracts/nft/ReputationBadge.sol
173: emit FeesWithdrawn(recipient, balance);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L173
File: contracts/token/ArcadeTokenDistributor.sol
81: emit Distribute(address(arcadeToken), _treasury, treasuryAmount);
97: emit Distribute(address(arcadeToken), _devPartner, devPartnerAmount);
113: emit Distribute(address(arcadeToken), _communityRewards, communityRewardsAmount);
129: emit Distribute(address(arcadeToken), _communityAirdrop, communityAirdropAmount);
146: emit Distribute(address(arcadeToken), _vestingTeam, vestingTeamAmount);
163: emit Distribute(address(arcadeToken), _vestingPartner, vestingPartnerAmount);
Setters should have initial value check to prevent assigning wrong value to the variable. Assignment of wrong value can lead to unexpected behavior of the contract.
There are 4 instances of this issue:
File: contracts/NFTBoostVault.sol
// @audit Not validated: tokenAddress
363: function setMultiplier(address tokenAddress, uint128 tokenId, uint128 multiplierValue) public override onlyManager {
364: if (multiplierValue > MAX_MULTIPLIER) revert NBV_MultiplierLimit();
365:
366: NFTBoostVaultStorage.AddressUintUint storage multiplierData = _getMultipliers()[tokenAddress][tokenId];
367: // set multiplier value
368: multiplierData.multiplier = multiplierValue;
369:
370: emit MultiplierSet(tokenAddress, tokenId, multiplierValue);
371: }
// @audit Not validated: newAirdropContract
392: function setAirdropContract(address newAirdropContract) external override onlyManager {
393: Storage.set(Storage.addressPtr("airdrop"), newAirdropContract);
394:
395: emit AirdropContractUpdated(newAirdropContract);
396: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L363-L371
File: contracts/nft/BadgeDescriptor.sol
// @audit Not validated: newBaseURI
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
58: baseURI = newBaseURI;
59:
60: emit SetBaseURI(msg.sender, newBaseURI);
61: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57-L61
File: contracts/token/ArcadeAirdrop.sol
// @audit Not validated: _merkleRoot
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
76: rewardsRoot = _merkleRoot;
77:
78: emit SetMerkleRoot(_merkleRoot);
79: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L75-L79
The braces denoting the body of a contract, library, functions and structs should:
-
Open on the same line as the declaration.
-
Close on their own line at the same indentation level as the beginning of the declaration.
-
The opening brace should be preceded by a single space.
There are 6 instances of this issue:
File: contracts/ARCDVestingVault.sol
16: import {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L16
File: contracts/ArcadeTreasury.sol
11: import {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L11
File: contracts/NFTBoostVault.sol
15: import {
686: {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L15
File: contracts/nft/ReputationBadge.sol
14: import {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L14
File: contracts/token/ArcadeToken.sol
11: import {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L11
Consider removing unused modifier or moving it to a higher level contract.
There are 1 instances of this issue:
File: contracts/BaseVotingVault.sol
187: modifier onlyManager() {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L187
Using import declarations of the form import {<identifier_name>} from "some/file.sol" avoids polluting the symbol namespace making flattened files smaller, and speeds up compilation
There are 46 instances of this issue:
File: contracts/ARCDVestingVault.sol
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
7: import "./external/council/libraries/History.sol";
8: import "./external/council/libraries/Storage.sol";
10: import "./libraries/ARCDVestingVaultStorage.sol";
11: import "./libraries/HashedStorageReentrancyBlock.sol";
13: import "./interfaces/IARCDVestingVault.sol";
14: import "./BaseVotingVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L5
File: contracts/ArcadeGSCCoreVoting.sol
5: import "./external/council/CoreVoting.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L5
File: contracts/ArcadeGSCVault.sol
5: import "./external/council/vaults/GSCVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L5
File: contracts/ArcadeTreasury.sol
5: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
6: import "@openzeppelin/contracts/access/AccessControl.sol";
7: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
9: import "./interfaces/IArcadeTreasury.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L5
File: contracts/BaseVotingVault.sol
5: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
7: import "./external/council/libraries/History.sol";
8: import "./external/council/libraries/Storage.sol";
10: import "./libraries/HashedStorageReentrancyBlock.sol";
12: import "./interfaces/IBaseVotingVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L5
File: contracts/ImmutableVestingVault.sol
5: import "./ARCDVestingVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L5
File: contracts/NFTBoostVault.sol
5: import "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
8: import "./external/council/libraries/History.sol";
9: import "./external/council/libraries/Storage.sol";
11: import "./libraries/NFTBoostVaultStorage.sol";
12: import "./interfaces/INFTBoostVault.sol";
13: import "./BaseVotingVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L5
File: contracts/nft/BadgeDescriptor.sol
5: import "@openzeppelin/contracts/access/Ownable.sol";
6: import "@openzeppelin/contracts/utils/Strings.sol";
8: import "../interfaces/IBadgeDescriptor.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L5
File: contracts/nft/ReputationBadge.sol
5: import "@openzeppelin/contracts/token/ERC1155/ERC1155.sol";
6: import "@openzeppelin/contracts/token/ERC1155/extensions/ERC1155Burnable.sol";
7: import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
8: import "@openzeppelin/contracts/access/AccessControl.sol";
9: import "@openzeppelin/contracts/utils/Strings.sol";
11: import "../interfaces/IReputationBadge.sol";
12: import "../interfaces/IBadgeDescriptor.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L5
File: contracts/token/ArcadeAirdrop.sol
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
7: import "../external/council/libraries/Authorizable.sol";
9: import "../libraries/ArcadeMerkleRewards.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L5
File: contracts/token/ArcadeToken.sol
5: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
6: import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
7: import "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
9: import "../interfaces/IArcadeToken.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L5
File: contracts/token/ArcadeTokenDistributor.sol
5: import "@openzeppelin/contracts/access/Ownable.sol";
6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
8: import "../interfaces/IArcadeToken.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeTokenDistributor.sol#L5
Consider commenting out the empty function body.
To improve the code, either the mentioned blocks should be refactored to serve a useful purpose such as emitting an event or reverting, or they should be eliminated entirely. If the contract is intended for extension, it should be declared as abstract and include function signatures without any default implementation. If an empty if-statement block is used to avoid subsequent checks in else-if/else conditions, it is recommended to nest the else-if/else conditions under the negation of the if-statement. This separation helps to prevent potential errors when modifying the code later on (e.g., if (x) {...} else if (y) {...} else {...} => if (!x) { if (y) {...} else {...} }). Lastly, unused empty receive()/fallback() payable functions can be removed to reduce deployment gas consumption.
There are 4 instances of this issue:
File: contracts/ArcadeGSCCoreVoting.sol
26: constructor(
27: address timelock,
28: uint256 baseQuorum,
29: uint256 minProposalPower,
30: address gsc,
31: address[] memory votingVaults
32: ) CoreVoting(timelock, baseQuorum, minProposalPower, gsc, votingVaults) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L26-L32
File: contracts/ArcadeGSCVault.sol
35: constructor(
36: ICoreVoting coreVoting,
37: uint256 votingPowerBound,
38: address owner
39: ) GSCVault(coreVoting, votingPowerBound, owner) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L35-L39
File: contracts/ArcadeTreasury.sol
397: receive() external payable {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L397
File: contracts/ImmutableVestingVault.sol
28: constructor(
29: IERC20 _token,
30: uint256 _stale,
31: address manager_,
32: address timelock_
33: ) ARCDVestingVault(_token, _stale, manager_, timelock_) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L28-L33
The braces denoting the body of a contract, library, functions and structs should:
-
Open on the same line as the declaration.
-
Close on their own line at the same indentation level as the beginning of the declaration.
-
The opening brace should be preceded by a single space.
There are 20 instances of this issue:
File: contracts/ARCDVestingVault.sol
91: function addGrantAndDelegate(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L91
File: contracts/ArcadeTreasury.sol
108: function gscSpend(
130: function smallSpend(
149: function mediumSpend(
168: function largeSpend(
189: function gscApprove(
211: function approveSmallSpend(
230: function approveMediumSpend(
249: function approveLargeSpend(
333: function batchCalls(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L108
File: contracts/NFTBoostVault.sol
114: function addNftAndDelegate(
139: function airdropReceive(
462: function _registerAndDelegate(
608: function _getWithdrawableAmount(
627: function _currentVotingPower(
650: function _lockTokens(
682: function _getMultipliers()
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L114
File: contracts/nft/ReputationBadge.sol
98: function mint(
204: function _verifyClaim(
217: function supportsInterface(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L98
The uint type is an alias for uint256. It is recommended to use uint256 instead of uint for better readability.
There are 1 instances of this issue:
File: contracts/BaseVotingVault.sol
146: * @return balance A struct containing the balance uint.
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L146
It is important to have both the old and the new value in the event. This way, the user can easily track the changes.
There are 7 instances of this issue:
File: contracts/ArcadeTreasury.sol
322: emit GSCAllowanceUpdated(token, newAllowance);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L322
File: contracts/NFTBoostVault.sol
370: emit MultiplierSet(tokenAddress, tokenId, multiplierValue);
395: emit AirdropContractUpdated(newAirdropContract);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L370
File: contracts/nft/BadgeDescriptor.sol
60: emit SetBaseURI(msg.sender, newBaseURI);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L60
File: contracts/nft/ReputationBadge.sol
189: emit SetDescriptor(msg.sender, _descriptor);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L189
File: contracts/token/ArcadeAirdrop.sol
78: emit SetMerkleRoot(_merkleRoot);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L78
File: contracts/token/ArcadeToken.sol
136: emit MinterUpdated(minter);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L136
Constants should be in uppercase as stated Solidity style guide.
There are 6 instances of this issue:
File: contracts/token/ArcadeTokenDistributor.sol
32: uint256 public constant treasuryAmount = 25_500_000 ether;
37: uint256 public constant devPartnerAmount = 600_000 ether;
42: uint256 public constant communityRewardsAmount = 15_000_000 ether;
47: uint256 public constant communityAirdropAmount = 10_000_000 ether;
52: uint256 public constant vestingTeamAmount = 16_200_000 ether;
57: uint256 public constant vestingPartnerAmount = 32_700_000 ether;
Consider limiting the number of iterations in loops with an explicit revert reason to improve the user experience.
The function would eventually revert if out of gas anyway, but by limiting it the user avoids wasting too much gas, as the loop doesn't execute if an excessive value is provided.
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
339: for (uint256 i = 0; i < targets.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L339
File: contracts/NFTBoostVault.sol
345: for (uint256 i = 0; i < userAddresses.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L345
File: contracts/nft/ReputationBadge.sol
144: for (uint256 i = 0; i < _claimData.length; i++) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L144
Performing double type casting in Solidity contracts should be avoided to prevent unintended consequences and ensure accurate data representation. Successive type casts can introduce unexpected truncation, rounding errors, or loss of precision. Additionally, excessive type casting can make the code less readable and harder to maintain, increasing the likelihood of errors and misunderstandings during development and debugging. To ensure precise and consistent data handling, it is recommended for developers to utilize appropriate data types and minimize unnecessary or excessive type casting, promoting a more robust and dependable execution of the contract.
There are 5 instances of this issue:
File: contracts/ARCDVestingVault.sol
148: emit VoteChange(grant.delegatee, who, int256(uint256(newVotingPower)));
281: emit VoteChange(to, msg.sender, int256(uint256(grant.latestVotingPower)));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L148
File: contracts/NFTBoostVault.sol
195: emit VoteChange(msg.sender, registration.delegatee, -1 * int256(uint256(registration.latestVotingPower)));
509: emit VoteChange(user, _delegatee, int256(uint256(newVotingPower)));
585: int256 change = int256(newVotingPower) - int256(uint256(registration.latestVotingPower));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L195
It is important to log critical parameters change to be able to track the state of the contract.
There are 1 instances of this issue:
File: contracts/token/ArcadeToken.sol
145: function mint(address _to, uint256 _amount) external onlyMinter {
146: if (block.timestamp < mintingAllowedAfter) revert AT_MintingNotStarted(mintingAllowedAfter, block.timestamp);
147: if (_to == address(0)) revert AT_ZeroAddress("to");
148: if (_amount == 0) revert AT_ZeroMintAmount();
149:
150: // record the mint
151: mintingAllowedAfter = block.timestamp + MIN_TIME_BETWEEN_MINTS;
152:
153: // inflation cap enforcement - 2% of total supply
154: uint256 mintCapAmount = (totalSupply() * MINT_CAP) / PERCENT_DENOMINATOR;
155: if (_amount > mintCapAmount) {
156: revert AT_MintingCapExceeded(totalSupply(), mintCapAmount, _amount);
157: }
158:
159: _mint(_to, _amount);
160: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L145-L160
In scenarios where actions are initiated by users, the absence of the ability to filter events based on the initiator of the action can significantly complicate event processing. By including the msg.sender in events associated with such actions, events become considerably more valuable for end users, particularly when msg.sender differs from tx.origin.
There are 2 instances of this issue:
File: contracts/ArcadeTreasury.sol
290: emit SpendThresholdsUpdated(token, thresholds);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L290
File: contracts/token/ArcadeToken.sol
136: emit MinterUpdated(minter);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L136
Doing so will prevent typo bugs
There are 78 instances of this issue:
File: contracts/ARCDVestingVault.sol
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
68: if (timelock_ == address(0)) revert AVV_ZeroAddress("timelock");
101: if (who == address(0)) revert AVV_ZeroAddress("who");
102: if (amount == 0) revert AVV_InvalidAmount();
105: if (startTime == 0) {
122: if (grant.allocation != 0) revert AVV_HasGrant();
162: if (grant.allocation == 0) revert AVV_NoGrantSet();
229: if (amount == 0) revert AVV_InvalidAmount();
233: if (grant.allocation == 0) revert AVV_NoGrantSet();
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L67
File: contracts/ArcadeTreasury.sol
88: if (_timelock == address(0)) revert T_ZeroAddress("timelock");
113: if (destination == address(0)) revert T_ZeroAddress("destination");
114: if (amount == 0) revert T_ZeroAmount();
135: if (destination == address(0)) revert T_ZeroAddress("destination");
136: if (amount == 0) revert T_ZeroAmount();
154: if (destination == address(0)) revert T_ZeroAddress("destination");
155: if (amount == 0) revert T_ZeroAmount();
173: if (destination == address(0)) revert T_ZeroAddress("destination");
174: if (amount == 0) revert T_ZeroAmount();
194: if (spender == address(0)) revert T_ZeroAddress("spender");
195: if (amount == 0) revert T_ZeroAmount();
216: if (spender == address(0)) revert T_ZeroAddress("spender");
217: if (amount == 0) revert T_ZeroAmount();
235: if (spender == address(0)) revert T_ZeroAddress("spender");
236: if (amount == 0) revert T_ZeroAmount();
254: if (spender == address(0)) revert T_ZeroAddress("spender");
255: if (amount == 0) revert T_ZeroAmount();
271: if (token == address(0)) revert T_ZeroAddress("token");
273: if (thresholds.small == 0) revert T_ZeroAmount();
304: if (token == address(0)) revert T_ZeroAddress("token");
305: if (newAllowance == 0) revert T_ZeroAmount();
340: if (spendThresholds[targets[i]].small != 0) revert T_InvalidTarget(targets[i]);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L88
File: contracts/BaseVotingVault.sol
53: if (address(_token) == address(0)) revert BVV_ZeroAddress("token");
69: if (timelock_ == address(0)) revert BVV_ZeroAddress("timelock");
81: if (manager_ == address(0)) revert BVV_ZeroAddress("manager");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L53
File: contracts/NFTBoostVault.sol
88: if (timelock == address(0)) revert NBV_ZeroAddress("timelock");
89: if (manager == address(0)) revert NBV_ZeroAddress("manager");
120: if (amount == 0) revert NBV_ZeroAmount();
144: if (amount == 0) revert NBV_ZeroAmount();
145: if (user == address(0)) revert NBV_ZeroAddress("user");
152: if (registration.delegatee == address(0)) {
183: if (to == address(0)) revert NBV_ZeroAddress("to");
224: if (getIsLocked() == 1) revert NBV_Locked();
225: if (amount == 0) revert NBV_ZeroAmount();
247: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
267: if (amount == 0) revert NBV_ZeroAmount();
273: if (registration.delegatee == address(0)) revert NBV_NoRegistration();
306: if (newTokenAddress == address(0) || newTokenId == 0) revert NBV_InvalidNft(newTokenAddress, newTokenId);
314: if (registration.delegatee == address(0)) revert NBV_NoRegistration();
317: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
379: if (getIsLocked() != 1) revert NBV_AlreadyUnlocked();
422: if (tokenAddress == address(0) || tokenId == 0) {
472: if (_tokenAddress != address(0) && _tokenId != 0) {
477: if (multiplier == 0) revert NBV_NoMultiplierSet();
488: if (registration.delegatee != address(0)) revert NBV_HasRegistration();
552: if (registration.tokenAddress == address(0) || registration.tokenId == 0)
588: if (change == 0) return;
632: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
659: if (tokenAddress != address(0) && tokenId != 0) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L88
File: contracts/nft/ReputationBadge.sol
76: if (_owner == address(0)) revert RB_ZeroAddress("owner");
77: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
141: if (_claimData.length == 0) revert RB_NoClaimData();
164: if (recipient == address(0)) revert RB_ZeroAddress("recipient");
185: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L76
File: contracts/token/ArcadeAirdrop.sol
49: if (_governance == address(0)) revert AA_ZeroAddress("governance");
64: if (destination == address(0)) revert AA_ZeroAddress("destination");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L49
File: contracts/token/ArcadeToken.sol
114: if (_minter == address(0)) revert AT_ZeroAddress("minter");
115: if (_initialDistribution == address(0)) revert AT_ZeroAddress("initialDistribution");
133: if (_newMinter == address(0)) revert AT_ZeroAddress("newMinter");
147: if (_to == address(0)) revert AT_ZeroAddress("to");
148: if (_amount == 0) revert AT_ZeroMintAmount();
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L114
File: contracts/token/ArcadeTokenDistributor.sol
75: if (_treasury == address(0)) revert AT_ZeroAddress("treasury");
91: if (_devPartner == address(0)) revert AT_ZeroAddress("devPartner");
107: if (_communityRewards == address(0)) revert AT_ZeroAddress("communityRewards");
123: if (_communityAirdrop == address(0)) revert AT_ZeroAddress("communityAirdrop");
140: if (_vestingTeam == address(0)) revert AT_ZeroAddress("vestingTeam");
157: if (_vestingPartner == address(0)) revert AT_ZeroAddress("vestingPartner");
175: if (address(_arcadeToken) == address(0)) revert AT_ZeroAddress("arcadeToken");
176: if (address(arcadeToken) != address(0)) revert AT_TokenAlreadySet();
This saves deployment gas cost.
There are 4 instances of this issue:
File: contracts/ARCDVestingVault.sol
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L67
File: contracts/NFTBoostVault.sol
88: if (timelock == address(0)) revert NBV_ZeroAddress("timelock");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L88
File: contracts/nft/ReputationBadge.sol
76: if (_owner == address(0)) revert RB_ZeroAddress("owner");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L76
File: contracts/token/ArcadeToken.sol
114: if (_minter == address(0)) revert AT_ZeroAddress("minter");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L114
Magic numbers are numbers that appear without explanation in the code. They should be replaced with named constants.
There are 3 instances of this issue:
File: contracts/NFTBoostVault.sol
343: if (userAddresses.length > 50) revert NBV_ArrayTooManyElements();
380: Storage.set(Storage.uint256Ptr("locked"), 2);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L343
File: contracts/nft/ReputationBadge.sol
142: if (_claimData.length > 50) revert RB_ArrayTooLarge();
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L142
The use of underscore at the end of the variable name is uncommon and also suggests that the variable name was not completely changed.
Consider refactoring variableName_ to variableName.
There are 4 instances of this issue:
File: contracts/ARCDVestingVault.sol
66: constructor(IERC20 _token, uint256 _stale, address manager_, address timelock_) BaseVotingVault(_token, _stale) {
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
68: if (timelock_ == address(0)) revert AVV_ZeroAddress("timelock");
69:
70: Storage.set(Storage.addressPtr("manager"), manager_);
71: Storage.set(Storage.addressPtr("timelock"), timelock_);
72: Storage.set(Storage.uint256Ptr("entered"), 1);
73: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L66-L73
File: contracts/BaseVotingVault.sol
68: function setTimelock(address timelock_) external onlyTimelock {
69: if (timelock_ == address(0)) revert BVV_ZeroAddress("timelock");
70:
71: Storage.set(Storage.addressPtr("timelock"), timelock_);
72: }
80: function setManager(address manager_) external onlyTimelock {
81: if (manager_ == address(0)) revert BVV_ZeroAddress("manager");
82:
83: Storage.set(Storage.addressPtr("manager"), manager_);
84: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L68-L72
File: contracts/ImmutableVestingVault.sol
28: constructor(
29: IERC20 _token,
30: uint256 _stale,
31: address manager_,
32: address timelock_
33: ) ARCDVestingVault(_token, _stale, manager_, timelock_) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L28-L33
There are 2 instances of this issue:
File: contracts/token/ArcadeToken.sol
102: event MinterUpdated(address newMinter);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L102
File: contracts/token/ArcadeTokenDistributor.sol
64: event Distribute(address token, address recipient, uint256 amount);
Functions that alter state should emit events to notify users of the state change. This is especially important for functions that alter state and do not return a value.
There are 2 instances of this issue:
File: contracts/token/ArcadeToken.sol
145: function mint(address _to, uint256 _amount) external onlyMinter {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L145
File: contracts/token/ArcadeTokenDistributor.sol
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
There are 1 instances of this issue:
File: contracts/BaseVotingVault.sol
41: event VoteChange(address indexed from, address indexed to, int256 amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L41
Immutables should be in uppercase as stated Solidity style guide.
There are 2 instances of this issue:
File: contracts/BaseVotingVault.sol
33: IERC20 public immutable token;
36: uint256 public immutable staleBlockLag;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L33
Solidity follows two's complement rules for its integers, meaning that the most significant bit for signed integers is used to denote the sign, and converting between the two requires inverting all of the bits and adding one. Because of this, casting an unsigned integer to a signed one may result in a change of the sign and or magnitude of the value. For example, int8(type(uint8).max) is not equal to type(int8).max, but is equal to -1. type(uint8).max in binary is 11111111, which if cast to a signed value, means the first binary 1 indicates a negative value, and the binary 1s, invert to all zeroes, and when one is added, it becomes one, but negative, and therefore the decimal value of binary 11111111 is -1.
There are 5 instances of this issue:
File: contracts/ARCDVestingVault.sol
148: emit VoteChange(grant.delegatee, who, int256(uint256(newVotingPower)));
350: int256 change = int256(newVotingPower) - int256(grant.latestVotingPower);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L148
File: contracts/NFTBoostVault.sol
211: emit VoteChange(msg.sender, to, int256(addedVotingPower));
509: emit VoteChange(user, _delegatee, int256(uint256(newVotingPower)));
585: int256 change = int256(newVotingPower) - int256(uint256(registration.latestVotingPower));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L211
State variables should be commented to explain their purpose.
There are 3 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
19:
20: event SetBaseURI(address indexed caller, string baseURI);
23:
24: string public baseURI;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L19-L20
File: contracts/token/ArcadeAirdrop.sol
25:
26: event SetMerkleRoot(bytes32 indexed merkleRoot);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L25-L26
To avoid mistakenly accepting empty bytes as a valid value, consider to check for empty bytes. This helps ensure that empty bytes are not treated as valid inputs, preventing potential issues.
There are 2 instances of this issue:
File: contracts/token/ArcadeAirdrop.sol
42: constructor(
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L42
Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
391: IERC20(token).approve(spender, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L391
Typically, the contract’s owner is the account that deploys the contract. As a result, the owner is able to perform certain privileged activities. The OpenZeppelin’s Ownable used in this project contract implements renounceOwnership. This can represent a certain risk if the ownership is renounced for any other reason than by design. Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.
There are 2 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
17: contract BadgeDescriptor is IBadgeDescriptor, Ownable {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L17
File: contracts/token/ArcadeTokenDistributor.sol
23: contract ArcadeTokenDistributor is Ownable {
Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>).
Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>).
There are 1 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
49: return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L49
Strings should be quoted with double quotes (") instead of single quotes (').
There are 2 instances of this issue:
File: contracts/ArcadeGSCVault.sol
11: * The Arcade GSC Voting Vault contract enables a 'council' of delegates who act as a steering committee
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L11
File: contracts/BaseVotingVault.sol
100: // Find the historical data and clear everything more than 'staleBlockLag' into the past
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L100
Source: https://docs.soliditylang.org/en/v0.8.17/style-guide.html#order-of-layout
There are 4 instances of this issue:
File: contracts/ARCDVestingVault.sol
341: // @audit internal function can not go after internal view function
342: function _syncVotingPower(address who, ARCDVestingVaultStorage.Grant storage grant) internal {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L341-L342
File: contracts/NFTBoostVault.sol
378: // @audit external function can not go after public function
379: function unlock() external override onlyTimelock {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L378-L379
File: contracts/BaseVotingVault.sol
159: // @audit internal view function can not go after internal pure function
160: function _timelock() internal view returns (Storage.Address storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L159-L160
File: contracts/nft/ReputationBadge.sol
140: // @audit external function can not go after public view function
141: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L140-L141
Inside each contract, library or interface, use the following order:
- Type declarations
- State variables
- Events
- Errors
- Modifiers
- Functions
Source: https://docs.soliditylang.org/en/v0.8.17/style-guide.html#order-of-layout
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
397: // @audit receive function can not go after internal function
398: receive() external payable {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L397-L398
File: contracts/token/ArcadeToken.sol
167: // @audit modifier definition can not go after external function
168: modifier onlyMinter() {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L167-L168
File: contracts/nft/BadgeDescriptor.sol
24: // @audit state variable declaration can not go after event definition
25: string public baseURI;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L24-L25
There are 4 instances of this issue:
File: contracts/ARCDVestingVault.sol
341: // @audit internal function can not go after internal view function
342: function _syncVotingPower(address who, ARCDVestingVaultStorage.Grant storage grant) internal {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L341-L342
File: contracts/NFTBoostVault.sol
378: // @audit external function can not go after public function
379: function unlock() external override onlyTimelock {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L378-L379
File: contracts/BaseVotingVault.sol
159: // @audit internal view function can not go after internal pure function
160: function _timelock() internal view returns (Storage.Address storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L159-L160
File: contracts/nft/ReputationBadge.sol
140: // @audit external function can not go after public view function
141: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L140-L141
The contract's interface should be imported first, followed by each of the interfaces it uses, followed by all other files. The examples below do not follow this layout.
There are 12 instances of this issue:
File: contracts/ARCDVestingVault.sol
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L5
File: contracts/ArcadeGSCCoreVoting.sol
5: import "./external/council/CoreVoting.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L5
File: contracts/ArcadeGSCVault.sol
5: import "./external/council/vaults/GSCVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L5
File: contracts/ArcadeTreasury.sol
5: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L5
File: contracts/BaseVotingVault.sol
5: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L5
File: contracts/ImmutableVestingVault.sol
5: import "./ARCDVestingVault.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L5
File: contracts/NFTBoostVault.sol
5: import "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L5
File: contracts/nft/BadgeDescriptor.sol
5: import "@openzeppelin/contracts/access/Ownable.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L5
File: contracts/nft/ReputationBadge.sol
5: import "@openzeppelin/contracts/token/ERC1155/ERC1155.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L5
File: contracts/token/ArcadeAirdrop.sol
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L5
File: contracts/token/ArcadeToken.sol
5: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L5
File: contracts/token/ArcadeTokenDistributor.sol
5: import "@openzeppelin/contracts/access/Ownable.sol";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeTokenDistributor.sol#L5
Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract’s value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don’t get out of sync.
There are 2 instances of this issue:
File: contracts/ArcadeTreasury.sol
48: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L48
File: contracts/nft/ReputationBadge.sol
44: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L44
There are 12 instances of this issue:
File: contracts/ARCDVestingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L3
File: contracts/ArcadeGSCCoreVoting.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L3
File: contracts/ArcadeGSCVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L3
File: contracts/ArcadeTreasury.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L3
File: contracts/BaseVotingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L3
File: contracts/ImmutableVestingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L3
File: contracts/NFTBoostVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L3
File: contracts/nft/BadgeDescriptor.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L3
File: contracts/nft/ReputationBadge.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L3
File: contracts/token/ArcadeAirdrop.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L3
File: contracts/token/ArcadeToken.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L3
File: contracts/token/ArcadeTokenDistributor.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeTokenDistributor.sol#L3
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining.
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads.
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings.
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(,).
There are 12 instances of this issue:
File: contracts/ARCDVestingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L3
File: contracts/ArcadeGSCCoreVoting.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L3
File: contracts/ArcadeGSCVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L3
File: contracts/ArcadeTreasury.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L3
File: contracts/BaseVotingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L3
File: contracts/ImmutableVestingVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L3
File: contracts/NFTBoostVault.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L3
File: contracts/nft/BadgeDescriptor.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L3
File: contracts/nft/ReputationBadge.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L3
File: contracts/token/ArcadeAirdrop.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L3
File: contracts/token/ArcadeToken.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L3
File: contracts/token/ArcadeTokenDistributor.sol
3: pragma solidity 0.8.18;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeTokenDistributor.sol#L3
Functions below do not change the state of the contract and can be declared as pure. It will save gas and make the code more readable.
There are 8 instances of this issue:
File: /contracts/BaseVotingVault.sol
159 : function _timelock() internal view returns (Storage.Address storage) {
170 : function _manager() internal view returns (Storage.Address storage) {
159 : function _timelock() internal view returns (Storage.Address storage) {
170 : function _manager() internal view returns (Storage.Address storage) {
159 : function _timelock() internal view returns (Storage.Address storage) {
170 : function _manager() internal view returns (Storage.Address storage) {
159 : function _timelock() internal view returns (Storage.Address storage) {
170 : function _manager() internal view returns (Storage.Address storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main//contracts/BaseVotingVault.sol#L 159
The IR-based code generator was introduced with an aim to not only allow code generation to be more transparent and auditable but also to enable more powerful optimization passes that span across functions.
You can enable it on the command line using --via-ir or with the option {"viaIR": true}.
This will take longer to compile, but you can just simple test it before deploying and if you got a better benchmark then you can add --via-ir to your deploy command
https://docs.soliditylang.org/en/v0.8.20/ir-breaking-changes.html#solidity-ir-based-codegen-changes
There are 1 instances of this issue:
File: All files
While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.
There are 1 instances of this issue:
File: All files
Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement invariant fuzzing tests. Invariant fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and invariant fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.
There are 1 instances of this issue:
File: All files
It is recommend to document all functions with NatSpec comments. This helps to understand the purpose of the function and its parameters. It also helps to generate documentation for the smart contract.
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
215:
216: /// @notice function override
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L215-L216
There are 46 instances of this issue:
File: contracts/ARCDVestingVault.sol
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
68: if (timelock_ == address(0)) revert AVV_ZeroAddress("timelock");
101: if (who == address(0)) revert AVV_ZeroAddress("who");
125: delegatee = delegatee == address(0) ? who : delegatee;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L67
File: contracts/ArcadeTreasury.sol
88: if (_timelock == address(0)) revert T_ZeroAddress("timelock");
113: if (destination == address(0)) revert T_ZeroAddress("destination");
135: if (destination == address(0)) revert T_ZeroAddress("destination");
154: if (destination == address(0)) revert T_ZeroAddress("destination");
173: if (destination == address(0)) revert T_ZeroAddress("destination");
194: if (spender == address(0)) revert T_ZeroAddress("spender");
216: if (spender == address(0)) revert T_ZeroAddress("spender");
235: if (spender == address(0)) revert T_ZeroAddress("spender");
254: if (spender == address(0)) revert T_ZeroAddress("spender");
271: if (token == address(0)) revert T_ZeroAddress("token");
304: if (token == address(0)) revert T_ZeroAddress("token");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L88
File: contracts/NFTBoostVault.sol
88: if (timelock == address(0)) revert NBV_ZeroAddress("timelock");
89: if (manager == address(0)) revert NBV_ZeroAddress("manager");
145: if (user == address(0)) revert NBV_ZeroAddress("user");
152: if (registration.delegatee == address(0)) {
183: if (to == address(0)) revert NBV_ZeroAddress("to");
273: if (registration.delegatee == address(0)) revert NBV_NoRegistration();
306: if (newTokenAddress == address(0) || newTokenId == 0) revert NBV_InvalidNft(newTokenAddress, newTokenId);
314: if (registration.delegatee == address(0)) revert NBV_NoRegistration();
422: if (tokenAddress == address(0) || tokenId == 0) {
491: _delegatee = _delegatee == address(0) ? user : _delegatee;
552: if (registration.tokenAddress == address(0) || registration.tokenId == 0)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L88
File: contracts/BaseVotingVault.sol
53: if (address(_token) == address(0)) revert BVV_ZeroAddress("token");
69: if (timelock_ == address(0)) revert BVV_ZeroAddress("timelock");
81: if (manager_ == address(0)) revert BVV_ZeroAddress("manager");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L53
File: contracts/token/ArcadeAirdrop.sol
49: if (_governance == address(0)) revert AA_ZeroAddress("governance");
64: if (destination == address(0)) revert AA_ZeroAddress("destination");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L49
File: contracts/token/ArcadeToken.sol
114: if (_minter == address(0)) revert AT_ZeroAddress("minter");
115: if (_initialDistribution == address(0)) revert AT_ZeroAddress("initialDistribution");
133: if (_newMinter == address(0)) revert AT_ZeroAddress("newMinter");
147: if (_to == address(0)) revert AT_ZeroAddress("to");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L114
File: contracts/token/ArcadeTokenDistributor.sol
75: if (_treasury == address(0)) revert AT_ZeroAddress("treasury");
91: if (_devPartner == address(0)) revert AT_ZeroAddress("devPartner");
107: if (_communityRewards == address(0)) revert AT_ZeroAddress("communityRewards");
123: if (_communityAirdrop == address(0)) revert AT_ZeroAddress("communityAirdrop");
140: if (_vestingTeam == address(0)) revert AT_ZeroAddress("vestingTeam");
157: if (_vestingPartner == address(0)) revert AT_ZeroAddress("vestingPartner");
175: if (address(_arcadeToken) == address(0)) revert AT_ZeroAddress("arcadeToken");
File: contracts/nft/ReputationBadge.sol
76: if (_owner == address(0)) revert RB_ZeroAddress("owner");
77: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
164: if (recipient == address(0)) revert RB_ZeroAddress("recipient");
185: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L76
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 4 instances of this issue:
File: contracts/BaseVotingVault.sol
159: function _timelock() internal view returns (Storage.Address storage) {
170: function _manager() internal view returns (Storage.Address storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L159
File: contracts/NFTBoostVault.sol
521: function _grantVotingPower(address delegatee, uint128 newVotingPower) internal {
608: function _getWithdrawableAmount(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L521
Saves 5 gas per iteration.
There are 15 instances of this issue:
File: contracts/nft/ReputationBadge.sol
144: for (uint256 i = 0; i < _claimData.length; i++) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L144
File: contracts/token/ArcadeToken.sol
29: * :--====-::
31: * .=#%%#*+=-----=+*%%%%%%*.
34: * .-+#%%%%%%%%%%%%%%#+-. :%%%%%=
35: * .-+#%%%%%%%%%%%%%%%%%%%%%%#+-. %%%%%*
36: * .-+#%%%%%%%%%%%%#+::=*%%%%%%%%%%%%#+-. .%%%%%*
37: * :+#%%%%%%%%%%%%#+- :=*%%%%%%%%%%%%#+: -%%%%%+
40: * *%%%%%%%%%%%%%%+- :=*%%%%%%%%%%%%%* :%%%%%%=
48: * - *%%%%%%*: +%%%%%%= -+%%%%%%%%%-
51: * #%# -+%%%%%%%%%%%#=*%%%%%%++#%%%%%%%%%%%%%*.
55: * +%%%: -+%%%%%%%%%%%%%%%*:
57: * :%%%%: .-+%%%%%%%%%%*-
59: * +%%%%%#*+===+*#%%%%%%%%%%%%#+-
60: * :*%%%%%%%%%%%%%%%%%%%%#+-.
61: * -+#%%%%%%%%%%#*+-:
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L29
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
167: uint256 balance = address(this).balance;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L167
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Use a larger size then downcast where needed.
There are 41 instances of this issue:
File: contracts/ARCDVestingVault.sol
93: uint128 amount,
94: uint128 cliffAmount,
95: uint128 startTime,
96: uint128 expiration,
97: uint128 cliff,
106: startTime = uint128(block.number);
128: uint128 newVotingPower = amount;
166: grant.withdrawn += uint128(withdrawable);
171: grant.withdrawn += uint128(remaining);
242: grant.withdrawn += uint128(withdrawable);
244: grant.withdrawn += uint128(amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L93
File: contracts/ArcadeTreasury.sol
56: uint48 public constant SET_ALLOWANCE_COOL_DOWN = 7 days;
59: mapping(address => uint48) public lastAllowanceSet;
308: if (uint48(block.timestamp) < lastAllowanceSet[token] + SET_ALLOWANCE_COOL_DOWN) {
319: lastAllowanceSet[token] = uint48(block.timestamp);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L56
File: contracts/NFTBoostVault.sol
66: uint128 public constant MAX_MULTIPLIER = 1.5e3;
69: uint128 public constant MULTIPLIER_DENOMINATOR = 1e3;
115: uint128 amount,
116: uint128 tokenId,
141: uint128 amount,
208: registration.latestVotingPower = uint128(addedVotingPower);
223: function withdraw(uint128 amount) external override nonReentrant {
266: function addTokens(uint128 amount) external override nonReentrant {
305: function updateNft(uint128 newTokenId, address newTokenAddress) external override nonReentrant {
363: function setMultiplier(address tokenAddress, uint128 tokenId, uint128 multiplierValue) public override onlyManager {
418: function getMultiplier(address tokenAddress, uint128 tokenId) public view override returns (uint128) {
464: uint128 _amount,
465: uint128 _tokenId,
469: uint128 multiplier = 1e3;
494: uint128 newVotingPower = (_amount * multiplier) / MULTIPLIER_DENOMINATOR;
521: function _grantVotingPower(address delegatee, uint128 newVotingPower) internal {
596: registration.latestVotingPower = uint128(newVotingPower);
630: uint128 locked = registration.amount - registration.withdrawn;
654: uint128 tokenId,
655: uint128 nftAmount
673: function _lockNft(address from, address tokenAddress, uint128 tokenId, uint128 nftAmount) internal {
685: returns (mapping(address => mapping(uint128 => NFTBoostVaultStorage.AddressUintUint)) storage)
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L66
File: contracts/nft/ReputationBadge.sol
55: mapping(uint256 => uint48) public claimExpirations;
106: uint48 claimExpiration = claimExpirations[tokenId];
108: if (block.timestamp > claimExpiration) revert RB_ClaimingExpired(claimExpiration, uint48(block.timestamp));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L55
File: contracts/token/ArcadeToken.sol
80: uint48 public constant MIN_TIME_BETWEEN_MINTS = 365 days;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L80
Using unchecked blocks saves just a tiny bit of gas, but in instances where its clearly safe already it’s possible to avoid this unnecessary check.
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
339: for (uint256 i = 0; i < targets.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L339
File: contracts/NFTBoostVault.sol
345: for (uint256 i = 0; i < userAddresses.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L345
File: contracts/nft/ReputationBadge.sol
144: for (uint256 i = 0; i < _claimData.length; i++) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L144
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost
There are 10 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57
File: contracts/token/ArcadeAirdrop.sol
62: function reclaim(address destination) external onlyOwner {
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L62
File: contracts/token/ArcadeTokenDistributor.sol
73: function toTreasury(address _treasury) external onlyOwner {
89: function toDevPartner(address _devPartner) external onlyOwner {
105: function toCommunityRewards(address _communityRewards) external onlyOwner {
121: function toCommunityAirdrop(address _communityAirdrop) external onlyOwner {
138: function toTeamVesting(address _vestingTeam) external onlyOwner {
155: function toPartnerVesting(address _vestingPartner) external onlyOwner {
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.
There are 10 instances of this issue:
File: contracts/ARCDVestingVault.sol
66: constructor(IERC20 _token, uint256 _stale, address manager_, address timelock_) BaseVotingVault(_token, _stale) {
67: if (manager_ == address(0)) revert AVV_ZeroAddress("manager");
68: if (timelock_ == address(0)) revert AVV_ZeroAddress("timelock");
69:
70: Storage.set(Storage.addressPtr("manager"), manager_);
71: Storage.set(Storage.addressPtr("timelock"), timelock_);
72: Storage.set(Storage.uint256Ptr("entered"), 1);
73: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L66-L73
File: contracts/ArcadeGSCCoreVoting.sol
26: constructor(
27: address timelock,
28: uint256 baseQuorum,
29: uint256 minProposalPower,
30: address gsc,
31: address[] memory votingVaults
32: ) CoreVoting(timelock, baseQuorum, minProposalPower, gsc, votingVaults) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCCoreVoting.sol#L26-L32
File: contracts/ArcadeGSCVault.sol
35: constructor(
36: ICoreVoting coreVoting,
37: uint256 votingPowerBound,
38: address owner
39: ) GSCVault(coreVoting, votingPowerBound, owner) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeGSCVault.sol#L35-L39
File: contracts/ArcadeTreasury.sol
87: constructor(address _timelock) {
88: if (_timelock == address(0)) revert T_ZeroAddress("timelock");
89:
90: _setupRole(ADMIN_ROLE, _timelock);
91: _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
92: _setRoleAdmin(GSC_CORE_VOTING_ROLE, ADMIN_ROLE);
93: _setRoleAdmin(CORE_VOTING_ROLE, ADMIN_ROLE);
94: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L87-L94
File: contracts/ImmutableVestingVault.sol
28: constructor(
29: IERC20 _token,
30: uint256 _stale,
31: address manager_,
32: address timelock_
33: ) ARCDVestingVault(_token, _stale, manager_, timelock_) {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L28-L33
File: contracts/NFTBoostVault.sol
82: constructor(
83: IERC20 token,
84: uint256 staleBlockLag,
85: address timelock,
86: address manager
87: ) BaseVotingVault(token, staleBlockLag) {
88: if (timelock == address(0)) revert NBV_ZeroAddress("timelock");
89: if (manager == address(0)) revert NBV_ZeroAddress("manager");
90:
91: Storage.set(Storage.uint256Ptr("initialized"), 1);
92: Storage.set(Storage.addressPtr("timelock"), timelock);
93: Storage.set(Storage.addressPtr("manager"), manager);
94: Storage.set(Storage.uint256Ptr("entered"), 1);
95: Storage.set(Storage.uint256Ptr("locked"), 1);
96: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L82-L96
File: contracts/nft/BadgeDescriptor.sol
34: constructor(string memory _baseURI) {
35: // Empty baseURI is allowed
36: baseURI = _baseURI;
37: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L34-L37
File: contracts/nft/ReputationBadge.sol
75: constructor(address _owner, address _descriptor) ERC1155("") {
76: if (_owner == address(0)) revert RB_ZeroAddress("owner");
77: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
78:
79: _setupRole(ADMIN_ROLE, _owner);
80: _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
81: _setRoleAdmin(BADGE_MANAGER_ROLE, ADMIN_ROLE);
82: _setRoleAdmin(RESOURCE_MANAGER_ROLE, ADMIN_ROLE);
83:
84: descriptor = IBadgeDescriptor(_descriptor);
85: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L75-L85
File: contracts/token/ArcadeAirdrop.sol
42: constructor(
43: address _governance,
44: bytes32 _merkleRoot,
45: IERC20 _token,
46: uint256 _expiration,
47: INFTBoostVault _votingVault
48: ) ArcadeMerkleRewards(_merkleRoot, _token, _expiration, _votingVault) {
49: if (_governance == address(0)) revert AA_ZeroAddress("governance");
50:
51: owner = _governance;
52: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L42-L52
File: contracts/token/ArcadeToken.sol
113: constructor(address _minter, address _initialDistribution) ERC20("Arcade", "ARCD") ERC20Permit("Arcade") {
114: if (_minter == address(0)) revert AT_ZeroAddress("minter");
115: if (_initialDistribution == address(0)) revert AT_ZeroAddress("initialDistribution");
116:
117: minter = _minter;
118:
119: mintingAllowedAfter = block.timestamp + MIN_TIME_BETWEEN_MINTS;
120:
121: // mint the initial amount of tokens for distribution
122: _mint(_initialDistribution, INITIAL_MINT_AMOUNT);
123: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L113-L123
The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas. Similarly for <=.
There are 1 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
49: return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L49
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot.
There are 6 instances of this issue:
File: contracts/ArcadeTreasury.sol
59: mapping(address => uint48) public lastAllowanceSet;
62: mapping(address => SpendThreshold) public spendThresholds;
65: mapping(address => uint256) public gscAllowance;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L59
File: contracts/nft/ReputationBadge.sol
52: mapping(uint256 => bytes32) public claimRoots;
55: mapping(uint256 => uint48) public claimExpirations;
58: mapping(uint256 => uint256) public mintPrices;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L52
Ensure that the storage variables are not updated with the same value as it will result in unnecessary gas consumption.
There are 4 instances of this issue:
File: contracts/nft/BadgeDescriptor.sol
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
58: baseURI = newBaseURI;
59:
60: emit SetBaseURI(msg.sender, newBaseURI);
61: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L57-L61
File: contracts/nft/ReputationBadge.sol
184: function setDescriptor(address _descriptor) external onlyRole(RESOURCE_MANAGER_ROLE) {
185: if (_descriptor == address(0)) revert RB_ZeroAddress("descriptor");
186:
187: descriptor = IBadgeDescriptor(_descriptor);
188:
189: emit SetDescriptor(msg.sender, _descriptor);
190: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L184-L190
File: contracts/token/ArcadeToken.sol
132: function setMinter(address _newMinter) external onlyMinter {
133: if (_newMinter == address(0)) revert AT_ZeroAddress("newMinter");
134:
135: minter = _newMinter;
136: emit MinterUpdated(minter);
137: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L132-L137
File: contracts/token/ArcadeTokenDistributor.sol
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
175: if (address(_arcadeToken) == address(0)) revert AT_ZeroAddress("arcadeToken");
176: if (address(arcadeToken) != address(0)) revert AT_TokenAlreadySet();
177:
178: arcadeToken = _arcadeToken;
179: }
Using assembly to calculate hashes can save 80 gas per instance.
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
211: bytes32 leafHash = keccak256(abi.encodePacked(recipient, tokenId, totalClaimable));
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L211
There are 2 instances of this issue:
File: contracts/BaseVotingVault.sol
187: modifier onlyManager() {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L187
File: contracts/NFTBoostVault.sol
701: modifier onlyAirdrop() {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L701
We can use assembly to emit events efficiently by utilizing scratch space and the free memory pointer. This will allow us to potentially avoid memory expansion costs.
Note: In order to do this optimization safely, we will need to cache and restore the free memory pointer.
There are 28 instances of this issue:
File: contracts/ARCDVestingVault.sol
148: emit VoteChange(grant.delegatee, who, int256(uint256(newVotingPower)));
269: emit VoteChange(grant.delegatee, msg.sender, -1 * int256(grant.latestVotingPower));
281: emit VoteChange(to, msg.sender, int256(uint256(grant.latestVotingPower)));
356: emit VoteChange(grant.delegatee, who, change);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L148
File: contracts/ArcadeTreasury.sol
284: emit GSCAllowanceUpdated(token, thresholds.small);
290: emit SpendThresholdsUpdated(token, thresholds);
322: emit GSCAllowanceUpdated(token, newAllowance);
372: emit TreasuryTransfer(token, destination, amount);
393: emit TreasuryApproval(token, spender, amount);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L284
File: contracts/NFTBoostVault.sol
195: emit VoteChange(msg.sender, registration.delegatee, -1 * int256(uint256(registration.latestVotingPower)));
211: emit VoteChange(msg.sender, to, int256(addedVotingPower));
370: emit MultiplierSet(tokenAddress, tokenId, multiplierValue);
382: emit WithdrawalsUnlocked();
395: emit AirdropContractUpdated(newAirdropContract);
509: emit VoteChange(user, _delegatee, int256(uint256(newVotingPower)));
598: emit VoteChange(who, registration.delegatee, change);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L195
File: contracts/nft/BadgeDescriptor.sol
60: emit SetBaseURI(msg.sender, newBaseURI);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L60
File: contracts/nft/ReputationBadge.sol
155: emit RootsPublished(_claimData);
173: emit FeesWithdrawn(recipient, balance);
189: emit SetDescriptor(msg.sender, _descriptor);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L155
File: contracts/token/ArcadeAirdrop.sol
78: emit SetMerkleRoot(_merkleRoot);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L78
File: contracts/token/ArcadeToken.sol
136: emit MinterUpdated(minter);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L136
File: contracts/token/ArcadeTokenDistributor.sol
81: emit Distribute(address(arcadeToken), _treasury, treasuryAmount);
97: emit Distribute(address(arcadeToken), _devPartner, devPartnerAmount);
113: emit Distribute(address(arcadeToken), _communityRewards, communityRewardsAmount);
129: emit Distribute(address(arcadeToken), _communityAirdrop, communityAirdropAmount);
146: emit Distribute(address(arcadeToken), _vestingTeam, vestingTeamAmount);
163: emit Distribute(address(arcadeToken), _vestingPartner, vestingPartnerAmount);
If you are not modifying the function parameters, consider using calldata instead of memory. This will save gas.
There are 6 instances of this issue:
File: contracts/ARCDVestingVault.sol
317: function _getWithdrawableAmount(ARCDVestingVaultStorage.Grant memory grant) internal view returns (uint256) {
318: // if before cliff or created date, no tokens have unlocked
319: if (block.number < grant.cliff) {
320: return 0;
321: }
322: // if after expiration, return the full allocation minus what has already been withdrawn
323: if (block.number >= grant.expiration) {
324: return grant.allocation - grant.withdrawn;
325: }
326: // if after cliff, return vested amount minus what has already been withdrawn
327: uint256 postCliffAmount = grant.allocation - grant.cliffAmount;
328: uint256 blocksElapsedSinceCliff = block.number - grant.cliff;
329: uint256 totalBlocksPostCliff = grant.expiration - grant.cliff;
330: uint256 unlocked = grant.cliffAmount + (postCliffAmount * blocksElapsedSinceCliff) / totalBlocksPostCliff;
331:
332: return unlocked - grant.withdrawn;
333: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L317-L333
File: contracts/ArcadeTreasury.sol
269: function setThreshold(address token, SpendThreshold memory thresholds) external onlyRole(ADMIN_ROLE) {
270: // verify that the token is not the zero address
271: if (token == address(0)) revert T_ZeroAddress("token");
272: // verify small threshold is not zero
273: if (thresholds.small == 0) revert T_ZeroAmount();
274:
275: // verify thresholds are ascending from small to large
276: if (thresholds.large < thresholds.medium || thresholds.medium < thresholds.small) {
277: revert T_ThresholdsNotAscending();
278: }
279:
280: // if gscAllowance is greater than new small threshold, set it to the new small threshold
281: if (thresholds.small < gscAllowance[token]) {
282: gscAllowance[token] = thresholds.small;
283:
284: emit GSCAllowanceUpdated(token, thresholds.small);
285: }
286:
287: // Overwrite the spend limits for specified token
288: spendThresholds[token] = thresholds;
289:
290: emit SpendThresholdsUpdated(token, thresholds);
291: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L269-L291
File: contracts/NFTBoostVault.sol
608: function _getWithdrawableAmount(
609: NFTBoostVaultStorage.Registration memory registration
610: ) internal pure returns (uint256) {
611: if (registration.withdrawn == registration.amount) {
612: return 0;
613: }
614:
615: return registration.amount - registration.withdrawn;
616: }
627: function _currentVotingPower(
628: NFTBoostVaultStorage.Registration memory registration
629: ) internal view virtual returns (uint256) {
630: uint128 locked = registration.amount - registration.withdrawn;
631:
632: if (registration.tokenAddress != address(0) && registration.tokenId != 0) {
633: return (locked * getMultiplier(registration.tokenAddress, registration.tokenId)) / MULTIPLIER_DENOMINATOR;
634: }
635:
636: return locked;
637: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L608-L616
File: contracts/nft/BadgeDescriptor.sol
34: constructor(string memory _baseURI) {
35: // Empty baseURI is allowed
36: baseURI = _baseURI;
37: }
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
58: baseURI = newBaseURI;
59:
60: emit SetBaseURI(msg.sender, newBaseURI);
61: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L34-L37
Caching the array length outside a loop saves reading it on each iteration.
There are 1 instances of this issue:
File: contracts/ArcadeTreasury.sol
339: for (uint256 i = 0; i < targets.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L339
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants.
It saves ~3500 deployment gas cost.
There are 21 instances of this issue:
File: contracts/ArcadeTreasury.sol
48: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
49: bytes32 public constant GSC_CORE_VOTING_ROLE = keccak256("GSC_CORE_VOTING");
50: bytes32 public constant CORE_VOTING_ROLE = keccak256("CORE_VOTING");
56: uint48 public constant SET_ALLOWANCE_COOL_DOWN = 7 days;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L48
File: contracts/BaseVotingVault.sol
33: IERC20 public immutable token;
36: uint256 public immutable staleBlockLag;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L33
File: contracts/NFTBoostVault.sol
66: uint128 public constant MAX_MULTIPLIER = 1.5e3;
69: uint128 public constant MULTIPLIER_DENOMINATOR = 1e3;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L66
File: contracts/nft/ReputationBadge.sol
44: bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
45: bytes32 public constant BADGE_MANAGER_ROLE = keccak256("BADGE_MANAGER");
46: bytes32 public constant RESOURCE_MANAGER_ROLE = keccak256("RESOURCE_MANAGER");
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L44
File: contracts/token/ArcadeToken.sol
80: uint48 public constant MIN_TIME_BETWEEN_MINTS = 365 days;
83: uint256 public constant MINT_CAP = 2;
86: uint256 public constant PERCENT_DENOMINATOR = 100;
89: uint256 public constant INITIAL_MINT_AMOUNT = 100_000_000 ether;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L80
File: contracts/token/ArcadeTokenDistributor.sol
32: uint256 public constant treasuryAmount = 25_500_000 ether;
37: uint256 public constant devPartnerAmount = 600_000 ether;
42: uint256 public constant communityRewardsAmount = 15_000_000 ether;
47: uint256 public constant communityAirdropAmount = 10_000_000 ether;
52: uint256 public constant vestingTeamAmount = 16_200_000 ether;
57: uint256 public constant vestingPartnerAmount = 32_700_000 ether;
Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 1 instances of this issue:
File: contracts/token/ArcadeToken.sol
// @audit cache minter
167: modifier onlyMinter() {
168: if (msg.sender != minter) revert AT_MinterNotCaller(minter);
169: _;
170: }
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L167-L170
Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.
There are 3 instances of this issue:
File: contracts/ArcadeTreasury.sol
339: for (uint256 i = 0; i < targets.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L339
File: contracts/NFTBoostVault.sol
345: for (uint256 i = 0; i < userAddresses.length; ++i) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L345
File: contracts/nft/ReputationBadge.sol
144: for (uint256 i = 0; i < _claimData.length; i++) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L144
You can use selfbalance() instead of address(this).balance when getting your contract’s balance of ETH to save gas. Additionally, you can use balance(address) instead of address.balance() when getting an external contract’s balance of ETH.
Example: https://solodit.xyz/issues/5567.
There are 1 instances of this issue:
File: contracts/nft/ReputationBadge.sol
167: uint256 balance = address(this).balance;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L167
There are 9 instances of this issue:
File: contracts/BaseVotingVault.sol
56: token = _token;
57: staleBlockLag = _staleBlockLag;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L56
File: contracts/nft/ReputationBadge.sol
84: descriptor = IBadgeDescriptor(_descriptor);
187: descriptor = IBadgeDescriptor(_descriptor);
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L84
File: contracts/token/ArcadeToken.sol
117: minter = _minter;
119: mintingAllowedAfter = block.timestamp + MIN_TIME_BETWEEN_MINTS;
135: minter = _newMinter;
151: mintingAllowedAfter = block.timestamp + MIN_TIME_BETWEEN_MINTS;
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L117
File: contracts/token/ArcadeTokenDistributor.sol
178: arcadeToken = _arcadeToken;
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See example.
There are 6 instances of this issue:
File: contracts/token/ArcadeTokenDistributor.sol
77: treasurySent = true;
93: devPartnerSent = true;
109: communityRewardsSent = true;
125: communityAirdropSent = true;
142: vestingTeamSent = true;
159: vestingPartnerSent = true;
Public/external function names and public member variable names can be optimized to save gas. You can refer to this link for an example of how the optimization works.
Below are the interfaces/abstract contracts that can be optimized to ensure that the most frequently-called functions use the least amount of gas possible during method lookup. By having method IDs with two leading zero bytes, you can save 128 gas each during deployment. Additionally, renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted.
There are 83 instances of this issue:
File: contracts/ARCDVestingVault.sol
91: function addGrantAndDelegate(
157: function revokeGrant(address who) external virtual onlyManager {
197: function deposit(uint256 amount) external onlyManager {
211: function withdraw(uint256 amount, address recipient) external override onlyManager {
228: function claim(uint256 amount) external override nonReentrant {
260: function delegate(address to) external {
293: function claimable(address who) external view returns (uint256) {
304: function getGrant(address who) external view returns (ARCDVestingVaultStorage.Grant memory) {
317: function _getWithdrawableAmount(ARCDVestingVaultStorage.Grant memory grant) internal view returns (uint256) {
341: function _syncVotingPower(address who, ARCDVestingVaultStorage.Grant storage grant) internal {
368: function _grants() internal pure returns (mapping(address => ARCDVestingVaultStorage.Grant) storage) {
381: function _unassigned() internal pure returns (Storage.Uint256 storage) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ARCDVestingVault.sol#L91
File: contracts/ArcadeTreasury.sol
108: function gscSpend(
130: function smallSpend(
149: function mediumSpend(
168: function largeSpend(
189: function gscApprove(
211: function approveSmallSpend(
230: function approveMediumSpend(
249: function approveLargeSpend(
269: function setThreshold(address token, SpendThreshold memory thresholds) external onlyRole(ADMIN_ROLE) {
303: function setGSCAllowance(address token, uint256 newAllowance) external onlyRole(ADMIN_ROLE) {
333: function batchCalls(
358: function _spend(address token, uint256 amount, address destination, uint256 limit) internal {
384: function _approve(address token, address spender, uint256 amount, uint256 limit) internal {
397: receive() external payable {}
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L108
File: contracts/BaseVotingVault.sol
68: function setTimelock(address timelock_) external onlyTimelock {
80: function setManager(address manager_) external onlyTimelock {
96: function queryVotePower(address user, uint256 blockNumber, bytes calldata) external override returns (uint256) {
112: function queryVotePowerView(address user, uint256 blockNumber) external view returns (uint256) {
126: function timelock() public view returns (address) {
137: function manager() public view returns (address) {
148: function _balance() internal pure returns (Storage.Uint256 storage) {
159: function _timelock() internal view returns (Storage.Address storage) {
170: function _manager() internal view returns (Storage.Address storage) {
179: function _votingPower() internal pure returns (History.HistoricalBalances memory) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/BaseVotingVault.sol#L68
File: contracts/ImmutableVestingVault.sol
40: function revokeGrant(address) public pure override {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ImmutableVestingVault.sol#L40
File: contracts/NFTBoostVault.sol
114: function addNftAndDelegate(
139: function airdropReceive(
182: function delegate(address to) external override {
223: function withdraw(uint128 amount) external override nonReentrant {
266: function addTokens(uint128 amount) external override nonReentrant {
293: function withdrawNft() external override nonReentrant {
305: function updateNft(uint128 newTokenId, address newTokenAddress) external override nonReentrant {
342: function updateVotingPower(address[] calldata userAddresses) public override {
363: function setMultiplier(address tokenAddress, uint128 tokenId, uint128 multiplierValue) public override onlyManager {
378: function unlock() external override onlyTimelock {
392: function setAirdropContract(address newAirdropContract) external override onlyManager {
405: function getIsLocked() public view override returns (uint256) {
418: function getMultiplier(address tokenAddress, uint128 tokenId) public view override returns (uint128) {
436: function getRegistration(address who) external view override returns (NFTBoostVaultStorage.Registration memory) {
445: function getAirdropContract() external view override returns (address) {
462: function _registerAndDelegate(
521: function _grantVotingPower(address delegatee, uint128 newVotingPower) internal {
539: function _getRegistrations() internal pure returns (mapping(address => NFTBoostVaultStorage.Registration) storage) {
548: function _withdrawNft() internal {
579: function _syncVotingPower(address who, NFTBoostVaultStorage.Registration storage registration) internal {
608: function _getWithdrawableAmount(
627: function _currentVotingPower(
650: function _lockTokens(
673: function _lockNft(address from, address tokenAddress, uint128 tokenId, uint128 nftAmount) internal {
682: function _getMultipliers()
697: function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/NFTBoostVault.sol#L114
File: contracts/nft/BadgeDescriptor.sol
48: function tokenURI(uint256 tokenId) external view override returns (string memory) {
57: function setBaseURI(string memory newBaseURI) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/BadgeDescriptor.sol#L48
File: contracts/nft/ReputationBadge.sol
98: function mint(
129: function uri(uint256 tokenId) public view override(ERC1155, IReputationBadge) returns (string memory) {
140: function publishRoots(ClaimData[] calldata _claimData) external onlyRole(BADGE_MANAGER_ROLE) {
163: function withdrawFees(address recipient) external onlyRole(BADGE_MANAGER_ROLE) {
184: function setDescriptor(address _descriptor) external onlyRole(RESOURCE_MANAGER_ROLE) {
204: function _verifyClaim(
217: function supportsInterface(
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/nft/ReputationBadge.sol#L98
File: contracts/token/ArcadeAirdrop.sol
62: function reclaim(address destination) external onlyOwner {
75: function setMerkleRoot(bytes32 _merkleRoot) external onlyOwner {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeAirdrop.sol#L62
File: contracts/token/ArcadeToken.sol
132: function setMinter(address _newMinter) external onlyMinter {
145: function mint(address _to, uint256 _amount) external onlyMinter {
https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/token/ArcadeToken.sol#L132
File: contracts/token/ArcadeTokenDistributor.sol
73: function toTreasury(address _treasury) external onlyOwner {
89: function toDevPartner(address _devPartner) external onlyOwner {
105: function toCommunityRewards(address _communityRewards) external onlyOwner {
121: function toCommunityAirdrop(address _communityAirdrop) external onlyOwner {
138: function toTeamVesting(address _vestingTeam) external onlyOwner {
155: function toPartnerVesting(address _vestingPartner) external onlyOwner {
174: function setToken(IArcadeToken _arcadeToken) external onlyOwner {
I would say Mediums #1,2,3,7 are valid. We also addressed fixes for them during our fix period when fixing other issues in the report.