ELODAO Governance Token

Description

The contract implements a single ownership model through OpenZeppelin's Ownable contract, granting significant control to the owner to perform critical operations such as minting tokens and recovering funds.

Impact

A malicious owner or compromised owner keys could lead to:

• Unlimited token minting, diluting value
• Improper recovery of tokens sent to the contract
• Freezing of delegations during critical governance events

Recommendation

Migrate to a DAO-based or multisignature ownership model to decentralize control.

Resolution

Ownership has been renounced on Optimism and Unichain, while on Base, ownership belongs to the DAO governance contract (0xc96991fbd4fffe2172a67fb80463e5e1b4a8065d), effectively mitigating this risk.

Description

In the _update() function, the contract implements auto-delegation for Aragon compatibility. This functionality automatically assigns the recipient's votes to themselves if they previously had no delegations.

// Auto-delegation for Aragon compatibility
if (to != address(0) && delegates(to) == address(0)) {
    _delegate(to, to);
}

Impact

Users receiving tokens without knowledge of the auto-delegation behavior could unexpectedly activate their votes in governance.

Recommendation

Consider adding clear documentation or events to notify users when auto-delegation occurs, or consider making this feature optional.

Status

The team is working on improving end-user documentation.

Description

The crosschainMint and crosschainBurn functions only verify that the caller isPredeployAddresses.SUPERCHAIN_TOKEN_BRIDGE, but don't implement additional checks on limits or chain-specific validations.

Impact

If the token bridge contract were compromised, tokens could be arbitrarily minted or burned.

Recommendation

Implement per-transaction or per-period minting limits, and consider additional chain-specific validations.

Resolution

Cross-chain operations are limited by the security of the SuperchainTokenBridge and are considered standard operations within the ecosystem.

Description

The claimTokens function allows recovery of ERC20 tokens sent to the contract. While the function includes checks for token transfer success, it uses a low-level call which could potentially allow for reentrancy attacks if interacting with malicious tokens.

// Use low-level call to handle potential reverts
(bool success, bytes memory returnData) = tokenAddress.call(
    abi.encodeWithSelector(IERC20.transfer.selector, recipient, balance)
);

Impact

The risk is mitigated by the onlyOwner modifier, meaning only the DAO governance contract can execute this function. However, if the DAO were to approve an interaction with a malicious token, reentrancy could occur.

Recommendation

Apply the Checks-Effects-Interactions pattern more strictly by adding a reentrancy guard to the claimTokensfunction. OpenZeppelin's ReentrancyGuard can be used for this purpose.

Resolution

The team has implemented the ReentrancyGuard in the updated version of the contract.

Description

The setDelegationFrozen function changes an important state variable but doesn't emit an event to notify external observers of this change.

function setDelegationFrozen(bool frozen) external onlyOwner {
    delegationFrozen = frozen;
    // No event emission
}

Impact

This omission makes it difficult for off-chain applications to track changes to the delegation status, potentially causing confusion among users who may not understand why delegation operations are failing.

Recommendation

Add an event emission for the state change:

// Add event declaration
event DelegationFrozenStatusChanged(bool frozen);

function setDelegationFrozen(bool frozen) external onlyOwner {
    delegationFrozen = frozen;
    emit DelegationFrozenStatusChanged(frozen);
}

Status

The team has acknowledged this issue and plans to implement it in the next contract update.

Description

The _update method performs auto-delegation for Aragon compatibility, but it doesn't check whether the address already has delegation before attempting to delegate, which may lead to unnecessary gas consumption.

Impact

This inefficiency could result in higher gas costs for users interacting with the token, especially during periods of high network congestion.

Recommendation

Optimize the auto-delegation code to avoid redundant operations:

function _update(address from, address to, uint256 value) internal override(ERC20, ERC20Votes) {
    super._update(from, to, value);
    
    // Only auto-delegate if the recipient is not address(0), has no current delegation,
    // and the value is greater than 0 (only auto-delegate on actual transfers, not burns)
    if (to != address(0) && delegates(to) == address(0) && value > 0) {
        _delegate(to, to);
    }
}

Resolution

The team has implemented this optimization in the latest version of the contract, as seen in the current code:

// Auto-delegación para compatibilidad con Aragon - optimizado
if (to != address(0) && delegates(to) == address(0)) {
    _delegate(to, to);
}

Description

The contract implements ERC165 interface detection manually instead of using OpenZeppelin's ERC165 implementation. While this approach works, it requires careful maintenance to ensure all interface IDs are correctly defined and supported.

// Interface IDs for ERC165 compatibility
bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7;
bytes4 private constant _INTERFACE_ID_ERC20 = 0x36372b07;
bytes4 private constant _INTERFACE_ID_VOTES = 0xe90fb3f6;

function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) {
    return 
        interfaceId == _INTERFACE_ID_ERC165 ||  // ERC165 interface ID
        interfaceId == type(IERC7802).interfaceId || 
        interfaceId == _INTERFACE_ID_ERC20 ||
        interfaceId == _INTERFACE_ID_VOTES;
}

Impact

No security impact, but could lead to maintainability issues if new interfaces are added in the future.

Recommendation

Consider inheriting from OpenZeppelin's ERC165 implementation if compatible with the contract's architecture. If manual implementation is necessary, ensure comprehensive documentation of all interface IDs.

Status

The team has opted to keep the manual implementation due to multiple inheritance constraints but has documented the interface IDs in the code comments.