Skip to content

Instantly share code, notes, and snippets.

@romeroadrian
Created September 20, 2023 20:56
Show Gist options
  • Save romeroadrian/a2045e828aa87418a66e9ab47d811292 to your computer and use it in GitHub Desktop.
Save romeroadrian/a2045e828aa87418a66e9ab47d811292 to your computer and use it in GitHub Desktop.
2023-09-asymmetry Automated Findings

Report

Gas Optimizations

Issue Instances
GAS-1 Using bools for storage incurs overhead 2
GAS-2 Cache array length outside of loop 3
GAS-3 For Operations that will not overflow, you could use unchecked 210
GAS-4 Don't initialize variables with default value 3
GAS-5 Functions guaranteed to revert when called by normal users can be marked payable 9
GAS-6 ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 5
GAS-7 Using private rather than public for constants, saves gas 2
GAS-8 Use != 0 instead of > 0 for unsigned integer comparison 6

[GAS-1] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (2):

File: contracts/AfEth.sol

28:     bool public pauseDeposit;

29:     bool public pauseWithdraw;

Link to code

[GAS-2] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (3):

File: contracts/strategies/votium/VotiumStrategy.sol

77:         for (uint256 i = 0; i < lockedBalances.length; i++) {

184:         for (uint256 i = 0; i < lockedBalances.length; i++) {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

274:         for (uint256 i = 0; i < _swapsData.length; i++) {

Link to code

[GAS-3] For Operations that will not overflow, you could use unchecked

Instances (210):

File: contracts/AfEth.sol

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

5: import "contracts/strategies/votium/VotiumStrategy.sol";

5: import "contracts/strategies/votium/VotiumStrategy.sol";

5: import "contracts/strategies/votium/VotiumStrategy.sol";

6: import "contracts/external_interfaces/IVotiumStrategy.sol";

6: import "contracts/external_interfaces/IVotiumStrategy.sol";

7: import "contracts/strategies/AbstractStrategy.sol";

7: import "contracts/strategies/AbstractStrategy.sol";

16:     address public vEthAddress; // Votium Strategy Address

16:     address public vEthAddress; // Votium Strategy Address

69:         @notice - Initialize values for the contracts

70:         @dev - This replaces the constructor for upgradeable contracts

86:         @notice - Sets the target ratio of safEth to votium. 

88:         @param _newRatio - New ratio of safEth to votium

95:         @notice - Sets the protocol fee address which takes a percentage of the rewards.

96:         @param _newFeeAddress - New protocol fee address to collect rewards

103:         @notice - Sets the protocol fee which takes a percentage of the rewards.

104:         @param _newFee - New protocol fee

112:         @notice - Enables/Disables depositing

112:         @notice - Enables/Disables depositing

113:         @param _pauseDeposit - Bool to set pauseDeposit

121:         @notice - Enables/Disables withdrawing & requesting to withdraw

121:         @notice - Enables/Disables withdrawing & requesting to withdraw

122:         @param _pauseWithdraw - Bool to set pauseWithdraw

129:         @notice - Get's the price of afEth

130:         @dev - Checks each strategy and calculates the total value in ETH divided by supply of afETH tokens

131:         @return - Price of afEth

136:         uint256 safEthValueInEth = (ISafEth(SAF_ETH_ADDRESS).approxPrice(true) *

137:             safEthBalanceMinusPending()) / 1e18;

138:         uint256 vEthValueInEth = (vEthStrategy.price() *

139:             vEthStrategy.balanceOf(address(this))) / 1e18;

140:         return ((vEthValueInEth + safEthValueInEth) * 1e18) / totalSupply();

140:         return ((vEthValueInEth + safEthValueInEth) * 1e18) / totalSupply();

140:         return ((vEthValueInEth + safEthValueInEth) * 1e18) / totalSupply();

144:         @notice - Deposits into each strategy

145:         @dev - This is the entry into the protocol

146:         @param _minout - Minimum amount of afEth to mint

156:         uint256 sValue = (amount * ratio) / 1e18;

156:         uint256 sValue = (amount * ratio) / 1e18;

160:         uint256 vValue = (amount * (1e18 - ratio)) / 1e18;

160:         uint256 vValue = (amount * (1e18 - ratio)) / 1e18;

160:         uint256 vValue = (amount * (1e18 - ratio)) / 1e18;

162:         totalValue +=

163:             (sMinted * ISafEth(SAF_ETH_ADDRESS).approxPrice(true)) +

163:             (sMinted * ISafEth(SAF_ETH_ADDRESS).approxPrice(true)) +

164:             (vMinted * vStrategy.price());

166:         uint256 amountToMint = totalValue / priceBeforeDeposit;

172:         @notice - Request to close position

173:         @param _amount - Amount of afEth to withdraw

178:         latestWithdrawId++;

178:         latestWithdrawId++;

184:         uint256 withdrawRatio = (_amount * 1e18) /

184:         uint256 withdrawRatio = (_amount * 1e18) /

185:             (totalSupply() - afEthBalance);

190:         uint256 votiumWithdrawAmount = (withdrawRatio * votiumBalance) / 1e18;

190:         uint256 votiumWithdrawAmount = (withdrawRatio * votiumBalance) / 1e18;

197:         uint256 safEthWithdrawAmount = (withdrawRatio * safEthBalance) / 1e18;

197:         uint256 safEthWithdrawAmount = (withdrawRatio * safEthBalance) / 1e18;

199:         pendingSafEthWithdraws += safEthWithdrawAmount;

218:         @notice - Checks if withdraw can be executed from withdrawId

219:         @param _withdrawId - Id of the withdraw request for SafEth

220:         @return - Bool if withdraw can be executed

230:         @notice - Get's the withdraw time for an amount of AfEth

231:         @param _amount - Amount of afETH to withdraw

232:         @return - Highest withdraw time of the strategies

239:         @notice - Withdraw from each strategy

240:         @param _withdrawId - Id of the withdraw request

241:         @param _minout - Minimum amount of ETH to receive

257:         uint256 ethReceived = ethBalanceAfter - ethBalanceBefore;

259:         pendingSafEthWithdraws -= withdrawInfo.safEthWithdrawAmount;

274:         uint256 feeAmount = (_amount * protocolFee) / 1e18;

274:         uint256 feeAmount = (_amount * protocolFee) / 1e18;

280:         uint256 amount = _amount - feeAmount;

281:         uint256 safEthTvl = (ISafEth(SAF_ETH_ADDRESS).approxPrice(true) *

282:             safEthBalanceMinusPending()) / 1e18;

283:         uint256 votiumTvl = ((votiumStrategy.cvxPerVotium() *

284:             votiumStrategy.ethPerCvx(true)) *

285:             IERC20(vEthAddress).balanceOf(address(this))) / 1e36;

286:         uint256 totalTvl = (safEthTvl + votiumTvl);

287:         uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;

287:         uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;

297:             IERC20(SAF_ETH_ADDRESS).balanceOf(address(this)) -

Link to code

File: contracts/strategies/AbstractStrategy.sol

4: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

Link to code

File: contracts/strategies/votium/VotiumStrategy.sol

4: import "../AbstractStrategy.sol";

5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

5: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

6: import "./VotiumStrategyCore.sol";

32:         return (cvxPerVotium() * ethPerCvx(false)) / 1e18;

32:         return (cvxPerVotium() * ethPerCvx(false)) / 1e18;

44:         mintAmount = ((cvxAmount * 1e18) / priceBefore);

44:         mintAmount = ((cvxAmount * 1e18) / priceBefore);

57:         latestWithdrawId++;

57:         latestWithdrawId++;

71:         uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;

71:         uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;

72:         cvxUnlockObligations += cvxAmount;

74:         uint256 totalLockedBalancePlusUnlockable = unlockable +

77:         for (uint256 i = 0; i < lockedBalances.length; i++) {

77:         for (uint256 i = 0; i < lockedBalances.length; i++) {

78:             totalLockedBalancePlusUnlockable += lockedBalances[i].amount;

83:                 uint256 timeDifference = lockedBalances[i].unlockTime -

85:                 uint256 epochOffset = timeDifference /

87:                 uint256 withdrawEpoch = currentEpoch + epochOffset;

123:         cvxUnlockObligations -= cvxWithdrawAmount;

143:             ? cvxBalance - cvxUnlockObligations

180:         uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;

180:         uint256 cvxAmount = (_amount * _priceInCvx) / 1e18;

181:         uint256 totalLockedBalancePlusUnlockable = unlockable +

184:         for (uint256 i = 0; i < lockedBalances.length; i++) {

184:         for (uint256 i = 0; i < lockedBalances.length; i++) {

185:             totalLockedBalancePlusUnlockable += lockedBalances[i].amount;

189:                 cvxUnlockObligations + cvxAmount

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

5: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

6: import "../../external_interfaces/IWETH.sol";

6: import "../../external_interfaces/IWETH.sol";

6: import "../../external_interfaces/IWETH.sol";

7: import "../../external_interfaces/ISwapRouter.sol";

7: import "../../external_interfaces/ISwapRouter.sol";

7: import "../../external_interfaces/ISwapRouter.sol";

8: import "../../external_interfaces/IVotiumMerkleStash.sol";

8: import "../../external_interfaces/IVotiumMerkleStash.sol";

8: import "../../external_interfaces/IVotiumMerkleStash.sol";

9: import "../../external_interfaces/ISnapshotDelegationRegistry.sol";

9: import "../../external_interfaces/ISnapshotDelegationRegistry.sol";

9: import "../../external_interfaces/ISnapshotDelegationRegistry.sol";

10: import "../../external_interfaces/ILockedCvx.sol";

10: import "../../external_interfaces/ILockedCvx.sol";

10: import "../../external_interfaces/ILockedCvx.sol";

11: import "../../external_interfaces/IClaimZap.sol";

11: import "../../external_interfaces/IClaimZap.sol";

11: import "../../external_interfaces/IClaimZap.sol";

12: import "../../external_interfaces/ICrvEthPool.sol";

12: import "../../external_interfaces/ICrvEthPool.sol";

12: import "../../external_interfaces/ICrvEthPool.sol";

13: import "../../external_interfaces/IAfEth.sol";

13: import "../../external_interfaces/IAfEth.sol";

13: import "../../external_interfaces/IAfEth.sol";

14: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

14: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

14: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

14: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

14: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

15: import "../AbstractStrategy.sol";

16: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

16: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

16: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

16: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

16: import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

17: import "../../external_interfaces/ISafEth.sol";

17: import "../../external_interfaces/ISafEth.sol";

17: import "../../external_interfaces/ISafEth.sol";

18: import "hardhat/console.sol";

73:         @notice - Sets the address for the chainlink feed

74:         @param _cvxEthFeedAddress - Address of the chainlink feed

94:         @notice - Function to initialize values for the contracts

95:         @dev - This replaces the constructor for upgradeable contracts

96:         @param _owner - Address of the owner of the contract (asym multisig)

97:         @param _rewarder - Address of the rewarder contract (reward oracle)

98:         @param _manager - Address of the manager contract (afEth)

137:         return total + IERC20(CVX_ADDRESS).balanceOf(address(this));

148:         return ((totalCvx - cvxUnlockObligations) * 1e18) / supply;

148:         return ((totalCvx - cvxUnlockObligations) * 1e18) / supply;

148:         return ((totalCvx - cvxUnlockObligations) * 1e18) / supply;

152:         @notice - Eth per cvx (chainlink)

153:         @param _validate - Whether or not to validate the chainlink response

154:         @return - Price of cvx in eth

161:             uint256 /* startedAt */,

161:             uint256 /* startedAt */,

161:             uint256 /* startedAt */,

161:             uint256 /* startedAt */,

163:             uint80 /* answeredInRound */

163:             uint80 /* answeredInRound */

163:             uint80 /* answeredInRound */

163:             uint80 /* answeredInRound */

180:                     block.timestamp - cl.updatedAt <= 25 hours))

239:             0 // this is handled at the afEth level

239:             0 // this is handled at the afEth level

242:         cvxAmountOut = cvxBalanceAfter - cvxBalanceBefore;

262:             0 // this is handled at the afEth level

262:             0 // this is handled at the afEth level

264:         ethAmountOut = address(this).balance - ethBalanceBefore;

274:         for (uint256 i = 0; i < _swapsData.length; i++) {

274:         for (uint256 i = 0; i < _swapsData.length; i++) {

300:         uint256 ethReceived = ethBalanceAfter - ethBalanceBefore;

Link to code

[GAS-4] Don't initialize variables with default value

Instances (3):

File: contracts/strategies/votium/VotiumStrategy.sol

77:         for (uint256 i = 0; i < lockedBalances.length; i++) {

184:         for (uint256 i = 0; i < lockedBalances.length; i++) {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

274:         for (uint256 i = 0; i < _swapsData.length; i++) {

Link to code

[GAS-5] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Instances (9):

File: contracts/AfEth.sol

81:     function setStrategyAddress(address _vEthAddress) external onlyOwner {

90:     function setRatio(uint256 _newRatio) public onlyOwner {

98:     function setFeeAddress(address _newFeeAddress) public onlyOwner {

106:     function setProtocolFee(uint256 _newFee) public onlyOwner {

116:     function setPauseDeposit(bool _pauseDeposit) external onlyOwner {

124:     function setPauseWithdraw(bool _pauseWithdraw) external onlyOwner {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

125:     function setRewarder(address _rewarder) external onlyOwner {

215:     function withdrawStuckTokens(address _token) public onlyOwner {

272:     function applyRewards(SwapData[] calldata _swapsData) public onlyRewarder {

Link to code

[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (5):

File: contracts/AfEth.sol

178:         latestWithdrawId++;

Link to code

File: contracts/strategies/votium/VotiumStrategy.sol

57:         latestWithdrawId++;

77:         for (uint256 i = 0; i < lockedBalances.length; i++) {

184:         for (uint256 i = 0; i < lockedBalances.length; i++) {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

274:         for (uint256 i = 0; i < _swapsData.length; i++) {

Link to code

[GAS-7] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (2):

File: contracts/AfEth.sol

14:     address public constant SAF_ETH_ADDRESS =

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

27:     address public constant SNAPSHOT_DELEGATE_REGISTRY =

Link to code

[GAS-8] Use != 0 instead of > 0 for unsigned integer comparison

Instances (6):

File: contracts/AfEth.sol

157:         uint256 sMinted = sValue > 0

161:         uint256 vMinted = vValue > 0 ? vStrategy.deposit{value: vValue}() : 0;

275:         if (feeAmount > 0) {

Link to code

File: contracts/strategies/votium/VotiumStrategy.sol

139:         if (unlockable > 0)

145:         if (cvxAmountToRelock > 0) {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

281:                 if (allowance > 0) {

Link to code

Non Critical Issues

Issue Instances
NC-1 Return values of approve() not checked 6

[NC-1] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

Instances (6):

File: contracts/strategies/votium/VotiumStrategy.sol

42:         IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmount);

146:             IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmountToRelock);

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

205:         IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmount);

256:         IERC20(CVX_ADDRESS).approve(CVX_ETH_CRV_POOL_ADDRESS, _cvxAmountIn);

282:                     IERC20(_swapsData[i].sellToken).approve(

287:                 IERC20(_swapsData[i].sellToken).approve(

Link to code

Low Issues

Issue Instances
L-1 Empty Function Body - Consider commenting why 2
L-2 Initializers could be front-run 5
L-3 Unsafe ERC20 operation(s) 7

[L-1] Empty Function Body - Consider commenting why

Instances (2):

File: contracts/AfEth.sol

301:     receive() external payable {}

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

336:     receive() external payable {}

Link to code

[L-2] Initializers could be front-run

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Instances (5):

File: contracts/AfEth.sol

72:     function initialize() external initializer {

72:     function initialize() external initializer {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

100:     function initialize(

104:     ) external initializer {

114:         __ERC20_init("Votium AfEth Strategy", "vAfEth");

Link to code

[L-3] Unsafe ERC20 operation(s)

Instances (7):

File: contracts/strategies/votium/VotiumStrategy.sol

42:         IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmount);

146:             IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmountToRelock);

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

205:         IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmount);

216:         IERC20(_token).transfer(

256:         IERC20(CVX_ADDRESS).approve(CVX_ETH_CRV_POOL_ADDRESS, _cvxAmountIn);

282:                     IERC20(_swapsData[i].sellToken).approve(

287:                 IERC20(_swapsData[i].sellToken).approve(

Link to code

Medium Issues

Issue Instances
M-1 Centralization Risk for trusted owners 9

[M-1] Centralization Risk for trusted owners

Impact:

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

Instances (9):

File: contracts/AfEth.sol

81:     function setStrategyAddress(address _vEthAddress) external onlyOwner {

90:     function setRatio(uint256 _newRatio) public onlyOwner {

98:     function setFeeAddress(address _newFeeAddress) public onlyOwner {

106:     function setProtocolFee(uint256 _newFee) public onlyOwner {

116:     function setPauseDeposit(bool _pauseDeposit) external onlyOwner {

124:     function setPauseWithdraw(bool _pauseWithdraw) external onlyOwner {

Link to code

File: contracts/strategies/votium/VotiumStrategyCore.sol

78:     ) public onlyOwner {

125:     function setRewarder(address _rewarder) external onlyOwner {

215:     function withdrawStuckTokens(address _token) public onlyOwner {

Link to code

@toshiSat
Copy link

This contract is built upon the trust of the owners, like many contracts in defi. I guess it is valid, but definitely acknowledged

@0xleastwood
Copy link

I would class M-1 as QA and not medium risk but understand it is being acknowledged and intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment