Skip to content

Commit

Permalink
fix(StakeManager): initial MP should not increase when locking
Browse files Browse the repository at this point in the history
When an account calls `lock()` on the `StakeManager` it ultimately
called `_mintInitialMP()` which besides increasing the accounts
`currentMP`, it also increases the accounts `initialMP`.

This seems incorrect, as `initialMP` should only increase at first
stake.

Also, notice how inside `lock()` it always passes an `amount` of `0` to
to `_mintInitialMP()`, so it only ever uses one code path of that
function.

This commit adjust `lock()` such that it no longer uses
`_mintInitialMP()`. Instead it makes use of `_getMaxMPToMint` based on
the lock up time delta.

It also removes a test that claims that `initialMP` should increase on
`lock()` and instead adds a test an a CVL rule ensure `initialMP`
doesn't increase when locking.
  • Loading branch information
0x-r4bbit committed Jun 19, 2024
1 parent 3c4c463 commit cef950a
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 20 deletions.
20 changes: 20 additions & 0 deletions certora/specs/StakeManager.spec
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,26 @@ rule stakingGreaterLockupTimeMeansGreaterMPs {
satisfy to_mathint(multiplierPointsAfter1) > to_mathint(multiplierPointsAfter2);
}

rule increasingLockupTimeNeverIncreasesInitialMPs {

env e;
uint256 amount;
uint256 lockupTime1;
uint256 lockupTime2;
uint256 multiplierPointsAfter1;
uint256 multiplierPointsAfter2;

storage initalStorage = lastStorage;

stake(e, amount, lockupTime1);
multiplierPointsAfter1 = getAccountInitialMultiplierPoints(e.msg.sender);

lock(e, lockupTime2);
multiplierPointsAfter2 = getAccountInitialMultiplierPoints(e.msg.sender);

assert to_mathint(multiplierPointsAfter1) == to_mathint(multiplierPointsAfter2);
}

/**
@title when there is no migration - some functions must revert.
Other function should have non reverting cases
Expand Down
17 changes: 13 additions & 4 deletions contracts/StakeManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,18 @@ contract StakeManager is Ownable {
if (deltaTime < MIN_LOCKUP_PERIOD || deltaTime > MAX_LOCKUP_PERIOD) {
revert StakeManager__InvalidLockTime();
}
_mintInitialMP(account, _timeToIncrease, 0);
//update account storage

uint256 mpToMint = _getMaxMPToMint(
_getMPToMint(account.balance, _timeToIncrease),
account.balance,
account.initialMP,
account.currentMP
);

totalSupplyMP += mpToMint;
account.currentMP += mpToMint;
account.lockUntil = lockUntil;
account.lastMint = block.timestamp;
}

/**
Expand Down Expand Up @@ -433,9 +442,9 @@ contract StakeManager is Ownable {
);

//update storage
account.lastMint = processTime;
account.currentMP += increasedMP;
totalSupplyMP += increasedMP;
account.currentMP += increasedMP;
account.lastMint = processTime;
epoch.totalSupply += increasedMP;
}

Expand Down
22 changes: 6 additions & 16 deletions test/StakeManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -413,23 +413,13 @@ contract LockTest is StakeManagerTest {
userVault.lock(minLockup - 1);
}

function test_ShouldIncreaseInitialMP() public {
uint256 stakeAmount = 100;
uint256 lockTime = stakeManager.MAX_LOCKUP_PERIOD();
StakeVault userVault = _createStakingAccount(testUser, stakeAmount);
(, uint256 balance, uint256 initialMP, uint256 currentMP,,,) = stakeManager.accounts(address(userVault));
uint256 totalSupplyMPBefore = stakeManager.totalSupplyMP();

function test_InitialMPDoesNotIncreaseAfterLock() public {
uint256 minLockup = stakeManager.MIN_LOCKUP_PERIOD();
StakeVault userVault = _createStakingAccount(testUser, 1000, 0, 1000);
vm.startPrank(testUser);
userVault.lock(lockTime);

(, uint256 newBalance, uint256 newInitialMP, uint256 newCurrentMP,,,) =
stakeManager.accounts(address(userVault));
uint256 totalSupplyMPAfter = stakeManager.totalSupplyMP();
assertGt(totalSupplyMPAfter, totalSupplyMPBefore, "totalSupplyMP");
assertGt(newInitialMP, initialMP, "initialMP");
assertGt(newCurrentMP, currentMP, "currentMP");
assertEq(newBalance, balance, "balance");
userVault.lock(minLockup);
(, uint256 balance, uint256 initialMP,,,,) = stakeManager.accounts(address(userVault));
assertEq(initialMP, balance);
}
}

Expand Down

0 comments on commit cef950a

Please sign in to comment.