Lines of code
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L357>
The changeRewardSpeed() function computes rewardsEndTimestamp incorrectly for the case block.timestamp < prevEndTime. For example, it may increase the rewardsEndTimestamp by several years forward despite the fact that the fund will be enough for one day only. As a result, some users will claim extra rewards at the expense of other users.
The function changeRewardSpeed() calculates rewardsEndTimestamp via
_calcRewardsEnd(
prevEndTime > block.timestamp ? prevEndTime : block.timestamp.safeCastTo32(),
rewardsPerSecond,
remainder
)
If the prevEndTime > block.timestamp then it can be reduced to _calcRewardsEnd(prevEndTime, rewardsPerSecond, remainder) where the remainder is the contractβs reward token balance.
Then, in the _calcRewardsEnd() the first condition is met and the amount value is increased from remainder to remainder + rewardsPerSecond * (timeLeft):
if (rewardsEndTimestamp > block.timestamp)
amount += uint256(rewardsPerSecond) * (rewardsEndTimestamp - block.timestamp);
Thus, the amount becomes larger the original contractβs reward token balance and the new rewardsEndTimestamp is calculated incorrectly:
return (block.timestamp + (amount / uint256(rewardsPerSecond))).safeCastTo32();
You can use the following forge test to check for the error. Put the following test in ./test/ folder and run with forge test --mc IncorrectComputation. The test fails because rewardsEndTimestamp calculations are wrong:
// SPDX-License-Identifier: GPL-3.0
// Docgen-SOLC: 0.8.15
pragma solidity ^0.8.15;
import { Test } from "forge-std/Test.sol";
import { SafeCastLib } from "solmate/utils/SafeCastLib.sol";
import { MockERC20 } from "./utils/mocks/MockERC20.sol";
import { IMultiRewardEscrow } from "../src/interfaces/IMultiRewardEscrow.sol";
import { MultiRewardStaking, IERC20 } from "../src/utils/MultiRewardStaking.sol";
import { MultiRewardEscrow } from "../src/utils/MultiRewardEscrow.sol";
import { RewardInfo, EscrowInfo } from "../src/interfaces/IMultiRewardStaking.sol";
contract IncorrectComputation is Test {
using SafeCastLib for uint256;
MockERC20 stakingToken;
MockERC20 rewardToken;
MultiRewardStaking staking;
MultiRewardEscrow escrow;
address alice = address(0xABCD);
address bob = address(0xDCBA);
address feeRecipient = address(0x9999);
uint160 constant rps = 1 ether;
uint constant funds = 100 ether;
function setUp() public {
vm.label(alice, "alice");
vm.label(bob, "bob");
stakingToken = new MockERC20("Staking Token", "STKN", 18);
rewardToken = new MockERC20("RewardsToken1", "RTKN1", 18);
escrow = new MultiRewardEscrow(address(this), feeRecipient);
staking = new MultiRewardStaking();
staking.initialize(IERC20(address(stakingToken)), IMultiRewardEscrow(address(escrow)), address(this));
rewardToken.mint(address(this), 1000 ether);
rewardToken.approve(address(staking), 1000 ether);
staking.addRewardToken(
// rewardToken
IERC20(address(rewardToken)),
// rewardsPerSecond
rps,
// amount
funds,
// useEscrow
false,
// escrowPercentage
0,
// escrowDuration
0,
// offset
0
);
}
function testIncorrectComputation() public {
stakingToken.mint(address(bob), 1);
vm.prank(bob);
stakingToken.approve(address(staking), 1);
vm.prank(bob);
staking.deposit(1);
(
/*uint64 ONE1*/,
/*uint160 rewardsPerSecond1*/,
uint32 rewardsEndTimestamp1,
/*uint224 index1*/,
/*uint32 lastUpdatedTimestamp1*/
) = staking.rewardInfos(IERC20(address(rewardToken)));
assert (rewardsEndTimestamp1 == block.timestamp + funds / rps);
// now we change the speed to the same number
// so it shouldn't change anything
// (but it incorrectly does)
staking.changeRewardSpeed(IERC20(address(rewardToken)), rps);
(
/*uint64 ONE2*/,
/*uint160 rewardsPerSecond2*/,
uint32 rewardsEndTimestamp2,
/*uint224 index2*/,
/*uint32 lastUpdatedTimestamp2*/
) = staking.rewardInfos(IERC20(address(rewardToken)));
// THIS WILL INCORRECTLY REVERT BECAUSE COMPUTATIONS IN changeRewardSpeed() are wrong!
assert (rewardsEndTimestamp2 == rewardsEndTimestamp1);
}
}
Manual analysis
Fix rewardsEndTimestamp calculations
The text was updated successfully, but these errors were encountered:
All reactions