From 1547d84ea913f3144e41b18c7865763b44545137 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Sat, 24 Feb 2024 16:41:52 -0300 Subject: [PATCH] fix(StakeManager): use mul by PRECISION and div back later to avoid precision loss in int divisions --- .gas-report | 57 ++++++++++++++++++++------------------ .gas-snapshot | 32 +++++++++++---------- contracts/StakeManager.sol | 14 ++++++---- 3 files changed, 55 insertions(+), 48 deletions(-) diff --git a/.gas-report b/.gas-report index 3834445..efc9352 100644 --- a/.gas-report +++ b/.gas-report @@ -1,26 +1,29 @@ | contracts/StakeManager.sol:StakeManager contract | | | | | | |--------------------------------------------------|-----------------|--------|--------|--------|---------| | Deployment Cost | Deployment Size | | | | | -| 2011229 | 10261 | | | | | +| 2038857 | 10399 | | | | | | Function Name | min | avg | median | max | # calls | +| EPOCH_SIZE | 285 | 285 | 285 | 285 | 9 | | MAX_LOCKUP_PERIOD | 405 | 405 | 405 | 405 | 2 | | MIN_LOCKUP_PERIOD | 264 | 264 | 264 | 264 | 3 | -| accounts | 1406 | 1406 | 1406 | 1406 | 2 | -| currentEpoch | 341 | 2118 | 2341 | 2341 | 9 | -| executeAccount | 1311 | 1311 | 1311 | 1311 | 1 | -| isVault | 517 | 2335 | 2517 | 2517 | 11 | +| accounts | 1406 | 1406 | 1406 | 1406 | 22 | +| currentEpoch | 341 | 1571 | 2341 | 2341 | 13 | +| epochEnd | 627 | 627 | 627 | 627 | 56 | +| executeAccount | 1311 | 54676 | 59362 | 105262 | 63 | +| executeEpoch | 87833 | 95166 | 87833 | 109833 | 3 | +| isVault | 517 | 2117 | 2517 | 2517 | 15 | | lock | 2614 | 2614 | 2614 | 2614 | 1 | | migrateTo | 1041 | 1713 | 1041 | 2721 | 5 | | oldManager | 240 | 240 | 240 | 240 | 8 | -| owner | 2341 | 2341 | 2341 | 2341 | 8 | -| pendingReward | 2386 | 2386 | 2386 | 2386 | 8 | -| setVault | 22606 | 22606 | 22606 | 22606 | 10 | -| stake | 2638 | 128658 | 188409 | 189093 | 13 | -| stakedToken | 260 | 260 | 260 | 260 | 22 | -| totalSupply | 561 | 561 | 561 | 561 | 8 | -| totalSupplyBalance | 362 | 1592 | 2362 | 2362 | 13 | -| totalSupplyMP | 384 | 1984 | 2384 | 2384 | 10 | -| unstake | 1730 | 18924 | 8964 | 107445 | 8 | +| owner | 2364 | 2364 | 2364 | 2364 | 8 | +| pendingReward | 386 | 1243 | 386 | 2386 | 21 | +| setVault | 22606 | 22606 | 22606 | 22606 | 12 | +| stake | 2638 | 136886 | 188409 | 189250 | 17 | +| stakedToken | 260 | 260 | 260 | 260 | 26 | +| totalSupply | 561 | 561 | 561 | 561 | 17 | +| totalSupplyBalance | 385 | 1615 | 2385 | 2385 | 13 | +| totalSupplyMP | 384 | 1614 | 2384 | 2384 | 13 | +| unstake | 1730 | 20389 | 8592 | 128444 | 9 | | contracts/StakeVault.sol:StakeVault contract | | | | | | @@ -30,10 +33,10 @@ | Function Name | min | avg | median | max | # calls | | acceptMigration | 1726 | 1726 | 1726 | 1726 | 2 | | leave | 1712 | 1712 | 1712 | 1712 | 1 | -| owner | 362 | 362 | 362 | 362 | 10 | -| stake | 3433 | 156693 | 219184 | 219868 | 13 | +| owner | 362 | 362 | 362 | 362 | 14 | +| stake | 3433 | 165565 | 219184 | 220025 | 17 | | stakedToken | 212 | 212 | 212 | 212 | 2 | -| unstake | 2588 | 24583 | 14285 | 111766 | 7 | +| unstake | 2588 | 28550 | 14913 | 132765 | 8 | | contracts/VaultFactory.sol:VaultFactory contract | | | | | | @@ -41,7 +44,7 @@ | Deployment Cost | Deployment Size | | | | | | 1043406 | 5305 | | | | | | Function Name | min | avg | median | max | # calls | -| createVault | 670954 | 674489 | 675454 | 675454 | 14 | +| createVault | 670954 | 674204 | 675454 | 675454 | 18 | | setStakeManager | 2518 | 5317 | 4644 | 8790 | 3 | | stakeManager | 368 | 1868 | 2368 | 2368 | 4 | @@ -51,18 +54,18 @@ | Deployment Cost | Deployment Size | | | | | | 649818 | 3562 | | | | | | Function Name | min | avg | median | max | # calls | -| approve | 24603 | 24603 | 24603 | 24603 | 11 | -| balanceOf | 561 | 1132 | 561 | 2561 | 42 | -| transfer | 3034 | 3034 | 3034 | 3034 | 5 | -| transferFrom | 27530 | 27530 | 27530 | 27530 | 12 | +| approve | 24603 | 24603 | 24603 | 24603 | 15 | +| balanceOf | 561 | 819 | 561 | 2561 | 139 | +| transfer | 3034 | 8340 | 3034 | 22934 | 15 | +| transferFrom | 27530 | 27530 | 27530 | 27530 | 16 | | script/Deploy.s.sol:Deploy contract | | | | | | |-------------------------------------|-----------------|---------|---------|---------|---------| | Deployment Cost | Deployment Size | | | | | -| 5095726 | 26758 | | | | | +| 5123362 | 26896 | | | | | | Function Name | min | avg | median | max | # calls | -| run | 4790797 | 4790797 | 4790797 | 4790797 | 30 | +| run | 4818454 | 4818454 | 4818454 | 4818454 | 32 | | script/DeploymentConfig.s.sol:DeploymentConfig contract | | | | | | @@ -70,7 +73,7 @@ | Deployment Cost | Deployment Size | | | | | | 1634091 | 8548 | | | | | | Function Name | min | avg | median | max | # calls | -| activeNetworkConfig | 455 | 455 | 455 | 455 | 60 | +| activeNetworkConfig | 455 | 455 | 455 | 455 | 64 | | test/mocks/BrokenERC20.s.sol:BrokenERC20 contract | | | | | | @@ -86,9 +89,9 @@ | test/script/DeployBroken.s.sol:DeployBroken contract | | | | | | |------------------------------------------------------|-----------------|---------|---------|---------|---------| | Deployment Cost | Deployment Size | | | | | -| 3868460 | 20556 | | | | | +| 3896102 | 20694 | | | | | | Function Name | min | avg | median | max | # calls | -| run | 3630620 | 3630620 | 3630620 | 3630620 | 1 | +| run | 3658277 | 3658277 | 3658277 | 3658277 | 1 | diff --git a/.gas-snapshot b/.gas-snapshot index 4d2a4db..d8f62f9 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,31 +1,33 @@ CreateVaultTest:testDeployment() (gas: 9774) CreateVaultTest:test_createVault() (gas: 692923) -ExecuteAccountTest:testDeployment() (gas: 26335) -ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1051620) -LeaveTest:testDeployment() (gas: 26335) +ExecuteAccountTest:testDeployment() (gas: 26381) +ExecuteAccountTest:test_ExecuteAccountMintMP() (gas: 3638137) +ExecuteAccountTest:test_RevertWhen_InvalidLimitEpoch() (gas: 1051753) +LeaveTest:testDeployment() (gas: 26381) LeaveTest:test_RevertWhen_NoPendingMigration() (gas: 1052239) LeaveTest:test_RevertWhen_SenderIsNotVault() (gas: 10794) -LockTest:testDeployment() (gas: 26335) +LockTest:testDeployment() (gas: 26381) LockTest:test_RevertWhen_InvalidLockupPeriod() (gas: 865463) LockTest:test_RevertWhen_SenderIsNotVault() (gas: 10630) -MigrateTest:testDeployment() (gas: 26335) +MigrateTest:testDeployment() (gas: 26381) MigrateTest:test_RevertWhen_NoPendingMigration() (gas: 1049846) MigrateTest:test_RevertWhen_SenderIsNotVault() (gas: 10794) SetStakeManagerTest:testDeployment() (gas: 9774) SetStakeManagerTest:test_RevertWhen_InvalidStakeManagerAddress() (gas: 20481) SetStakeManagerTest:test_SetStakeManager() (gas: 19869) -StakeManagerTest:testDeployment() (gas: 26107) -StakeTest:testDeployment() (gas: 26335) +StakeManagerTest:testDeployment() (gas: 26153) +StakeTest:testDeployment() (gas: 26381) StakeTest:test_RevertWhen_InvalidLockupPeriod() (gas: 883366) StakeTest:test_RevertWhen_SenderIsNotVault() (gas: 10650) StakeTest:test_RevertWhen_StakeTokenTransferFails() (gas: 175040) -StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 948606) +StakeTest:test_StakeWithoutLockUpTimeMintsMultiplierPoints() (gas: 949234) StakedTokenTest:testStakeToken() (gas: 7616) -UnstakeTest:testDeployment() (gas: 26357) -UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1051778) -UnstakeTest:test_RevertWhen_SenderIsNotVault() (gas: 10631) -UnstakeTest:test_UnstakeShouldReturnFunds() (gas: 946735) -UserFlowsTest:testDeployment() (gas: 26335) -UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1026340) -UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 1825381) +UnstakeTest:testDeployment() (gas: 26403) +UnstakeTest:test_RevertWhen_FundsLocked() (gas: 1051935) +UnstakeTest:test_RevertWhen_SenderIsNotVault() (gas: 10653) +UnstakeTest:test_UnstakeShouldBurnMultiplierPoints() (gas: 3600584) +UnstakeTest:test_UnstakeShouldReturnFunds() (gas: 947354) +UserFlowsTest:testDeployment() (gas: 26381) +UserFlowsTest:test_StakeWithLockUpTimeLocksStake() (gas: 1047519) +UserFlowsTest:test_StakedSupplyShouldIncreaseAndDecreaseAgain() (gas: 1826706) VaultFactoryTest:testDeployment() (gas: 9774) \ No newline at end of file diff --git a/contracts/StakeManager.sol b/contracts/StakeManager.sol index 3b61f98..4d78b8f 100644 --- a/contracts/StakeManager.sol +++ b/contracts/StakeManager.sol @@ -40,7 +40,7 @@ contract StakeManager is Ownable { uint256 public constant MAX_LOCKUP_PERIOD = 4 * YEAR; // 4 years uint256 public constant MP_APY = 1; uint256 public constant MAX_BOOST = 4; - + uint256 public constant PRECISION = 1000; mapping(address index => Account value) public accounts; mapping(uint256 index => Epoch value) public epochs; mapping(bytes32 codehash => bool approved) public isVault; @@ -162,8 +162,10 @@ contract StakeManager is Ownable { } _processAccount(account, currentEpoch); - uint256 reducedMP = ((_amount * account.currentMP) / account.balance); //TODO: fix precision loss - uint256 reducedInitialMP = ((_amount * account.initialMP) / account.balance); //TODO: fix precision loss + uint256 reducedMP = ((_amount * account.currentMP * PRECISION) / account.balance) / PRECISION; //TODO: fix + // precision loss + uint256 reducedInitialMP = ((_amount * account.initialMP * PRECISION) / account.balance) / PRECISION; //TODO: + // fix precision loss //update storage account.balance -= _amount; @@ -324,8 +326,8 @@ contract StakeManager is Ownable { //mint multiplier points to that epoch _mintMP(account, iEpoch.startTime + EPOCH_SIZE, iEpoch); uint256 userSupply = account.balance + account.currentMP; - uint256 userShare = userSupply / iEpoch.totalSupply; //TODO: fix precision loss; - uint256 userEpochReward = userShare * iEpoch.epochReward; + uint256 userShare = (userSupply * PRECISION) / iEpoch.totalSupply; //TODO: fix precision loss; + uint256 userEpochReward = (userShare * iEpoch.epochReward) / PRECISION; userReward += userEpochReward; iEpoch.epochReward -= userEpochReward; iEpoch.totalSupply -= userSupply; @@ -432,7 +434,7 @@ contract StakeManager is Ownable { * @return _increasedMP increased multiplier points */ function _getIncreasedMP(uint256 _balance, uint256 _deltaTime) private pure returns (uint256 _increasedMP) { - return _balance * ((_deltaTime / YEAR) * MP_APY); //TODO: fix precision loss + return (_balance * ((_deltaTime * PRECISION / YEAR) * MP_APY)) / PRECISION; //TODO: fix precision loss } /**