Merge pull request #143 from BeanstalkFarms/s2-upgr-well-remediations

Stable2 Well Function and Upgradeable Well Remediations.

.gitignore

@@ -2,6 +2,10 @@
 cache/
 out/
 
+# Python virtual environments
+env/
+venv/
+
 .vscode
 
 # Ignores development broadcast logs

foundry.toml

@@ -4,7 +4,7 @@ out = 'out'
 libs = ['lib', 'node_modules']
 fuzz = { runs = 256 }
 optimizer = true
-optimizer_runs = 400
+optimizer_runs = 200
 remappings = [
   '@openzeppelin/=node_modules/@openzeppelin/',
 ]
@@ -27,7 +27,6 @@ ignore = ["src/libraries/LibClone.sol", "src/utils/Clone.sol", "src/libraries/AB
 int_types = "long"
 line_length = 120
 multiline_func_header = "params_first"
-number_underscore = "thousands"
 override_spacing = false
 quote_style = "double"
 tab_width = 4

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@beanstalk/wells",
-  "version": "1.2.0-prerelease1",
+  "version": "1.2.0-prerelease3",
   "description": "A [{Well}](/src/Well.sol) is a constant function AMM that allows the provisioning of liquidity into a single pooled on-chain liquidity position.",
   "main": "index.js",
   "directories": {

src/WellUpgradeable.sol

@@ -17,15 +17,13 @@ contract WellUpgradeable is Well, UUPSUpgradeable, OwnableUpgradeable {
     address private immutable ___self = address(this);
 
     /**
-     * @notice verifies that the execution is called through an minimal proxy or is not a delegate call.
+     * @notice Verifies that the execution is called through an minimal proxy.
      */
     modifier notDelegatedOrIsMinimalProxy() {
         if (address(this) != ___self) {
             address aquifer = aquifer();
             address wellImplmentation = IAquifer(aquifer).wellImplementation(address(this));
             require(wellImplmentation == ___self, "Function must be called by a Well bored by an aquifer");
-        } else {
-            revert("UUPSUpgradeable: must not be called through delegatecall");
         }
         _;
     }
@@ -62,7 +60,7 @@ contract WellUpgradeable is Well, UUPSUpgradeable, OwnableUpgradeable {
      * @notice Check that the execution is being performed through a delegatecall call and that the execution context is
      * a proxy contract with an ERC1167 minimal proxy from an aquifier, pointing to a well implmentation.
      */
-    function _authorizeUpgrade(address newImplmentation) internal view override {
+    function _authorizeUpgrade(address newImplementation) internal view override onlyOwner {
         // verify the function is called through a delegatecall.
         require(address(this) != ___self, "Function must be called through delegatecall");
 
@@ -73,13 +71,21 @@ contract WellUpgradeable is Well, UUPSUpgradeable, OwnableUpgradeable {
 
         // verify the new implmentation is a well bored by an aquifier.
         require(
-            IAquifer(aquifer).wellImplementation(newImplmentation) != address(0),
+            IAquifer(aquifer).wellImplementation(newImplementation) != address(0),
             "New implementation must be a well implmentation"
         );
 
+        // verify the new well uses the same tokens in the same order.
+        IERC20[] memory _tokens = tokens();
+        IERC20[] memory newTokens = WellUpgradeable(newImplementation).tokens();
+        require(_tokens.length == newTokens.length, "New well must use the same number of tokens");
+        for (uint256 i; i < _tokens.length; ++i) {
+            require(_tokens[i] == newTokens[i], "New well must use the same tokens in the same order");
+        }
+
         // verify the new implmentation is a valid ERC-1967 implmentation.
         require(
-            UUPSUpgradeable(newImplmentation).proxiableUUID() == _IMPLEMENTATION_SLOT,
+            UUPSUpgradeable(newImplementation).proxiableUUID() == _IMPLEMENTATION_SLOT,
             "New implementation must be a valid ERC-1967 implmentation"
         );
     }
@@ -115,7 +121,7 @@ contract WellUpgradeable is Well, UUPSUpgradeable, OwnableUpgradeable {
      * are ERC-1167 minimal immutable clones and cannot delgate to another proxy. Thus, `proxiableUUID` was updated to support
      * this specific usecase.
      */
-    function proxiableUUID() external view override notDelegatedOrIsMinimalProxy returns (bytes32) {
+    function proxiableUUID() public view override notDelegatedOrIsMinimalProxy returns (bytes32) {
         return _IMPLEMENTATION_SLOT;
     }
 

src/functions/Stable2.sol

@@ -26,6 +26,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
     struct PriceData {
         uint256 targetPrice;
         uint256 currentPrice;
+        uint256 newPrice;
         uint256 maxStepSize;
         ILookupTable.PriceData lutData;
     }
@@ -41,7 +42,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
 
     // price threshold. more accurate pricing requires a lower threshold,
     // at the cost of higher execution costs.
-    uint256 constant PRICE_THRESHOLD = 100; // 0.01%
+    uint256 constant PRICE_THRESHOLD = 10; // 0.001%
 
     address immutable lookupTable;
     uint256 immutable a;
@@ -83,23 +84,39 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
         uint256 Ann = a * N * N;
 
         uint256 sumReserves = scaledReserves[0] + scaledReserves[1];
-        if (sumReserves == 0) return 0;
         lpTokenSupply = sumReserves;
         for (uint256 i = 0; i < 255; i++) {
+            bool stableOscillation;
             uint256 dP = lpTokenSupply;
             // If division by 0, this will be borked: only withdrawal will work. And that is good
             dP = dP * lpTokenSupply / (scaledReserves[0] * N);
             dP = dP * lpTokenSupply / (scaledReserves[1] * N);
             uint256 prevReserves = lpTokenSupply;
             lpTokenSupply = (Ann * sumReserves / A_PRECISION + (dP * N)) * lpTokenSupply
                 / (((Ann - A_PRECISION) * lpTokenSupply / A_PRECISION) + ((N + 1) * dP));
+
             // Equality with the precision of 1
+            // If the difference between the current lpTokenSupply and the previous lpTokenSupply is 2,
+            // Check that the oscillation is stable, and if so, return the average between the two.
             if (lpTokenSupply > prevReserves) {
+                if (lpTokenSupply - prevReserves == 2) {
+                    if (stableOscillation) {
+                        return lpTokenSupply - 1;
+                    }
+                    stableOscillation = true;
+                }
                 if (lpTokenSupply - prevReserves <= 1) return lpTokenSupply;
             } else {
+                if (prevReserves - lpTokenSupply == 2) {
+                    if (stableOscillation) {
+                        return lpTokenSupply + 1;
+                    }
+                    stableOscillation = true;
+                }
                 if (prevReserves - lpTokenSupply <= 1) return lpTokenSupply;
             }
         }
+        revert("Non convergence: calcLpTokenSupply");
     }
 
     /**
@@ -140,7 +157,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
                 }
             }
         }
-        revert("did not find convergence");
+        revert("Non convergence: calcReserve");
     }
 
     /**
@@ -196,7 +213,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
         uint256 parityReserve = lpTokenSupply / 2;
 
         // update `scaledReserves` based on whether targetPrice is closer to low or high price:
-        if (pd.lutData.highPrice - pd.targetPrice > pd.targetPrice - pd.lutData.lowPrice) {
+        if (percentDiff(pd.lutData.highPrice, pd.targetPrice) > percentDiff(pd.lutData.lowPrice, pd.targetPrice)) {
             // targetPrice is closer to lowPrice.
             scaledReserves[i] = parityReserve * pd.lutData.lowPriceI / pd.lutData.precision;
             scaledReserves[j] = parityReserve * pd.lutData.lowPriceJ / pd.lutData.precision;
@@ -211,21 +228,36 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
         }
 
         // calculate max step size:
-        if (pd.lutData.lowPriceJ > pd.lutData.highPriceJ) {
-            pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
-        } else {
-            pd.maxStepSize = scaledReserves[j] * (pd.lutData.highPriceJ - pd.lutData.lowPriceJ) / pd.lutData.highPriceJ;
-        }
+        // lowPriceJ will always be larger than highPriceJ so a check here is unnecessary.
+        pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
 
         for (uint256 k; k < 255; k++) {
             scaledReserves[j] = updateReserve(pd, scaledReserves[j]);
 
             // calculate scaledReserve[i]:
             scaledReserves[i] = calcReserve(scaledReserves, i, lpTokenSupply, abi.encode(18, 18));
-            // calc currentPrice:
-            pd.currentPrice = _calcRate(scaledReserves, i, j, lpTokenSupply);
+            // calculate new price from reserves:
+            pd.newPrice = _calcRate(scaledReserves, i, j, lpTokenSupply);
+
+            // if the new current price is either lower or higher than both the previous current price and the target price,
+            // (i.e the target price lies between the current price and the previous current price),
+            // recalibrate high/low price.
+            if (pd.newPrice > pd.currentPrice && pd.newPrice > pd.targetPrice) {
+                pd.lutData.highPriceJ = scaledReserves[j] * 1e18 / parityReserve;
+                pd.lutData.highPriceI = scaledReserves[i] * 1e18 / parityReserve;
+                pd.lutData.highPrice = pd.newPrice;
+            } else if (pd.newPrice < pd.currentPrice && pd.newPrice < pd.targetPrice) {
+                pd.lutData.lowPriceJ = scaledReserves[j] * 1e18 / parityReserve;
+                pd.lutData.lowPriceI = scaledReserves[i] * 1e18 / parityReserve;
+                pd.lutData.lowPrice = pd.newPrice;
+            }
+
+            // update max step size based on new scaled reserve.
+            pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
+
+            pd.currentPrice = pd.newPrice;
 
-            // check if new price is within 1 of target price:
+            // check if new price is within PRICE_THRESHOLD:
             if (pd.currentPrice > pd.targetPrice) {
                 if (pd.currentPrice - pd.targetPrice <= PRICE_THRESHOLD) {
                     return scaledReserves[j] / (10 ** (18 - decimals[j]));
@@ -236,6 +268,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
                 }
             }
         }
+        revert("Non convergence: calcReserveAtRatioSwap");
     }
 
     /**
@@ -264,7 +297,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
 
         // update scaledReserve[j] such that calcRate(scaledReserves, i, j) = low/high Price,
         // depending on which is closer to targetPrice.
-        if (pd.lutData.highPrice - pd.targetPrice > pd.targetPrice - pd.lutData.lowPrice) {
+        if (percentDiff(pd.lutData.highPrice, pd.targetPrice) > percentDiff(pd.lutData.lowPrice, pd.targetPrice)) {
             // targetPrice is closer to lowPrice.
             scaledReserves[j] = scaledReserves[i] * pd.lutData.lowPriceJ / pd.lutData.precision;
 
@@ -279,16 +312,29 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
         }
 
         // calculate max step size:
-        if (pd.lutData.lowPriceJ > pd.lutData.highPriceJ) {
-            pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
-        } else {
-            pd.maxStepSize = scaledReserves[j] * (pd.lutData.highPriceJ - pd.lutData.lowPriceJ) / pd.lutData.highPriceJ;
-        }
+        // lowPriceJ will always be larger than highPriceJ so a check here is unnecessary.
+        pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
 
         for (uint256 k; k < 255; k++) {
             scaledReserves[j] = updateReserve(pd, scaledReserves[j]);
             // calculate new price from reserves:
-            pd.currentPrice = calcRate(scaledReserves, i, j, abi.encode(18, 18));
+            pd.newPrice = calcRate(scaledReserves, i, j, abi.encode(18, 18));
+
+            // if the new current price is either lower or higher than both the previous current price and the target price,
+            // (i.e the target price lies between the current price and the previous current price),
+            // recalibrate high/lowPrice and continue.
+            if (pd.newPrice > pd.targetPrice && pd.targetPrice > pd.currentPrice) {
+                pd.lutData.highPriceJ = scaledReserves[j] * 1e18 / scaledReserves[i];
+                pd.lutData.highPrice = pd.newPrice;
+            } else if (pd.newPrice < pd.targetPrice && pd.targetPrice < pd.currentPrice) {
+                pd.lutData.lowPriceJ = scaledReserves[j] * 1e18 / scaledReserves[i];
+                pd.lutData.lowPrice = pd.newPrice;
+            }
+
+            // update max step size based on new scaled reserve.
+            pd.maxStepSize = scaledReserves[j] * (pd.lutData.lowPriceJ - pd.lutData.highPriceJ) / pd.lutData.lowPriceJ;
+
+            pd.currentPrice = pd.newPrice;
 
             // check if new price is within PRICE_THRESHOLD:
             if (pd.currentPrice > pd.targetPrice) {
@@ -301,6 +347,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
                 }
             }
         }
+        revert("Non convergence: calcReserveAtRatioLiquidity");
     }
 
     /**
@@ -314,7 +361,7 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
         if (decimal0 == 0) {
             decimal0 = 18;
         }
-        if (decimal0 == 0) {
+        if (decimal1 == 0) {
             decimal1 = 18;
         }
         if (decimal0 > 18 || decimal1 > 18) revert InvalidTokenDecimals();
@@ -397,4 +444,19 @@ contract Stable2 is ProportionalLPToken2, IBeanstalkWellFunction {
                 + pd.maxStepSize * (pd.currentPrice - pd.targetPrice) / (pd.lutData.highPrice - pd.lutData.lowPrice);
         }
     }
+
+    /**
+     * @notice Calculate the percentage difference between two numbers.
+     * @return The percentage difference as a fixed-point number with 18 decimals.
+     * @dev This function calculates the absolute percentage difference:
+     *      |(a - b)| / ((a + b) / 2) * 100
+     *      The result is scaled by 1e18 for precision.
+     */
+    function percentDiff(uint256 _a, uint256 _b) internal pure returns (uint256) {
+        if (_a == _b) return 0;
+        uint256 difference = _a > _b ? _a - _b : _b - _a;
+        uint256 average = (_a + _b) / 2;
+        // Multiply by 100 * 1e18 to get percentage with 18 decimal places
+        return (difference * 100 * 1e18) / average;
+    }
 }

test/Stable2/Well.Stable2.AddLiquidity.t.sol

@@ -158,7 +158,9 @@ contract WellStable2AddLiquidityTest is LiquidityHelper {
         // amounts to add as liquidity
         uint256[] memory amounts = new uint256[](2);
         amounts[0] = bound(x, 0, type(uint104).max);
-        amounts[1] = bound(y, 0, type(uint104).max);
+        // reserve 1 must be at least 1/600th of the value of amounts[0].
+        uint256 reserve1MinValue = (amounts[0] / 6e2) < 10e18 ? 10e18 : amounts[0] / 6e2;
+        amounts[1] = bound(y, reserve1MinValue, type(uint104).max);
         mintTokens(user, amounts);
 
         Snapshot memory before;