From a34ae066a382ad058ad58a70137fa8f3a8e7f2f3 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 3 Oct 2024 00:24:11 -0300 Subject: [PATCH] refactor(StakeManager): optimize stake and lock functionality by removing mintBonusMP function --- .gas-report | 36 ++++++++++---------- .gas-snapshot | 70 +++++++++++++++++++------------------- contracts/StakeManager.sol | 60 +++++++++++++++----------------- 3 files changed, 80 insertions(+), 86 deletions(-) diff --git a/.gas-report b/.gas-report index 3c8d934..cc34c99 100644 --- a/.gas-report +++ b/.gas-report @@ -1,21 +1,21 @@ | contracts/StakeManager.sol:StakeManager contract | | | | | | |--------------------------------------------------|-----------------|--------|--------|--------|---------| | Deployment Cost | Deployment Size | | | | | -| 2483377 | 13123 | | | | | +| 2495816 | 13179 | | | | | | Function Name | min | avg | median | max | # calls | | EPOCH_SIZE | 263 | 263 | 263 | 263 | 1498 | | MAX_BOOST | 264 | 264 | 264 | 264 | 637 | | MAX_LOCKUP_PERIOD | 383 | 383 | 383 | 383 | 4 | | MIN_LOCKUP_PERIOD | 264 | 264 | 264 | 264 | 12 | | YEAR | 263 | 263 | 263 | 263 | 637 | -| accounts | 1616 | 1616 | 1616 | 1616 | 144345 | +| accounts | 1616 | 1616 | 1616 | 1616 | 144285 | | calculateMPToMint | 740 | 740 | 740 | 740 | 1276 | | currentEpoch | 384 | 1050 | 384 | 2384 | 54 | -| epochEnd | 649 | 649 | 649 | 2649 | 23687 | +| epochEnd | 649 | 649 | 649 | 2649 | 23677 | | epochReward | 1381 | 2881 | 1381 | 5881 | 3 | | executeAccount(address) | 149300 | 149300 | 149300 | 149300 | 2 | -| executeAccount(address,uint256) | 26562 | 72242 | 74122 | 200087 | 141932 | -| executeEpoch() | 23480 | 120709 | 121865 | 900380 | 23576 | +| executeAccount(address,uint256) | 26562 | 72246 | 74122 | 200087 | 141872 | +| executeEpoch() | 23480 | 120708 | 121865 | 900380 | 23566 | | executeEpoch(uint256) | 23861 | 24497 | 23861 | 26090 | 7 | | expiredStakeStorage | 437 | 2346 | 2437 | 2437 | 22 | | isTrustedCodehash | 541 | 949 | 541 | 2541 | 680 | @@ -25,7 +25,7 @@ | migrationInitialize | 24624 | 24624 | 24624 | 24624 | 1 | | newEpoch | 441 | 441 | 441 | 441 | 5 | | owner | 2432 | 2432 | 2432 | 2432 | 13 | -| pendingMPToBeMinted | 363 | 363 | 363 | 363 | 46456 | +| pendingMPToBeMinted | 363 | 363 | 363 | 363 | 46436 | | pendingReward | 408 | 1442 | 2408 | 2408 | 29 | | previousManager | 275 | 275 | 275 | 275 | 13 | | setTrustedCodehash | 47960 | 47960 | 47960 | 47960 | 139 | @@ -35,7 +35,7 @@ | startTime | 306 | 306 | 306 | 306 | 21 | | totalSupply | 784 | 1965 | 2784 | 2784 | 22 | | totalSupplyBalance | 407 | 1807 | 2407 | 2407 | 20 | -| totalSupplyMP | 384 | 384 | 384 | 2384 | 46477 | +| totalSupplyMP | 384 | 384 | 384 | 2384 | 46457 | | unstake | 23841 | 23841 | 23841 | 23841 | 1 | @@ -46,9 +46,9 @@ | Function Name | min | avg | median | max | # calls | | acceptMigration | 35311 | 35311 | 35311 | 35311 | 2 | | leave | 35297 | 35297 | 35297 | 35297 | 1 | -| lock | 43285 | 95698 | 64379 | 204197 | 7 | +| lock | 43285 | 90487 | 61938 | 180284 | 7 | | owner | 362 | 362 | 362 | 362 | 679 | -| stake | 27265 | 282180 | 265700 | 351859 | 684 | +| stake | 27265 | 282115 | 265681 | 351644 | 684 | | stakedToken | 212 | 212 | 212 | 212 | 2 | | unstake | 40180 | 96354 | 78700 | 229598 | 11 | @@ -68,7 +68,7 @@ | Deployment Cost | Deployment Size | | | | | | 0 | 0 | | | | | | Function Name | min | avg | median | max | # calls | -| getExpiredMP | 2427 | 2427 | 2427 | 2427 | 23737 | +| getExpiredMP | 2427 | 2427 | 2427 | 2427 | 23727 | | transferOwnership | 28533 | 28533 | 28533 | 28533 | 1 | @@ -77,24 +77,24 @@ | Deployment Cost | Deployment Size | | | | | | 0 | 0 | | | | | | Function Name | min | avg | median | max | # calls | -| approve | 46175 | 46236 | 46199 | 46367 | 679 | -| balanceOf | 561 | 2107 | 2561 | 2561 | 30756 | +| approve | 46175 | 46239 | 46199 | 46367 | 679 | +| balanceOf | 561 | 2107 | 2561 | 2561 | 30746 | | script/Deploy.s.sol:Deploy contract | | | | | | |-------------------------------------|-----------------|---------|---------|---------|---------| | Deployment Cost | Deployment Size | | | | | -| 6136611 | 29620 | | | | | +| 6149062 | 29676 | | | | | | Function Name | min | avg | median | max | # calls | -| run | 5332750 | 5332750 | 5332750 | 5332750 | 66 | +| run | 5343965 | 5343965 | 5343965 | 5343965 | 66 | | script/DeployMigrationStakeManager.s.sol:DeployMigrationStakeManager contract | | | | | | |-------------------------------------------------------------------------------|-----------------|---------|---------|---------|---------| | Deployment Cost | Deployment Size | | | | | -| 3300146 | 16388 | | | | | +| 3312594 | 16444 | | | | | | Function Name | min | avg | median | max | # calls | -| run | 2319079 | 2319079 | 2319079 | 2319079 | 19 | +| run | 2330294 | 2330294 | 2330294 | 2330294 | 19 | | script/DeploymentConfig.s.sol:DeploymentConfig contract | | | | | | @@ -117,9 +117,9 @@ | test/script/DeployBroken.s.sol:DeployBroken contract | | | | | | |------------------------------------------------------|-----------------|---------|---------|---------|---------| | Deployment Cost | Deployment Size | | | | | -| 4821371 | 23418 | | | | | +| 4833800 | 23474 | | | | | | Function Name | min | avg | median | max | # calls | -| run | 4172572 | 4172572 | 4172572 | 4172572 | 1 | +| run | 4183787 | 4183787 | 4183787 | 4183787 | 1 | diff --git a/.gas-snapshot b/.gas-snapshot index dad3de3..d90857a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,41 +1,41 @@ CreateVaultTest:testDeployment() (gas: 9774) CreateVaultTest:test_createVault() (gas: 713999) ExecuteAccountTest:testDeployment() (gas: 28828) -ExecuteAccountTest:test_ExecuteAccountLimit() (gas: 1579172) -ExecuteAccountTest:test_ExecuteAccountMintMP() (gas: 5295647) -ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1787335) -ExecuteAccountTest:test_ShouldNotMintMoreThanCap() (gas: 321024135) +ExecuteAccountTest:test_ExecuteAccountLimit() (gas: 1579141) +ExecuteAccountTest:test_ExecuteAccountMintMP() (gas: 5295554) +ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1787120) +ExecuteAccountTest:test_ShouldNotMintMoreThanCap() (gas: 321023887) ExecuteEpochTest:testDeployment() (gas: 28829) ExecuteEpochTest:testNewDeployment() (gas: 30901) -ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterEpochEnd() (gas: 1367896) -ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsJumoMany() (gas: 1385583) -ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTime() (gas: 1630994) -ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTimeJumpMany() (gas: 1395298) -ExecuteEpochTest:test_ExecuteEpochExecuteEpochAfterEnd() (gas: 1928384) -ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochs() (gas: 2511226) -ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsJumoMany() (gas: 1479103) -ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTime() (gas: 2520962) -ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTimeJumpMany() (gas: 1488795) +ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterEpochEnd() (gas: 1367865) +ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsJumoMany() (gas: 1385552) +ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTime() (gas: 1630963) +ExecuteEpochTest:test_ExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTimeJumpMany() (gas: 1395267) +ExecuteEpochTest:test_ExecuteEpochExecuteEpochAfterEnd() (gas: 1928353) +ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochs() (gas: 2511195) +ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsJumoMany() (gas: 1479072) +ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTime() (gas: 2520931) +ExecuteEpochTest:test_ExecuteEpochExecuteEpochExecuteAccountAfterManyEpochsWithBrokenTimeJumpMany() (gas: 1488764) ExecuteEpochTest:test_ExecuteEpochNewEpoch() (gas: 1083687) ExecuteEpochTest:test_ExecuteEpochShouldIncreaseEpoch() (gas: 92344) ExecuteEpochTest:test_ExecuteEpochShouldIncreasePendingReward() (gas: 256338) ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochBeforeEnd() (gas: 39028) ExecuteEpochTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 149748) LeaveTest:testDeployment() (gas: 28806) -LeaveTest:test_RevertWhen_NoPendingMigration() (gas: 1329762) +LeaveTest:test_RevertWhen_NoPendingMigration() (gas: 1329731) LeaveTest:test_RevertWhen_SenderIsNotVault() (gas: 31995) LockTest:testDeployment() (gas: 28806) -LockTest:test_NewLockupPeriod() (gas: 1330751) -LockTest:test_RevertWhen_InvalidNewLockupPeriod() (gas: 1303059) -LockTest:test_RevertWhen_InvalidUpdateLockupPeriod() (gas: 1543826) +LockTest:test_NewLockupPeriod() (gas: 1328279) +LockTest:test_RevertWhen_InvalidNewLockupPeriod() (gas: 1303028) +LockTest:test_RevertWhen_InvalidUpdateLockupPeriod() (gas: 1543611) LockTest:test_RevertWhen_SenderIsNotVault() (gas: 31812) -LockTest:test_ShouldIncreaseBonusMP() (gas: 1313344) -LockTest:test_UpdateLockupPeriod() (gas: 1634292) +LockTest:test_ShouldIncreaseBonusMP() (gas: 1310872) +LockTest:test_UpdateLockupPeriod() (gas: 1579753) MigrateTest:testDeployment() (gas: 28806) -MigrateTest:test_RevertWhen_NoPendingMigration() (gas: 1293784) +MigrateTest:test_RevertWhen_NoPendingMigration() (gas: 1293753) MigrateTest:test_RevertWhen_SenderIsNotVault() (gas: 32007) MigrationInitializeTest:testDeployment() (gas: 28806) -MigrationInitializeTest:test_RevertWhen_MigrationPending() (gas: 5216837) +MigrationInitializeTest:test_RevertWhen_MigrationPending() (gas: 5241726) MigrationStakeManagerTest:testDeployment() (gas: 28806) MigrationStakeManagerTest:testNewDeployment() (gas: 30945) MigrationStakeManagerTest:test_ExecuteEpochShouldNotIncreaseEpochInMigration() (gas: 149713) @@ -44,24 +44,24 @@ SetStakeManagerTest:test_RevertWhen_InvalidStakeManagerAddress() (gas: 63105) SetStakeManagerTest:test_SetStakeManager() (gas: 41301) StakeManagerTest:testDeployment() (gas: 28578) StakeTest:testDeployment() (gas: 28784) -StakeTest:test_RevertWhen_InvalidLockupPeriod() (gas: 1082353) -StakeTest:test_RevertWhen_Restake() (gas: 1318545) -StakeTest:test_RevertWhen_RestakeWithLock() (gas: 1322559) +StakeTest:test_RevertWhen_InvalidLockupPeriod() (gas: 1078067) +StakeTest:test_RevertWhen_Restake() (gas: 1318488) +StakeTest:test_RevertWhen_RestakeWithLock() (gas: 1322318) StakeTest:test_RevertWhen_SenderIsNotVault() (gas: 32018) -StakeTest:test_RevertWhen_StakeIsTooLow() (gas: 819467) +StakeTest:test_RevertWhen_StakeIsTooLow() (gas: 817324) StakeTest:test_RevertWhen_StakeTokenTransferFails() (gas: 211363) -StakeTest:test_StakeWithLockBonusMP() (gas: 2359257) -StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 1314761) +StakeTest:test_StakeWithLockBonusMP() (gas: 2356570) +StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 1314730) StakedTokenTest:testStakeToken() (gas: 7616) UnstakeTest:testDeployment() (gas: 28828) -UnstakeTest:test_RevertWhen_AmountMoreThanBalance() (gas: 1299127) -UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1343877) +UnstakeTest:test_RevertWhen_AmountMoreThanBalance() (gas: 1299096) +UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1343662) UnstakeTest:test_RevertWhen_SenderIsNotVault() (gas: 31879) -UnstakeTest:test_UnstakeShouldBurnMultiplierPoints() (gas: 6450051) -UnstakeTest:test_UnstakeShouldReturnFund_NoLockUp() (gas: 1321289) -UnstakeTest:test_UnstakeShouldReturnFund_WithLockUp() (gas: 1438949) +UnstakeTest:test_UnstakeShouldBurnMultiplierPoints() (gas: 6450026) +UnstakeTest:test_UnstakeShouldReturnFund_NoLockUp() (gas: 1321258) +UnstakeTest:test_UnstakeShouldReturnFund_WithLockUp() (gas: 1438734) UserFlowsTest:testDeployment() (gas: 28806) -UserFlowsTest:test_PendingMPToBeMintedCannotBeGreaterThanTotalSupplyMP(uint8,uint128) (runs: 106, μ: 130995099, ~: 130391755) -UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1480058) -UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 2496473) +UserFlowsTest:test_PendingMPToBeMintedCannotBeGreaterThanTotalSupplyMP(uint8,uint128) (runs: 106, μ: 130945931, ~: 130391929) +UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1479843) +UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 2496411) VaultFactoryTest:testDeployment() (gas: 9774) \ No newline at end of file diff --git a/contracts/StakeManager.sol b/contracts/StakeManager.sol index 5cce273..e184553 100644 --- a/contracts/StakeManager.sol +++ b/contracts/StakeManager.sol @@ -155,8 +155,7 @@ contract StakeManager is TrustedCodehashAccess { */ function stake(uint256 _amount, uint256 _secondsToLock) external onlyTrustedCodehash noPendingMigration { finalizeEpoch(newEpoch()); - Account storage account = accounts[msg.sender]; - if (account.balance > 0 || account.lockUntil != 0) { + if (accounts[msg.sender].balance > 0) { revert StakeManager__AlreadyStaked(); } if (_secondsToLock != 0 && (_secondsToLock < MIN_LOCKUP_PERIOD || _secondsToLock > MAX_LOCKUP_PERIOD)) { @@ -173,16 +172,26 @@ contract StakeManager is TrustedCodehashAccess { uint256 epochAmountToReachMpLimit = (maxMpToMint) / mpPerEpoch; uint256 mpLimitEpoch = currentEpoch + epochAmountToReachMpLimit; uint256 lastEpochAmountToMint = ((mpPerEpoch * (epochAmountToReachMpLimit + 1)) - maxMpToMint); + uint256 bonusMP = _amount; + if (_secondsToLock > 0) { + //bonus for lock time + bonusMP += _getMPToMint(_amount, _secondsToLock); + } // account initialization - account.lockUntil = block.timestamp + _secondsToLock; - account.epoch = currentEpoch; //starts in current epoch - account.rewardAddress = StakeVault(msg.sender).owner(); - _mintBonusMP(account, _secondsToLock, _amount); //TODO: remove this function competely - account.balance = _amount; - account.mpLimitEpoch = mpLimitEpoch; + accounts[msg.sender] = Account({ + rewardAddress: StakeVault(msg.sender).owner(), + balance: _amount, + bonusMP: bonusMP, + totalMP: bonusMP, + lastMint: block.timestamp, + lockUntil: block.timestamp + _secondsToLock, + epoch: currentEpoch, + mpLimitEpoch: mpLimitEpoch + }); //update global storage + totalSupplyMP += bonusMP; totalSupplyBalance += _amount; currentEpochTotalExpiredMP += currentEpochExpiredMP; totalMPPerEpoch += mpPerEpoch; @@ -245,18 +254,27 @@ contract StakeManager is TrustedCodehashAccess { uint256 lockUntil = account.lockUntil; uint256 deltaTime; if (lockUntil < block.timestamp) { + //if unlocked, increase from now lockUntil = block.timestamp + _secondsToIncreaseLock; deltaTime = _secondsToIncreaseLock; } else { + //if locked, increase from lock until lockUntil += _secondsToIncreaseLock; deltaTime = lockUntil - block.timestamp; } + //checks if the lock time is in range if (deltaTime < MIN_LOCKUP_PERIOD || deltaTime > MAX_LOCKUP_PERIOD) { revert StakeManager__InvalidLockTime(); } - _mintBonusMP(account, _secondsToIncreaseLock, 0); + //mints bonus multiplier points for seconds increased + uint256 bonusMP = _getMPToMint(account.balance, _secondsToIncreaseLock); + //update account storage account.lockUntil = lockUntil; + account.bonusMP += bonusMP; + account.totalMP += bonusMP; + //update global storage + totalSupplyMP += bonusMP; } /** @@ -451,30 +469,6 @@ contract StakeManager is TrustedCodehashAccess { } } - /** - * @notice Mint bonus multiplier points for given staking amount and time - * @dev if amount is greater 0, it increases difference of amount for current remaining lock time - * @dev if increased lock time, increases difference of total new balance for increased lock time - * @param account Account to mint multiplier points - * @param increasedLockTime increased lock time - * @param amount amount to stake - */ - function _mintBonusMP(Account storage account, uint256 increasedLockTime, uint256 amount) private { - uint256 mpToMint; - if (amount > 0) { - mpToMint += amount; //initial multiplier points - } - if (increasedLockTime > 0) { - //bonus for increased lock time - mpToMint += _getMPToMint(account.balance + amount, increasedLockTime); - } - //update storage - totalSupplyMP += mpToMint; - account.bonusMP += mpToMint; - account.totalMP += mpToMint; - account.lastMint = block.timestamp; - } - /** * @notice Mint multiplier points for given account and epoch * @param account Account earning multiplier points