fix: final audit GN-1628 changes (#13)

gelatodigital · Dec 12, 2022 · a7e6152 · a7e6152
1 parent 768648e
commit a7e6152
Show file tree

Hide file tree

Showing 22 changed files with 73 additions and 138 deletions.
diff --git a/.gitignore b/.gitignore
@@ -11,6 +11,9 @@ coverage
 # cache
 .eslintcache
 
+# local deployments
+deployments/localhost
+
 # hardhat
 artifacts
 cache

diff --git a/contracts/GelatoRelayContext.sol b/contracts/GelatoRelayContext.sol
@@ -67,7 +67,6 @@ abstract contract GelatoRelayContext is GelatoRelayBase {
         _getFeeToken().transfer(_getFeeCollector(), fee);
     }
 
-    // Do not confuse with OZ Context.sol _msgData()
     function _getMsgData() internal view returns (bytes calldata) {
         return
             _isGelatoRelay(msg.sender)

diff --git a/contracts/GelatoRelayContextERC2771.sol b/contracts/GelatoRelayContextERC2771.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.1;
 
-import {GelatoRelayBase} from "./base/GelatoRelayBase.sol";
+import {GelatoRelayERC2771Base} from "./base/GelatoRelayERC2771Base.sol";
 import {TokenUtils} from "./lib/TokenUtils.sol";
 
 uint256 constant _FEE_COLLECTOR_START = 92; // offset: address + address + uint256 + address
@@ -66,16 +66,17 @@ function _getMsgSenderRelayContextERC2771() pure returns (address _msgSender) {
  *     fee: - 52 bytes
  *     _msgSender: - 20 bytes
  */
+
 /// @dev Do not use with GelatoRelayFeeCollectorERC2771 - pick only one
-abstract contract GelatoRelayContextERC2771 is GelatoRelayBase {
+abstract contract GelatoRelayContextERC2771 is GelatoRelayERC2771Base {
     using TokenUtils for address;
 
-    // DANGER! Only use with onlyGelatoRelay `_isGelatoRelay` before transferring
+    // DANGER! Only use with onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
     function _transferRelayFee() internal {
         _getFeeToken().transfer(_getFeeCollector(), _getFee());
     }
 
-    // DANGER! Only use with onlyGelatoRelay `_isGelatoRelay` before transferring
+    // DANGER! Only use with onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
     function _transferRelayFeeCapped(uint256 _maxFee) internal {
         uint256 fee = _getFee();
         require(
@@ -85,30 +86,32 @@ abstract contract GelatoRelayContextERC2771 is GelatoRelayBase {
         _getFeeToken().transfer(_getFeeCollector(), fee);
     }
 
-    // Do not confuse with OZ Context.sol _msgData()
-    function _getMsgData() internal view returns (bytes calldata) {
+    function _getMsgData() internal view virtual returns (bytes calldata) {
         return
-            _isGelatoRelay(msg.sender)
+            _isGelatoRelayERC2771(msg.sender)
                 ? msg.data[:msg.data.length - _FEE_COLLECTOR_START]
                 : msg.data;
     }
 
-    // Only use with GelatoRelayBase onlyGelatoRelay or `_isGelatoRelay` checks
+    function _getMsgSender() internal view virtual returns (address) {
+        return
+            _isGelatoRelayERC2771(msg.sender)
+                ? _getMsgSenderRelayContextERC2771()
+                : msg.sender;
+    }
+
+    // Only use with GelatoRelayERC2771Base onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
     function _getFeeCollector() internal pure returns (address) {
         return _getFeeCollectorRelayContextERC2771();
     }
 
-    // Only use with previous onlyGelatoRelay or `_isGelatoRelay` checks
+    // Only use with GelatoRelayERC2771Base onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
     function _getFeeToken() internal pure returns (address) {
         return _getFeeTokenRelayContextERC2771();
     }
 
-    // Only use with previous onlyGelatoRelay or `_isGelatoRelay` checks
+    // Only use with GelatoRelayERC2771Base onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
     function _getFee() internal pure returns (uint256) {
         return _getFeeRelayContextERC2771();
     }
-
-    function _getMsgSender() internal pure returns (address) {
-        return _getMsgSenderRelayContextERC2771();
-    }
 }
diff --git a/contracts/GelatoRelayFeeCollector.sol b/contracts/GelatoRelayFeeCollector.sol
@@ -24,9 +24,8 @@ function __getFeeCollector() pure returns (address feeCollector) {
  * 20bytes start offsets from calldatasize:
  *    feeCollector: -20
  */
-/// @dev Do not use with GelatoRelayFeeCollector - pick only one
+/// @dev Do not use with GelatoRelayContext - pick only one
 abstract contract GelatoRelayFeeCollector is GelatoRelayBase {
-    // Do not confuse with OZ Context.sol _msgData()
     function _getMsgData() internal view returns (bytes calldata) {
         return
             _isGelatoRelay(msg.sender)

diff --git a/contracts/GelatoRelayFeeCollectorERC2771.sol b/contracts/GelatoRelayFeeCollectorERC2771.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.1;
 
-import {GelatoRelayBase} from "./base/GelatoRelayBase.sol";
+import {GelatoRelayERC2771Base} from "./base/GelatoRelayERC2771Base.sol";
 
 uint256 constant _FEE_COLLECTOR_START = 40; // offset: address + address
 uint256 constant _MSG_SENDER_START = 20; // offset: address
@@ -36,22 +36,25 @@ function _getMsgSenderFeeCollectorERC2771() pure returns (address _msgSender) {
  *    feeCollector: - 40 bytes
  *    _msgSender: - 20 bytes
  */
+
 /// @dev Do not use with GelatoRelayFeeCollectorERC2771 - pick only one
-abstract contract GelatoRelayFeeCollectorERC2771 is GelatoRelayBase {
-    // Do not confuse with OZ Context.sol _msgData()
+abstract contract GelatoRelayFeeCollectorERC2771 is GelatoRelayERC2771Base {
     function _getMsgData() internal view returns (bytes calldata) {
         return
-            _isGelatoRelay(msg.sender)
+            _isGelatoRelayERC2771(msg.sender)
                 ? msg.data[:msg.data.length - _FEE_COLLECTOR_START]
                 : msg.data;
     }
 
-    // Only use with GelatoRelayBase onlyGelatoRelay or `_isGelatoRelay` checks
-    function _getFeeCollector() internal pure returns (address) {
-        return _getFeeCollectorERC2771();
+    function _getMsgSender() internal view returns (address) {
+        return
+            _isGelatoRelayERC2771(msg.sender)
+                ? _getMsgSenderFeeCollectorERC2771()
+                : msg.sender;
     }
 
-    function _getMsgSender() internal pure returns (address) {
-        return _getMsgSenderFeeCollectorERC2771();
+    // Only use with GelatoRelayERC2771Base onlyGelatoRelayERC2771 or `_isGelatoRelayERC2771` checks
+    function _getFeeCollector() internal pure returns (address) {
+        return _getFeeCollectorERC2771();
     }
 }
diff --git a/contracts/__mocks__/MockGelatoRelayContextERC2771.sol b/contracts/__mocks__/MockGelatoRelayContextERC2771.sol
@@ -31,5 +31,5 @@ contract MockGelatoRelayContextERC2771 is GelatoRelayContextERC2771 {
     }
 
     // solhint-disable-next-line no-empty-blocks
-    function testOnlyGelatoRelay() external onlyGelatoRelay {}
+    function testOnlyGelatoRelayERC2771() external onlyGelatoRelayERC2771 {}
 }
diff --git a/contracts/__mocks__/MockGelatoRelayFeeCollectorERC2771.sol b/contracts/__mocks__/MockGelatoRelayFeeCollectorERC2771.sol
@@ -18,5 +18,5 @@ contract MockGelatoRelayFeeCollectorERC2771 is GelatoRelayFeeCollectorERC2771 {
     }
 
     // solhint-disable-next-line no-empty-blocks
-    function testOnlyGelatoRelay() external onlyGelatoRelay {}
+    function testOnlyGelatoRelayERC2771() external onlyGelatoRelayERC2771 {}
 }
diff --git a/contracts/base/GelatoRelayBase.sol b/contracts/base/GelatoRelayBase.sol
@@ -1,28 +1,15 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.1;
 
-import {GELATO_RELAY, GELATO_RELAY_ERC2771} from "../constants/GelatoRelay.sol";
+import {GELATO_RELAY} from "../constants/GelatoRelay.sol";
 
 abstract contract GelatoRelayBase {
     modifier onlyGelatoRelay() {
         require(_isGelatoRelay(msg.sender), "onlyGelatoRelay");
         _;
     }
 
-    modifier onlyGelatoRelayERC2771() {
-        require(_isGelatoRelayERC2771(msg.sender), "onlyGelatoRelayERC2771");
-        _;
-    }
-
     function _isGelatoRelay(address _forwarder) internal pure returns (bool) {
         return _forwarder == GELATO_RELAY;
     }
-
-    function _isGelatoRelayERC2771(address _forwarder)
-        internal
-        pure
-        returns (bool)
-    {
-        return _forwarder == GELATO_RELAY_ERC2771;
-    }
 }
diff --git a/contracts/base/GelatoRelayERC2771Base.sol b/contracts/base/GelatoRelayERC2771Base.sol
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.1;
+
+import {GELATO_RELAY_ERC2771} from "../constants/GelatoRelay.sol";
+
+abstract contract GelatoRelayERC2771Base {
+    modifier onlyGelatoRelayERC2771() {
+        require(_isGelatoRelayERC2771(msg.sender), "onlyGelatoRelayERC2771");
+        _;
+    }
+
+    function _isGelatoRelayERC2771(address _forwarder)
+        internal
+        pure
+        returns (bool)
+    {
+        return _forwarder == GELATO_RELAY_ERC2771;
+    }
+}
diff --git a/contracts/constants/GelatoRelay.sol b/contracts/constants/GelatoRelay.sol
@@ -2,4 +2,4 @@
 pragma solidity ^0.8.1;
 
 address constant GELATO_RELAY = 0xaBcC9b596420A9E9172FD5938620E265a0f9Df92;
-address constant GELATO_RELAY_ERC2771 = 0x1Cc587d239AF07C23D8f28Bc6DCdF73BE1994cA1;
+address constant GELATO_RELAY_ERC2771 = 0xBf175FCC7086b4f9bd59d5EAE8eA67b8f940DE0d;
diff --git a/contracts/vendor/Context.sol b/contracts/vendor/Context.sol
diff --git a/contracts/vendor/ERC2771Context.sol b/contracts/vendor/ERC2771Context.sol
diff --git a/deploy/MockGelatoRelayContext.deploy.ts b/deploy/MockGelatoRelayContext.deploy.ts
@@ -19,6 +19,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
 func.skip = async (hre: HardhatRuntimeEnvironment) => {
   return hre.network.name !== "hardhat";
 };
+func.dependencies = ["MockRelay"];
 func.tags = ["MockGelatoRelayContext"];
 
 export default func;
diff --git a/deploy/MockGelatoRelayContextERC2771.deploy.ts b/deploy/MockGelatoRelayContextERC2771.deploy.ts
@@ -19,6 +19,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
 func.skip = async (hre: HardhatRuntimeEnvironment) => {
   return hre.network.name !== "hardhat";
 };
+func.dependencies = ["MockRelay"];
 func.tags = ["MockGelatoRelayContextERC2771"];
 
 export default func;
diff --git a/deploy/MockGelatoRelayFeeCollector.deploy.ts b/deploy/MockGelatoRelayFeeCollector.deploy.ts
@@ -19,6 +19,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
 func.skip = async (hre: HardhatRuntimeEnvironment) => {
   return hre.network.name !== "hardhat";
 };
+func.dependencies = ["MockRelay"];
 func.tags = ["MockGelatoRelayFeeCollector"];
 
 export default func;
diff --git a/deploy/MockGelatoRelayFeeCollectorERC2771.deploy.ts b/deploy/MockGelatoRelayFeeCollectorERC2771.deploy.ts
@@ -19,6 +19,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
 func.skip = async (hre: HardhatRuntimeEnvironment) => {
   return hre.network.name !== "hardhat";
 };
+func.dependencies = ["MockRelay"];
 func.tags = ["MockGelatoRelayFeeCollectorERC2771"];
 
 export default func;
diff --git a/hardhat.config.ts b/hardhat.config.ts
@@ -13,7 +13,7 @@ const config: HardhatUserConfig = {
       default: 0,
     },
   },
-  solidity: "0.8.1",
+  solidity: "0.8.9",
   typechain: {
     outDir: "typechain",
     target: "ethers-v5",

diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
   },
   "license": "MIT",
   "dependencies": {
-    "@openzeppelin/contracts": "4.7.3"
+    "@openzeppelin/contracts": "4.8.0"
   },
   "devDependencies": {
     "@nomiclabs/hardhat-ethers": "npm:hardhat-deploy-ethers",

diff --git a/test/MockRelayContext.test.ts b/test/MockRelayContext.test.ts
@@ -43,7 +43,7 @@ describe("Test MockGelatoRelayContext Smart Contract", function () {
     mockERC20 = await hre.ethers.getContract("MockERC20");
 
     targetRelayContext = mockRelayContext.address;
-    salt = 42069;
+    salt = Date.now();
     deadline = 2664381086;
     feeToken = mockERC20.address;
   });

diff --git a/test/MockRelayContextERC2771.test.ts b/test/MockRelayContextERC2771.test.ts
@@ -18,7 +18,6 @@ import { INIT_TOKEN_BALANCE as FEE } from "./constants";
 import { utils } from "ethers";
 
 const FEE_COLLECTOR = "0x3AC05161b76a35c1c28dC99Aa01BEd7B24cEA3bf";
-const MSG_SENDER = "0xDA9644C2c2B6F50426EaBa9ce1B853e99f2D4fCa";
 const correlationId = utils.formatBytes32String("CORRELATION_ID");
 const dummySig = utils.randomBytes(32);
 
@@ -29,6 +28,7 @@ describe("Test MockGelatoRelayContextERC2771 Smart Contract", function () {
   let mockERC20: MockERC20;
 
   let targetAddress: string;
+  let MSG_SENDER: string;
   let salt: number;
   let deadline: number;
   let feeToken: string;
@@ -49,6 +49,7 @@ describe("Test MockGelatoRelayContextERC2771 Smart Contract", function () {
     mockERC20 = await hre.ethers.getContract("MockERC20");
 
     targetAddress = mockRelayContextERC2771.address;
+    MSG_SENDER = mockRelay.address;
     salt = 42069;
     deadline = 2664381086;
     feeToken = mockERC20.address;
@@ -258,7 +259,7 @@ describe("Test MockGelatoRelayContextERC2771 Smart Contract", function () {
 
   it("#6: testOnlyGelatoRelay reverts if not GelatoRelay", async () => {
     await expect(
-      mockRelayContextERC2771.testOnlyGelatoRelay()
+      mockRelayContextERC2771.testOnlyGelatoRelayERC2771()
     ).to.be.revertedWith("onlyGelatoRelay");
   });
 });