refactor(StakeManager): Remove debug code and optimize epoch finalization

This is a work in progress. This commit will be split in many other PRs to address each one of the changes.

Remove debug code and optimize the epoch finalization process in the StakeManager contract. The debug code related to the `nextEpoch` variable has been removed. The `finalizeEpoch` modifier has been optimized to process epochs more efficiently . The `executeEpoch` function has been updated to execute epochs up to a specified limit. The `_processAccount` function has been modified to use a `while` loop instead of a `for` loop to fix a problem with the code.
This commit is contained in:
Ricardo Guilherme Schmidt 2024-09-11 06:35:33 -03:00
parent d3b4e95ab3
commit fcb620743b
4 changed files with 236 additions and 130 deletions

View File

@ -0,0 +1,122 @@
| contracts/StakeManager.sol:StakeManager contract | | | | | |
|--------------------------------------------------|-----------------|--------|--------|--------|---------|
| Deployment Cost | Deployment Size | | | | |
| 2393736 | 12447 | | | | |
| Function Name | min | avg | median | max | # calls |
| EPOCH_SIZE | 307 | 307 | 307 | 307 | 815 |
| MAX_BOOST | 285 | 285 | 285 | 285 | 1 |
| MAX_LOCKUP_PERIOD | 361 | 361 | 361 | 361 | 4 |
| MIN_LOCKUP_PERIOD | 287 | 287 | 287 | 287 | 10 |
| YEAR | 263 | 263 | 263 | 263 | 1 |
| accounts | 1597 | 1597 | 1597 | 1597 | 23752 |
| calculateMPToMint | 740 | 740 | 740 | 740 | 2 |
| currentEpoch | 406 | 1739 | 2406 | 2406 | 27 |
| epochEnd | 627 | 639 | 627 | 4627 | 633 |
| epochReward | 1403 | 2903 | 1403 | 5903 | 3 |
| executeAccount | 31046 | 95466 | 95222 | 246424 | 21314 |
| executeEpoch | 23458 | 168653 | 163967 | 203767 | 573 |
| isVault | 540 | 569 | 540 | 2540 | 18810 |
| lock | 23862 | 23862 | 23862 | 23862 | 1 |
| migrateTo | 23891 | 23897 | 23897 | 23903 | 2 |
| migration | 439 | 1439 | 1439 | 2439 | 4 |
| migrationInitialize | 24602 | 24602 | 24602 | 24602 | 1 |
| owner | 2432 | 2432 | 2432 | 2432 | 13 |
| pendingMPToBeMinted | 364 | 364 | 364 | 364 | 512 |
| pendingReward | 364 | 1398 | 2364 | 2364 | 29 |
| previousManager | 275 | 275 | 275 | 275 | 13 |
| setVault | 46239 | 46239 | 46239 | 46239 | 278 |
| stake | 23983 | 23983 | 23983 | 23983 | 1 |
| stakeRewardEstimate | 436 | 2269 | 2436 | 2436 | 12 |
| stakedToken | 295 | 295 | 295 | 295 | 18826 |
| startMigration | 108208 | 108216 | 108220 | 108220 | 3 |
| totalSupply | 740 | 1921 | 2740 | 2740 | 22 |
| totalSupplyBalance | 362 | 1762 | 2362 | 2362 | 20 |
| totalSupplyMP | 362 | 414 | 362 | 2362 | 533 |
| unstake | 23819 | 23819 | 23819 | 23819 | 1 |
| contracts/StakeManager.sol:StakeRewardEstimate contract | | | | | |
|---------------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| getExpiredMP | 2427 | 2427 | 2427 | 2427 | 635 |
| transferOwnership | 28533 | 28533 | 28533 | 28533 | 1 |
| contracts/StakeVault.sol:StakeVault contract | | | | | |
|----------------------------------------------|-----------------|--------|--------|--------|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| acceptMigration | 35280 | 35280 | 35280 | 35280 | 2 |
| leave | 35266 | 35266 | 35266 | 35266 | 1 |
| lock | 45740 | 135281 | 87993 | 246408 | 7 |
| owner | 362 | 362 | 362 | 362 | 18809 |
| stake | 27265 | 268920 | 267737 | 353784 | 18814 |
| stakedToken | 212 | 212 | 212 | 212 | 2 |
| unstake | 42312 | 103845 | 80560 | 268260 | 11 |
| contracts/VaultFactory.sol:VaultFactory contract | | | | | |
|--------------------------------------------------|-----------------|--------|--------|--------|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| createVault | 696553 | 696553 | 696553 | 696553 | 18813 |
| setStakeManager | 23710 | 26669 | 26076 | 30222 | 3 |
| stakeManager | 368 | 1868 | 2368 | 2368 | 4 |
| lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol:ERC20 contract | | | | | |
|---------------------------------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| approve | 46175 | 46198 | 46199 | 46211 | 18809 |
| balanceOf | 561 | 602 | 561 | 2561 | 44154 |
| script/Deploy.s.sol:Deploy contract | | | | | |
|-------------------------------------|-----------------|---------|---------|---------|---------|
| Deployment Cost | Deployment Size | | | | |
| 5994574 | 28944 | | | | |
| Function Name | min | avg | median | max | # calls |
| run | 5250241 | 5250241 | 5250241 | 5250241 | 54 |
| script/DeployMigrationStakeManager.s.sol:DeployMigrationStakeManager contract | | | | | |
|-------------------------------------------------------------------------------|-----------------|---------|---------|---------|---------|
| Deployment Cost | Deployment Size | | | | |
| 3158144 | 15712 | | | | |
| Function Name | min | avg | median | max | # calls |
| run | 2235911 | 2235911 | 2235911 | 2235911 | 9 |
| script/DeploymentConfig.s.sol:DeploymentConfig contract | | | | | |
|---------------------------------------------------------|-----------------|-----|--------|-----|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| activeNetworkConfig | 455 | 455 | 455 | 455 | 108 |
| test/mocks/BrokenERC20.s.sol:BrokenERC20 contract | | | | | |
|---------------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost | Deployment Size | | | | |
| 0 | 0 | | | | |
| Function Name | min | avg | median | max | # calls |
| approve | 46175 | 46175 | 46175 | 46175 | 1 |
| balanceOf | 561 | 1227 | 561 | 2561 | 3 |
| test/script/DeployBroken.s.sol:DeployBroken contract | | | | | |
|------------------------------------------------------|-----------------|---------|---------|---------|---------|
| Deployment Cost | Deployment Size | | | | |
| 4679350 | 22742 | | | | |
| Function Name | min | avg | median | max | # calls |
| run | 4090063 | 4090063 | 4090063 | 4090063 | 1 |

View File

@ -1,55 +1,55 @@
CreateVaultTest:testDeployment() (gas: 9774)
CreateVaultTest:test_createVault() (gas: 692936)
ExecuteAccountTest:testDeployment() (gas: 28720)
ExecuteAccountTest:test_ExecuteAccountMintMP() (gas: 3856635)
ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1154886)
ExecuteAccountTest:test_ShouldNotMintMoreThanCap() (gas: 110522900)
ExecuteEpochTest:testDeployment() (gas: 28720)
ExecuteEpochTest:testNewDeployment() (gas: 30815)
ExecuteEpochTest:test_ExecuteEpochShouldIncreaseEpoch() (gas: 94810)
ExecuteEpochTest:test_ExecuteEpochShouldIncreasePendingReward() (gas: 253041)
ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochBeforeEnd() (gas: 17972)
ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 105698)
LeaveTest:testDeployment() (gas: 28720)
LeaveTest:test_RevertWhen_NoPendingMigration() (gas: 1154760)
LeaveTest:test_RevertWhen_SenderIsNotVault() (gas: 10750)
LockTest:testDeployment() (gas: 28720)
LockTest:test_NewLockupPeriod() (gas: 1143587)
LockTest:test_RevertWhen_InvalidNewLockupPeriod() (gas: 1135204)
LockTest:test_RevertWhen_InvalidUpdateLockupPeriod() (gas: 1231813)
LockTest:test_RevertWhen_SenderIsNotVault() (gas: 10630)
LockTest:test_ShouldIncreaseBonusMP() (gas: 1123687)
LockTest:test_UpdateLockupPeriod() (gas: 1281200)
MigrateTest:testDeployment() (gas: 28720)
MigrateTest:test_RevertWhen_NoPendingMigration() (gas: 1152399)
MigrateTest:test_RevertWhen_SenderIsNotVault() (gas: 10750)
MigrationInitializeTest:testDeployment() (gas: 28720)
MigrationInitializeTest:test_RevertWhen_MigrationPending() (gas: 5716968)
MigrationStakeManagerTest:testDeployment() (gas: 28720)
MigrationStakeManagerTest:testNewDeployment() (gas: 30859)
MigrationStakeManagerTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 105686)
CreateVaultTest:test_createVault() (gas: 692958)
ExecuteAccountTest:testDeployment() (gas: 28696)
ExecuteAccountTest:test_ExecuteAccountMintMP() (gas: 3856428)
ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1154266)
ExecuteAccountTest:test_ShouldNotMintMoreThanCap() (gas: 110674231)
ExecuteEpochTest:testDeployment() (gas: 28696)
ExecuteEpochTest:testNewDeployment() (gas: 30791)
ExecuteEpochTest:test_ExecuteEpochShouldIncreaseEpoch() (gas: 94294)
ExecuteEpochTest:test_ExecuteEpochShouldIncreasePendingReward() (gas: 252628)
ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochBeforeEnd() (gas: 18007)
ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 105891)
LeaveTest:testDeployment() (gas: 28696)
LeaveTest:test_RevertWhen_NoPendingMigration() (gas: 1154811)
LeaveTest:test_RevertWhen_SenderIsNotVault() (gas: 10772)
LockTest:testDeployment() (gas: 28696)
LockTest:test_NewLockupPeriod() (gas: 1143421)
LockTest:test_RevertWhen_InvalidNewLockupPeriod() (gas: 1135379)
LockTest:test_RevertWhen_InvalidUpdateLockupPeriod() (gas: 1330763)
LockTest:test_RevertWhen_SenderIsNotVault() (gas: 10652)
LockTest:test_ShouldIncreaseBonusMP() (gas: 1123475)
LockTest:test_UpdateLockupPeriod() (gas: 1488967)
MigrateTest:testDeployment() (gas: 28696)
MigrateTest:test_RevertWhen_NoPendingMigration() (gas: 1152428)
MigrateTest:test_RevertWhen_SenderIsNotVault() (gas: 10772)
MigrationInitializeTest:testDeployment() (gas: 28696)
MigrationInitializeTest:test_RevertWhen_MigrationPending() (gas: 4502656)
MigrationStakeManagerTest:testDeployment() (gas: 28696)
MigrationStakeManagerTest:testNewDeployment() (gas: 30835)
MigrationStakeManagerTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 105879)
SetStakeManagerTest:testDeployment() (gas: 9774)
SetStakeManagerTest:test_RevertWhen_InvalidStakeManagerAddress() (gas: 20481)
SetStakeManagerTest:test_SetStakeManager() (gas: 19869)
StakeManagerTest:testDeployment() (gas: 28492)
StakeTest:testDeployment() (gas: 28720)
StakeTest:test_RevertWhen_InvalidLockupPeriod() (gas: 892125)
StakeTest:test_RevertWhen_Restake() (gas: 1158072)
StakeTest:test_RevertWhen_RestakeWithLock() (gas: 1160132)
StakeManagerTest:testDeployment() (gas: 28468)
StakeTest:testDeployment() (gas: 28696)
StakeTest:test_RevertWhen_InvalidLockupPeriod() (gas: 888153)
StakeTest:test_RevertWhen_Restake() (gas: 1158199)
StakeTest:test_RevertWhen_RestakeWithLock() (gas: 1159483)
StakeTest:test_RevertWhen_SenderIsNotVault() (gas: 10651)
StakeTest:test_RevertWhen_StakeIsTooLow() (gas: 745253)
StakeTest:test_RevertWhen_StakeIsTooLow() (gas: 743278)
StakeTest:test_RevertWhen_StakeTokenTransferFails() (gas: 175040)
StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 1029215)
StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 1029251)
StakedTokenTest:testStakeToken() (gas: 7616)
UnstakeTest:testDeployment() (gas: 28742)
UnstakeTest:test_RevertWhen_AmountMoreThanBalance() (gas: 1133014)
UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1158758)
UnstakeTest:testDeployment() (gas: 28718)
UnstakeTest:test_RevertWhen_AmountMoreThanBalance() (gas: 1133170)
UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1158287)
UnstakeTest:test_RevertWhen_SenderIsNotVault() (gas: 10653)
UnstakeTest:test_UnstakeShouldBurnMultiplierPoints() (gas: 5497531)
UnstakeTest:test_UnstakeShouldReturnFund_NoLockUp() (gas: 1026695)
UnstakeTest:test_UnstakeShouldReturnFund_WithLockUp() (gas: 1115820)
UserFlowsTest:testDeployment() (gas: 28720)
UserFlowsTest:test_PendingMPToBeMintedCannotBeGreaterThanTotalSupplyMP(uint8) (runs: 1001, μ: 69673434, ~: 32945058)
UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1116708)
UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 1951147)
UnstakeTest:test_UnstakeShouldBurnMultiplierPoints() (gas: 5480115)
UnstakeTest:test_UnstakeShouldReturnFund_NoLockUp() (gas: 1026754)
UnstakeTest:test_UnstakeShouldReturnFund_WithLockUp() (gas: 1174411)
UserFlowsTest:testDeployment() (gas: 28696)
UserFlowsTest:test_PendingMPToBeMintedCannotBeGreaterThanTotalSupplyMP(uint8) (runs: 1001, μ: 70845652, ~: 30154363)
UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1175240)
UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 1951242)
VaultFactoryTest:testDeployment() (gas: 9774)

View File

@ -60,7 +60,6 @@ contract StakeManager is Ownable {
uint256 epochReward;
uint256 totalSupply;
uint256 estimatedMP;
uint256 nextEpoch; //DEBUG: should be removed.
}
uint256 public constant EPOCH_SIZE = 1 weeks;
@ -137,48 +136,21 @@ contract StakeManager is Ownable {
_;
}
function _finalizeEpoch() internal returns(bool finalized) {
Epoch storage epoch = epochs[currentEpoch];
uint256 epochEnd = epoch.startTime + EPOCH_SIZE;
finalized = block.timestamp >= epochEnd;
if(finalized) {
console.log("##### FINALIZE EPOCH", currentEpoch);
uint256 expiredMP = stakeRewardEstimate.getExpiredMP(currentEpoch);
if (expiredMP > 0) {
totalMPPerEpoch -= expiredMP;
stakeRewardEstimate.deleteExpiredMP(currentEpoch);
}
epoch.estimatedMP = totalMPPerEpoch - currentEpochTotalExpiredMP;
delete currentEpochTotalExpiredMP;
pendingMPToBeMinted += epoch.estimatedMP;
//finalize current epoch
epoch.epochReward = epochReward();
epoch.totalSupply = totalSupply();
pendingReward += epoch.epochReward;
//create new epoch
currentEpoch++;
console.log("##### NEW EPOCH", currentEpoch);
epochs[currentEpoch].startTime = epochEnd;
epochs[currentEpoch].nextEpoch = currentEpoch+1; //DEBUG: should be removed
}
}
/**
* @notice Process epoch if it has ended
*/
modifier finalizeEpoch() {
//during migration the epoch should not be updated
if (address(migration) == address(0)) {
do {} while(_finalizeEpoch());
while (_finalizeEpoch()) {
continue;
}
}
_;
}
constructor(address _stakedToken, address _previousManager) {
epochs[0].startTime = block.timestamp;
epochs[0].nextEpoch = 1; //DEBUG: should be removed
previousManager = StakeManager(_previousManager);
stakedToken = ERC20(_stakedToken);
if (address(previousManager) != address(0)) {
@ -323,15 +295,24 @@ contract StakeManager is Ownable {
account.totalMP += bonusMP;
//update global storage
totalSupplyMP += bonusMP;
//account.lastMint = block.timestamp;
}
/**
* @notice Release rewards for current epoch and increase epoch.
* @dev only executes the prerequisite modifier finalizeEpoch
*/
function executeEpoch() external noPendingMigration finalizeEpoch {
return; //see modifier finalizeEpoch
function executeEpoch() external noPendingMigration {
while (_finalizeEpoch()) {
continue;
}
}
function executeEpoch(uint256 _limitEpoch) public noPendingMigration {
while (currentEpoch < _limitEpoch) {
if (!_finalizeEpoch()) {
break;
}
}
}
/**
@ -348,7 +329,6 @@ contract StakeManager is Ownable {
finalizeEpoch
{
_processAccount(accounts[_vault], _limitEpoch);
}
/**
@ -483,18 +463,17 @@ contract StakeManager is Ownable {
uint256 userReward;
uint256 userEpoch = account.epoch;
uint256 mpDifference = account.totalMP;
for (Epoch storage iEpoch = epochs[userEpoch]; userEpoch < _limitEpoch; userEpoch++) {
while (userEpoch < _limitEpoch) {
Epoch storage iEpoch = epochs[userEpoch];
//mint multiplier points to that epoch
console.log("userEpoch", userEpoch);
console.log("iEpoch.nextEpoch", iEpoch.nextEpoch);
_mintMP(account, iEpoch.startTime + EPOCH_SIZE, iEpoch);
uint256 userSupply = account.balance + account.totalMP;
console.log("iEpoch.nextEpoch (2)", iEpoch.nextEpoch);
uint256 userEpochReward = Math.mulDiv(userSupply, iEpoch.epochReward, iEpoch.totalSupply);
userReward += userEpochReward;
iEpoch.epochReward -= userEpochReward;
iEpoch.totalSupply -= userSupply;
//TODO: remove epoch when iEpoch.totalSupply reaches zero
userEpoch++;
}
account.epoch = userEpoch;
if (userReward > 0) {
@ -507,6 +486,32 @@ contract StakeManager is Ownable {
}
}
function _finalizeEpoch() internal returns (bool finalized) {
Epoch storage epoch = epochs[currentEpoch];
uint256 thisEpochEnd = epoch.startTime + EPOCH_SIZE;
if (block.timestamp < thisEpochEnd) {
return false;
}
uint256 expiredMP = stakeRewardEstimate.getExpiredMP(currentEpoch);
if (expiredMP > 0) {
totalMPPerEpoch -= expiredMP;
stakeRewardEstimate.deleteExpiredMP(currentEpoch);
}
epoch.estimatedMP = totalMPPerEpoch - currentEpochTotalExpiredMP;
delete currentEpochTotalExpiredMP;
pendingMPToBeMinted += epoch.estimatedMP;
//finalize current epoch
epoch.epochReward = epochReward();
epoch.totalSupply = totalSupply();
pendingReward += epoch.epochReward;
currentEpoch++;
epochs[currentEpoch].startTime = thisEpochEnd;
return true;
}
/**
* @notice Mint multiplier points for given account and epoch
* @param account Account earning multiplier points

View File

@ -288,7 +288,6 @@ contract LockTest is StakeManagerTest {
uint256 minLockup = stakeManager.MIN_LOCKUP_PERIOD();
//start stake with minimum time allowed
uint256 i = 0;
console.log("call", i++);
StakeVault userVault = _createStakingAccount(testUser, 1000, minLockup, 1000);
(, uint256 balance, uint256 bonusMP, uint256 totalMP,, uint256 currentLockUntil,,) =
stakeManager.accounts(address(userVault));
@ -301,7 +300,6 @@ contract LockTest is StakeManagerTest {
//lock again with new minimum lockup period
//as account still locked, it should increase this lock from the previous lock until.
vm.startPrank(testUser);
console.log("call", i++);
userVault.lock(minLockup);
expectedLockUntil = currentLockUntil + minLockup;
//stakeManager.executeAccount(address(userVault), stakeManager.currentEpoch());
@ -311,52 +309,33 @@ contract LockTest is StakeManagerTest {
//fast forward to exact time when account is unlocked
vm.warp(currentLockUntil);
//locks again, but this time account is unlocked. next lock until is increased from block.timestamp.
console.log("call", i++);
//locks again, but this time account is unlocked.
userVault.lock(minLockup);
//next lock until is increased from block.timestamp.
expectedLockUntil = block.timestamp + minLockup;
//As its the exact time the account gets unlocked, it would be the same as increasing from last lock period
assertEq(currentLockUntil + minLockup, expectedLockUntil, "sanity check did not match");
(, balance, bonusMP, totalMP,, currentLockUntil,,) = stakeManager.accounts(address(userVault));
assertEq(currentLockUntil, expectedLockUntil, "Lock until did not match when increased lock time while unlocked (exact time of unlock)");
assertEq(
currentLockUntil,
expectedLockUntil,
"Lock until did not match when increased lock time while unlocked (exact time of unlock)"
);
console.log("call", i++);
vm.warp(block.timestamp + minLockup);
//fast forward to after the time when account is unlocked
vm.warp(currentLockUntil + 1);
//locks again, but this time account is after the lock until time
userVault.lock(minLockup);
}
function test_ExecAcc() public {
uint256 minLockup = stakeManager.MIN_LOCKUP_PERIOD();
//start stake with minimum time allowed
uint256 i = 0;
console.log("call", i++);
StakeVault userVault = _createStakingAccount(testUser, 1000, minLockup, 1000);
(, uint256 balance, uint256 bonusMP, uint256 totalMP,, uint256 currentLockUntil,,) =
stakeManager.accounts(address(userVault));
vm.warp(block.timestamp + stakeManager.MIN_LOCKUP_PERIOD());
console.log("call", i++);
stakeManager.executeAccount(address(userVault), stakeManager.currentEpoch());
//next lock should be incrased from block.timestamp
expectedLockUntil = block.timestamp + minLockup;
//just assuring the test is not broken, the next lock should be bigger than the last lock + minLockup
assertLt(currentLockUntil + minLockup, expectedLockUntil, "sanity check 2 did not match");
(, balance, bonusMP, totalMP,, currentLockUntil,,) = stakeManager.accounts(address(userVault));
vm.warp(currentLockUntil);
console.log("call", i++);
stakeManager.executeAccount(address(userVault), stakeManager.currentEpoch());
(, balance, bonusMP, totalMP,, currentLockUntil,,) = stakeManager.accounts(address(userVault));
vm.warp(block.timestamp + minLockup);
console.log("call", i++);
stakeManager.executeAccount(address(userVault), stakeManager.currentEpoch());
assertEq(
currentLockUntil,
expectedLockUntil,
"Lock until did not match when increased lock time while unlocked (after unlock time)"
);
}
function test_RevertWhen_InvalidUpdateLockupPeriod() public {