From 6b09fbcf6f2989eb8af6c392579855a8ec0b442f Mon Sep 17 00:00:00 2001 From: franz Date: Thu, 23 Jan 2025 18:00:59 +0100 Subject: [PATCH 01/12] wip rate provider --- rate-providers/BeefyUsdcSiloRateprovider.md | 44 +++++++++++++++++++++ rate-providers/registry.json | 9 +++++ 2 files changed, 53 insertions(+) create mode 100644 rate-providers/BeefyUsdcSiloRateprovider.md diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md new file mode 100644 index 0000000..0f39bc4 --- /dev/null +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -0,0 +1,44 @@ +# Rate Provider: `ERC4626RateProvider` + +## Details +- Reviewed by: @franzns +- Checked by: @danielmkm +- Deployed at: + - [sonic:0x5fded3206608d3f33175a8865576431906cdb43b](https://sonicscan.org/address/0x5fded3206608d3f33175a8865576431906cdb43b#code) + + +## Context +The ERC4626 Rate Provider fetches the rate of the Beefy vault for USDC deposited into Silo v2. The rate provider was created using the ERC4626 Rateprovider factory which calls convertToAssets on the ERC4626 to expose the rate. The rate of the ERC4626 is calculated by `shares.mulDiv(totalAssets() + 1, totalSupply() + 10 ** _decimalsOffset(), rounding)`. + +## Review Checklist: Bare Minimum Compatibility +Each of the items below represents an absolute requirement for the Rate Provider. If any of these is unchecked, the Rate Provider is unfit to use. + +- [x] Implements the [`IRateProvider`](https://github.com/balancer/balancer-v2-monorepo/blob/bc3b3fee6e13e01d2efe610ed8118fdb74dfc1f2/pkg/interfaces/contracts/pool-utils/IRateProvider.sol) interface. +- [x] `getRate` returns an 18-decimal fixed point number (i.e., 1 == 1e18) regardless of underlying token decimals. + +## Review Checklist: Common Findings +Each of the items below represents a common red flag found in Rate Provider contracts. + +If none of these is checked, then this might be a pretty great Rate Provider! If any of these is checked, we must thoroughly elaborate on the conditions that lead to the potential issue. Decision points are not binary; a Rate Provider can be safe despite these boxes being checked. A check simply indicates that thorough vetting is required in a specific area, and this vetting should be used to inform a holistic analysis of the Rate Provider. + +### Administrative Privileges +- [ ] The Rate Provider is upgradeable (e.g., via a proxy architecture or an `onlyOwner` function that updates the price source address). + + +### Oracles +- [ ] Price data is provided by an off-chain source (e.g., a Chainlink oracle, a multisig, or a network of nodes). + +- [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price). + +### Common Manipulation Vectors +- [ ] The Rate Provider is susceptible to donation attacks. + + +## Additional Findings +To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. + + +## Conclusion +**Summary judgment: SAFE** + +Overall this Rate Provider should work well in pool operations with Balancer pools. diff --git a/rate-providers/registry.json b/rate-providers/registry.json index 82a3acc..c49f873 100644 --- a/rate-providers/registry.json +++ b/rate-providers/registry.json @@ -2871,6 +2871,15 @@ "entrypoint": "0xD31E89Ffb929b38bA60D1c7dBeB68c7712EAAb0a", "implementationReviewed": "0xb9fa01cbd690dfd5be3d8d667c54bbdd9e41e57d" }] + }, + "0x5fded3206608d3f33175a8865576431906cdb43b": { + "asset": "0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a", + "name": "Beefy USDC SiloV2 Rateprovider", + "summary": "safe", + "review": "./BeefyUsdcSiloRateprovider.md", + "warnings": [""], + "factory": "0x00de97829d01815346e58372be55aefd84ca2457", + "upgradeableComponents": [] } } } From 1763a7f637426e200eb3af1e2c98e037e911c355 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 10:40:42 +0100 Subject: [PATCH 02/12] add beefy 4626 --- erc4626/BeefyWrapperReview.md | 39 +++++++++++++++++++++++++++++++++++ erc4626/registry.json | 7 +++++++ 2 files changed, 46 insertions(+) create mode 100644 erc4626/BeefyWrapperReview.md diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md new file mode 100644 index 0000000..b8ba667 --- /dev/null +++ b/erc4626/BeefyWrapperReview.md @@ -0,0 +1,39 @@ +# ERC4626 Vault: `BeefyWrapper` + +## Details +- Reviewed by: @franzns +- Checked by: @danielmkm +- Deployed at: + - [sonic:0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a](https://sonicscan.org/address/0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a#code) + + +## Context +A 4626 wrapper that can wrap the various Beefy vaults. Its created using their factory at [sonic:0x234f7f81434e340910a84f45f8e89d07fa86611a](https://sonicscan.org/address/0x234f7f81434e340910a84f45f8e89d07fa86611a). + +## Review Checklist: Bare Minimum Compatibility +Each of the items below represents an absolute requirement for the ERC4626. If any of these is unchecked, the the ERC4626 is unfit to use. + +- [x] Tests based on the [balancer-v3-monorepo](https://github.com/balancer/balancer-v3-monorepo/tree/main/pkg/vault/test/foundry/fork) pass for the given ERC4626 vaults, which can be found [here](https://github.com/balancer/balancer-v3-erc4626-tests/blob/main/test/sonic/ERC4626BeefyUsdcSilo.sol). +- [x] The required Vault implements the required operational ERC4626 Interface + +## Review Checklist: Common Findings +Each of the items below represents a common red flag found in ERC4626 contracts. + +If none of these is checked, then this might be a pretty great ERC4626! If any of these is checked, we must thoroughly elaborate on the conditions that lead to the potential issue. Decision points are not binary; a ERC4626 can be safe despite these boxes being checked. A check simply indicates that thorough vetting is required in a specific area, and this vetting should be used to inform a holistic analysis of the ERC4626. + +### Administrative Privileges +- [ ] The ERC4626 Vault is upgradeable. + + +### Common Manipulation Vectors +- [ ] The ERC4626 Vault is susceptible to donation attacks. + +## Additional Findings +To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. + +The Wrapper is a minimal proxy which means that it has a hardcoded implementation address so by definition nothing can be upgraded. + +## Conclusion +**Summary judgment: USABLE** + +The outlined ERC4626 Vaults should work well with Balancer pools. \ No newline at end of file diff --git a/erc4626/registry.json b/erc4626/registry.json index f833343..45aa41c 100644 --- a/erc4626/registry.json +++ b/erc4626/registry.json @@ -157,6 +157,13 @@ "summary": "safe", "review": "./StaticATokenLMAvalonReview.md", "warnings": [] + }, + "0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a": { + "asset": "0x541FD749419CA806a8bc7da8ac23D346f2dF8B77", + "name": "Beefy USDC Wrapper for SiloV2", + "summary": "safe", + "review": "./BeefyWrapperReview.md", + "warnings": [] } }, "sepolia": { From 7f273cef877e3ff90dedae9910835436ef7281fc Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 10:45:38 +0100 Subject: [PATCH 03/12] fix asset --- erc4626/registry.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erc4626/registry.json b/erc4626/registry.json index 45aa41c..b2c2c8f 100644 --- a/erc4626/registry.json +++ b/erc4626/registry.json @@ -159,7 +159,7 @@ "warnings": [] }, "0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a": { - "asset": "0x541FD749419CA806a8bc7da8ac23D346f2dF8B77", + "asset": "0x29219dd400f2Bf60E5a23d13Be72B486D4038894", "name": "Beefy USDC Wrapper for SiloV2", "summary": "safe", "review": "./BeefyWrapperReview.md", From f2613bc67fbacd0597a678969a888c802bcc6dc4 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 14:09:04 +0100 Subject: [PATCH 04/12] adding audits --- erc4626/BeefyWrapperReview.md | 2 ++ rate-providers/BeefyUsdcSiloRateprovider.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index b8ba667..747032b 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -5,6 +5,8 @@ - Checked by: @danielmkm - Deployed at: - [sonic:0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a](https://sonicscan.org/address/0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a#code) +- Audits: + - [4626 wrapper audit](https://github.com/beefyfinance/beefy-audits/blob/master/2023-08-03-Beefy-Zellic-4626-Wrapper-Audit.pdf) ## Context diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md index 0f39bc4..2eb9e29 100644 --- a/rate-providers/BeefyUsdcSiloRateprovider.md +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -5,6 +5,8 @@ - Checked by: @danielmkm - Deployed at: - [sonic:0x5fded3206608d3f33175a8865576431906cdb43b](https://sonicscan.org/address/0x5fded3206608d3f33175a8865576431906cdb43b#code) +- Audits: + - [4626 wrapper audit](https://github.com/beefyfinance/beefy-audits/blob/master/2023-08-03-Beefy-Zellic-4626-Wrapper-Audit.pdf) ## Context From 3f2b4be5b85515d76896ddf3f1e99ba8529369d7 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 14:38:55 +0100 Subject: [PATCH 05/12] adding donation vector --- erc4626/BeefyWrapperReview.md | 10 +++++++++- rate-providers/BeefyUsdcSiloRateprovider.md | 11 +++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index 747032b..f960f7b 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -28,7 +28,15 @@ If none of these is checked, then this might be a pretty great ERC4626! If any o ### Common Manipulation Vectors -- [ ] The ERC4626 Vault is susceptible to donation attacks. +- [x] The Rate Provider is susceptible to donation attacks. + - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via + ```solidity + // + function totalAssets() public view virtual override returns (uint256) { + return _asset.balanceOf(address(this)); + } + ``` + which is part of the `totalAssets` used in the `getRate` calculation. ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md index 2eb9e29..b7def6f 100644 --- a/rate-providers/BeefyUsdcSiloRateprovider.md +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -33,8 +33,15 @@ If none of these is checked, then this might be a pretty great Rate Provider! If - [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price). ### Common Manipulation Vectors -- [ ] The Rate Provider is susceptible to donation attacks. - +- [x] The Rate Provider is susceptible to donation attacks. + - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via + ```solidity + // + function totalAssets() public view virtual override returns (uint256) { + return _asset.balanceOf(address(this)); + } + ``` + which is part of the `totalAssets` used in the `getRate` calculation. ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. From b6bf73a3e5e6a4bc6f3d4ea643c2e8784239acef Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 14:39:08 +0100 Subject: [PATCH 06/12] remove checked by --- erc4626/BeefyWrapperReview.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index f960f7b..f3ae30f 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -2,7 +2,7 @@ ## Details - Reviewed by: @franzns -- Checked by: @danielmkm +- Checked by: - Deployed at: - [sonic:0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a](https://sonicscan.org/address/0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a#code) - Audits: From 8e4be28949cf45960aa1241200f6ba339385d4c4 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 14:50:59 +0100 Subject: [PATCH 07/12] remove daniel --- rate-providers/BeefyUsdcSiloRateprovider.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md index b7def6f..a1e7549 100644 --- a/rate-providers/BeefyUsdcSiloRateprovider.md +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -2,7 +2,7 @@ ## Details - Reviewed by: @franzns -- Checked by: @danielmkm +- Checked by: - Deployed at: - [sonic:0x5fded3206608d3f33175a8865576431906cdb43b](https://sonicscan.org/address/0x5fded3206608d3f33175a8865576431906cdb43b#code) - Audits: From 5859e1f4602edc317c8e96ecfe12c3045c69a999 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 15:29:48 +0100 Subject: [PATCH 08/12] donation vector --- erc4626/BeefyWrapperReview.md | 25 ++++++++++++++--- .../AffineLiquidRestakingRateProvider.md | 28 +++++++++++++++---- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index f3ae30f..5815885 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -29,14 +29,31 @@ If none of these is checked, then this might be a pretty great ERC4626! If any o ### Common Manipulation Vectors - [x] The Rate Provider is susceptible to donation attacks. - - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via + - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `getRate` calculation. + ```solidity - // + /** + * @notice Fetches the total assets held by the vault + * @dev Returns the total assets held by the vault, not only the wrapper + * @return totalAssets the total balance of assets held by the vault + */ function totalAssets() public view virtual override returns (uint256) { - return _asset.balanceOf(address(this)); + return IVault(vault).balance(); } ``` - which is part of the `totalAssets` used in the `getRate` calculation. + The vault calculates it based on underlying balance inside the vault plus the balance inside the strategy. + ```solidity + /** + * @dev It calculates the total underlying value of {token} held by the system. + * It takes into account the vault contract balance, the strategy contract balance + * and the balance deployed in other contracts as part of the strategy. + */ + function balance() public view returns (uint) { + return want().balanceOf(address(this)) + IStrategyV7(strategy).balanceOf(); + } + ``` + + The underlying balance can be inflated by donating underlying assets to the vault. ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. diff --git a/rate-providers/AffineLiquidRestakingRateProvider.md b/rate-providers/AffineLiquidRestakingRateProvider.md index fb4794e..be4b072 100644 --- a/rate-providers/AffineLiquidRestakingRateProvider.md +++ b/rate-providers/AffineLiquidRestakingRateProvider.md @@ -47,14 +47,32 @@ If none of these is checked, then this might be a pretty great Rate Provider! If ### Common Manipulation Vectors - [x] The Rate Provider is susceptible to donation attacks. - - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via + - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `getRate` calculation. + ```solidity - // - function vaultAssets() public view returns (uint256) { - return IERC20MetadataUpgradeable(asset()).balanceOf(address(this)); + /** + * @notice Fetches the total assets held by the vault + * @dev Returns the total assets held by the vault, not only the wrapper + * @return totalAssets the total balance of assets held by the vault + */ + function totalAssets() public view virtual override returns (uint256) { + return IVault(vault).balance(); } ``` - which is part of the `totalAssets` used in the `getRate` calculation. + The vault calculates it based on underlying balance inside the vault plus the balance inside the strategy. + ```solidity + /** + * @dev It calculates the total underlying value of {token} held by the system. + * It takes into account the vault contract balance, the strategy contract balance + * and the balance deployed in other contracts as part of the strategy. + */ + function balance() public view returns (uint) { + return want().balanceOf(address(this)) + IStrategyV7(strategy).balanceOf(); + } + ``` + + The underlying balance can be inflated by donating underlying assets to the vault. + ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. From 6b02f6f59cc809b6ff096fa5434f9459e968f0fd Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 15:32:58 +0100 Subject: [PATCH 09/12] adapt context for 4626 --- erc4626/BeefyWrapperReview.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index 5815885..5ab415f 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -28,8 +28,8 @@ If none of these is checked, then this might be a pretty great ERC4626! If any o ### Common Manipulation Vectors -- [x] The Rate Provider is susceptible to donation attacks. - - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `getRate` calculation. +- [x] The ERC4626 Vault is susceptible to donation attacks. + - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `converToAssets` calculation. ```solidity /** From c7236359abab9ae66bdb067009f23c0365e2029a Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 15:36:08 +0100 Subject: [PATCH 10/12] refine --- rate-providers/AffineLiquidRestakingRateProvider.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rate-providers/AffineLiquidRestakingRateProvider.md b/rate-providers/AffineLiquidRestakingRateProvider.md index be4b072..0333f9e 100644 --- a/rate-providers/AffineLiquidRestakingRateProvider.md +++ b/rate-providers/AffineLiquidRestakingRateProvider.md @@ -47,7 +47,7 @@ If none of these is checked, then this might be a pretty great Rate Provider! If ### Common Manipulation Vectors - [x] The Rate Provider is susceptible to donation attacks. - - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `getRate` calculation. + - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `converToAsset` call and therefore in the `getRate` calculation. ```solidity /** From 41c80377e1c9ece0754966b2af27d18ba7d3de04 Mon Sep 17 00:00:00 2001 From: franz Date: Fri, 24 Jan 2025 15:38:08 +0100 Subject: [PATCH 11/12] adjust --- .../AffineLiquidRestakingRateProvider.md | 28 ++++--------------- rate-providers/BeefyUsdcSiloRateprovider.md | 25 ++++++++++++++--- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/rate-providers/AffineLiquidRestakingRateProvider.md b/rate-providers/AffineLiquidRestakingRateProvider.md index 0333f9e..fb4794e 100644 --- a/rate-providers/AffineLiquidRestakingRateProvider.md +++ b/rate-providers/AffineLiquidRestakingRateProvider.md @@ -47,32 +47,14 @@ If none of these is checked, then this might be a pretty great Rate Provider! If ### Common Manipulation Vectors - [x] The Rate Provider is susceptible to donation attacks. - - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `converToAsset` call and therefore in the `getRate` calculation. - + - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via ```solidity - /** - * @notice Fetches the total assets held by the vault - * @dev Returns the total assets held by the vault, not only the wrapper - * @return totalAssets the total balance of assets held by the vault - */ - function totalAssets() public view virtual override returns (uint256) { - return IVault(vault).balance(); + // + function vaultAssets() public view returns (uint256) { + return IERC20MetadataUpgradeable(asset()).balanceOf(address(this)); } ``` - The vault calculates it based on underlying balance inside the vault plus the balance inside the strategy. - ```solidity - /** - * @dev It calculates the total underlying value of {token} held by the system. - * It takes into account the vault contract balance, the strategy contract balance - * and the balance deployed in other contracts as part of the strategy. - */ - function balance() public view returns (uint) { - return want().balanceOf(address(this)) + IStrategyV7(strategy).balanceOf(); - } - ``` - - The underlying balance can be inflated by donating underlying assets to the vault. - + which is part of the `totalAssets` used in the `getRate` calculation. ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md index a1e7549..8dc2e29 100644 --- a/rate-providers/BeefyUsdcSiloRateprovider.md +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -34,14 +34,31 @@ If none of these is checked, then this might be a pretty great Rate Provider! If ### Common Manipulation Vectors - [x] The Rate Provider is susceptible to donation attacks. - - comment: The rate can be influenced by donating to the vault as the vault's total assets are measured via + - comment: The ERC4626 wrapper calls the vaults balance for totalAssets() which is part of the `totalAssets` used in the `converToAssets` call and therefore in the `getRate` calculation. + ```solidity - // + /** + * @notice Fetches the total assets held by the vault + * @dev Returns the total assets held by the vault, not only the wrapper + * @return totalAssets the total balance of assets held by the vault + */ function totalAssets() public view virtual override returns (uint256) { - return _asset.balanceOf(address(this)); + return IVault(vault).balance(); } ``` - which is part of the `totalAssets` used in the `getRate` calculation. + The vault calculates it based on underlying balance inside the vault plus the balance inside the strategy. + ```solidity + /** + * @dev It calculates the total underlying value of {token} held by the system. + * It takes into account the vault contract balance, the strategy contract balance + * and the balance deployed in other contracts as part of the strategy. + */ + function balance() public view returns (uint) { + return want().balanceOf(address(this)) + IStrategyV7(strategy).balanceOf(); + } + ``` + + The underlying balance can be inflated by donating underlying assets to the vault. ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. From e90910ec5341a122123db2292b354eaae815bc0b Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 24 Jan 2025 15:50:30 +0100 Subject: [PATCH 12/12] Add checked by --- erc4626/BeefyWrapperReview.md | 4 +--- rate-providers/BeefyUsdcSiloRateprovider.md | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/erc4626/BeefyWrapperReview.md b/erc4626/BeefyWrapperReview.md index 5ab415f..51c190c 100644 --- a/erc4626/BeefyWrapperReview.md +++ b/erc4626/BeefyWrapperReview.md @@ -2,7 +2,7 @@ ## Details - Reviewed by: @franzns -- Checked by: +- Checked by: @danielmkm - Deployed at: - [sonic:0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a](https://sonicscan.org/address/0x7870ddFd5ACA4E977B2287e9A212bcbe8FC4135a#code) - Audits: @@ -58,8 +58,6 @@ If none of these is checked, then this might be a pretty great ERC4626! If any o ## Additional Findings To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users. -The Wrapper is a minimal proxy which means that it has a hardcoded implementation address so by definition nothing can be upgraded. - ## Conclusion **Summary judgment: USABLE** diff --git a/rate-providers/BeefyUsdcSiloRateprovider.md b/rate-providers/BeefyUsdcSiloRateprovider.md index 8dc2e29..c297d8b 100644 --- a/rate-providers/BeefyUsdcSiloRateprovider.md +++ b/rate-providers/BeefyUsdcSiloRateprovider.md @@ -2,7 +2,7 @@ ## Details - Reviewed by: @franzns -- Checked by: +- Checked by: @danielmkm - Deployed at: - [sonic:0x5fded3206608d3f33175a8865576431906cdb43b](https://sonicscan.org/address/0x5fded3206608d3f33175a8865576431906cdb43b#code) - Audits: