Sparrow dom/balancer amo

contracts/contracts/proxies/Proxies.sol

@@ -204,3 +204,10 @@ contract ConvexFrxEthWethStrategyProxy is
 contract ConvexFrxETHAMOStrategyProxy is InitializeGovernedUpgradeabilityProxy {
 
 }
+
+/**
+ * @notice BalancerEthAMOStrategyProxy delegates calls to a BalancerEthAMOStrategy implementation
+ */
+contract BalancerEthAMOStrategyProxy is InitializeGovernedUpgradeabilityProxy {
+
+}

contracts/contracts/strategies/FraxETHStrategy.sol

@@ -68,7 +68,6 @@ contract FraxETHStrategy is Generalized4626Strategy {
 
     /**
      * @dev Retuns bool indicating whether asset is supported by strategy
-     * @param _asset Address of the asset
      */
     function supportsAsset(address _asset) public view override returns (bool) {
         return _asset == address(assetToken) || _asset == weth;

contracts/contracts/strategies/amo/BalancerEthAMOStrategy.sol

@@ -9,6 +9,7 @@ pragma solidity ^0.8.0;
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import { BaseBalancerAMOStrategy } from "./BaseBalancerAMOStrategy.sol";
+import { VaultReentrancyLib } from "../balancer/VaultReentrancyLib.sol";
 import { IRateProvider } from "../../interfaces/balancer/IRateProvider.sol";
 import { IERC4626 } from "../../../lib/openzeppelin/interfaces/IERC4626.sol";
 import { StableMath } from "../../utils/StableMath.sol";
@@ -94,8 +95,9 @@ contract BalancerEthAMOStrategy is BaseBalancerAMOStrategy {
      * @param _asset  Address of the Vault asset. eg WETH
      * @return balance  the amount of vault assets
      *
-     * IMPORTANT if this function is overridden it needs to have a whenNotInBalancerVaultContext
-     * modifier on it or it is susceptible to read-only re-entrancy attack
+     * IMPORTANT if this function is overridden it needs to do a call to:
+     *  - VaultReentrancyLib.ensureNotInVaultContext(balancerVault);
+     * to prevent a read only re-entrancy vulnerability.
      *
      * @dev it is important that this function is not affected by reporting inflated
      * values of assets in case of any pool manipulation. Such a manipulation could easily
@@ -112,6 +114,18 @@ contract BalancerEthAMOStrategy is BaseBalancerAMOStrategy {
         override
         returns (uint256 balance)
     {
+        /**
+         * @dev Ensure we are not in a Vault context when this function is called, by attempting a no-op internal
+         * balance operation. If we are already in a Vault transaction (e.g., a swap, join, or exit), the Vault's
+         * reentrancy protection will cause this function to revert.
+         *
+         * Use this with any function that can cause a state change in a pool and is either public itself,
+         * or called by a public function *outside* a Vault operation (e.g., join, exit, or swap).
+         *
+         * This is to protect against Balancer's read-only re-entrancy vulnerability:
+         * https://www.notion.so/originprotocol/Balancer-read-only-reentrancy-c686e72c82414ef18fa34312bb02e11b
+         */
+        VaultReentrancyLib.ensureNotInVaultContext(balancerVault);
         require(_asset == address(asset), "Unsupported asset");
 
         uint256 bptBalance = IERC4626(auraRewardPool).maxRedeem(address(this));

contracts/contracts/strategies/amo/BaseAMOStrategy.sol

@@ -35,6 +35,22 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
     // for OUSD/3CRV, 3CRV = 1
     // for frxETH/OUSD, frxETH = 0
     uint128 public immutable assetCoinIndex;
+    // Weight of the oToken in the pool. Denominated in 1e18
+    // for OETH/ETH = 50% (0.5 * 1e18)
+    // for OUSD/3CRV = 50% (0.5 * 1e18)
+    // for OETH/WETH Balancer = 80% (0.8 * 1e18)
+    uint256 public immutable oTokenWeight;
+    // Weight of the asset in the pool. Denominated in 1e18
+    // for OETH/ETH = 50% (0.5 * 1e18)
+    // for OUSD/3CRV = 50% (0.5 * 1e18)
+    // for OETH/WETH Balancer = 20% (0.2 * 1e18)
+    uint256 public immutable assetWeight;
+
+    /// @dev Verifies that the caller is the Strategist
+    modifier onlyStrategist() {
+        callerNotStrategist();
+        _;
+    }
 
     /// @notice Validates the vault asset is supported by this strategy.
     modifier onlyAsset(address _vaultAsset) {
@@ -48,33 +64,70 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         _;
     }
 
-    /// @dev Verifies that the caller is the Strategist
-    modifier onlyStrategist() {
+    modifier improvePoolBalance() {
+        int256 diffBefore = improvePoolBalancePreCheck();
+        _;
+        improvePoolBalancePostCheck(diffBefore);
+    }
+
+    /// @dev This is separate internal function to save on contract deployment size
+    function callerNotStrategist() internal view {
         require(
             msg.sender == IVault(vaultAddress).strategistAddr(),
             "Caller is not the Strategist"
         );
-        _;
     }
 
     /**
      * @dev Checks the AMO pool's balances have improved and the balances
-     * have not tipped to the other side.
+     * have not tipped to the other side. Also This is separate internal function
+     * to save on contract deployment size
+     * 
      */
-    modifier improvePoolBalance() {
+    function improvePoolBalancePreCheck()
+        internal
+        view
+        returns (int256 diffBefore)
+    {
         // Get the asset and OToken balances in the AMO pool
         uint256[2] memory balancesBefore = _getBalances();
-        // diff = asset balance - OToken balance
-        int256 diffBefore = int256(balancesBefore[assetCoinIndex]) -
-            int256(balancesBefore[oTokenCoinIndex]);
-
-        _;
+        // get value of assets normalized to oTokens / units
+        uint256 assetUnitBalanceBefore = _toOTokens(
+            balancesBefore[assetCoinIndex]
+        );
+        // oTokens are already normalized to units
+        uint256 totalBalanceBefore = assetUnitBalanceBefore +
+            balancesBefore[oTokenCoinIndex];
+
+        // int256 will throw an exception in case of overflow
+        diffBefore =
+            // to get the diff add up the asset balance from the ideal pool asset balance...
+            int256(assetUnitBalanceBefore) -
+            int256(totalBalanceBefore.mulTruncate(assetWeight)) +
+            // ... and oToken balance from the ideal pool oToken balance
+            int256(balancesBefore[oTokenCoinIndex]) -
+            int256(totalBalanceBefore.mulTruncate(oTokenWeight));
+    }
 
+    /// @dev This is separate internal function to save on contract deployment size
+    function improvePoolBalancePostCheck(int256 diffBefore) internal view {
         // Get the asset and OToken balances in the AMO pool
         uint256[2] memory balancesAfter = _getBalances();
-        // diff = asset balance - OToken balance
-        int256 diffAfter = int256(balancesAfter[assetCoinIndex]) -
-            int256(balancesAfter[oTokenCoinIndex]);
+        // get value of assets normalized to oTokens / units
+        uint256 assetUnitBalanceAfter = _toOTokens(
+            balancesAfter[assetCoinIndex]
+        );
+        // oTokens are already normalized to units
+        uint256 totalBalanceAfter = assetUnitBalanceAfter +
+            balancesAfter[oTokenCoinIndex];
+
+        // int256 will throw an exception in case of overflow
+        int256 diffAfter = // to get the diff add up the asset balance from the ideal pool asset balance...
+            int256(totalBalanceAfter) -
+                int256(totalBalanceAfter.mulTruncate(assetWeight)) +
+                // ... and oToken balance from the ideal pool oToken balance
+                int256(balancesAfter[oTokenCoinIndex]) -
+                int256(totalBalanceAfter.mulTruncate(oTokenWeight));
 
         if (diffBefore <= 0) {
             // If the pool was originally imbalanced in favor of the OToken, then
@@ -94,8 +147,10 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
     struct AMOConfig {
         address oTokenAddress; // Address of the OToken. eg OETH or OUSD
         address assetAddress; // Address of the asset token. eg WETH, frxETH or 3CRV
-        uint128 oTokenCoinIndex;
-        uint128 assetCoinIndex;
+        uint128 oTokenCoinIndex; // index of OToken in the pool
+        uint128 assetCoinIndex; // index of the other asset in the pool
+        uint256 oTokenWeight; // oToken share of the pool the strategy considers balanced
+        uint256 assetWeight; // asset share of the pool the strategy considers balanced
     }
 
     constructor(
@@ -108,6 +163,8 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         asset = IERC20(_amoConfig.assetAddress);
         oTokenCoinIndex = _amoConfig.oTokenCoinIndex;
         assetCoinIndex = _amoConfig.assetCoinIndex;
+        oTokenWeight = _amoConfig.oTokenWeight;
+        assetWeight = _amoConfig.assetWeight;
     }
 
     /***************************************
@@ -116,7 +173,7 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
 
     /// @dev Validates the vault asset is supported by this strategy.
     /// The default implementation is the vault asset matches the pool asset.
-    /// This needs to be overriden for OUSD AMO as the vault assets are DAI, USDC and USDT
+    /// This needs to be overridden for OUSD AMO as the vault assets are DAI, USDC and USDT
     /// while the pool asset is 3CRV.
     /// @param _vaultAsset Address of the vault asset
     function _isVaultAsset(address _vaultAsset)
@@ -150,11 +207,11 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
     /// @dev Converts Vault assets to a pool assets.
     /// @param vaultAsset The address of the Vault asset to convert. eg WETH, frxETH, DAI
     /// @param vaultAssetAmount The amount of vault assets to convert.
-    /// @return poolAssets The amount of pool assets. eg ETH, frxETH or 3CRV
+    /// @return poolAssetAmount The amount of pool assets. eg ETH, frxETH, 3CRV
     function _toPoolAsset(address vaultAsset, uint256 vaultAssetAmount)
         internal
         virtual
-        returns (uint256 poolAssets);
+        returns (uint256 poolAssetAmount);
 
     /// @dev Calculates the required amount of pool assets to be removed from
     /// the pool in order to get the specified amount of vault assets.
@@ -164,10 +221,12 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         virtual
         returns (uint256 poolAssets);
 
-    /// @dev Convert pool asset amount to an oToken amount.
+    /// @dev Convert pool asset amount to an oToken amount in other words normalizing the asset
+    /// amount to units - dollars in OUSD and ether in OETH.
     /// @param poolAssetAmount The amount of pool assets to convert. eg ETH, 3CRV or frxETH
     function _toOTokens(uint256 poolAssetAmount)
         internal
+        view
         virtual
         returns (uint256 oTokenAmount);
 
@@ -285,9 +344,11 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         external
         override
         onlyVault
-        onlyAsset(_vaultAsset)
         nonReentrant
     {
+        // checks that it is a supported asset
+        require(_isVaultAsset(_vaultAsset), "Unsupported asset");
+
         emit Deposit(_vaultAsset, address(lpToken), _vaultAssetAmount);
         uint256 poolAssetAmount = _toPoolAsset(_vaultAsset, _vaultAssetAmount);
         _deposit(poolAssetAmount);
@@ -305,7 +366,6 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         uint256[] memory _vaultAssetAmounts
     ) external virtual onlyVault onlyAssets(_vaultAssets) nonReentrant {
         // validate the number of assets matches the number of amounts
-        // The onlyAssets modified ensures the correct number of assets are supported.
         // Most AMOs will be just one asset but for OUSD's 3CRV AMO it will be 3 assets.
         require(
             _vaultAssets.length == _vaultAssetAmounts.length,
@@ -407,7 +467,8 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         address _recipient,
         address _vaultAsset,
         uint256 _vaultAssetAmount
-    ) external override onlyVault onlyAsset(_vaultAsset) nonReentrant {
+    ) external override onlyAsset(_vaultAsset) onlyVault nonReentrant {
+        // Ensures that the asset is supported.
         _withdraw(_recipient, _vaultAsset, _vaultAssetAmount);
     }
 
@@ -426,7 +487,6 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
         uint256[] memory _vaultAssetAmounts
     ) external virtual onlyVault onlyAssets(_vaultAssets) nonReentrant {
         // validate the number of assets matches the number of amounts
-        // The onlyAssets modified ensures the correct number of assets are supported.
         // Most AMOs will be just one asset but for OUSD's 3CRV AMO it will be 3 assets.
         require(
             _vaultAssets.length == _vaultAssetAmounts.length,
@@ -547,12 +607,7 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
      * The asset value of the strategy and vault is increased.
      * @param _oTokens The amount of OTokens to be minted and added to the pool.
      */
-    function mintAndAddOTokens(uint256 _oTokens)
-        external
-        onlyStrategist
-        nonReentrant
-        improvePoolBalance
-    {
+    function mintAndAddOTokens(uint256 _oTokens) onlyStrategist improvePoolBalance external nonReentrant {
         IVault(vaultAddress).mintForStrategy(_oTokens);
 
         uint256[2] memory _amounts;
@@ -583,12 +638,7 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
      * The asset value of the strategy and vault is reduced.
      * @param _lpTokens The amount of AMO pool LP tokens to be burned for OTokens.
      */
-    function removeAndBurnOTokens(uint256 _lpTokens)
-        external
-        onlyStrategist
-        nonReentrant
-        improvePoolBalance
-    {
+    function removeAndBurnOTokens(uint256 _lpTokens) improvePoolBalance onlyStrategist external nonReentrant {
         // Withdraw AMO pool LP tokens from the rewards pool and remove OTokens from the AMO pool
         uint256 oTokenToBurn = _withdrawAndRemoveFromPool(
             _lpTokens,
@@ -618,12 +668,7 @@ abstract contract BaseAMOStrategy is InitializableAbstractStrategy {
      * is a gas intensive process. It's easier for the trusted strategist to
      * caclulate the amount of AMO pool LP tokens required off-chain.
      */
-    function removeOnlyAssets(uint256 _lpTokens)
-        external
-        onlyStrategist
-        nonReentrant
-        improvePoolBalance
-    {
+    function removeOnlyAssets(uint256 _lpTokens) onlyStrategist improvePoolBalance external nonReentrant {
         // Withdraw AMO pool LP tokens from rewards pool and remove asset from the AMO pool
         _withdrawAndRemoveFromPool(_lpTokens, address(asset));
 

contracts/contracts/strategies/amo/BaseBalancerAMOStrategy.sol

@@ -78,6 +78,7 @@ abstract contract BaseBalancerAMOStrategy is BaseAMOStrategy {
         uint256 minMintAmount
     ) internal override returns (uint256 lpDeposited) {
         // TODO do we need to check if the tokens in the pool have changed?
+        // TODO move this to initialize these values don't ever change
         (IERC20[] memory tokens, , ) = balancerVault.getPoolTokens(
             balancerPoolId
         );

contracts/contracts/strategies/amo/ConvexFrxETHAMOStrategy.sol

@@ -94,9 +94,10 @@ contract ConvexFrxETHAMOStrategy is BaseConvexAMOStrategy {
         public
         view
         override
-        onlyAsset(_asset)
         returns (uint256 balance)
     {
+        require(_isVaultAsset(_asset), "Unsupported asset");
+
         // Get the Curve LP tokens staked in the Convex pool
         uint256 curveLpTokens = cvxRewardStaker.balanceOf(address(this));
 

contracts/contracts/strategies/amo/ConvexOUSDMetaStrategy.sol

@@ -185,7 +185,7 @@ contract ConvexOUSDMetaStrategy is BaseConvexAMOStrategy {
         uint256[3] memory _amounts = [uint256(0), uint256(0), uint256(0)];
         _amounts[coinIndex] = _vaultAssetAmount;
 
-        // 3Pool LP required when removing reuiqred vault assets ignoring fees
+        // 3Pool LP required when removing required vault assets ignoring fees
         uint256 lpRequiredNoFees = curve3Pool.calc_token_amount(
             _amounts,
             false
@@ -344,7 +344,7 @@ contract ConvexOUSDMetaStrategy is BaseConvexAMOStrategy {
         address,
         address[] memory _vaultAssets,
         uint256[] memory
-    ) external override onlyVault onlyAssets(_vaultAssets) nonReentrant {
+    ) external override onlyAssets(_vaultAssets) onlyVault nonReentrant {
         // TODO add support for withdrawing multiple 3Pool assets
         revert("Not supported");
     }
@@ -423,6 +423,7 @@ contract ConvexOUSDMetaStrategy is BaseConvexAMOStrategy {
         onlyAsset(_asset)
         returns (uint256 balance)
     {
+
         // 3Pool LP tokens (3Crv) in this strategy contract.
         // This should generally be nothing as we should always stake
         // the full balance in the Gauge, but include for safety

contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol

@@ -70,6 +70,8 @@ contract BalancerMetaPoolStrategy is BaseAuraStrategy {
                 address(this)
             );
         }
+
+        // TODO Make sure all assets are in the correct order
         _deposit(strategyAssets, strategyAmounts);
     }
 
@@ -89,6 +91,7 @@ contract BalancerMetaPoolStrategy is BaseAuraStrategy {
             "Array length missmatch"
         );
 
+        // TODO move this to initialize, this doesn't change
         (IERC20[] memory tokens, , ) = balancerVault.getPoolTokens(
             balancerPoolId
         );
@@ -127,6 +130,8 @@ contract BalancerMetaPoolStrategy is BaseAuraStrategy {
             // Convert IERC20 type to address
             poolAssets[i] = address(tokens[i]);
 
+            // TODO remove nested loop when assets in correct order
+
             // For each of the mapped assets
             for (uint256 j = 0; j < strategyAssetsToPoolAssets.length; ++j) {
                 // If the pool asset is the same as the mapped asset

contracts/deploy/071_balancer_rETH_WETH.js

@@ -66,7 +66,7 @@ module.exports = deploymentWithGovernanceProposal(
     );
 
     // 3. Encode the init data
-    const initFunction = "initialize(address[],address[],address[])";
+    const initFunction = "initialize(address[])"; // [_rewardTokenAddresses]
     const initData = cOETHBalancerMetaPoolStrategy.interface.encodeFunctionData(
       initFunction,
       [

contracts/deploy/078_frax_amo.js

@@ -59,6 +59,8 @@ module.exports = deploymentWithGovernanceProposal(
           addresses.mainnet.frxETH, // assetAddress (frxETH)
           1, // Curve pool index for OToken OETH
           0, // Curve pool index for asset frxETH
+          `${0.5 * 1e18}`, // oToken weight
+          `${0.5 * 1e18}`, // asset weight
         ],
         [
           addresses.mainnet.CVXBooster, // cvxDepositorAddress,