Rebase ComposableStablePool

pkg/balancer-js/src/utils/errors.ts

@@ -85,6 +85,7 @@ const balancerErrorCodes: Record<string, string> = {
   '355': 'INVALID_CIRCUIT_BREAKER_BOUNDS',
   '356': 'CIRCUIT_BREAKER_TRIPPED',
   '357': 'MALICIOUS_QUERY_REVERT',
+  '358': 'UNHANDLED_BY_STABLE_POOL',
   '400': 'REENTRANCY',
   '401': 'SENDER_NOT_ALLOWED',
   '402': 'PAUSED',

pkg/interfaces/contracts/solidity-utils/helpers/BalancerErrors.sol

@@ -198,6 +198,7 @@ library Errors {
     uint256 internal constant INVALID_CIRCUIT_BREAKER_BOUNDS = 355;
     uint256 internal constant CIRCUIT_BREAKER_TRIPPED = 356;
     uint256 internal constant MALICIOUS_QUERY_REVERT = 357;
+    uint256 internal constant UNHANDLED_BY_STABLE_POOL = 358;
 
     // Lib
     uint256 internal constant REENTRANCY = 400;

pkg/pool-stable/contracts/ComposableStablePoolProtocolFees.sol

@@ -307,20 +307,14 @@ abstract contract ComposableStablePoolProtocolFees is
     }
 
     /**
-     * @dev Inheritance rules still require us to override this in the most derived contract, even though
+     * @dev Inheritance rules still require us to override this in this derived contract, even though
      * it only calls super.
      */
     function _isOwnerOnlyAction(bytes32 actionId)
         internal
         view
         virtual
-        override(
-            // Our inheritance pattern creates a small diamond that requires explicitly listing the parents here.
-            // Each parent calls the `super` version, so linearization ensures all implementations are called.
-            BasePool,
-            BasePoolAuthorization,
-            ComposableStablePoolRates
-        )
+        override(BasePoolAuthorization, ComposableStablePoolRates)
         returns (bool)
     {
         return super._isOwnerOnlyAction(actionId);

pkg/pool-stable/contracts/ComposableStablePoolRates.sol

@@ -13,13 +13,15 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 pragma solidity ^0.7.0;
+pragma experimental ABIEncoderV2;
 
 import "@balancer-labs/v2-interfaces/contracts/solidity-utils/openzeppelin/IERC20.sol";
 import "@balancer-labs/v2-interfaces/contracts/pool-utils/IRateProvider.sol";
 
 import "@balancer-labs/v2-solidity-utils/contracts/helpers/ERC20Helpers.sol";
 import "@balancer-labs/v2-solidity-utils/contracts/helpers/InputHelpers.sol";
 import "@balancer-labs/v2-pool-utils/contracts/rates/PriceRateCache.sol";
+import "@balancer-labs/v2-pool-utils/contracts/lib/PoolRegistrationLib.sol";
 
 import "./ComposableStablePoolStorage.sol";
 
@@ -41,7 +43,7 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
     // [ uint32  |  uint32  |  uint96  |   uint96     ]
 
     // Since we never need just one cache but all of them at once, instead of making the mapping go from token address
-    // to cache, we go from token index (including BPT), i.e. an array. We use a mapping however instead of a native
+    // to cache, we go from token index (not including BPT), i.e. an array. We use a mapping however instead of a native
     // array to skip the extra read associated with the out-of-bounds check, as we have cheaper ways to guarantee the
     // indices are valid.
     mapping(uint256 => bytes32) internal _tokenRateCaches;
@@ -56,61 +58,41 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
             rateParams.tokenRateCacheDurations.length
         );
 
-        IERC20[] memory registeredTokens = _insertSorted(rateParams.tokens, IERC20(this));
-        uint256 bptIndex;
-        for (
-            bptIndex = registeredTokens.length - 1;
-            bptIndex > 0 && registeredTokens[bptIndex] > IERC20(this);
-            bptIndex--
-        ) {
-            // solhint-disable-previous-line no-empty-blocks
-        }
-
-        uint256 skipBpt = 0;
         for (uint256 i = 0; i < rateParams.tokens.length; i++) {
-            if (i == bptIndex) {
-                skipBpt = 1;
-            }
-
-            uint256 k = i + skipBpt;
             if (rateParams.rateProviders[i] != IRateProvider(0)) {
-                _updateTokenRateCache(k, rateParams.rateProviders[i], rateParams.tokenRateCacheDurations[i]);
+                _updateTokenRateCache(i, rateParams.rateProviders[i], rateParams.tokenRateCacheDurations[i]);
 
-                emit TokenRateProviderSet(k, rateParams.rateProviders[i], rateParams.tokenRateCacheDurations[i]);
+                emit TokenRateProviderSet(i, rateParams.rateProviders[i], rateParams.tokenRateCacheDurations[i]);
 
                 // Initialize the old rates as well, in case they are referenced before the first join.
-                _updateOldRate(k);
+                _updateOldRate(i);
             }
         }
     }
 
     /**
-     * @dev Updates the old rate for the token at `index` (including BPT). Assumes `index` is valid.
+     * @dev Updates the old rate for the token at `poolTokenIndex` (not including BPT). Assumes index is valid.
      */
-    function _updateOldRate(uint256 index) internal {
-        bytes32 cache = _tokenRateCaches[index];
-        _tokenRateCaches[index] = cache.updateOldRate();
+    function _updateOldRate(uint256 poolTokenIndex) internal {
+        bytes32 cache = _tokenRateCaches[poolTokenIndex];
+        _tokenRateCaches[poolTokenIndex] = cache.updateOldRate();
     }
 
     /**
      * @dev Returns the rate for a given token. All token rates are fixed-point values with 18 decimals.
-     * If there is no rate provider for the provided token, it returns FixedPoint.ONE.
+     * If there is no rate provider for the provided token, or this is called with the pool token, it
+     * returns FixedPoint.ONE. It will revert if called with an invalid token.
      */
     function getTokenRate(IERC20 token) external view returns (uint256) {
-        return _getTokenRate(_getTokenIndex(token));
-    }
-
-    function _getTokenRate(uint256 index) internal view virtual returns (uint256) {
-        // We optimize for the scenario where all tokens have rate providers, except the BPT (which never has a rate
-        // provider). Therefore, we return early if `token` is the BPT, and otherwise optimistically read the cache
-        // expecting that it will not be empty (instead of e.g. fetching the provider to avoid a cache read, since
-        // we don't need the provider at all).
-
-        if (index == getBptIndex()) {
+        if (token == IERC20(this)) {
             return FixedPoint.ONE;
         }
 
-        bytes32 tokenRateCache = _tokenRateCaches[index];
+        return _getTokenRate(_getPoolTokenIndex(token));
+    }
+
+    function _getTokenRate(uint256 poolTokenIndex) internal view virtual returns (uint256) {
+        bytes32 tokenRateCache = _tokenRateCaches[poolTokenIndex];
         return tokenRateCache == bytes32(0) ? FixedPoint.ONE : tokenRateCache.getCurrentRate();
     }
 
@@ -128,7 +110,11 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
             uint256 expires
         )
     {
-        bytes32 cache = _tokenRateCaches[_getTokenIndex(token)];
+        bytes32 cache;
+
+        if (token != IERC20(this)) {
+            cache = _tokenRateCaches[_getPoolTokenIndex(token)];
+        }
 
         // A zero cache indicates that the token doesn't have a rate provider associated with it.
         _require(cache != bytes32(0), Errors.TOKEN_DOES_NOT_HAVE_RATE_PROVIDER);
@@ -144,79 +130,82 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
      * @param duration Number of seconds until the current token rate is fetched again.
      */
     function setTokenRateCacheDuration(IERC20 token, uint256 duration) external authenticate {
-        uint256 index = _getTokenIndex(token);
-        IRateProvider provider = _getRateProvider(index);
-        _require(address(provider) != address(0), Errors.TOKEN_DOES_NOT_HAVE_RATE_PROVIDER);
-        _updateTokenRateCache(index, provider, duration);
-        emit TokenRateProviderSet(index, provider, duration);
+        (uint256 poolTokenIndex, IRateProvider provider) = _getTokenIndexAndRateProvider(token);
+
+        _updateTokenRateCache(poolTokenIndex, provider, duration);
+        emit TokenRateProviderSet(poolTokenIndex, provider, duration);
     }
 
     /**
      * @dev Forces a rate cache hit for a token.
      * It will revert if the requested token does not have an associated rate provider.
      */
     function updateTokenRateCache(IERC20 token) external {
-        uint256 index = _getTokenIndex(token);
+        (uint256 poolTokenIndex, IRateProvider provider) = _getTokenIndexAndRateProvider(token);
+
+        uint256 duration = _tokenRateCaches[poolTokenIndex].getDuration();
+        _updateTokenRateCache(poolTokenIndex, provider, duration);
+    }
+
+    function _getTokenIndexAndRateProvider(IERC20 token) private view returns (uint256, IRateProvider) {
+        uint256 poolTokenIndex;
+        IRateProvider provider;
 
-        IRateProvider provider = _getRateProvider(index);
+        if (token != IERC20(this)) {
+            poolTokenIndex = _getPoolTokenIndex(token);
+            provider = _getRateProvider(poolTokenIndex);
+        }
         _require(address(provider) != address(0), Errors.TOKEN_DOES_NOT_HAVE_RATE_PROVIDER);
-        uint256 duration = _tokenRateCaches[index].getDuration();
-        _updateTokenRateCache(index, provider, duration);
+
+        return (poolTokenIndex, provider);
     }
 
     /**
      * @dev Internal function to update a token rate cache for a known provider and duration.
      * It trusts the given values, and does not perform any checks.
      */
     function _updateTokenRateCache(
-        uint256 index,
+        uint256 poolTokenIndex,
         IRateProvider provider,
         uint256 duration
     ) internal virtual {
         uint256 rate = provider.getRate();
-        bytes32 cache = _tokenRateCaches[index];
+        bytes32 cache = _tokenRateCaches[poolTokenIndex];
 
-        _tokenRateCaches[index] = cache.updateRateAndDuration(rate, duration);
+        _tokenRateCaches[poolTokenIndex] = cache.updateRateAndDuration(rate, duration);
 
-        emit TokenRateCacheUpdated(index, rate);
+        emit TokenRateCacheUpdated(poolTokenIndex, rate);
     }
 
     /**
-     * @dev Caches the rates of all tokens if necessary
+     * @dev Caches the rates of all pool tokens, if necessary.
      */
     function _cacheTokenRatesIfNecessary() internal {
-        uint256 totalTokens = _getTotalTokens();
-        for (uint256 i = 0; i < totalTokens; ++i) {
+        uint256 totaPoollTokens = _getTotalPoolTokens();
+        for (uint256 i = 0; i < totaPoollTokens; ++i) {
             _cacheTokenRateIfNecessary(i);
         }
     }
 
     /**
      * @dev Caches the rate for a token if necessary. It ignores the call if there is no provider set.
      */
-    function _cacheTokenRateIfNecessary(uint256 index) internal {
-        // We optimize for the scenario where all tokens have rate providers, except the BPT (which never has a rate
-        // provider). Therefore, we return early if token is BPT, and otherwise optimistically read the cache expecting
-        // that it will not be empty (instead of e.g. fetching the provider to avoid a cache read in situations where
-        // we might not need the provider if the cache is still valid).
-
-        if (index == getBptIndex()) return;
-
-        bytes32 cache = _tokenRateCaches[index];
+    function _cacheTokenRateIfNecessary(uint256 poolTokenIndex) internal {
+        bytes32 cache = _tokenRateCaches[poolTokenIndex];
         if (cache != bytes32(0)) {
             (uint256 duration, uint256 expires) = cache.getTimestamps();
             if (block.timestamp > expires) {
                 // solhint-disable-previous-line not-rely-on-time
-                _updateTokenRateCache(index, _getRateProvider(index), duration);
+                _updateTokenRateCache(poolTokenIndex, _getRateProvider(poolTokenIndex), duration);
             }
         }
     }
 
     // To compute the yield protocol fees, we need the oldRate for all tokens, even if the exempt flag is not set.
     // We do need to ensure the token has a rate provider before updating; otherwise it will not be in the cache.
     function _updateOldRates() internal {
-        uint256 totalTokens = _getTotalTokens();
-        for (uint256 i = 0; i < totalTokens; ++i) {
+        uint256 totalPoolTokens = _getTotalPoolTokens();
+        for (uint256 i = 0; i < totalPoolTokens; ++i) {
             if (_hasRateProvider(i)) _updateOldRate(i);
         }
     }
@@ -230,14 +219,12 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
         view
         returns (uint256[] memory)
     {
-        uint256 totalTokensWithoutBpt = balances.length;
-        uint256[] memory adjustedBalances = new uint256[](totalTokensWithoutBpt);
-
-        for (uint256 i = 0; i < totalTokensWithoutBpt; ++i) {
-            uint256 skipBptIndex = i >= getBptIndex() ? i + 1 : i;
-            adjustedBalances[i] = _isTokenExemptFromYieldProtocolFee(skipBptIndex) ||
-                (ignoreExemptFlags && _hasRateProvider(skipBptIndex))
-                ? _adjustedBalance(balances[i], _tokenRateCaches[skipBptIndex])
+        uint256 totalPoolTokens = balances.length;
+        uint256[] memory adjustedBalances = new uint256[](totalPoolTokens);
+
+        for (uint256 i = 0; i < totalPoolTokens; ++i) {
+            adjustedBalances[i] = _isTokenExemptFromYieldProtocolFee(i) || (ignoreExemptFlags && _hasRateProvider(i))
+                ? _adjustedBalance(balances[i], _tokenRateCaches[i])
                 : balances[i];
         }
 
@@ -252,15 +239,18 @@ abstract contract ComposableStablePoolRates is ComposableStablePoolStorage {
     // Scaling Factors
 
     /**
-     * @dev Overrides scaling factor getter to compute the tokens' rates.
+     * @dev Overrides scaling factor getter to compute the tokens' rates. This is the rare exception where we
+     * need to include the BPT token.
      */
-    function _scalingFactors() internal view virtual override returns (uint256[] memory) {
-        // There is no need to check the arrays length since both are based on `_getTotalTokens`
-        uint256 totalTokens = _getTotalTokens();
-        uint256[] memory scalingFactors = new uint256[](totalTokens);
-
-        for (uint256 i = 0; i < totalTokens; ++i) {
-            scalingFactors[i] = _getScalingFactor(i).mulDown(_getTokenRate(i));
+    function getScalingFactors() public view virtual override returns (uint256[] memory) {
+        uint256 totalPoolTokens = _getTotalPoolTokens();
+        uint256[] memory scalingFactors = new uint256[](totalPoolTokens + 1);
+        // Set the BPT scaling factor to ONE.
+        scalingFactors[PoolRegistrationLib.COMPOSABLE_BPT_INDEX] = FixedPoint.ONE;
+
+        // Set the scaling factors of the pool tokens.
+        for (uint256 i = 0; i < totalPoolTokens; ++i) {
+            scalingFactors[i + 1] = _getScalingFactor(i).mulDown(_getTokenRate(i));
         }
 
         return scalingFactors;