hats-finance / Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb

0 stars 1 forks source link

Double-counting of CVG rewards #60

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x66a4594cd04f9c4e80d1eb581fbfa2efc18032b360ecaf99bda6d1b2c40d10aa Severity: high

Description: Description\ The condition nextClaimableCvg <= nextClaimableCvx in the CVG calculation part can lead to double-counting of CVG rewards for the same cycle, resulting in incorrect reward calculations.

The function uses two variables, nextClaimableCvg and nextClaimableCvx, to keep track of the next claimable cycles for CVG and CVX rewards, respectively.

https://github.com/hats-finance/Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb/blob/246e3ac71f3f2e4ab7eded0f347ad8d070410262/contracts/Staking/Convex/StakingServiceBase.sol#L361

The condition nextClaimableCvg <= nextClaimableCvxis used to determine whether CVG rewards should be calculated for the current cycle (nextClaimableCvx).

However, this condition can lead to double-counting of CVG rewards if nextClaimableCvg and nextClaimableCvx have the same value.

In such a case, the condition nextClaimableCvg <= nextClaimableCvxwill be true, and CVG rewards will be calculated and added to _cvgClaimable for that cycle, even though the user has already claimed CVG rewards for that cycle.

Let's say nextClaimableCvg and nextClaimableCvx are both equal to 10, indicating that the user has already claimed both CVG and CVX rewards up to and including cycle 10.

With the current condition nextClaimableCvg (10) <= nextClaimableCvx (10), the function will calculate and add CVG rewards for cycle 10 to _cvgClaimable again, even though the user has already claimed those rewards.

This double-counting of CVG rewards for the same cycle will lead to incorrect reward calculations and potential overpayment of rewards to the user.

  1. Revised Code File (Optional)
if (nextClaimableCvg < nextClaimableCvx)
PlamenTSV commented 1 month ago

At the end of the function both values for nextClaimable get updated to the current cycle. Any attempt to claim again would revert due to require(actualCycle > nextClaimableCvx, "ALL_CVX_CLAIMED_FOR_NOW") The 2 variables are supposed to be insync after claiming. If you provide a PoC where rewards are unfairly earned will reconsider.