Compensator Audit-Readiness Assessment 202504

1

Summary

Timeline: From 2025-04-28 To 2025-04-30

Total Issues: 25 (0 resolved)

Critical Severity Issues: 1 (0 resolved)

High Severity Issues: 1 (0 resolved)

Medium Severity Issues: 3 (0 resolved)

Low Severity Issues: 7 (0 resolved)

Notes & Additional Information: 13 (0 resolved)

Scope

This report evaluates the audit readiness of the in-scope contracts, highlighting concerns with the current design and providing recommendations for improvement.

Please note that the issues identified in this assessment are not exhaustive. To minimize delays arising from code revisions, only the most readily apparent and critical issues were included.

The OpenZeppelin team was asked to perform an assessment of the audit readiness of the camconrad/compensator repository at commit a436522.

In scope were the following files:

contracts
├── Compensator.sol
└── CompensatorFactory.sol

Overview

The Compensator contract allows COMP token holders to delegate their voting power in exchange for rewards, encouraging increased participation in proposal votes. The contract is ERC-20-compliant and utilizes user balances and total supply for internal bookkeeping, with disabled transfer and transferFrom functionality. Delegates fund rewards for delegators by supplying COMP tokens into the Compensator contract and setting a reward rate to distribute COMP rewards based on the proportional amount of tokens deposited by each delegator. A factory contract is utilized to deploy a unique Compensator contract for each deployer, which delegates all voting power of its holdings to the delegate.

To prevent any single delegate from accumulating excessive voting power, a delegation cap of 5% of the total COMP supply is enforced. In addition, delegators can incentivize delegates to vote for or against specific proposals by staking their COMP tokens. After the voting period ends, delegators who staked for the winning option will pass their stake to the delegate, while those who staked for the losing option will have their stake returned.

Critical Severity

_updateRewardsIndex Has Faulty Logic

The _updateRewardsIndex function is the core function responsible for updating the reward index, which serves as the basis for calculating each user’s rewards. However, the mechanism that it uses to update availableRewards and totalPendingRewards is incorrect. The contract assumes the difference between availableRewards and totalPendingRewards to be equal to the available rewards inside the contract. Yet, when updating these values, the shared rewards are both subtracted from availableRewards and added to totalPendingRewards, which causes the result of availableRewards - totalPendingRewards to decrease by twice the intended amount. This causes the internal bookkeeping of the protocol to be incorrect.

Consider fixing the logic of the _updateRewardsIndex function and tracking the values correctly.

High Severity

Staking Mechanism Relies Solely on Delegate Without Independent Verification

The staking mechanism allows users to incentivize a delegate to vote for or against a certain proposal. The mechanism consists of three different stages:

  1. Users call stakeForProposal to lock in some COMP to incentivize a delegate with high voting power to vote in line with the users’ wishes.
  2. The delegate calls distributeStakes to announce the proposal’s outcome and claims the staked amount that is in line with the result of the proposal execution.
  3. Users who staked some amount on the other side of the proposal execution results can reclaim their locked stakes.

While the staking mechanism can be useful for incentivizing the delegate, it is too reliant on the delegate’s honest behavior. In a better mechanism, the delegate would have the power to take advantage of any of the following scenarios:

  • The distributeStakes function does not check to see if the delegate actually provided the correct results to the proposal execution as the input winningSupport is completely reliant on the delegate’s input.
  • The smart contract does not check to see whether the delegate actually voted for or against the staked proposal. Instead, it sends the staked amount to the delegate solely based on the result of the proposal execution.
  • In case the delegate never calls distributeStakes after a proposal is executed, none of the users will be able to reclaim their staked amounts by calling reclaimLosingStake.

Consider redesigning the staking mechanism in a manner that is less dependent on the delegate’s actions. One option could be to have the Compensator contract verify the results of the proposal execution using the Compound Governor via the state function.

Medium Severity

Compensator Does Not Account for Rewards When availableRewards Is Less Than totalPendingRewards

The Compensator contract distributes rewards among all users based on the totalSupply, ensuring that each token held by users earns an equal amount. However, when availableRewards is less than totalPendingRewards, no rewards are distributed among the delegators. This means that even if the contract is later funded by the delegate, it does not account for the users’ rewards earned from the earlier period.

Consider fixing the internal bookkeeping to ensure that rewards are correctly tracked and distributed to each user.

Compensator Rewards Can be Exploited by Withdrawing Before Voting Starts

The Compensator contract allows users to opt-in via delegatorDeposit or withdraw their COMP tokens via delegatorWithdraw at any time, rewarding them based on the time their COMP tokens remained locked in the contract. However, since (1) there is no lock time for the funds in the contract and delegators have full control over their delegated funds, and because (2) Compound governance only considers the number of votes held by an account at the time of vote start, malicious users can withdraw their COMP tokens just before a proposal’s startBlock, thereby retaining voting power and still receiving rewards from the Compensator contract.

Consider mitigating this issue by ensuring that users cannot avoid participating in certain proposals with their voting power. One potential solution could be to lock users’ funds for a certain amount of time.

Incomplete and Failing Test Coverage

The current test suite has multiple failing cases and does not consistently run as expected. In addition, the tests are not comprehensive and fail to cover critical edge cases and core contract behaviors. This affects confidence in the system’s reliability and makes it difficult to ensure correctness in various scenarios.

Consider fixing the failing tests and expanding the test coverage to include a wider range of potential edge cases.

Low Severity

rewardsUntil Output Is Incorrect When Reward Rate Is Zero

The rewardsUntil function returns the timestamp until which rewards will be distributed. However, if rewardRate is zero, it returns the current block.timestamp instead of the lastRewarded timestamp, which may lead to confusing results.

To ensure accurate behavior, consider updating rewardsUntil to return lastRewarded when rewardRate is zero.

getCompensators Should Implement Pagination

The getCompensators function returns an array of all Compensator contract addresses. However, as the number of Compensators grows indefinitely, retrieving the full list may eventually become infeasible.

To ensure scalability, consider implementing pagination in getCompensators to allow for fetching a subset of addresses at a time.

Floating Pragma

Pragma directives should be fixed to clearly identify the Solidity version with which the contracts will be compiled.

Throughout the codebase, multiple instances of floating pragma directives were identified:

Consider using fixed pragma directives.

Incomplete Docstrings

Throughout the codebase, multiple instances of incomplete docstrings were identified:

  • In Compensator.sol, the DelegateDeposit event. For example, the delegate and amount parameters are not documented.
  • In Compensator.sol, the DelegateWithdraw event. For example, the delegate and amount parameters are not documented.
  • In Compensator.sol, the RewardRateUpdate event. For example, the delegate and newRate parameters are not documented.
  • In Compensator.sol, the DelegatorDeposit event. For example, the delegator and amount parameters are not documented.
  • In Compensator.sol, the DelegatorWithdraw event. For example, the delegator and amount parameters are not documented.
  • In Compensator.sol, the ClaimRewards event. For example, the delegator and amount parameters are not documented.
  • In Compensator.sol, the ProposalStaked event. For example, the staker, proposalId, support, and amount parameters are not documented.
  • In Compensator.sol, the ProposalStakeDistributed event. For example, the proposalId and winningSupport parameters are not documented.
  • In Compensator.sol, the LosingStakeReclaimed event. For example, the delegator, proposalId, and amount parameters are not documented
  • In CompensatorFactory.sol, the CompensatorCreated event. For example, the delegatee and compensator parameters are not documented.

Consider thoroughly documenting all functions/events (and their parameters or return values) that are part of a contract’s public API. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

Redundant State Updates and Misleading Events

When a setter function updates a state variable without first checking whether the new value differs from the current one, it will overwrite the storage with the same data. This operation still consumes gas and, more importantly, emits an event signaling state change—even though no actual change occurred.

The setRewardRate function sets the rewardRate variable and emits an event without checking if the value has changed.

Consider adding a check that reverts the transaction if the value being set is identical to the existing value.

Improper Use of Local @openzeppelin/contracts Folder

The project contains a folder named @openzeppelin/contracts which should not exist as a local directory. This setup can lead to confusion, versioning issues, and potential misuse of core library files.

Consider removing the folder and relying solely on package-managed dependencies to ensure proper functioning and compliance with best practices.

Unnecessary Use of Initializable in Non-Upgradeable Contract

The Compensator contract inherits from Initializable. This inheritance pattern is intended for upgradeable contracts. However, since the contract is only deployed via a factory and the initialize function is called immediately after deployment, this pattern is unnecessary and could be replaced with a constructor.

Consider removing the initialize function and using a constructor instead. Alternatively, to benefit from the upgradeable pattern and reduce gas costs, consider having the factory deploy minimal proxies pointing to a single implementation contract.

Notes & Additional Information

Functions Updating State Without Event Emissions

Throughout the codebase, multiple instances of functions updating the state without an event emission were identified:

Consider emitting events whenever there are state changes to improve the clarity of the codebase and make it less error-prone.

Redundant Getter Function

When state variables use public visibility in a contract, a getter method is automatically generated by the compiler.

Within the CompensatorFactory contract in CompensatorFactory.sol, the getCompensator function is redundant because the delegateeToCompensator state variable, being a public variable, already has a getter. In addition, the getCompensator function is redundant because the delegateeToCompensator state variable, being a public variable, already has a getter.

To improve the overall clarity, intent, and readability of the codebase, consider removing the redundant getter functions.

Lack of Security Contact

Providing a specific security contact (such as an email or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice is quite beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for their maintainers to contact the appropriate person about the problem and provide mitigation instructions.

Throughout the codebase, multiple instances of contracts not having a security contact were identified:

Consider adding a NatSpec comment containing a security contact above each contract definition. Using the @custom:security-contact convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.

Missing Named Parameters in Mappings

Since Solidity 0.8.18, developers can utilize named parameters in mappings. This means mappings can take the form of mapping(KeyType KeyName? => ValueType ValueName?). This updated syntax provides a more transparent representation of a mapping’s purpose.

Throughout the codebase, multiple instances of mappings without named parameters were identified:

Consider adding named parameters to mappings in order to improve the readability and maintainability of the codebase.

Non-Explicit Imports

The use of non-explicit imports in the codebase can affect code clarity and may create naming conflicts between locally defined and imported variables. This is particularly relevant when multiple contracts exist within the same Solidity file or when inheritance chains are long.

Throughout the codebase, multiple instances of non-explicit imports were identified:

Following the principle that clearer code is better code, consider using the named import syntax (import {A, B, C} from "X") to explicitly declare which contracts are being imported.

Lack of Indexed Event Parameter

Within Compensator.sol, the ProposalStakeDistributed event does not have indexed parameters.

To improve the ability of off-chain services to search and filter for specific events, consider indexing event parameters.

Inconsistent Order Within a Contract

The Compensator contract deviates from the Solidity Style Guide due to having inconsistent ordering of functions.

To improve the project’s overall legibility, consider standardizing ordering throughout the codebase as recommended by the Solidity Style Guide (Order of Functions).

Hardcoded Addresses

In Compensator.sol, multiple instances of addresses hardcoded address values were identified:

Consider declaring hardcoded addresses as immutable variables and initializing them through constructor arguments. This allows code to remain the same across deployments on different networks and mitigates situations where contracts need to be redeployed due to having incorrect hardcoded addresses.

Magic Numbers

Within Compensator.sol, multiple instances of literal values with unexplained meanings were identified.

  • The 1e18 literal number in lines 154, 188, 390, 430 and 456
  • The 100 literal number in line 158.

Consider defining and using constant variables instead of using literals to improve the readability of the codebase.

Function Visibility Overly Permissive

Throughout the codebase, multiple instances of functions with unnecessarily permissive visibility were identified:

  • The initialize function in Compensator.sol with public visibility could be limited to external.
  • The _updateUserRewards function in Compensator.sol with internal visibility could be limited to private.
  • The _updateRewardsIndex function in Compensator.sol with internal visibility could be limited to private.
  • The _getCurrentRewardsIndex function in Compensator.sol with internal visibility could be limited to private.
  • The getCompensators function in CompensatorFactory.sol with public visibility could be limited to external.

To better convey the intended use of functions and to potentially realize some additional gas savings, consider changing a function’s visibility to be only as permissive as required.

Multiple Optimizable State Reads

In Compensator.sol, multiple opportunities for optimizing storage reads were identified:

Consider reducing SLOAD operations that consume unnecessary amounts of gas by caching the values in a memory variable.

Constants Not Using UPPER_CASE Format

In Compensator.sol, multiple instances of constants not being declared using the UPPER_CASE format were identified.

  • The compToken constant declared in line 32
  • The compoundGovernor constant declared in line 35

According to the Solidity Style Guide, constants should be named with all capital letters with underscores separating words. For better readability, consider following this convention.

Use calldata Instead of memory

When dealing with the parameters of external functions, it is more gas-efficient to read their arguments directly from calldata instead of storing them to memory. calldata is a read-only region of memory that contains the arguments of incoming external function calls. This makes using calldata as the data location for such parameters cheaper and more efficient compared to memory. Thus, using calldata in such situations will generally save gas and improve the performance of a smart contract.

Within CompensatorFactory.sol, the delegateeName parameter should use calldata instead of memory.

Consider using calldata as the data location for the parameters of external functions to optimize gas usage.

Conclusion

The codebase in its current state is not yet ready for a comprehensive security audit due to the technical and design issues identified in this assessment, as well as the failing test suite. To improve its audit readiness, the system should integrate Compound Governor’s trust assumptions regarding voting delegation into the deposit/withdrawal and staking logic while enabling permissionless querying of proposal states through the designated contract. Additionally, the test suite must be revised to ensure that all tests pass and to achieve sufficient coverage for sensitive functions and edge cases.