Merge pull request #143 from meTokens/feat/slippage-check

Slippage Check

contracts/facets/HubFacet.sol

@@ -27,9 +27,10 @@ contract HubFacet is IHubFacet, Modifiers {
         require(s.vaultRegistry.isApproved(address(vault)), "vault !approved");
         require(refundRatio < s.MAX_REFUND_RATIO, "refundRatio > MAX");
         require(refundRatio > 0, "refundRatio == 0");
+        require(asset != address(0), "asset !valid");
 
         // Ensure asset is valid based on encoded args and vault validation logic
-        require(vault.isValid(asset, encodedVaultArgs), "asset !valid");
+        require(vault.isValid(encodedVaultArgs), "!encodedVaultArgs");
 
         // Store value set base parameters to `{CurveName}.sol`
         uint256 id = ++s.hubCount;

contracts/facets/MeTokenRegistryFacet.sol

@@ -121,7 +121,7 @@ contract MeTokenRegistryFacet is
             "!approved"
         );
         require(
-            IVault(migration).isValid(meToken, encodedMigrationArgs),
+            IVault(migration).isValid(encodedMigrationArgs),
             "Invalid encodedMigrationArgs"
         );
         meTokenInfo.startTime = block.timestamp + s.meTokenWarmup;
@@ -153,8 +153,8 @@ contract MeTokenRegistryFacet is
         require(LibMeta.msgSender() == meTokenInfo.owner, "!owner");
         require(meTokenInfo.targetHubId != 0, "!resubscribing");
         require(
-            !IMigration(meTokenInfo.migration).migrationStarted(meToken),
-            "Resubscription has started"
+            !IMigration(meTokenInfo.migration).isStarted(meToken),
+            "cannot cancel resubscribe"
         );
 
         meTokenInfo.startTime = 0;

contracts/interfaces/IMigration.sol

@@ -22,12 +22,4 @@ interface IMigration {
     ///         target hub
     /// @param meToken      Address of meToken
     function finishMigration(address meToken) external;
-
-    /// @notice Method returns bool if migration started
-    /// @param meToken  Address of meToken
-    /// @return started True if migration started else false
-    function migrationStarted(address meToken)
-        external
-        view
-        returns (bool started);
 }

contracts/interfaces/IVault.sol

@@ -89,10 +89,7 @@ interface IVault {
 
     /// @notice View to see if an asset with encoded arguments passed
     ///           when a vault is registered to a new hub
-    /// @param asset        Address of asset
     /// @param encodedArgs  Additional encoded arguments
-    /// @return             True if asset & encoded args are valid, else false
-    function isValid(address asset, bytes memory encodedArgs)
-        external
-        returns (bool);
+    /// @return             True if encoded args are valid, else false
+    function isValid(bytes memory encodedArgs) external pure returns (bool);
 }

contracts/migrations/SameAssetTransferMigration.sol

@@ -117,22 +117,8 @@ contract SameAssetTransferMigration is ReentrancyGuard, Vault, IMigration {
 
     /// @inheritdoc Vault
     function isValid(
-        address meToken,
         bytes memory /* encodedArgs */
-    ) external view override returns (bool) {
-        // MeToken not subscribed to a hub
-        if (IMeTokenRegistryFacet(diamond).getMeTokenInfo(meToken).hubId == 0)
-            return false;
+    ) external pure override returns (bool) {
         return true;
     }
-
-    /// @inheritdoc IMigration
-    function migrationStarted(address meToken)
-        external
-        view
-        override
-        returns (bool started)
-    {
-        return _sameAssetMigration[meToken].started;
-    }
 }

contracts/migrations/UniswapSingleTransferMigration.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity 0.8.9;
 
+import "hardhat/console.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
@@ -9,6 +10,7 @@ import {IMeTokenRegistryFacet} from "../interfaces/IMeTokenRegistryFacet.sol";
 import {IMigration} from "../interfaces/IMigration.sol";
 import {ISingleAssetVault} from "../interfaces/ISingleAssetVault.sol";
 import {IV3SwapRouter} from "@uniswap/swap-router-contracts/contracts/interfaces/IV3SwapRouter.sol";
+import {IQuoter} from "@uniswap/v3-periphery/contracts/interfaces/IQuoter.sol";
 import {HubInfo} from "../libs/LibHub.sol";
 import {MeTokenInfo} from "../libs/LibMeToken.sol";
 import {Vault} from "../vaults/Vault.sol";
@@ -35,12 +37,16 @@ contract UniswapSingleTransferMigration is ReentrancyGuard, Vault, IMigration {
     // github.com/Uniswap/uniswap-v3-periphery/blob/main/contracts/interfaces/ISwapRouter.sol
     IV3SwapRouter private immutable _router =
         IV3SwapRouter(0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45);
+    IQuoter private immutable _quoter =
+        IQuoter(0xb27308f9F90D607463bb33eA1BeBb41C27CE5AB6);
 
     // args for uniswap router
     uint24 public constant MINFEE = 500; // 0.05%
     uint24 public constant MIDFEE = 3000; // 0.3% (Default fee)
     uint24 public constant MAXFEE = 1e4; // 1%
 
+    uint256 public constant MAXSLIPPAGE = 95 * 1e16; // *0.95 = -5%
+
     modifier onlyDiamond() {
         require(msg.sender == diamond, "!diamond");
         _;
@@ -134,37 +140,18 @@ contract UniswapSingleTransferMigration is ReentrancyGuard, Vault, IMigration {
     }
 
     /// @inheritdoc Vault
-    function isValid(address meToken, bytes memory encodedArgs)
+    function isValid(bytes memory encodedArgs)
         external
-        view
+        pure
         override
         returns (bool)
     {
         // encodedArgs empty
         if (encodedArgs.length == 0) return false;
-
-        MeTokenInfo memory meTokenInfo = IMeTokenRegistryFacet(diamond)
-            .getMeTokenInfo(meToken);
         uint24 fee = abi.decode(encodedArgs, (uint24));
 
-        // MeToken not subscribed to a hub
-        if (meTokenInfo.hubId == 0) return false;
-        // Invalid fee
-        if (fee == MINFEE || fee == MIDFEE || fee == MAXFEE) {
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    /// @inheritdoc IMigration
-    function migrationStarted(address meToken)
-        external
-        view
-        override
-        returns (bool started)
-    {
-        return _uniswapSingleTransfers[meToken].started;
+        // Must have valid fees
+        return (fee == MINFEE || fee == MIDFEE || fee == MAXFEE);
     }
 
     /// @dev parent call must have reentrancy check
@@ -190,18 +177,29 @@ contract UniswapSingleTransferMigration is ReentrancyGuard, Vault, IMigration {
             return 0;
         }
 
+        // Calculate rough expected return based on 5% input
+        uint256 expectedPartialAmount = _quoter.quoteExactInputSingle(
+            hubInfo.asset,
+            targetHubInfo.asset,
+            usts.fee,
+            (amountIn * 5) / 100,
+            0
+        );
+
         // Approve router to spend
         IERC20(hubInfo.asset).safeApprove(address(_router), amountIn);
 
         // https://docs.uniswap.org/protocol/guides/swaps/single-swaps
+        // Have amountOutMinimum based on the 20 * expectedPartialAmount assuming swapping 5% has 0 slippage
         IV3SwapRouter.ExactInputSingleParams memory params = IV3SwapRouter
             .ExactInputSingleParams({
                 tokenIn: hubInfo.asset,
                 tokenOut: targetHubInfo.asset,
                 fee: usts.fee,
                 recipient: address(this),
                 amountIn: amountIn,
-                amountOutMinimum: 0,
+                amountOutMinimum: (expectedPartialAmount * 20 * MAXSLIPPAGE) /
+                    PRECISION,
                 sqrtPriceLimitX96: 0
             });
 

contracts/vaults/SingleAssetVault.sol

@@ -41,12 +41,8 @@ contract SingleAssetVault is Vault, ISingleAssetVault {
 
     /// @inheritdoc Vault
     function isValid(
-        address asset,
         bytes memory /*encodedArgs */
     ) public pure override returns (bool) {
-        if (asset == address(0)) {
-            return false;
-        }
         return true;
     }
 }

contracts/vaults/Vault.sol

@@ -104,9 +104,8 @@ contract Vault is IVault, ReentrancyGuard {
 
     /// @inheritdoc IVault
     function isValid(
-        address, /* meToken */
         bytes memory /* encodedArgs */
-    ) external virtual override returns (bool) {
+    ) external pure virtual override returns (bool) {
         return true;
     }
 }

test/contracts/facets/MeTokenRegistryFacet.ts

@@ -711,7 +711,7 @@ const setup = async () => {
         // should revert to cancelResubscribe now
         await expect(
           meTokenRegistry.cancelResubscribe(meTokenAddr0)
-        ).to.be.revertedWith("Resubscription has started");
+        ).to.be.revertedWith("cannot cancel resubscribe");
       });
     });
 

test/contracts/migrations/SameAssetTransferMigration.ts

@@ -138,19 +138,9 @@ const setup = async () => {
 
     describe("isValid()", () => {
       it("Returns true for valid encoding", async () => {
-        const isValid = await migration.isValid(
-          meToken.address,
-          encodedMigrationArgs
-        );
+        const isValid = await migration.isValid(encodedMigrationArgs);
         expect(isValid).to.be.true;
       });
-      it("Returns false for nonexistent meToken", async () => {
-        const isValid = await migration.isValid(
-          account0.address,
-          encodedMigrationArgs
-        );
-        expect(isValid).to.be.false;
-      });
     });
 
     describe("initMigration()", () => {

test/contracts/migrations/UniswapSingleTransfer.ts

@@ -173,43 +173,27 @@ const setup = async () => {
 
     describe("isValid()", () => {
       it("Returns false for invalid encoding", async () => {
-        const isValid = await migration.isValid(meToken.address, "0x");
+        const isValid = await migration.isValid("0x");
         expect(isValid).to.be.false;
       });
       it("Returns true for valid encoding", async () => {
-        const isValid = await migration.isValid(
-          meToken.address,
-          encodedMigrationArgs
-        );
+        const isValid = await migration.isValid(encodedMigrationArgs);
         expect(isValid).to.be.true;
       });
       it("Returns false when pass soonest", async () => {
         badEncodedMigrationArgs = ethers.utils.defaultAbiCoder.encode(
           ["uint256", "uint24"],
           [1000, fees] // invalid encoding
         );
-        const isValid = await migration.isValid(
-          meToken.address,
-          badEncodedMigrationArgs
-        );
-        expect(isValid).to.be.false;
-      });
-      it("Returns false for nonexistent meToken", async () => {
-        const isValid = await migration.isValid(
-          account0.address,
-          encodedMigrationArgs
-        );
+        const isValid = await migration.isValid(badEncodedMigrationArgs);
         expect(isValid).to.be.false;
       });
       it("Returns false for invalid fee", async () => {
         badEncodedMigrationArgs = ethers.utils.defaultAbiCoder.encode(
           ["uint24"],
           [2999]
         );
-        const isValid = await migration.isValid(
-          meToken.address,
-          badEncodedMigrationArgs
-        );
+        const isValid = await migration.isValid(badEncodedMigrationArgs);
         expect(isValid).to.be.false;
       });
     });