Compensator 2025-08 Audit

Summary

Timeline: From 2025-08-19 To 2025-08-27

Languages: Solidity

Total Issues: 40 (0 resolved)

Critical Severity Issues: 1 (0 resolved)

High Severity Issues: 1 (0 resolved)

Medium Severity Issues: 3 (0 resolved)

Low Severity Issues: 12 (0 resolved)

Notes & Additional Information: 23 (0 resolved)

Scope

OpenZeppelin audited the camconrad/compensator repository at commit fd69763.

In scope were the following files:

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

System Overview

The Compensator system introduces a standardized way for COMP token holders to delegate voting power, earn rewards, and participate in governance-aligned staking around Compound governance proposals. The design supports two primary components:

  • Compensator (ERC-20, non-transferable): A per-owner delegate contract that aggregates delegated COMP voting power, streams rewards to depositors, casts votes on Compound proposals, and manages proposal-related staking and resolution.
  • CompensatorFactory: A factory and registry contract that deploys new Compensator instances, tracks ownership, and ensures that each owner can only control a single Compensator at a time.

The Compensator contract, which holds delegated COMP, locks user deposits for a minimum period, and updates a reward index to distribute COMP rewards deposited by the owner. Rewards are distributed over time at a configurable rate, capped to prevent over-distribution. In parallel, users may stake COMP “for” or “against” active Compound governance proposals. Once a proposal gets resolved, stakes from the losing side are reclaimable by users, while winning-side stakes are awarded to the Compensator owner if their recorded vote matched the outcome, creating direct performance incentives.

The CompensatorFactory contract allows delegates to deploy Compensator instances and tracks their addresses and owners.

Security Model and Trust Assumptions

The following assumptions about roles, external dependencies, and timing are critical to preserving the safety of the delegators and the integrity of proposal resolution:

  • The Compensator owner is trusted to manage rewards, cast votes, configure parameters, and control the onlyOwner-guarded functions. Delegators assume that the Compensator owner will behave honestly in voting and reward management.
  • The system assumes that the provided COMP token and Compound Governor addresses are correct and immutable. It trusts their ERC-20 behavior, delegation mechanics, and governance state transitions (Active, Succeeded, Defeated, etc.).
  • Deposits are subject to a minimum 7-day lock, extended automatically if proposals are active or pending. This ensures that voting power remains stable during governance windows and prevents short-term delegation churn.
  • Depositors trust the Compensator owner not to act maliciously by withdrawing unaccrued rewards or reducing the reward rate shortly after the users deposit.
  • Proposal outcomes are determined by the Compound Governor. Auto-resolution after a 30-day timeout treats unresolved proposals as Defeated. Proposal-resolution logic assumes that the Governor correctly enforces proposal state transitions.
  • If the delegate’s vote matches the winning outcome, the owner may claim 100% of the winning stakes. If not, stakes remain with users. This introduces strong incentives but also shifts risk to delegators. Users must trust that the owner will aim to vote correctly.
  • Reward streaming is capped by available deposits and calculated through an index system. The system prevents over-distribution by halting accruals once pending rewards meet available balances.
  • Only the Compensators deployed by the Factory can call back into onOwnershipTransferred. This ensures registry integrity, prevents arbitrary contracts from corrupting the mapping, and enforces a one-owner-one-compensator rule.

Privileged Roles

The Compensator system defines three important roles: Owner, Delegator, and Staker.

  • Owner: Sole privileged operator. Manages rewards (funding, withdrawal, and rate changes), casts votes on Compound proposals, and controls configuration via the onlyOwner modifier. Delegators must trust the owner not to manipulate rewards after deposits.
  • Delegators: Deposit COMP tokens to delegate voting power and earn rewards. They are subject to a 7-day minimum lock period and can later withdraw and claim rewards. Their participation is incentivized by the reward distribution.
  • Stakers: Commit COMP tokens to active proposals. After resolution, losing stakes are reclaimable while winning stakes will be awarded to the owner if the delegate’s staking direction and the owner’s voting direction align with the proposal outcome.

Critical Severity

Incorrect Reward Accounting Can Lead to Compensator Insolvency

The Compensator contract tracks the rewards available for distribution among the delegators in the availableRewards state variable. Its value should correspond to the amount of COMP tokens the contract is holding for this purpose. This variable is increased by the ownerDeposit function and decreased by the ownerWithdraw function. However, it is not decreased when rewards are claimed by delegators.

The availableRewards value not decreasing when rewards are claimed leads to a misaccounting of the COMP balance that is actually available for distribution. This discrepancy causes several issues:

  • The rewardsUntil view function may return an inaccurate timestamp, as the contract will not have sufficient COMP tokens to pay rewards until the projected time.
  • The owner would be able to withdraw tokens using the ownerWithdraw function that should be reserved for delegator rewards.
  • The reward distribution cap will not function correctly. It may allow for the distribution of more rewards than are available in the contract’s balance, causing claimRewards transactions to revert due to insufficient funds, or it may allow delegators to claim COMP tokens as rewards that should account for another part of the system.
  • The _getCurrentRewardsIndex private function could calculate an incorrectly large reward index, which in turn affects the value returned by the getPendingRewards view function.

To ensure the correct accounting of distributable rewards and to prevent the Compensator contract from becoming insolvent, consider updating the availableRewards state variable within the claimRewards function.

High Severity

Winning Stakers Lose Funds if the Owner Votes Incorrectly

A flaw exists in the proposal resolution logic which surfaces when the owner of the Compensator contract, votes against the winning outcome of a proposal or fails to vote at all. In such a scenario, the _resolveProposalInternal function correctly withholds the reward from the owner but fails to create a mechanism to return the staked funds to the stakers who correctly staked on the winning side. The corresponding reclaimStake function is exclusively designed to return funds to stakers on the losing side of a proposal. This creates a situation where there is no code path for stakers who correctly staked on the winning side to withdraw their funds, as reclaimStake will revert their transaction, effectively trapping their assets in the contract.

The primary consequence of this issue is that stakers permanently lose their funds if the owner of the Compensator contract, casts an incorrect vote, regardless of the users having correctly staked on the winning outcome. This flaw severely undermines the trust and economic incentive of the staking mechanism, as it punishes the accurate stakers for a mistake made by the owner, over whom they have no direct control.

Consider modifying the reclaimStake function to allow stakers to reclaim their funds if they staked the losing side, or if the owner voted against the winning side.

Medium Severity

Queued State Not Treated as a Resolved Outcome in resolveProposal

The resolveProposal function only proceeds when the governor state is either Succeeded, Defeated, Expired, Canceled, or Executed and reverts otherwise. It omits Queued, which, in Compound-style governance, indicates that the proposal has passed voting and is in the timelock period, awaiting execution. As a result, proposals in Queued state are incorrectly considered as not resolved, causing ProposalNotResolvedYet reverts despite their voting outcome being finalized.

Excluding the Queued state blocks timely stake resolution and reward flows after voting concludes. Users cannot reclaim according to the final vote outcome, and the owner cannot receive winning-side distributions until execution or later state changes. This creates unnecessary delays during the timelock period and can degrade user experience, liquidity, and accounting clarity, especially for proposals that remain queued for extended windows.

Consider including the Queued state in the allowlist of resolvable states so that stake distribution and reclaim paths can proceed.

Voting Power Accounting Mismatch in castVote

The Compensator contract records voting power using the getCurrentVotes function on COMP_TOKEN at call time and stores/emits that value in the VoteInfo and delegateInfo variables. The Compound Governor counts votes using getPriorVotes(account, proposalSnapshot(proposalId)) at the proposal’s snapshot block (block at which the proposal voting starts). Vote delegations, COMP token transfers, or vote re-delegations in the Compensator contract, between the proposal snapshot and the castVote transaction, can cause these two values to diverge, making the contract’s internal record and emitted data inconsistent with what the Governor actually counted and voted with.

This mismatch can inflate the VoteInfo.votingPower, delegateInfo.totalVotingPowerUsed, and averageVotingPowerPerVote values, misleading dashboards and off-chain tools that rely on these them. In addition, it breaks the voting power state maintained by the Compensator contract in its VoteInfo and DelegateInfo structs.

Consider aligning the Compensator contract’s accounting of the voting power with that of the Governor by fetching the snapshot block via the COMPOUND_GOVERNOR contract and then getting the voting power at that block. In addition, consider using this aligned voting power for the VoteInfo.votingPower and DelegateInfo computations performed in the _castVote function.

Missing one owner → one compensator Invariant in CompensatorFactory During Ownership Transfer

The createCompensator implementation enforces that each user gets their own Compensator instance, deploys a new Compensator instance for each user, and makes sure that the transaction reverts with OwnerAlreadyHasCompensator if a user that already has a Compensator tries to deploy a second one. Therefore, the contract is expected to preserve the invariant (“one owner → one compensator”).

However, the onOwnershipTransferred function updates ownerToCompensator[newOwner] without first verifying that newOwner does not already have a compensator. As a result, a legitimate transfer can assign a compensator to a newOwner who is already mapped to another compensator, silently overwriting the registry entry and leaving multiple live compensators owned by the same address while the factory’s single-pointer mapping no longer reflects reality.

As a result:

  • the compensatorToOriginalOwner mapping will have two compensators with the same owner
  • the ownerToCompensator mapping will map the new owner to the compensator which changed the ownership

This breaks the CompensatorFactory contract’s core uniqueness invariant and corrupts the state used by off-chain services to discover an owner’s compensator, leading to unexpected behavior.

Consider enforcing the “one owner → one compensator” invariant during ownership transfers in onOwnershipTransferred, similar to how it is enforced in createCompensator.

Low Severity

Premature Resolution of Proposals in Succeeded State

The Compensator contract currently treats a proposal in the Succeeded state as finalized by setting winningSupport = 1 and distributing outcomes. However, in the Compound governance flow, Succeeded is not terminal: a proposal can later be queued and executed, but it may also be canceled or expire during the timelock. Resolving stakes at the Succeeded stage is therefore premature, as the ultimate outcome of the proposal is not yet guaranteed.

By resolving proposals in the Succeeded state, Compensator risks distributing rewards and allowing stake reclaims incorrectly if the proposal is later canceled or expired. This can lead to situations where the owner is rewarded as though the proposal succeeded, even if it never does. This discrepancy undermines fairness, breaks alignment with the proposal lifecycle, and may result in permanently incorrect accounting for staked funds.

Consider restricting resolution to truly terminal states only: Executed (map to “For won”), and Defeated, Canceled, or Expired (map to “Against won”). Exclude Succeeded from the resolution logic, or only resolve it if the contract’s intended behavior is to reward vote outcomes irrespective of later execution. This change ensures that distributions reflect final governance outcomes and prevents misaligned stake handling.

VerifyVote Fails After 256 Blocks Due to Reliance on blockhash

The verifyVote function depends on comparing stored block hashes against blockhash(blockNumber - 1). However, the EVM only makes block hashes accessible for the most recent 256 blocks. Once this limit is exceeded, blockhash returns 0x0, causing the verification to fail for older proposals. This design guarantees that all vote verifications will start returning false negatives after ~256 blocks. As a result, any governance checks that rely on verifyVote will become unusable over time, limiting the function’s effectiveness for verifying votes after a delay.

Consider removing the block hash comparison from verifyVote or replacing it with alternative verification mechanisms that do not expire.

getContractVotingPowerAt Returns Incorrect Historical Voting Power

The getContractVotingPowerAt function is intended to return the voting power of the Compensator contract at a specific, historical block number. However, the function’s implementation incorrectly retrieves the current voting power by calling getCurrentVotes on the COMP token contract instead of the voting power at the specified block. This results in the function returning an incorrect value.

Consider using the getPriorVotes function of the COMP token contract. It will correctly fetch the historical voting power at the given block number, aligning the function’s behavior with its intended purpose.

Inconsistent Proposal State Check in _castVote

The _castVote function verifies that a proposal’s state is either Pending or Active before allowing the execution to proceed. A similar check is performed in the verifyVote function. However, the subsequent external call to the COMPOUND_GOVERNOR.castVote function includes a more restrictive check, only allowing votes to be cast on proposals in the Active state. This inconsistency means that any transaction for a Pending proposal will pass the internal check but will always revert due to the external call’s requirement, potentially causing unexpected reverts.

It is advisable to remove the check for the Pending state from both the _castVote and verifyVote functions. This would align the contract’s internal logic with the external requirements and prevent misleading validations.

rewardsUntil Calculation Uses Incorrect Precision

The rewardsUntil function is used to determine the remaining time for reward distribution. However, the resulting remainingRewardsTime value is incorrectly scaled by a factor of 1e18. This causes the final units to be seconds * 1e18 instead of just seconds.

Consider removing the unnecessary scaling factor from the calculation to ensure that the resulting time value is correctly represented in seconds.

Incorrect handling of Pending proposals as Active in _updateLatestProposalId

The _updateLatestProposalId function currently marks proposals as active when the governor reports them as either Active or Pending. This conflates two different states and causes the contract’s activeProposals mapping to include proposals that have not yet reached the voting period. As a result, the internal tracking does not distinguish between proposals that are already open for voting and those that are only scheduled to begin.

By treating pending proposals as active, the contract introduces ambiguity. This causes premature activation events and incorrect lock period extensions for users, and it misleads external systems that rely on emitted events. Furthermore, since Pending proposals are already considered active in the activeProposals mapping, the separate pendingProposals mapping becomes redundant in _hasUserActiveStakes and _hasRelevantActiveProposals, where it adds no unique functionality and only complicates the logic.

The pendingProposals and activeProposals mappings are only used in the _hasUserActiveStakes and _hasRelevantActiveProposals functions. Since using internal accounting is error-prone and can give stale results, consider fetching the proposal state directly from the Governor contract on-demand when required.

Incorrect Default blocksPerDay Narrows the “within 1 day” Pending Window on Ethereum

The Compensator contract marks a proposal as about to start when startBlock > block.number && (startBlock - block.number) < blocksPerDay. The default blocksPerDay value is set to 6,500, which reflects a pre-Merge estimate. Presently, the Ethereum mainnet averages roughly 7,100–7,200 blocks/day. This means that the intended 24-hour window is effectively ~22 hours with the current configuration. Hence, proposals starting between ~22–24 hours from now may not be flagged in pendingProposals.

The underestimated window can cause proposals that are less than a day away to be missed by pendingProposals, which, in turn, slightly skews logic that references it (e.g., _hasUserActiveStakes and _hasRelevantActiveProposals) for lock extensions and withdrawal gating.

Consider setting the blocksPerDay variable to a default value that more accurately represents the full 24 hours (e.g., 7,100 for Ethereum mainnet). In addition, consider documenting that blocksPerDay must be explicitly configured at deployment for each network.

Misleading Documentation

Throughout the codebase, multiple instances of misleading documentation were identified:

  • The txHash member of the VoteInfo struct is described as the transaction hash of the vote. However, it is set to the blockhash of the previous block in the _castVote function.
  • The delegateInfo variable is described as a mapping in the NatSpec docstring.
  • The comments in lines 1039 and 1063 state that caching storage variables is a reentrancy-prevention technique. However, this is a gas-optimization technique.
  • The NatSpec for the lastRewarded variable states that the timestamp is updated when the rewards are claimed. However, the timestamp is updated when the rewards are accrued.
  • The NatSpec for the getPendingRewards function states that it accounts for both “claimed and unclaimed rewards”, whereas, it only accounts for unclaimed rewards.
  • The NatSpec for the canUserWithdraw function states that the returned reason variable denotes “the reason why withdrawal is blocked”. However, it denotes the reason when the withdrawal is allowed as well.
  • The NatSpec for the userWithdraw function states that the function claims rewards. However, it only accrues them by calling _updateUserRewards. Users may wrongly assume rewards are automatically transferred during withdrawal.

Consider correcting the aforementioned instances of misleading documentation to improve the clarity and maintainability of the codebase.

Possible Duplicate Event Emissions

When a setter function does not check if the value being set is different from the existing one, it becomes possible to set the same value repeatedly, creating a possibility for event spamming. Repeated emission of identical events can also confuse off-chain clients.

The _updateRewardsIndex function sets the rewardIndex variable and emits an event without checking if the value has changed. In addition, the emit statement in line 1085 emits a RewardIndexUpdated event without updating the rewardIndex.

Consider adding a check that reverts the transaction if the value being set is the same as the existing one.

Undocumented Code

In the getContractVotingPowerAt function, the input parameter is 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).

Missing Docstring

The voteCount state variable in the Compensator.sol contract does not have a docstring.

Consider thoroughly documenting all state variables to improve the overall clarity and maintainability of the codebase.

Factory Ownership Tracking Can Desynchronize

Within Compensator.sol, in line 1224, the overridden transferOwnership function is meant to keep the factory’s ownerToCompensator mapping in sync with the actual owner of the Compensator contract. After calling super.transferOwnership(newOwner), the function tries to notify the ownership transference to the factory. If the factory call reverts (due to a fault in the logic), the catch-all clause silently swallows the error.

At that moment, the Compensator has already changed hands, while the factory still believes that the old owner controls it. Any system component that relies on the factory’s mapping (e.g., front-ends, other contracts, permission checks, etc.) will now operate on stale data, leading to inconsistent or insecure behavior. Since the stated purpose of the call is to guarantee synchronization, letting it fail without reverting is a logical error.

Consider avoiding architectures where the state of one contract is tracked in another. Instead, any system that needs to ascertain the owner of a Compensator instance should query the contract directly. This practice eliminates data redundancy and removes the possibility of synchronization errors.

Notes & Additional Information

Unused Function With internal Visibility

In Compensator.sol, the verifyVote function is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider using or removing any currently unused functions.

Lock Period Extension Applies to All Funds Instead of Only New Deposits

In the userDeposit function, the Compensator contract maintains a single unlockTime per user. Upon each deposit, the function compares the new calculated unlock with the current one and updates it if the new one is higher. This means that subsequent deposits can push out the unlock time for the user’s entire balance, including older deposits that would otherwise have been withdrawable.

Effectively, all previously deposited funds inherit the new extended lock time, causing users to lose liquidity over funds that should have matured. A user who makes repeated small deposits could find their entire balance to be locked much longer than expected. This affects user experience by preventing partial withdrawals of matured deposits.

Consider refactoring the locking logic so that only new deposits receive a fresh or extended lock, while older deposits retain their original unlock schedule.

Redundant startRewardIndex Assignment in userWithdraw

The userWithdraw function performs an explicit assignment when setting startRewardIndex after updating user rewards via _updateUserRewards. However, this assignment is redundant because _updateUserRewards already sets the user’s checkpoint to the current rewardIndex when relevant, and in all other cases, the two values are already equal.

Consider removing the redundant assignment to optimize the implementation.

Redundant Storage Writes in _castVote

The private _castVote function is responsible for recording a user’s vote on a specific proposal. This process involves multiple state changes to track the vote details, such as the voter’s support and the transaction hash. However, the function’s current implementation contains several inefficient and redundant storage operations, which leads to unnecessary gas consumption for voters.

These redundant operations include the following:

  • The voter’s support is written to storage twice, first in line 587 and again in line 599.
  • The txHash within the proposal’s vote information is initialized to zero in line 599 and then updated with the correct value in a separate storage write in line 614.
  • The voteIndexToProposalId mapping and the voteCount variable appear to be used for informational purposes only. Storing this data on-chain increases the cost of every vote, when it could instead be tracked off-chain by indexing contract events.

Consider refactoring the _castVote function to optimize storage writes and reduce gas costs. It is advisable to eliminate redundant writes by setting variables to their final values in a single operation. Furthermore, consider evaluating if informational data, such as the vote count, can be emitted in events and tracked off-chain instead of being stored in the contract’s state.

Vote Reason Is Never Used

The internal _castVote function only calls the castVote function from the base Governor contract, which does not support a reason parameter. Consequently, any reason string provided by a user is ignored.

Consider updating the _castVote function to call the castVoteWithReason function of the Governor contract. When a vote is cast without a reason, an empty string can be passed to this function.

Unused Error

In Compensator.sol, the RewardRateMustBeNonNegative error is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider either using or removing any currently unused errors.

Redundant owner() Calls in Owner-Only Functions

In the following functions, the owner view function is called to retrieve the owner’s address. However, each of these calls results in a storage read (SLOAD), consuming unnecessary gas. This is because the onlyOwner modifier already guarantees that msg.sender is the owner.

To optimize gas consumption, consider using msg.sender instead of calling owner() within these functions.

Confusing or Non-Descriptive Naming

Throughout the Compensator contract, multiple instances of confusing naming were identified:

  • The startRewardIndex mapping tracks the last reward index paid per delegator, not just the starting one.
  • The “Delegator” nomenclature is mixed with “User” nomenclature in different parts of the contract. For example, the getPendingRewards function uses the “Delegator” nomenclature while userDeposit and userWithdraw employ the “User” nomenclature.
  • The _resolveProposalInternal function name includes both a leading underscore and the “Internal” suffix. The suffix is redundant as the underscore already signifies that the function is intended for internal use.
  • The remainingRewards, withdrawableAmount, availableForNewRewards, and remainingRewards variables are computed in the same way, but their nomenclature is different.
  • compensatorToOriginalOwner refers to the current owner and not the original owner.
  • The gasUsed variable in the _hasRelevantActiveProposals function captures the amount of gas remaining for the transaction before the loop starts, not the gas used. As such, it should be renamed to gasRemaining.

Consider addressing the aforementioned instances of unclear naming to improve the clarity and maintainability of the codebase.

Unsafe ERC-20 Token Transfers

The Compensator contract imports the SafeERC20 library and declares its usage for the IComp interface. However, the contract does not utilize the functions provided by this library. Instead, the contract makes direct calls to the transfer and transferFrom functions. Some ERC-20 tokens do not revert on failure and instead return false. Interacting with such tokens without checking the return value can lead to failed transfers being treated as successful, potentially causing accounting issues. However, this behavior might not pose an immediate risk if the contract is only ever used with the official COMP token, which reverts on failed transfers.

Consider removing the using SafeERC20 for IComp directive or using safeTransfer and safeTransferFrom for token transfers if compatibility with other ERC-20 tokens is required.

Interpretation of block.number Varies Across Networks

The block.number value carries different implications across various Layer 2 (L2) networks. For example, on the Optimism network, block.number represents the L2 block number, whereas, on Arbitrum, it corresponds to the L1 block number.

Within Compensator.sol, multiple instances were block.number is used were identified:

Consider being careful when using the block.number keyword due to the Compensator contract being deployed on multiple networks.

Inconsistent Calculation of Remaining Rewards

The Compensator contract calculates the remaining rewards available for distribution in multiple parts of the code, including in line 380, line 517, line 1089, and line 1129. However, the implementation of this calculation in line 380 is different from the logic used in the other instances. This inconsistency introduces code repetition and creates a risk of implementation divergences, which could lead to incorrect reward accounting and can increase future maintenance overhead.

Consider refactoring the calculation into a single internal function. This will centralize the logic, protect against implementation divergences, and reduce code repetition, thereby improving the overall maintainability and reliability of the contract.

Interface Definition Does Not Follow Best Practices

In Compensator.sol, an interface named CompensatorFactory is defined to allow high-level calls to the onOwnershipTransferred function. However, defining an interface within the same file as a contract, and using a name that collides with an actual contract implementation, is not a common best practice. This approach can lead to confusion for developers, reduce the codebase’s modularity, and cause potential issues with development tools.

Consider adhering to standard Solidity conventions and refactoring the code by:

  • renaming the interface to ICompensatorFactory to clearly distinguish it as an interface
  • moving the new ICompensatorFactory interface to a standalone file

Doing so will improve the project’s overall code clarity, maintainability, and alignment with community standards.

Custom Errors in require Statements

Since Solidity version 0.8.26, custom error support has been added to require statements. Initially, this feature was only available through the IR pipeline. However, Solidity 0.8.27 extended its support to the legacy pipeline as well.

Throughout the codebase, multiple instances of if-revert statements that could be replaced with require statements were identified:

For conciseness and gas savings, consider replacing if-revert statements with require ones.

Function Visibility Overly Permissive

Within Compensator.sol, multiple instances of functions with unnecessarily permissive visibility were identified:

  • The _resolveProposalInternal function in Compensator.sol with internal visibility could be limited to private.
  • The approve function in Compensator.sol with public visibility could be limited to external.
  • The transferOwnership function in Compensator.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.

Inconsistent Order Within Contracts

Throughout the codebase, multiple instances of contracts having an inconsistent member order were identified:

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

Off-By-One Validation Error in setBlocksPerDay

Within Compensator.sol, in line 1024, the guard clause rejects values equal to MAX_BLOCKS_PER_DAY even though the constant’s name and comment imply that a value equal to the limit itself should be acceptable. This results in the allowed range unintentionally becoming 1MAX_BLOCKS_PER_DAY-1.

To align the implementation with the intended logic, consider changing the comparison operator from >= to >. This will correctly enforce MAX_BLOCKS_PER_DAY as the inclusive maximum value.

Redundant Getter Function

When state variables use public visibility in a contract, a getter method for the variable is automatically included.

Within CompensatorFactory.sol, the getOriginalOwner function is redundant because the compensatorToOriginalOwner state variable already has a getter.

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

Variable Could Be immutable

If a variable is only ever assigned a value from within the constructor of a contract, then it could be declared as immutable.

The delegationCap state variable could be immutable.

To better convey the intended use of variables and to potentially save gas, consider adding the immutable keyword to variables that are only set in the constructor.

Unnecessary Caching of Storage Variables

Within several functions of the Compensator contract, storage variables are read and cached into local memory variables. This is a common gas-saving technique when a storage variable is accessed multiple times within the same function scope. However, there are multiple instances where a storage variable is loaded into memory but is then only used once:

If a storage variable is only ever read once, consider using it directly instead of redundantly caching its value in a memory variable.

Equivalent Events

The RewardIndexUpdated and RewardsDistributed events represent the same underlying operation, emitting both is redundant.

Consider emitting only one of these events.

Constant Gas limit in _hasUserActiveStakes May Prevent Proper Stake Validation

The _hasUserActiveStakes function enforces a fixed PROPOSAL_CHECK_GAS_LIMIT to cap gas consumption when iterating through recent proposals. If opcode gas costs increase in future EVM upgrades, this static threshold may cause the loop to exit prematurely, resulting in incomplete scans of user stakes. In such cases, the function could incorrectly return false despite the user still having active stakes in later proposals.

Users may be able to withdraw funds while they still have active stakes on unresolved proposals if the gas limit prematurely breaks the loop. This undermines the lock and staking logic, potentially leading to protocol-level inconsistencies and premature withdrawals. A similar issue is found in the _hasRelevantActiveProposals function as well.

Consider making the PROPOSAL_CHECK_GAS_LIMIT value updatable by the contract owner.

Redundant address(0) Check for owner

The createCompensator function performs input validation on the owner address, but the same check is already implemented inside the constructor of the Compensator contract, which reverts on a zero-address owner. This makes the check in the createCompensator function redundant.

Consider removing the redundant zero-address check from createCompensator function.

Unnecessary nonReentrant Modifier

The Compensator utilizes OpenZeppelin’s ReentrancyGuard, applying the nonReentrant modifier to multiple entrypoints as a preventative security measure. However, the external calls within these functions are made to the COMP_TOKEN and the COMPOUND_GOVERNOR. Neither of these contracts contains callbacks or hooks that would enable a reentrancy attack. Consequently, the use of the nonReentrant modifier in this context adds unnecessary gas overhead.

For gas optimization, consider removing the nonReentrant modifier from the relevant functions.

Conclusion

The Compensator contract introduces a new delegation-based voting mechanism for Compound proposals that combines reward distribution, staking on proposals, and tracked voting performance. The design clearly separates responsibilities between the owner, delegators, and stakers, while the CompensatorFactory contract provides registry and deployment assurances.

During the review, 1 critical-, 1 high-, and 3 medium-severity issues were identified, along with multiple low- and note-severity findings. The most significant risks relate to broken reward accounting and locked winning stake when the owner votes in the wrong direction. Multiple issues also surfaced as a result of the erroneous handling of the proposal status derived via the Compound Governor contract. Some issues were considered low-severity only because the relevant functionality was broken, unreachable, or unused.

It is critical to emphasize that the current state of the codebase exhibits security and design flaws that extend far beyond the identified vulnerabilities. Despite multiple security reviews, the codebase continues to suffer from pervasive implementation failures alongside fundamental misunderstandings of smart contract constraints and best practices. The presence of critical vulnerabilities despite a test suite that provides false confidence raises serious concerns about the development approach itself. Given the systemic nature of these deficiencies and the pattern observed across multiple review cycles, we will not be conducting a fix review. The issues identified are not amenable to incremental remediation and would require architectural changes affecting the majority of the codebase.

We strongly advise against any production use of this codebase under any circumstances. Should this functionality be required, we recommend beginning development anew with focus on basic correctness and proper validation.

The above findings were addressed some weeks ago and staking COMP for/against specific proposals is no longer supported. Thank you and best wishes to the OpenZeppelin team.

Thank you OZ team for your diligence with the repeated reviews of the Compensator code. You and Certora have both reviewed this code (in OZ’s case, multiple times) and came to materially the same conclusion that the code is unsuitable for production deployment as part of the Compound ecosystem.

On the advice of Compound’s security partners (outgoing and incoming), and in observance of a clear and sustained disconnect between the builder and the auditors in what constitutes production-ready code, CGP will be moving to terminate the grant for the Compensator project.

Your diligence in taking on this project in good faith as you wind down operations on behalf of the protocol speaks volumes to the integrity of your team. Best wishes for the future.

2 Likes

If any community member would like to review the latest assessment, here it is: https://drive.google.com/file/d/1SVvx0Egjwu8B5Rq2wWMT1GFk6n4loxiY/view?usp=sharing. The Compensator project will remain open source here: compensator-cgp · GitHub .