Comet Rewards V2 Audit 2025-03

1

Summary

Timeline: From 2025-02-17 To 2025-03-04

Total Issues: 9 (0 resolved)

Low Severity Issues: 1 (0 resolved)

Notes & Additional Information: 8 (0 resolved)

Scope

OpenZeppelin reviewed pull request #843 of the compound-finance/comet repository at commit 27443c2. This audit was focused on the changes made since the last audit performed at commit 8291a94. The changes can be found here.

In scope were the following files:

contracts
├── CometRewardsInterface.sol
├── CometRewardsV2.sol
└── MerkleProof.sol

System Overview

The Comet protocol currently incentivizes user participation in Comet markets by distributing token rewards. The protocol tracks user rewards by updating the baseTrackingAccrued variable of each user whenever there is a change to the principal amount or the account is explicitly accrued. Rewards can be claimed through the CometRewards.sol contract, which has to be regularly funded by the DAO via a governance proposal. The current implementation of CometRewards allows the distribution of rewards in a single token, which is set to the COMP token.

The main focus of the audit was on CometRewardsV2.sol, which introduces the concept of campaigns for reward distribution. Each campaign is a period of time during which participants in a specified Comet market can earn rewards in a range of tokens, adjusted via some predefined multipliers. Many campaigns can run in parallel for each Comet market and allow for distributing rewards in a variety of tokens. Reward distribution is similarly based on each user’s baseTrackingAccrued amount, and is designed to be inclusive of all users by using Merkle trees at the start and finish of each campaign. By using Merkle trees, the protocol allows users to claim their rewards by providing a proof that they are indeed part of the submitted root.

Security Model and Trust Assumptions

  • Creation of the Merkle Root: The CometRewardsV2 relies on the accurate and verifiable construction of Merkle trees so that campaigns can fairly distribute rewards for each user. For a campaign to be initiated, a Merkle tree has to be constructed with the information of all the Comet users that have a record in the Comet protocol itself. To generate the Merkle root, all account addresses must be sorted in ascending order and used as the leaves of the Merkle tree. Each leaf is then structured by including the account’s index, along with the accrued reward amount at the snapshot’s starting point.
  • Starting a Campaign: The contract’s governor can post the start Merkle root hash on the CometRewardsV2 contract, initiating a campaign for a range of reward tokens with their respective multipliers. The governor also sets the campaign duration, which is limited to a maximum of 180 days.
  • Finishing a Campaign: When a campaign is being started, the finish timestamp of the campaign is set based on the current timestamp and the duration of the campaign. Once the campaign is set, the root, asset configurations, or finish timestamp of the campaign cannot be altered. However, the governor has the ability to finish a campaign before the finish timestamp is reached by setting the campaign’s finish root.
  • Claiming Rewards: Participants in the Comet protocol can claim rewards from an ongoing campaign based on the difference between their current accrued reward and the amount recorded in the root at the campaign’s start. New users who join after the campaign begins are not included in the initial root and are assumed to have had zero accrued rewards at the start. Once the campaign timer expires, all users must wait for the governor to submit the final root, after which they can claim any remaining rewards based on the difference between the final and initial roots.

For the correct and transparent functioning of the system, certain trust assumptions have to be upheld:

  • The CometRewardsV2’s governor is set to the Compound Timelock. The governor has the ability to withdraw all reward tokens in the contract to any address, transfer the contract’s ownership, arbitrarily set the Merkle start and finish roots of campaigns, and re-adjust the amount claimed by each user on demand.
  • The start and finish roots of each Merkle tree are accurately constructed and submitted on-chain via a governance proposal. The community should independently verify the correctness of the Merkle roots, as the trees are constructed using off-chain scripts.
  • The CometRewardsV2 contract should be sufficiently funded to ensure that it can pay all claims to the users.
  • To ensure accurate reward distribution, the finish Merkle tree should be constructed based on a snapshot from the same block number as the finish timestamp of each campaign. Throughout the governance process, the users will be unable to claim, and unless the proposal is enacted, any pending rewards will not be able to be distributed.
  • The start Merkle tree should be ensured to include address(0x0) at index 0 and address(uint160(int160(-1))) at the last index of the Merkle tree. This is to ensure that all users who joined the Comet for the first time after the campaign was initialized can still claim their proportional rewards.
  • The tokens used for rewards should be compliant, returning a boolean value or reverting. If non-compliant tokens are used that return no data, they must revert on failure. The implementation of tokens used as rewards should be examined to be compatible and non-malicious.
  • The setRewardsClaimed function should not be used by the governor, unless during exceptional situations, and should be used with extreme caution in order to maintain the integrity of the system and the fairness of reward distribution.
  • Since the baseTrackingAccrued function only returns each user’s accrued reward at the last point of accrual, some users’ tracking will be outdated and inaccurate. Thus, it is essential that the Merkle trees are constructed using up-to-date accrued amounts for each user.
  • The data availability and full visibility at each step of the Merkle tree root creation are vital. When publishing the proposal, the root proposer should make all relevant data available, including the block number which the Merkle tree root is created with. The scripts used to generate the Merkle trees should be thoroughly reviewed, and a hash of the scripts should be committed on-chain to verify the authenticity of the scripts and allow the community to replicate the results. Ideally, users should also independently verify that they are correctly included in the Merkle roots and can prove their presence. All the relevant data should be logged on the compound-finance/comet and any changes to the scripts should be re-assessed. If a proposal with a misconfigured Merkle root is enacted, it would make it impossible for users to prove on-chain that the root is wrong.
  • The setRewardsClaimed function can arbitrarily set the claimed amount of users for each token in a campaign, increasing or decreasing the actual amount claimed and corrupting data. This function should be used with caution and only through the Timelock.

Low Severity

Incomplete Docstrings

Within CometRewardsV2.sol, multiple instances of incomplete docstrings were identified:

  • The setNewCampaign function does not document the duration parameter.
  • The getRewardsOwed function does not document the shouldAccrue parameter and the return value.
  • The getRewardsOwedBatch function does not document the shouldAccrue parameter.
  • The rewardConfig function does not document all the return values.

Also, multiple instances of docstrings missing key details were identified, which may lead to misunderstandings about their intended usage:

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).

Notes & Additional Information

Lack of Indexed Event Parameters

Within CometRewardsV2.sol, multiple instances of events not having indexed parameters were identified:

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

Multiple Functions With Incorrect Order of Modifiers

Function modifiers should be ordered as follows: visibility, mutability, virtual, override, and custom modifiers.

Within CometRewardsInterface.sol, multiple instances of functions having an incorrect order of modifiers were identified:

To improve the project’s overall legibility, consider reordering function modifiers as recommended by the Solidity Style Guide.

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 in 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 CometRewardsV2.sol, multiple instances where function parameters should use calldata instead of memory were identified. For instances, the campaignIDs parameter in lines 489, 566, 625, and 769.

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

Unused Named Return Variables

Named return variables should only be used when they improve readability or are modified implicitly. Otherwise, they can be removed to enhance clarity and consistency.

Throughout the codebase, multiple instances of unused named return variables were identified:

  • doTransferOut: The amountOut named return variable is declared but never assigned a value. The function instead returns amount on success and defaults to 0 on failure, which may be unclear.
  • getRewardsAccrued: The accrued named return variable is declared but is manually assigned before returning, making the named return unnecessary.
  • setNewCampaignWithCustomTokenMultiplier: The campaignId named return variable is explicitly assigned but does not leverage the named return mechanism. On a side note, consider changing the return statement on line 246 to match the format used in the rest of the code by using return variable; instead of return(variable);.
  • setNewCampaign: The campaignId named return variable is not modified in the function body and is directly returned from another function.

Consider removing the named return variables from these functions and explicitly returning the values to improve code clarity and maintain consistency across the codebase.

Inconsistent and Redundant Validation

Throughout the codebase, multiple instances of inconsistent and/or redundant validation were identified:

To improve code consistency, increase efficiency, and reduce gas costs, consider ensuring that all functions calling verifyNewMember or claimInternal only check that the number of campaigns of the comet, removing redundant validation from loops and consistently validating inputs in their corresponding functions.

Unused Error

Unused errors can negatively affect code clarity and maintainability. In CometRewardsV2.sol, the NotANewMember error is unused.

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

Inconsistent Parameter Naming

Throughout CometRewardsV2.sol, multiple instances of inconsistent naming were identified:

  • The verifyProof function accepts a single proof and verifies whether the leaf is in the Merkle Tree. However, the parameter is named as proofs.
  • The parameter that shows the campaign ID is named as both campaignId or campaignID. Consider keeping the naming consistent throughout the file.

Consider addressing the above instances to improve the readability and clarity of the codebase.

Redundant Two-Dimensional Token Array Risks Invalid Reward Tracking and Gas Inefficiency

The setRewardsClaimed function uses a two-dimensional tokens array, forcing the redundant submission of token lists that must mirror the campaign’s assets for every user. While the current checks enforce array length consistency, they do not validate that the token addresses in tokens[i] match the campaign’s assets. This allows accidental or malicious misalignment (e.g., including a token not in the campaign’s assets), resulting in claimed amounts being irreversibly recorded for invalid or unintended tokens.

Consider removing the tokens parameter entirely and deriving token addresses directly from the campaign’s assets, paired with a two-dimensional claimedAmounts array where each sub-array corresponds to the predefined assets in a fixed order. This ensures token consistency, eliminates redundancy, and prevents invalid token mappings.

Conclusion

OpenZeppelin reviewed the codebase and found it to be well-structured and without any major issues. The recent changes to CometRewardsV2.sol have improved both its readability and resilience against potential attacks. Nonetheless, various recommendations aimed at enhancing code clarity were made. We appreciate the Woof Software team for their responsiveness and collaboration throughout the process.

1 Like