Arcade.xyz - 3agle's results

Analysis Report for Arcade.xyz

Codebase quality

The overall quality of the codebase for Arcade.xyz can be classified as "Good".

Strengths

• The Nat-spec comments significantly enhance the understanding of the functionality.
• The code exhibits symmetry in updating variables and transferring tokens, which is commendable.
• The contracts are neatly divided into sections such as Admin functionality, User Functionality, Helpers, etc. This division facilitates quick and clear comprehension of the contracts.

Weaknesses

• The codebase lacks proper documentation, which could potentially hinder understanding.
• The Arcade Governance token is often referred to as ERC20 tokens, which can sometimes lead to confusion.
• The codebase exhibits redundancy and decreased readability due to the usage of identical if-conditions in one flow of code and unnecessary complexity in simple logic.

Mechanism Review

• The contracts audited are primarily associated with the Arcade Governance Council.
• The governance system is centered around ArcadeGSCCoreVoting.sol and CoreVoting.sol (Out-of-Scope). These contracts allow for the submission and voting of proposals.
• The voting power of a user is always considered 1 block before the proposal was submitted to prevent flash-loan type attacks.
• The governance token is ArcadeToken.sol, which assigns 1 unit of voting power for every 1 unit of ARCD Token. This voting power can be delegated to other users.
• The mint function of the ARCD Token is restricted to a minter role. The initial amount of 100M ARCD tokens will be minted and sent to ArcadeTokenDistributor.sol which will distribute a pre-decided percentage of tokens to various stakeholders and contracts like ArcadeTreasury.sol, ArcadeAirdrop.sol, Dev Partners, etc. After that, the tokens can be minted after a year at a 2% inflationary cap.
• The ArcadeTreasury.sol contract provides a mechanism for managing funds held in the Arcade treasury, with configurable spending thresholds, governance control, and an allowance system for smaller spending by the GSC.
• Two contracts, ARCDVestingVault.sol and NFTBoostVault.sol, handle and track the user's voting power. Both inherit from the BaseVotingVault.sol.
• ARCDVestingVault.sol- This contract allows a manager to deposit ARCD tokens and assign those tokens to a grant. The grant has voting power proportional to the number of tokens. It also includes a vesting schedule using a cliff mechanism. After the cliff, the user assigned to the grant can withdraw the ARCD tokens. The manager can also revoke the grant. It also has a variant, ImmutableVestingVault.sol, wherein a manager cannot revoke the grants after setting them.
• NFTBoostVault.sol- The protocol also has a feature for boosting the user's voting power by depositing ERC1155 NFTs with the ARCD Tokens in the NFTBoostVault.sol. It supports any and all the ERC1155s which have an assigned boost multiplier value (max 1.5x) in the NFTBoostVault.sol.
• ReputationBadge.sol- Mints the native ERC1155 NFTs which are used to boost the voting power. The user can mint these NFTs by providing a Merkle proof. The parameters (price, expiration, etc.) for minting are passed using a struct ClaimData to the publishRoots() function.

Centralization risks

• The primary centralization risk identified lies in ARCDVestingVault.sol where the manager role has complete control over all the tokens that have not yet been withdrawn by the user.
• This gives the manager control over a significant amount of ARCD Tokens which could potentially negatively impact the governance.

Approach

• The audit began with a comprehensive review of all the in-scope smart contracts and an understanding of their functionality.
• NFTBoostVault.sol was particularly interesting as it used ERC1155s to boost the user's voting power, which opened up the possibility for reentrancy.
• Despite the major functions being protected with nonReentrant modifiers, one function, delegate(), lacked the modifier and performed several major state changes.
• A scenario was tested where a function that changes the ERC1155 NFT (updateNft() or withdraw()), with a higher boost multiplier, could potentially reenter delegate() to change the registration.delegatee from another user to myself, effectively reducing my delegate's voting power.
• However, the codebase was robust in terms of updating the status of voting power. It maintained a consistent sync between the previous voting power and the current voting power.
• This was due to the consideration that a user's voting power may change as the owner can update the boost that a particular NFT provides. As a result, they either use the previous voting power or always update the voting power first with the _syncVotingPower() and then perform subsequent actions.
• After thorough testing of multiple variants of the above scenario, it was concluded that negative delegation was not possible.
• The audit then moved on to other contracts. There were a few minor issues such as improper validation and redundancies but overall, the system did not exhibit any major issues.

Architecture Recommendation

• In ArcadeTreasury.sol, the expenditure of tokens is recorded and capped per block.
• The current approach is as follows:

/// @notice mapping storing how much is spent or approved in each block.
mapping(uint256 => uint256) public blockExpenditure;

//In _spend() function
uint256 spentThisBlock = blockExpenditure[block.number];
if (amount + spentThisBlock > limit) revert T_BlockSpendLimit();

• It is recommended to track the spending limit for different tokens, individually. This will significantly improve the user experience and does not require major changes.
• Here's the updated code:

/// @notice mapping storing how much is spent or approved in each block.
mapping(uint256 => mapping(address => uint256)) public blockExpenditure;

//In _spend() function
uint256 spentThisBlock = blockExpenditure[block.number][token];
if (amount + spentThisBlock > limit) revert T_BlockSpendLimit();

Time spent:

54 hours