Skip to content

Commit

Permalink
feat: remove migration code (#389)
Browse files Browse the repository at this point in the history
### Description

fixes [L-1] Locking migration breaks withdrawals for unpaused MentoToken
by removing migration related code from locking contracts as we decided


### Other changes

no

### Tested

all tests pass

### Related issues

- Fixes #378
  • Loading branch information
baroooo authored Mar 6, 2024
1 parent 37e1e8a commit 220b5b8
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 204 deletions.
57 changes: 3 additions & 54 deletions contracts/governance/locking/Locking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity 0.8.18;
// solhint-disable func-name-mixedcase

import "./interfaces/INextVersionLock.sol";
import "./LockingBase.sol";
import "./LockingRelock.sol";
import "./LockingVotes.sol";
Expand Down Expand Up @@ -53,20 +52,9 @@ contract Locking is ILocking, LockingBase, LockingRelock, LockingVotes {
emit StartLocking(msg.sender);
}

/**
* @notice Begins the migration process to a new contract
* @dev Can only be called by the owner
* @param to Address of the new contract where future operations will be migrated to
*/
function startMigration(address to) external onlyOwner {
// slither-disable-next-line missing-zero-check
migrateTo = to;
emit StartMigration(msg.sender, to);
}

/**
* @notice Locks a specified amount of tokens for a given period
* @dev Can not be called when locking is stopped or migration is in progress
* @dev Can not be called when locking is stopped
* @param account Account for which tokens are being locked
* @param _delegate Address that will receive the voting power from the locked tokens
* If address(0) passed, voting power will be lost
Expand All @@ -81,7 +69,7 @@ contract Locking is ILocking, LockingBase, LockingRelock, LockingVotes {
uint96 amount,
uint32 slopePeriod,
uint32 cliff
) external override notStopped notMigrating returns (uint256) {
) external override notStopped returns (uint256) {
require(amount > 0, "zero amount");
require(cliff <= MAX_CLIFF_PERIOD, "cliff too big");
require(slopePeriod <= MAX_SLOPE_PERIOD, "period too big");
Expand Down Expand Up @@ -164,7 +152,7 @@ contract Locking is ILocking, LockingBase, LockingRelock, LockingVotes {
* @param id The unique identifier for the lock whose delegate is to be changed
* @param newDelegate The address to which the delegation will be transferred
*/
function delegateTo(uint256 id, address newDelegate) external notStopped notMigrating {
function delegateTo(uint256 id, address newDelegate) external notStopped {
address account = verifyLockOwner(id);
address _delegate = locks[id].delegate;
uint32 currentBlock = getBlockNumber();
Expand Down Expand Up @@ -205,45 +193,6 @@ contract Locking is ILocking, LockingBase, LockingRelock, LockingVotes {
return accounts[account].balance.actualValue(time, currentBlock);
}

/**
* @notice Migrates specified locks to a new contract
* @dev Performs the migration by transferring locked tokens and updating delegations as necessary
* @param id An array of lock IDs to be migrated
*/
function migrate(uint256[] memory id) external {
if (migrateTo == address(0)) {
return;
}
uint32 currentBlock = getBlockNumber();
uint32 time = roundTimestamp(currentBlock);
INextVersionLock nextVersionLock = INextVersionLock(migrateTo);
for (uint256 i = 0; i < id.length; ++i) {
address account = verifyLockOwner(id[i]);
address _delegate = locks[id[i]].delegate;
updateLines(account, _delegate, time);
//save data Line before remove
LibBrokenLine.Line memory line = accounts[account].locked.initiatedLines[id[i]];
// slither-disable-start unused-return
(uint96 residue, , ) = accounts[account].locked.remove(id[i], time, currentBlock);

accounts[account].amount = accounts[account].amount - (residue);

accounts[_delegate].balance.remove(id[i], time, currentBlock);
totalSupplyLine.remove(id[i], time, currentBlock);
// slither-disable-end unused-return
// slither-disable-start reentrancy-no-eth
// slither-disable-start reentrancy-events
// slither-disable-start calls-loop
nextVersionLock.initiateData(id[i], line, account, _delegate);

require(token.transfer(migrateTo, residue), "transfer failed");
// slither-disable-end reentrancy-no-eth
// slither-disable-end reentrancy-events
// slither-disable-end calls-loop
}
emit Migrate(msg.sender, id);
}

/**
* @notice Returns the name of the token
*/
Expand Down
39 changes: 7 additions & 32 deletions contracts/governance/locking/LockingBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @dev True if contract entered stopped state
*/
bool public stopped;
/**
* @dev Address to migrate locks to. Is zero if not in migration state
*/
address public migrateTo;
/**
* @dev Minimum cliff period in weeks
*/
Expand Down Expand Up @@ -125,10 +121,6 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @dev Emitted when withdraw amount of Rari, account - msg.sender, amount - amount Rari
*/
event Withdraw(address indexed account, uint256 amount);
/**
* @dev Emitted when migrate Locks with given id, account - msg.sender
*/
event Migrate(address indexed account, uint256[] id);
/**
* @dev Stop run contract functions, accept withdraw, account - msg.sender
*/
Expand All @@ -137,10 +129,6 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @dev Start run contract functions, accept withdraw, account - msg.sender
*/
event StartLocking(address indexed account);
/**
* @dev StartMigration initiate migration to another contract, account - msg.sender, to - address delegate to
*/
event StartMigration(address indexed account, address indexed to);
/**
* @dev set newMinCliffPeriod
*/
Expand Down Expand Up @@ -329,7 +317,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @notice Sets the starting point for the week-based time system
* @param newStartingPointWeek new starting point
*/
function setStartingPointWeek(uint32 newStartingPointWeek) public notStopped notMigrating onlyOwner {
function setStartingPointWeek(uint32 newStartingPointWeek) public notStopped onlyOwner {
require(newStartingPointWeek < roundTimestamp(getBlockNumber()), "wrong newStartingPointWeek");
startingPointWeek = newStartingPointWeek;

Expand All @@ -340,7 +328,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @notice Sets the minimum cliff period
* @param newMinCliffPeriod new minimum cliff period
*/
function setMinCliffPeriod(uint32 newMinCliffPeriod) external notStopped notMigrating onlyOwner {
function setMinCliffPeriod(uint32 newMinCliffPeriod) external notStopped onlyOwner {
require(newMinCliffPeriod <= MAX_CLIFF_PERIOD, "new cliff period > 2 years");
minCliffPeriod = newMinCliffPeriod;

Expand All @@ -351,7 +339,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @notice Sets the minimum slope period
* @param newMinSlopePeriod new minimum slope period
*/
function setMinSlopePeriod(uint32 newMinSlopePeriod) external notStopped notMigrating onlyOwner {
function setMinSlopePeriod(uint32 newMinSlopePeriod) external notStopped onlyOwner {
require(newMinSlopePeriod <= MAX_SLOPE_PERIOD, "new slope period > 2 years");
minSlopePeriod = newMinSlopePeriod;

Expand All @@ -374,20 +362,12 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
_;
}

/**
* @dev Throws if migration is active
*/
modifier notMigrating() {
require(migrateTo == address(0), "migrating");
_;
}

/**
* @notice Updates the broken lines for an account until a given week number
* @param account address of account to update
* @param time week number until which to update lines
*/
function updateAccountLines(address account, uint32 time) public notStopped notMigrating onlyOwner {
function updateAccountLines(address account, uint32 time) public notStopped onlyOwner {
accounts[account].balance.update(time);
accounts[account].locked.update(time);
}
Expand All @@ -396,7 +376,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @notice updates the total supply line until a given week number
* @param time week number until which to update lines
*/
function updateTotalSupplyLine(uint32 time) public notStopped notMigrating onlyOwner {
function updateTotalSupplyLine(uint32 time) public notStopped onlyOwner {
totalSupplyLine.update(time);
}

Expand All @@ -405,12 +385,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @param account address of account to update
* @param blockNumber block number until which to update lines
*/
function updateAccountLinesBlockNumber(address account, uint32 blockNumber)
external
notStopped
notMigrating
onlyOwner
{
function updateAccountLinesBlockNumber(address account, uint32 blockNumber) external notStopped onlyOwner {
uint32 time = roundTimestamp(blockNumber);
updateAccountLines(account, time);
}
Expand All @@ -419,7 +394,7 @@ abstract contract LockingBase is OwnableUpgradeable, IVotesUpgradeable {
* @notice Updates the total supply line until a given block number
* @param blockNumber block number until which to update line
*/
function updateTotalSupplyLineBlockNumber(uint32 blockNumber) external notStopped notMigrating onlyOwner {
function updateTotalSupplyLineBlockNumber(uint32 blockNumber) external notStopped onlyOwner {
uint32 time = roundTimestamp(blockNumber);
updateTotalSupplyLine(time);
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/governance/locking/LockingRelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ abstract contract LockingRelock is LockingBase {
uint96 newAmount,
uint32 newSlopePeriod,
uint32 newCliff
) external notStopped notMigrating returns (uint256) {
) external notStopped returns (uint256) {
address account = verifyLockOwner(id);
uint32 currentBlock = getBlockNumber();
uint32 time = roundTimestamp(currentBlock);
Expand Down
25 changes: 0 additions & 25 deletions contracts/governance/locking/interfaces/INextVersionLock.sol

This file was deleted.

92 changes: 0 additions & 92 deletions test/governance/Locking/locking.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -391,98 +391,6 @@ contract Lock_Locking_Test is Locking_Test {
locking.getPastTotalSupply(currentBlock);
}

function test_migrate_shouldMigrateLocks() public {
MockLocking mockLockingContract = new MockLocking();

mentoToken.mint(alice, 100000);
vm.prank(alice);
uint256 lockId = locking.lock(alice, alice, 60000, 30, 0);

uint256[] memory ids = new uint256[](1);
ids[0] = lockId;

vm.prank(owner);
locking.startMigration(address(mockLockingContract));
// (30 / 104) * 60000 = 17307
assertEq(locking.balanceOf(alice), 17307);
assertEq(locking.totalSupply(), 17307);

vm.prank(alice);
locking.migrate(ids);

assertEq(locking.balanceOf(alice), 0);
assertEq(locking.totalSupply(), 0);

assertEq(mentoToken.balanceOf(address(mockLockingContract)), 60000);
assertEq(locking.totalSupply(), 0);
}

function test_migrate_whenDelegatedAndInSlope_shouldMigrateLocks() public {
MockLocking newLockingContract = new MockLocking();

mentoToken.mint(alice, 100000);
vm.prank(alice);
uint256 lockId = locking.lock(alice, bob, 60000, 30, 0);

uint256[] memory ids = new uint256[](1);
ids[0] = lockId;

_incrementBlock(10 * weekInBlocks);

vm.prank(owner);
locking.startMigration(address(newLockingContract));

vm.prank(alice);
locking.migrate(ids);

assertEq(locking.balanceOf(bob), 0);
assertEq(mentoToken.balanceOf(address(locking)), 20000);
assertEq(mentoToken.balanceOf(address(newLockingContract)), 40000);
assertEq(mentoToken.balanceOf(alice), 40000);

assertEq(locking.totalSupply(), 0);

vm.prank(alice);
locking.withdraw();

assertEq(mentoToken.balanceOf(address(locking)), 0);
assertEq(locking.totalSupply(), 0);
}

function test_migrate_whenDelegatedAndInTail_shouldMigrateLocks() public {
MockLocking newLockingContract = new MockLocking();

mentoToken.mint(alice, 100);
vm.prank(alice);
uint256 lockId = locking.lock(alice, bob, 65, 11, 0);

uint256[] memory ids = new uint256[](1);
ids[0] = lockId;

_incrementBlock(10 * weekInBlocks);

vm.prank(owner);
locking.startMigration(address(newLockingContract));

vm.prank(alice);
locking.migrate(ids);

assertEq(mentoToken.balanceOf(address(locking)), 60);
assertEq(mentoToken.balanceOf(address(newLockingContract)), 5);
assertEq(mentoToken.balanceOf(alice), 35);

vm.prank(alice);
locking.withdraw();
assertEq(mentoToken.balanceOf(address(locking)), 0);
assertEq(mentoToken.balanceOf(address(newLockingContract)), 5);
assertEq(mentoToken.balanceOf(alice), 95);
}

function test_startMigration_whenNotOwner_shouldRevert() public {
vm.expectRevert("Ownable: caller is not the owner");
locking.startMigration(address(1));
}

function test_setMinCliffPeriod_setMinSlope_whenNotOwner_shouldRevert() public {
vm.expectRevert("Ownable: caller is not the owner");
locking.setMinCliffPeriod(3);
Expand Down

0 comments on commit 220b5b8

Please sign in to comment.