Submitted on May 20th 2024 at 13:23:00 UTC by @Norah for Boost | Alchemix
Report ID: #31494
Report type: Smart Contract
Report severity: High
Target: https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Voter.sol
Impacts:
- Permanent freezing of unclaimed yield
- During each epoch, the minter contract distributes emissions to various components of the Alchemix system, one of them being the voter contract.
- Anyone can call the
distribute()function on the voter contract. If the minter period/epoch is over, the rewards are distributed to the various gauges, andminter.updatePeriod()is called, which triggers the minter contract to distribute the emissions for the previous epoch.
- The problem arises if the
_distribute()function is called first and thenminter.updatePeriod(). I - In this scenario, during the first epoch, since there are no emissions at this point, no rewards will be distributed. - Afterward,minter.updatePeriod()will be triggered, sending the rewards to the voter contract. - However, the rewards distribution routine has already been called, so no reward will be transferred to the contract or updated in the claimable[_gauge] mapping for the gauges to claim later on.
- Essentially, there is emissions are now stuck in the
votercontract forever.
- Essentially, there is emissions are now stuck in the
- Ideally, minter.updatePeriod() should be called first, to allow the minter contract to send the rewards, and then the _distribute() routine should be called to distribute and update the rewards in claimable or send them directly to the gauge
- For more detail check POC.
- As a result, the rewards for the first epoch will be forever stuck in the voter contract, and gauges will be unable to claim these rewards.
- This will result in the permanent freezing of yield (emissions) for the gauges.
Add any relevant links to documentation or code
- Update the code of the
distribute()routine so thatIMinter(minter).updatePeriod()is called before_distribute(). - Also, remove the timestamp check from
_distribute()and place it indistribute().
function distribute() external {
+++ require( block.timestamp >= IMinter(minter).activePeriod() IMinter(minter).DURATION(), "can only distribute after period end" );
+++ IMinter(minter).updatePeriod();
uint256 start = 0;
uint256 finish = pools.length;
for (uint256 x = start; x < finish; x++) {
// We don't revert if gauge is not alive since pools.length is not reduced
if (isAlive[gauges[pools[x]]]) { _distribute(gauges[pools[x]]); }
--- IMinter(minter).updatePeriod();
}
function _distribute(address _gauge) internal {
--- require( block.timestamp >= IMinter(minter).activePeriod() + IMinter(minter).DURATION(), "can only distribute after period end");
uint256 _claimable = claimable[_gauge];
// Reset claimable amount
claimable[_gauge] = 0;
_updateFor(_gauge);
if (_claimable > 0) {
IBaseGauge(_gauge).notifyRewardAmount(_claimable);
}
IBribe(bribes[_gauge]).resetVoting();
emit DistributeReward(msg.sender, _gauge, _claimable);
}
I have created a test showcasing how the rewards sent during the first epoch are stuck in the voter contract for 3 epochs.
- For better understanding, re-run the test after implementing the recommended changes.
- Note that there will still be a lag between gauge_balance and claimable_amount (i.e., gauge_balance will be updated in the next epoch each time), but that is due to a different bug.
- Add the test below to the voting.t.sol file of the test suite and run it with the following command:
- forge test --fork-url https://eth-mainnet.g.alchemy.com/v2/{Alchemy-api-key} --match-test "testFirstEpochRewardsStuck" -vvv
function testFirstEpochRewardsStuck() public {
uint256 period = minter.activePeriod();
//first empty the gauge reciever (passthrough gauge) for better reward tracking
vm.startPrank(address(sushiGauge.receiver()));
alcx.transfer(address(0xdead),alcx.balanceOf(address(sushiGauge.receiver())));
vm.stopPrank();
//we create a voter who will vote during the first epoch.
//which will trigger the emission of the reward to the voter contract as the
//voting weight will be more than zero.
uint256 tokenId = createVeAlcx(admin, 100*TOKEN_1, MAXTIME, false);
address[] memory pools = new address[](1);
pools[0] = sushiPoolAddress;
uint256[] memory weights = new uint256[](1);
weights[0] = 5000;
address[] memory gauges = new address[](1);//(10023200000000000000000-5037600000000000000000)/10023200000000000000000
gauges[0] = address(sushiGauge);
address gauge_reciever = sushiGauge.receiver();
hevm.prank(admin);
voter.vote(tokenId, pools, weights, 0);
hevm.warp(period + nextEpoch);
hevm.roll(block.number + 1);
console2.log("1st call ");
voter.distribute();
//Ideally as Voter.distribute() is being called after epochs end,
//therefore the rewards emitted during this epoch should be sent to the gauge
//Instead of that, due to mentioned voter recieve the alcx tokens but these are
//neither transfer to the gauge nor the gauges claimable_amount is updated.
uint256 claimable_amount = voter.claimable(address(sushiGauge)); // this will be non zero amount
uint256 Gauge_balance = alcx.balanceOf(address(gauge_reciever));// Actual balance of Gauge is zero.
uint256 voter_balance = alcx.balanceOf(address(voter));//some amount
//As result,here claimable amount is zero
//first epoch emmision sent to the emissions are forever stuck in voter contract
// assertEq(claimable_amount,0);
// assertEq(Gauge_balance,0);
// assertGt(voter_balance,0);
console2.log("claimable amount :",claimable_amount);
console2.log("Gauge_balance :",Gauge_balance);
console2.log("voter_balance :",voter_balance);
console2.log("");
//During second epoch,
//Atleast rewards will get updated into the claimable balance for gauge.
//Due to another bug this are not being sent to the gauge at this instance (which they would have been).
//nonetheless,first epochs balance is still stuck in the voter contract.
hevm.warp(block.timestamp + nextEpoch);
console2.log("2nd call ");
voter.distribute();
claimable_amount = voter.claimable(address(sushiGauge));
Gauge_balance = alcx.balanceOf(address(gauge_reciever));
voter_balance = alcx.balanceOf(address(voter));
console2.log("claimable amount :",claimable_amount);
console2.log("Gauge_balance :",Gauge_balance);
console2.log("voter_balance :",voter_balance);
//first epoch reward stuck = voter_balance = - claimable_amount(to be sent)
console2.log("Rewards stuck in the voter during (2nd Epoch) :", voter_balance - claimable_amount);
console2.log("");
//Here we can see second epochs balance is now transfer to the gauge
//3rd epoch balance is updated in claimable amount.
//First epochs rewards are still stuck here
hevm.warp(block.timestamp + nextEpoch);//5037599999999989148379 + 4985599999999982279363 - 9919200000000010851621
console2.log("3rd call ");
voter.distribute();
claimable_amount = voter.claimable(address(sushiGauge));
Gauge_balance = alcx.balanceOf(address(gauge_reciever));
voter_balance = alcx.balanceOf(address(voter));
console2.log("claimable amount :",claimable_amount);
console2.log("Gauge_balance :",Gauge_balance);
console2.log("voter_balance :",voter_balance);
//first epoch reward stuck = voter_balance - claimable_amount(to be sent)
console2.log("Rewards stuck in the voter during (3rd Epoch) :", voter_balance - claimable_amount);
console2.log("");
//Now change the code as mentioned below for the distribute() routine of the Voter.sol and remove the timestamp check _distribute() routine
//and place it in distribute() function before calling update.period function.
//and re-run the test, there will only negible residue will stuck in the voter.sol only due to rounding error.
//There will be still lag between gauge_balance adn claimable_amount (i.e gauge_balance will be updated in the next epoch each time) due to different bug.
/*
function distribute() external {
uint256 start = 0;
uint256 finish = pools.length;
require(
block.timestamp >= IMinter(minter).activePeriod() + IMinter(minter).DURATION(),
"can only distribute after period end"
);
IMinter(minter).updatePeriod();
for (uint256 x = start; x < finish; x++) {
// We don't revert if gauge is not alive since pools.length is not reduced
if (isAlive[gauges[pools[x]]]) {
_distribute(gauges[pools[x]]);
}
}
//IMinter(minter).updatePeriod();
}
function _distribute(address _gauge) internal {
// Distribute once after epoch has ended
// require(
// block.timestamp >= IMinter(minter).activePeriod() + IMinter(minter).DURATION(),
// "can only distribute after period end"
// );
uint256 _claimable = claimable[_gauge];
// Reset claimable amount
claimable[_gauge] = 0;
_updateFor(_gauge);
if (_claimable > 0) {
IBaseGauge(_gauge).notifyRewardAmount(_claimable);
}
IBribe(bribes[_gauge]).resetVoting();
emit DistributeReward(msg.sender, _gauge, _claimable);
}
*/
}