diff --git a/contracts/base/Multicall.sol b/contracts/base/Multicall.sol index f9f08b38..c576ac57 100644 --- a/contracts/base/Multicall.sol +++ b/contracts/base/Multicall.sol @@ -12,9 +12,9 @@ import '../interfaces/IMulticall.sol'; */ abstract contract Multicall is IMulticall { /// @inheritdoc IMulticall - function multicall(bytes[] calldata data) public override returns (bytes[] memory results) { + function multicall(bytes[] calldata data) external override returns (bytes[] memory results) { results = new bytes[](data.length); - for (uint256 i = 0; i < data.length; i++) { + for (uint256 i = 0; i < data.length; ) { (bool success, bytes memory result) = address(this).delegatecall(data[i]); if (!success) { @@ -27,6 +27,10 @@ abstract contract Multicall is IMulticall { } results[i] = result; + unchecked { + // cannot realistically overflow + ++i; + } } } } diff --git a/contracts/keeper/KeeperOracles.sol b/contracts/keeper/KeeperOracles.sol index a7d6753f..a7f4c866 100644 --- a/contracts/keeper/KeeperOracles.sol +++ b/contracts/keeper/KeeperOracles.sol @@ -29,7 +29,7 @@ abstract contract KeeperOracles is Ownable2Step, EIP712, IKeeperOracles { constructor() Ownable2Step() EIP712('KeeperOracles', '1') {} /// @inheritdoc IKeeperOracles - function addOracle(address oracle) public override onlyOwner { + function addOracle(address oracle) external override onlyOwner { if (isOracle[oracle]) revert Errors.AlreadyAdded(); // SLOAD to memory diff --git a/contracts/keeper/KeeperRewards.sol b/contracts/keeper/KeeperRewards.sol index e68ed87f..6c481244 100644 --- a/contracts/keeper/KeeperRewards.sol +++ b/contracts/keeper/KeeperRewards.sol @@ -112,7 +112,10 @@ abstract contract KeeperRewards is KeeperOracles, IKeeperRewards { rewardsRoot = params.rewardsRoot; // cannot overflow on human timescales lastRewardsTimestamp = uint64(block.timestamp); - rewardsNonce = nonce + 1; + unchecked { + // cannot realistically overflow + rewardsNonce = nonce + 1; + } _osToken.setAvgRewardPerSecond(params.avgRewardPerSecond); @@ -128,11 +131,9 @@ abstract contract KeeperRewards is KeeperOracles, IKeeperRewards { /// @inheritdoc IKeeperRewards function canUpdateRewards() public view override returns (bool) { - // SLOAD to memory - uint256 _lastRewardsTimestamp = lastRewardsTimestamp; unchecked { // cannot overflow as lastRewardsTimestamp & rewardsDelay are uint64 - return _lastRewardsTimestamp + rewardsDelay < block.timestamp; + return lastRewardsTimestamp + rewardsDelay < block.timestamp; } } @@ -190,7 +191,7 @@ abstract contract KeeperRewards is KeeperOracles, IKeeperRewards { } // SLOAD to memory - Reward memory lastReward = rewards[msg.sender]; + Reward storage lastReward = rewards[msg.sender]; // check whether Vault's nonce is smaller that the current, otherwise it's already harvested if (lastReward.nonce >= currentNonce) return (0, 0, false); @@ -199,7 +200,8 @@ abstract contract KeeperRewards is KeeperOracles, IKeeperRewards { harvested = true; // update state - rewards[msg.sender] = Reward({nonce: currentNonce, assets: params.reward}); + lastReward.nonce = currentNonce; + lastReward.assets = params.reward; // check whether Vault has unlocked execution reward if (IVaultMev(msg.sender).mevEscrow() == _sharedMevEscrow) { diff --git a/contracts/keeper/KeeperValidators.sol b/contracts/keeper/KeeperValidators.sol index 6450e3d1..4c66e58c 100644 --- a/contracts/keeper/KeeperValidators.sol +++ b/contracts/keeper/KeeperValidators.sol @@ -41,7 +41,7 @@ abstract contract KeeperValidators is KeeperOracles, KeeperRewards, IKeeperValid } /// @inheritdoc IKeeperValidators - function setValidatorsMinOracles(uint256 _validatorsMinOracles) public override onlyOwner { + function setValidatorsMinOracles(uint256 _validatorsMinOracles) external override onlyOwner { _setValidatorsMinOracles(_validatorsMinOracles); } @@ -105,7 +105,10 @@ abstract contract KeeperValidators is KeeperOracles, KeeperRewards, IKeeperValid ); // update state - exitSignaturesNonces[vault] = nonce + 1; + unchecked { + // cannot realistically overflow + exitSignaturesNonces[vault] = nonce + 1; + } // emit event emit ExitSignaturesUpdated(msg.sender, vault, nonce, exitSignaturesIpfsHash); diff --git a/contracts/libraries/ExitQueue.sol b/contracts/libraries/ExitQueue.sol index 9cf3123d..54bb8986 100644 --- a/contracts/libraries/ExitQueue.sol +++ b/contracts/libraries/ExitQueue.sol @@ -60,7 +60,10 @@ library ExitQueue { if (_unsafeAccess(self.checkpoints, mid).totalTickets > positionTicket) { high = mid; } else { - low = mid + 1; + unchecked { + // cannot underflow as mid < high + low = mid + 1; + } } } return high; @@ -124,8 +127,10 @@ library ExitQueue { // cannot overflow as it is capped with underlying asset total supply burnedShares += sharesDelta; exitedAssets += Math.mulDiv(sharesDelta, checkpointAssets, checkpointShares); + + // cannot overflow as checkpoints are created max once per day + checkpointIdx++; } - checkpointIdx++; // stop when required shares collected or reached end of checkpoints list if (positionShares <= burnedShares || checkpointIdx >= length) { diff --git a/contracts/osToken/OsToken.sol b/contracts/osToken/OsToken.sol index 09227035..d1c34e8e 100644 --- a/contracts/osToken/OsToken.sol +++ b/contracts/osToken/OsToken.sol @@ -250,9 +250,7 @@ contract OsToken is ERC20, Ownable2Step, IOsToken { // check whether any profit accrued if (profitAccrued == 0) { - // SLOAD to memory - uint256 lastUpdateTimestamp = _lastUpdateTimestamp; - if (lastUpdateTimestamp != block.timestamp) { + if (_lastUpdateTimestamp != block.timestamp) { _lastUpdateTimestamp = uint64(block.timestamp); } return; diff --git a/contracts/osToken/PriceFeed.sol b/contracts/osToken/PriceFeed.sol index fcfcc41d..908bf16e 100644 --- a/contracts/osToken/PriceFeed.sol +++ b/contracts/osToken/PriceFeed.sol @@ -67,6 +67,6 @@ contract PriceFeed is IBalancerRateProvider, IChainlinkAggregator, IChainlinkV3A uint80 answeredInRound ) { - return (uint80(0), latestAnswer(), block.timestamp, block.timestamp, uint80(0)); + return (0, latestAnswer(), block.timestamp, block.timestamp, 0); } } diff --git a/contracts/vaults/ethereum/EthGenesisVault.sol b/contracts/vaults/ethereum/EthGenesisVault.sol index a49d7698..31a09d38 100644 --- a/contracts/vaults/ethereum/EthGenesisVault.sol +++ b/contracts/vaults/ethereum/EthGenesisVault.sol @@ -214,7 +214,7 @@ contract EthGenesisVault is Initializable, EthVault, IEthGenesisVault { */ function _pullAssets() private { uint256 escrowBalance = address(_poolEscrow).balance; - if (escrowBalance > 0) _poolEscrow.withdraw(payable(this), escrowBalance); + if (escrowBalance != 0) _poolEscrow.withdraw(payable(this), escrowBalance); } /** diff --git a/contracts/vaults/modules/VaultOsToken.sol b/contracts/vaults/modules/VaultOsToken.sol index 7249388c..34eb2fd8 100644 --- a/contracts/vaults/modules/VaultOsToken.sol +++ b/contracts/vaults/modules/VaultOsToken.sol @@ -48,7 +48,7 @@ abstract contract VaultOsToken is VaultImmutables, VaultState, VaultEnterExit, I /// @inheritdoc IVaultOsToken function osTokenPositions(address user) external view override returns (uint128 shares) { OsTokenPosition memory position = _positions[user]; - if (position.shares > 0) _syncPositionFee(position); + if (position.shares != 0) _syncPositionFee(position); return position.shares; } @@ -66,7 +66,7 @@ abstract contract VaultOsToken is VaultImmutables, VaultState, VaultEnterExit, I // fetch user position OsTokenPosition memory position = _positions[msg.sender]; - if (position.shares > 0) { + if (position.shares != 0) { _syncPositionFee(position); } else { position.cumulativeFeePerShare = SafeCast.toUint128(_osToken.cumulativeFeePerShare()); diff --git a/contracts/vaults/modules/VaultToken.sol b/contracts/vaults/modules/VaultToken.sol index 8794154a..ef57b423 100644 --- a/contracts/vaults/modules/VaultToken.sol +++ b/contracts/vaults/modules/VaultToken.sol @@ -40,7 +40,7 @@ abstract contract VaultToken is Initializable, ERC20Upgradeable, VaultState, IVa /// @inheritdoc VaultState function _updateExitQueue() internal virtual override returns (uint256 burnedShares) { burnedShares = super._updateExitQueue(); - if (burnedShares > 0) emit Transfer(address(this), address(0), burnedShares); + if (burnedShares != 0) emit Transfer(address(this), address(0), burnedShares); } /// @inheritdoc ERC20Upgradeable diff --git a/test/__snapshots__/EthErc20Vault.spec.ts.snap b/test/__snapshots__/EthErc20Vault.spec.ts.snap index f5d6b176..2a43b8dd 100644 --- a/test/__snapshots__/EthErc20Vault.spec.ts.snap +++ b/test/__snapshots__/EthErc20Vault.spec.ts.snap @@ -31,6 +31,6 @@ Object { exports[`EthErc20Vault update exit queue emits transfer event 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 137518, + "gasUsed": 137509, } `; diff --git a/test/__snapshots__/EthGenesisVault.spec.ts.snap b/test/__snapshots__/EthGenesisVault.spec.ts.snap index 8b2f482f..021cd574 100644 --- a/test/__snapshots__/EthGenesisVault.spec.ts.snap +++ b/test/__snapshots__/EthGenesisVault.spec.ts.snap @@ -17,7 +17,7 @@ Object { exports[`EthGenesisVault pulls assets on claim exited assets 1`] = ` Object { "calldataByteLength": 68, - "gasUsed": 64372, + "gasUsed": 64329, } `; @@ -38,20 +38,20 @@ Object { exports[`EthGenesisVault update state deducts rewards on first state update 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 145239, + "gasUsed": 145230, } `; exports[`EthGenesisVault update state splits penalty between rewardEthToken and vault 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 100632, + "gasUsed": 100623, } `; exports[`EthGenesisVault update state splits reward between rewardEthToken and vault 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 162339, + "gasUsed": 162330, } `; diff --git a/test/__snapshots__/EthVault.deposit.spec.ts.snap b/test/__snapshots__/EthVault.deposit.spec.ts.snap index de9849b1..8bc8c48a 100644 --- a/test/__snapshots__/EthVault.deposit.spec.ts.snap +++ b/test/__snapshots__/EthVault.deposit.spec.ts.snap @@ -24,6 +24,6 @@ Object { exports[`EthVault - deposit full vault: assets & shares update state and deposit 1`] = ` Object { "calldataByteLength": 260, - "gasUsed": 207931, + "gasUsed": 207922, } `; diff --git a/test/__snapshots__/EthVault.multicall.spec.ts.snap b/test/__snapshots__/EthVault.multicall.spec.ts.snap index 361458d5..eb3b7076 100644 --- a/test/__snapshots__/EthVault.multicall.spec.ts.snap +++ b/test/__snapshots__/EthVault.multicall.spec.ts.snap @@ -3,13 +3,13 @@ exports[`EthVault - multicall can update state, redeem and queue for exit 1`] = ` Object { "calldataByteLength": 676, - "gasUsed": 185901, + "gasUsed": 185776, } `; exports[`EthVault - multicall can update state, redeem and queue for exit 2`] = ` Object { "calldataByteLength": 516, - "gasUsed": 139838, + "gasUsed": 139713, } `; diff --git a/test/__snapshots__/EthVault.register.spec.ts.snap b/test/__snapshots__/EthVault.register.spec.ts.snap index c0e72d28..f21d7552 100644 --- a/test/__snapshots__/EthVault.register.spec.ts.snap +++ b/test/__snapshots__/EthVault.register.spec.ts.snap @@ -3,7 +3,7 @@ exports[`EthVault - register multiple validators succeeds 1`] = ` Object { "calldataByteLength": 3652, - "gasUsed": 722975, + "gasUsed": 723005, } `; diff --git a/test/__snapshots__/EthVault.state.spec.ts.snap b/test/__snapshots__/EthVault.state.spec.ts.snap index cf1f25b4..2496e5ee 100644 --- a/test/__snapshots__/EthVault.state.spec.ts.snap +++ b/test/__snapshots__/EthVault.state.spec.ts.snap @@ -3,20 +3,20 @@ exports[`EthVault - state allocates fee to recipient when delta is above zero 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 117797, + "gasUsed": 117825, } `; exports[`EthVault - state applies penalty when delta is below zero 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 115420, + "gasUsed": 115411, } `; exports[`EthVault - state updates exit queue 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 144912, + "gasUsed": 144903, } `; diff --git a/test/__snapshots__/EthVault.withdraw.spec.ts.snap b/test/__snapshots__/EthVault.withdraw.spec.ts.snap index a9b48539..317f08ee 100644 --- a/test/__snapshots__/EthVault.withdraw.spec.ts.snap +++ b/test/__snapshots__/EthVault.withdraw.spec.ts.snap @@ -3,28 +3,28 @@ exports[`EthVault - withdraw claim exited assets for single user in multiple checkpoints in multiple transactions 1`] = ` Object { "calldataByteLength": 68, - "gasUsed": 76885, + "gasUsed": 76834, } `; exports[`EthVault - withdraw claim exited assets for single user in multiple checkpoints in multiple transactions 2`] = ` Object { "calldataByteLength": 68, - "gasUsed": 49562, + "gasUsed": 49521, } `; exports[`EthVault - withdraw claim exited assets for single user in multiple checkpoints in single transaction 1`] = ` Object { "calldataByteLength": 68, - "gasUsed": 52411, + "gasUsed": 52309, } `; exports[`EthVault - withdraw claim exited assets for single user in single checkpoint 1`] = ` Object { "calldataByteLength": 68, - "gasUsed": 49562, + "gasUsed": 49521, } `; @@ -47,13 +47,13 @@ Object { exports[`EthVault - withdraw update exit queue adds checkpoint 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 103499, + "gasUsed": 103490, } `; exports[`EthVault - withdraw update exit queue for not all the queued shares 1`] = ` Object { "calldataByteLength": 196, - "gasUsed": 103495, + "gasUsed": 103486, } `; diff --git a/test/__snapshots__/KeeperRewards.spec.ts.snap b/test/__snapshots__/KeeperRewards.spec.ts.snap index fe19257c..80ccbb57 100644 --- a/test/__snapshots__/KeeperRewards.spec.ts.snap +++ b/test/__snapshots__/KeeperRewards.spec.ts.snap @@ -3,42 +3,42 @@ exports[`KeeperRewards harvest (own escrow) succeeds for latest rewards root 1`] = ` Object { "calldataByteLength": 292, - "gasUsed": 110889, + "gasUsed": 110938, } `; exports[`KeeperRewards harvest (own escrow) succeeds for previous rewards root 1`] = ` Object { "calldataByteLength": 292, - "gasUsed": 113058, + "gasUsed": 113092, } `; exports[`KeeperRewards harvest (own escrow) succeeds for previous rewards root 2`] = ` Object { "calldataByteLength": 196, - "gasUsed": 74469, + "gasUsed": 74497, } `; exports[`KeeperRewards harvest (shared escrow) succeeds for latest rewards root 1`] = ` Object { "calldataByteLength": 292, - "gasUsed": 144270, + "gasUsed": 144282, } `; exports[`KeeperRewards harvest (shared escrow) succeeds for previous rewards root 1`] = ` Object { "calldataByteLength": 292, - "gasUsed": 146439, + "gasUsed": 146436, } `; exports[`KeeperRewards harvest (shared escrow) succeeds for previous rewards root 2`] = ` Object { "calldataByteLength": 196, - "gasUsed": 90750, + "gasUsed": 90741, } `; @@ -52,20 +52,20 @@ Object { exports[`KeeperRewards update rewards succeeds 1`] = ` Object { "calldataByteLength": 772, - "gasUsed": 141099, + "gasUsed": 141074, } `; exports[`KeeperRewards update rewards succeeds 2`] = ` Object { "calldataByteLength": 772, - "gasUsed": 123987, + "gasUsed": 123962, } `; exports[`KeeperRewards update rewards succeeds with all signatures 1`] = ` Object { "calldataByteLength": 1156, - "gasUsed": 147303, + "gasUsed": 147278, } `; diff --git a/test/__snapshots__/KeeperValidators.spec.ts.snap b/test/__snapshots__/KeeperValidators.spec.ts.snap index a6a692fb..cddc6281 100644 --- a/test/__snapshots__/KeeperValidators.spec.ts.snap +++ b/test/__snapshots__/KeeperValidators.spec.ts.snap @@ -3,14 +3,14 @@ exports[`KeeperValidators register multiple validators succeeds 1`] = ` Object { "calldataByteLength": 3716, - "gasUsed": 724154, + "gasUsed": 724184, } `; exports[`KeeperValidators register multiple validators succeeds 2`] = ` Object { "calldataByteLength": 3716, - "gasUsed": 632475, + "gasUsed": 632505, } `; diff --git a/test/__snapshots__/RewardSplitter.spec.ts.snap b/test/__snapshots__/RewardSplitter.spec.ts.snap index f0206f12..1263c3fb 100644 --- a/test/__snapshots__/RewardSplitter.spec.ts.snap +++ b/test/__snapshots__/RewardSplitter.spec.ts.snap @@ -31,6 +31,6 @@ Object { exports[`RewardSplitter withdraw rewards can redeem, enter exit queue with multicall 1`] = ` Object { "calldataByteLength": 772, - "gasUsed": 219424, + "gasUsed": 219211, } `;