Conversation
…urves, force e18 granularity for currentSharePrice
…because this affects requirements for getAtomCost and getTripleCost -- these must be adjusted to be inclusive of fees and indirect deposits
… tests to use proxy.
…ew admin confirms
…subsidiary atom deposits on triple deposit and triple creation
… empty string as a curve name
|
|
||
| /// @notice The maximum number of shares that this curve can handle without overflowing. | ||
| /// @dev Checked by the EthMultiVault before transacting | ||
| function maxShares() public view virtual returns (uint256); |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
|
|
||
| /// @notice The maximum number of assets that this curve can handle without overflowing. | ||
| /// @dev Checked by the EthMultiVault before transacting | ||
| function maxAssets() public view virtual returns (uint256); |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalShares Total quantity of shares already awarded by the curve | ||
| /// @return shares The number of shares that would be minted | ||
| function previewDeposit(uint256 assets, uint256 totalAssets, uint256 totalShares) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalAssets Total quantity of assets already staked into the curve | ||
| /// @return assets The number of assets that would be returned | ||
| function previewRedeem(uint256 shares, uint256 totalShares, uint256 totalAssets) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalShares Total quantity of shares already awarded by the curve | ||
| /// @return shares The number of shares that would need to be redeemed | ||
| function previewWithdraw(uint256 assets, uint256 totalAssets, uint256 totalShares) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalAssets Total quantity of assets already staked into the curve | ||
| /// @return assets The number of assets that would be required to mint the shares | ||
| function previewMint(uint256 shares, uint256 totalShares, uint256 totalAssets) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalShares Total quantity of shares already awarded by the curve | ||
| /// @return shares The number of shares equivalent to the given assets | ||
| function convertToShares(uint256 assets, uint256 totalAssets, uint256 totalShares) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
| /// @param totalAssets Total quantity of assets already staked into the curve | ||
| /// @return assets The number of assets equivalent to the given shares | ||
| function convertToAssets(uint256 shares, uint256 totalShares, uint256 totalAssets) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made this external
|
|
||
| /// @inheritdoc BaseCurve | ||
| /// @notice Computes the 1:1 relationship between assets <--> shares. | ||
| function previewDeposit(uint256 assets, uint256 totalAssets, uint256 totalShares) |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made all of these external
| /// @dev or to say that another way: | ||
| /// $$\text{shares} = \sqrt{(s + o)^2 + \frac{2a}{m}} - (s + o)$$ | ||
| function previewDeposit(uint256 assets, uint256, /*totalAssets*/ uint256 totalShares) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made all of these external
| /// @dev or to say that another way: | ||
| /// $$\text{shares} = \sqrt{s^2 + \frac{2a}{m}} - s$$ | ||
| function previewDeposit(uint256 assets, uint256, /*totalAssets*/ uint256 totalShares) | ||
| public |
There was a problem hiding this comment.
3.20 - Functions in Curve Contracts Could Be external Instead of public
Made all of these external
| shares = assets; // 1:1 relationship | ||
|
|
||
| return shares; | ||
| return assets; // 1:1 relationship |
There was a problem hiding this comment.
3.21 - Preview Functions in LinearCurve Could Be Simplified
Simplified what was here.
| assets = shares; // 1:1 relationship | ||
|
|
||
| return assets; | ||
| return shares; // 1:1 relationship |
There was a problem hiding this comment.
3.21 - Preview Functions in LinearCurve Could Be Simplified
Simplified what was here.
| assets = shares; // 1:1 relationship | ||
|
|
||
| return assets; | ||
| return assets; // 1:1 relationship |
There was a problem hiding this comment.
3.21 - Preview Functions in LinearCurve Could Be Simplified
Simplified what was here.
| shares = assets; // 1:1 relationship | ||
|
|
||
| return shares; | ||
| return shares; // 1:1 relationship |
There was a problem hiding this comment.
3.21 - Preview Functions in LinearCurve Could Be Simplified
Simplified what was here.
| /// @notice Construct the curve with a unique name | ||
| /// | ||
| /// @param _name Unique name for the curve | ||
| constructor(string memory _name) { |
There was a problem hiding this comment.
3.22 - Unconventional Function Order in BaseCurve and Derived Contractsz
Constructor is first now.
Summary of Test Results if Merged To Main:
✅ All 134 tests passed! (0 skipped, Total: 134) Test Results for Merge
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1219-1252): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1248) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1418) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1247) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1714) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1759) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1760) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2166-2170) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2131-2135) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2067-2073) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1859-1886) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1926-1960) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2303-2307) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2105-2108) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2400-2412) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2403)Affected Files:
Recommended Actions
⛽ Gas Analysis📊 First gas snapshot created |
Summary of Test Results if Merged To Main:
✅ All 134 tests passed! (0 skipped, Total: 134) Test Results for Merge
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1219-1252): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1248) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1418) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1247) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1759) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1714) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1760) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2166-2170) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2131-2135) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2067-2073) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1859-1886) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1926-1960) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2303-2307) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2105-2108) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2400-2412) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2403)Affected Files:
Recommended Actions
⛽ Gas Analysis📊 First gas snapshot created |
* deploy better curve * update progressive curve args to 2,5e35 * add reinit to add bonding curve config when upgrading ethmultivault. Add mutator for bonding curve config to ethmultivault if admin wants to change it like any other config. add upgrade test to verify we can upgrade from 1.0 to 1.5. fix warnings. * address feedback --------- Co-authored-by: Mihailo Maksa <mihajlomaksa9+@gmail.com>
Summary of Test Results if Merged To Main:
✅ All 148 tests passed! (2 skipped, Total: 150) Test Results for Merge
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1244-1277): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1273) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1443) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1272) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1784) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1739) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1785) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2191-2195) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2156-2160) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2092-2098) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1884-1911) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1951-1985) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2328-2332) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2130-2133) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2425-2437) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2428)Affected Files:
Recommended Actions
⛽ Gas Analysis📊 First gas snapshot created |
Summary of Test Results if Merged To Main:
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1244-1277): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1273) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1443) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1272) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1784) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1739) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1785) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2191-2195) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2156-2160) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2092-2098) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1884-1911) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1951-1985) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2328-2332) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2130-2133) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2425-2437) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2428)Affected Files:
uninitialized-localImpact: EthMultiVault.batchCreateAtom(bytes[]).protocolDepositFeeTotal (src/EthMultiVault.sol#628) is a local variable never initialized Affected Files:
Recommended Actions
⛽ Gas Analysis |
Summary of Test Results if Merged To Main:
✅ All 148 tests passed! (2 skipped, Total: 150) Test Results for Merge
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDeposit(address,uint256[],uint256[]) (src/EthMultiVault.sol#1244-1277): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1273) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1443) State variables written after the call(s): - shares[i] = _deposit(receiver,termIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1272) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1784) - vaults[id].balanceOf[to] += amount (src/EthMultiVault.sol#1739) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1785) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2191-2195) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2156-2160) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2092-2098) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1884-1911) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1951-1985) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2328-2332) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2130-2133) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2425-2437) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2428)Affected Files:
uninitialized-localImpact: EthMultiVault.batchCreateAtom(bytes[]).protocolDepositFeeTotal (src/EthMultiVault.sol#628) is a local variable never initialized Affected Files:
Recommended Actions
⛽ Gas Analysis📊 First gas snapshot created |
* v1.5 upgrade prep script * run forge fmt * add deployment addresses to readme * space out addresses in readme * add more spacing to readme deployment notes
Summary of Test Results if Merged To Main:
✅ All 148 tests passed! (2 skipped, Total: 150) Test Results for Merge
🔒 Security AnalysisHigh Severity Issuesarbitrary-send-ethImpact: AtomWallet._call(address,uint256,bytes) (src/AtomWallet.sol#214-221) sends eth to arbitrary user Dangerous calls: - (success,result) = target.call{value: value}(data) (src/AtomWallet.sol#215) Affected Files:
View Detailed Findings
reentrancy-ethImpact: Reentrancy in EthMultiVault.batchDepositCurve(address,uint256[],uint256[],uint256[]) (src/EthMultiVault.sol#1291-1326): External calls: - _transferFeesToProtocolMultisig(protocolFee) (src/EthMultiVault.sol#1322) - (success,None) = address(generalConfig.protocolMultisig).call{value: value}() (src/EthMultiVault.sol#1440) State variables written after the call(s): - shares[i] = _depositCurve(receiver,termIds[i],curveIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1320) - bondingCurveVaults[id][curveId].totalAssets += assetsDelta (src/EthMultiVault.sol#1798) - bondingCurveVaults[id][curveId].balanceOf[to] += amount (src/EthMultiVault.sol#1740) - bondingCurveVaults[id][curveId].totalShares += sharesDelta (src/EthMultiVault.sol#1799) EthMultiVault.bondingCurveVaults (src/EthMultiVault.sol#132) can be used in cross function reentrancies: - EthMultiVault.bondingCurveVaults (src/EthMultiVault.sol#132) - EthMultiVault.convertToAssetsCurve(uint256,uint256,uint256) (src/EthMultiVault.sol#2216-2221) - EthMultiVault.convertToSharesCurve(uint256,uint256,uint256) (src/EthMultiVault.sol#2181-2187) - EthMultiVault.currentSharePriceCurve(uint256,uint256) (src/EthMultiVault.sol#2113-2117) - EthMultiVault.getCurveVaultState(uint256,uint256) (src/EthMultiVault.sol#2353-2355) - EthMultiVault.getDepositSharesAndFeesCurve(uint256,uint256,uint256) (src/EthMultiVault.sol#1922-1949) - EthMultiVault.getRedeemAssetsAndFeesCurve(uint256,uint256,uint256) (src/EthMultiVault.sol#1996-2030) - EthMultiVault.getVaultStateForUserCurve(uint256,uint256,address) (src/EthMultiVault.sol#2343-2351) - EthMultiVault.maxRedeemCurve(address,uint256,uint256) (src/EthMultiVault.sol#2149-2152) - shares[i] = _depositCurve(receiver,termIds[i],curveIds[i],userDepositAfterprotocolFee) (src/EthMultiVault.sol#1320) - vaults[id].totalAssets = totalAssets (src/EthMultiVault.sol#1781) - vaults[id].totalShares = totalShares (src/EthMultiVault.sol#1782) EthMultiVault.vaults (src/EthMultiVault.sol#97) can be used in cross function reentrancies: - EthMultiVault.convertToAssets(uint256,uint256) (src/EthMultiVault.sol#2200-2204) - EthMultiVault.convertToShares(uint256,uint256) (src/EthMultiVault.sol#2165-2169) - EthMultiVault.currentSharePrice(uint256) (src/EthMultiVault.sol#2101-2107) - EthMultiVault.getDepositSharesAndFees(uint256,uint256) (src/EthMultiVault.sol#1893-1920) - EthMultiVault.getRedeemAssetsAndFees(uint256,uint256) (src/EthMultiVault.sol#1960-1994) - EthMultiVault.getVaultStateForUser(uint256,address) (src/EthMultiVault.sol#2337-2341) - EthMultiVault.maxRedeem(address,uint256) (src/EthMultiVault.sol#2139-2142) - EthMultiVault.vaults (src/EthMultiVault.sol#97) Affected Files:
View Detailed Findings
Medium Severity IssuesView Medium Severity Issues##### incorrect-equality **Impact**: EthMultiVault._validateTimelock(bytes32) (src/EthMultiVault.sol#2434-2446) uses a dangerous strict equality: - timelock.readyTime == 0 (src/EthMultiVault.sol#2437)Affected Files:
uninitialized-localImpact: EthMultiVault.batchCreateAtom(bytes[]).protocolDepositFeeTotal (src/EthMultiVault.sol#625) is a local variable never initialized Affected Files:
Recommended Actions
⛽ Gas Analysis📊 First gas snapshot created |
3 participants
No description provided.