Omniscia DappRadar Audit

RadarStakingLogic Manual Review Findings

RSL-01M: Improper Stake Cooldown Accounting

Type	Severity	Location
Logical Fault		RadarStakingLogic.sol:L113

Description:

The cooldownSeconds associated with a particular stake will remain the same regardless of whether the configurational variable cooldownSeconds of the RadarStakingLogic contract was adjusted via the RadarStakingLogic::setCooldownSeconds function.

Impact:

Presently, if a user has triggered their unstake cooldown and the cooldown of the RadarStakingLogic is increased, the staker who initiated their cooldown will retain the original lower-value cooldown and vice versa.

Example:

contracts/RadarStakingLogic.sol

107// unstake your tokens + rewards after the cooldown has passed
108function unstake(uint256 amount) external nonReentrant {
109    require(amount >= 0, "RadarStakingLogic: Amount cannot be lower than 0");
110    iRadarStake.Stake memory myStake = radarStakeContract.getStake(_msgSender());
111
112    require(myStake.cooldownTriggeredAtTimestamp > 0, "RadarStakingLogic: Cooldown not yet triggered");
113    require(block.timestamp >= myStake.cooldownTriggeredAtTimestamp + myStake.cooldownSeconds, "RadarStakingLogic: Can't unstake during the cooldown period");
114    
115    require(myStake.totalStaked >= amount, "RadarStakingLogic: Amount you want to unstake exceeds your staked amount");
116    require((myStake.totalStaked - amount >= stakeForDappRadarPro) || (myStake.totalStaked - amount == 0), "RadarStakingLogic: Either unstake all or keep more than the minimum stake required");
117    
118    // calculate rewards
119    uint256 tokenReward = calculateReward(_msgSender());
120
121    // unstake
122    radarStakeContract.removeFromStake(amount, _msgSender());
123
124    // transfer the rewards from the rewardAddress
125    radarTokenContract.transferFrom(rewardAddress, _msgSender(), tokenReward);
126
127    // transfer the stake from the radarStakeContract
128    radarTokenContract.transferFrom(address(radarStakeContract), _msgSender(), amount);
129
130    emit TokensUnstaked(_msgSender(), amount);
131}

Recommendation:

We advise the system to instead only track the cooldown trigger timestamp at the RadarStake contract and to apply the cooldownSeconds in the referenced conditional directly from the RadarStakingLogic contract rather than the RadarStake contract.

Alleviation (8614454f02):

The cooldown value of RadarStake is being utilized by the code, however, the code still stores the cooldown duration per stake rather than at a contract-level.

Alleviation (9193d77dde):

The DappRadar team has stated that it is an intended design for users to keep the original cooldown they initiated. As such, we consider this exhibit adequately alleviated in the original remediation.

RSL-02M: Incorrect Calculation of Compounding Reward

Type	Severity	Location
Mathematical Operations		RadarStakingLogic.sol:L171, L182-L186

Description:

The RadarStakingLogic::calculateCompoundingReward is meant to calculate an auto-compounding daily reward by iterating through all days passed during a reward period and compounding the reward. While the mechanism in the function is correct, its input argument principal is set as the accruedReward. As such, it will calculate the total reward of a period X and then proceed to calculate the compounding reward daily by using the reward at the end of the period which is incorrect.

Impact:

The RadarStakingLogic will calculate a compoundingReward greater-than the actual one as it does not replicate an auto-compounding system properly. To better illustrate the issue, let us consider an APR of 1%, a total reward duration of a year, and a totalStaked value of 1_000.

The code will calculate the APR of the whole year which is 1% of 1_000 (10) at the end of it. The code will then proceed to restart the rewards from the start of the year, iterating through each day with the value of 10 and auto-compounding it.

The correct auto-compounding value would be 10.050028723668065, however, the code of RadarStakingLogic would produce a value of 10.100223568782742 as it assumes the auto-compounding portion of the reward is the actual final reward rather than the reward due per day. The flaw can be exacerbated the longer the duration in which we calculate the compounding APR in.

Example:

contracts/RadarStakingLogic.sol

133// calculate the total rewards a user has already earned.
134function calculateReward(address addr) public view returns(uint256 reward) {
135    require(addr != address(0), "RadarStakingLogic: Cannot use the null address");
136
137    iRadarStake.Stake memory myStake = radarStakeContract.getStake(addr);
138    uint256 totalStaked = myStake.totalStaked;
139
140    // return 0 if the user has no stake
141    if (totalStaked <= 0 ) return 0;
142
143    iRadarStake.Apr[] memory allAprs = radarStakeContract.getAllAprs();
144    for (uint256 i = 0; i < allAprs.length; i++) {
145        iRadarStake.Apr memory currentApr = allAprs[i];
146
147        // jump over APRs, which are in the past for this user/address
148        if (currentApr.endTime > 0 && currentApr.endTime < myStake.lastStakedTimestamp) continue;
149
150        uint256 startTime = (myStake.lastStakedTimestamp > currentApr.startTime) ? myStake.lastStakedTimestamp : currentApr.startTime;
151        uint256 endTime = (currentApr.endTime < block.timestamp)? currentApr.endTime : block.timestamp;
152
153        // use current timestamp if the APR is still active (aka. has no endTime yet)
154        if (endTime <= 0) endTime = block.timestamp;
155
156        // once the cooldown is triggered, don't accrue any further from that point in time
157        if (myStake.cooldownTriggeredAtTimestamp > 0) {
158            endTime = myStake.cooldownTriggeredAtTimestamp;
159        }
160
161        // protect against subtraction errors
162        if (endTime <= startTime) continue;
163
164        uint256 secondsWithCurrentApr = endTime - startTime;
165        uint256 daysPassed = secondsWithCurrentApr/1 days;
166
167        // calculate accrued reward for each APR period (per second)
168        uint256 accruedReward = totalStaked * currentApr.apr/10_000 * secondsWithCurrentApr/(365 days);
169
170        // calculate compounding rewards (per day)
171        uint256 compoundingReward = calculateCompoundingReward(accruedReward, currentApr.apr, daysPassed);
172        
173        // compound the rewards for each APR period
174        reward += accruedReward + compoundingReward;
175        totalStaked += reward;
176    }
177
178    return reward;
179}
180
181// calculate compounding interest without running into floating point issues
182function calculateCompoundingReward(uint256 principal, uint256 aprToUse, uint256 daysPassed) internal pure returns(uint256 compoundingReward) {
183    for (uint256 i = 0; i < daysPassed; i++) {
184        compoundingReward += (principal + compoundingReward) * aprToUse/10_000/365;
185    }
186}

Recommendation:

We advise the compounding reward mechanism to calculate the reward due for each day as well, ensuring that it properly calculates an auto-compounding reward.

Alleviation (8614454f02ec2934ccfdc3f17974be01ca6c1709):

The compounding reward calculation was corrected, ensuring that rewards are calculated only once by advancing time rather than twice by first advancing time and then restarting from the start of the period.

RSL-03M: Incorrect Accumulation of Total Staked

Type	Severity	Location
Logical Fault		RadarStakingLogic.sol:L175

Description:

The totalStaked variable within RadarStakingLogic::calculateReward is incorrectly incremented on each iteration as the reward value it is incremented by is cumulative.

Impact:

All reward calculations via RadarStakingLogic::calculateReward are improperly diluted, causing users to receive less rewards than expected.

Example:

contracts/RadarStakingLogic.sol

167// calculate accrued reward for each APR period (per second)
168uint256 accruedReward = totalStaked * currentApr.apr/10_000 * secondsWithCurrentApr/(365 days);
169
170// calculate compounding rewards (per day)
171uint256 compoundingReward = calculateCompoundingReward(accruedReward, currentApr.apr, daysPassed);
172
173// compound the rewards for each APR period
174reward += accruedReward + compoundingReward;
175totalStaked += reward;

Recommendation:

We advise the totalStaked value to be increased by accruedReward + compoundingReward or the accruedReward calculation to utilize totalStaked + reward, either of which we consider an adequate resolution to this exhibit and the latter of which we advise.

Alleviation (8614454f02ec2934ccfdc3f17974be01ca6c1709):

The totalStaked variable is now incremented by the compoundingReward variable rather than the total cumulative reward variable, ensuring that the totalStaked value correctly corresponds to the total stakes plus any reward that has accumulated per iteration.

Omniscia DappRadar Audit

RadarStakingLogic Manual Review Findings

RadarStakingLogic Manual Review Findings

RSL-01M: Improper Stake Cooldown Accounting

Description:

Impact:

Example:

Recommendation:

Alleviation (8614454f02):

Alleviation (9193d77dde):

RSL-02M: Incorrect Calculation of Compounding Reward

Description:

Impact:

Example:

Recommendation:

Alleviation (8614454f02ec2934ccfdc3f17974be01ca6c1709):

RSL-03M: Incorrect Accumulation of Total Staked

Description:

Impact:

Example:

Recommendation:

Alleviation (8614454f02ec2934ccfdc3f17974be01ca6c1709):

RadarStakingLogic.sol (RSL-S)

RadarStake.sol (RSE-C)

On this page