Merge pull request #640 from lidofinance/fix/embedded-eip712

Feat: embedded EIP712 for StETH

contracts/0.4.24/StETHPermit.sol

@@ -100,7 +100,7 @@ contract StETHPermit is IERC2612, StETH {
             abi.encode(PERMIT_TYPEHASH, _owner, _spender, _value, _useNonce(_owner), _deadline)
         );
 
-        bytes32 hash = IEIP712(getEIP712StETH()).hashTypedDataV4(structHash);
+        bytes32 hash = IEIP712(getEIP712StETH()).hashTypedDataV4(address(this), structHash);
 
         address signer = ECDSA.recover(hash, _v, _r, _s);
         require(signer == _owner, "ERC20Permit: invalid signature");
@@ -124,7 +124,7 @@ contract StETHPermit is IERC2612, StETH {
      */
     // solhint-disable-next-line func-name-mixedcase
     function DOMAIN_SEPARATOR() external view returns (bytes32) {
-        return IEIP712(getEIP712StETH()).domainSeparatorV4();
+        return IEIP712(getEIP712StETH()).domainSeparatorV4(address(this));
     }
 
     /**

contracts/0.8.9/EIP712StETH.sol

@@ -1,27 +1,88 @@
-// SPDX-FileCopyrightText: 2023 Lido <info@lido.fi>
-// SPDX-License-Identifier: GPL-3.0
+// SPDX-FileCopyrightText: 2023 OpenZeppelin, Lido <info@lido.fi>
+// SPDX-License-Identifier: MIT
 
 /* See contracts/COMPILERS.md */
 pragma solidity 0.8.9;
 
-import {EIP712} from "@openzeppelin/contracts-v4.4/utils/cryptography/draft-EIP712.sol";
+import {ECDSA} from "@openzeppelin/contracts-v4.4/utils/cryptography/ECDSA.sol";
 
 import {IEIP712} from "../common/interfaces/IEIP712.sol";
 
 /**
- * Helper contract exposes OpenZeppelin's EIP712 message utils implementation.
+ * NOTE: The code below is taken from "@openzeppelin/contracts-v4.4/utils/cryptography/draft-EIP712.sol"
+ * With a main difference to store the stETH contract address internally and use it for signing.
  */
-contract EIP712StETH is IEIP712, EIP712 {
+
+/**
+ * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data.
+ *
+ * The encoding specified in the EIP is very generic, and such a generic implementation in Solidity is not feasible,
+ * thus this contract does not implement the encoding itself. Protocols need to implement the type-specific encoding
+ * they need in their contracts using a combination of `abi.encode` and `keccak256`.
+ *
+ * This contract implements the EIP 712 domain separator ({_domainSeparatorV4}) that is used as part of the encoding
+ * scheme, and the final step of the encoding to obtain the message digest that is then signed via ECDSA
+ * ({_hashTypedDataV4}).
+ *
+ * The implementation of the domain separator was designed to be as efficient as possible while still properly updating
+ * the chain id to protect against replay attacks on an eventual fork of the chain.
+ *
+ * NOTE: This contract implements the version of the encoding known as "v4", as implemented by the JSON RPC method
+ * https://docs.metamask.io/guide/signing-data.html[`eth_signTypedDataV4` in MetaMask].
+ *
+ */
+contract EIP712StETH is IEIP712 {
+    /* solhint-disable var-name-mixedcase */
+    // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to
+    // invalidate the cached domain separator if the chain id changes.
+    bytes32 private immutable _CACHED_DOMAIN_SEPARATOR;
+    uint256 private immutable _CACHED_CHAIN_ID;
+    address private immutable _CACHED_STETH;
+
+    bytes32 private immutable _HASHED_NAME;
+    bytes32 private immutable _HASHED_VERSION;
+    bytes32 private immutable _TYPE_HASH;
+
+    error ZeroStETHAddress();
+
     /**
      * @dev Constructs specialized EIP712 instance for StETH token, version "2".
      */
-    constructor() EIP712("Liquid staked Ether 2.0", "2") {}
+    constructor(address _stETH) {
+        if (_stETH == address(0)) { revert ZeroStETHAddress(); }
+
+        bytes32 hashedName = keccak256(bytes("Liquid staked Ether 2.0"));
+        bytes32 hashedVersion = keccak256(bytes("2"));
+        bytes32 typeHash = keccak256(
+            "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
+        );
+
+        _HASHED_NAME = hashedName;
+        _HASHED_VERSION = hashedVersion;
+        _CACHED_CHAIN_ID = block.chainid;
+        _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(typeHash, hashedName, hashedVersion, _stETH);
+        _CACHED_STETH = _stETH;
+        _TYPE_HASH = typeHash;
+    }
 
     /**
      * @dev Returns the domain separator for the current chain.
      */
-    function domainSeparatorV4() external view override returns (bytes32) {
-        return _domainSeparatorV4();
+    function domainSeparatorV4(address _stETH) public view override returns (bytes32) {
+        if (_stETH == _CACHED_STETH && block.chainid == _CACHED_CHAIN_ID) {
+            return _CACHED_DOMAIN_SEPARATOR;
+        } else {
+            return _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION, _stETH);
+        }
+    }
+
+    function _buildDomainSeparator(
+        bytes32 _typeHash,
+        bytes32 _nameHash,
+        bytes32 _versionHash,
+        address _stETH
+    ) private view returns (bytes32) {
+        return keccak256(abi.encode(_typeHash, _nameHash, _versionHash, block.chainid, _stETH));
     }
 
     /**
@@ -31,15 +92,15 @@ contract EIP712StETH is IEIP712, EIP712 {
      * This hash can be used together with {ECDSA-recover} to obtain the signer of a message. For example:
      *
      * ```solidity
-     * bytes32 digest = hashTypedDataV4(keccak256(abi.encode(
+     * bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(
      *     keccak256("Mail(address to,string contents)"),
      *     mailTo,
      *     keccak256(bytes(mailContents))
      * )));
      * address signer = ECDSA.recover(digest, signature);
      * ```
      */
-    function hashTypedDataV4(bytes32 _structHash) external view override returns (bytes32) {
-        return _hashTypedDataV4(_structHash);
+    function hashTypedDataV4(address _stETH, bytes32 _structHash) external view override returns (bytes32) {
+        return ECDSA.toTypedDataHash(domainSeparatorV4(_stETH), _structHash);
     }
 }

contracts/common/interfaces/IEIP712.sol

@@ -15,7 +15,7 @@ interface IEIP712 {
     /**
      * @dev Returns the domain separator for the current chain.
      */
-    function domainSeparatorV4() external view returns (bytes32);
+    function domainSeparatorV4(address _stETH) external view returns (bytes32);
 
     /**
      * @dev Given an already https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct[hashed struct], this
@@ -32,5 +32,5 @@ interface IEIP712 {
      * address signer = ECDSA.recover(digest, signature);
      * ```
      */
-    function hashTypedDataV4(bytes32 _structHash) external view returns (bytes32);
+    function hashTypedDataV4(address _stETH, bytes32 _structHash) external view returns (bytes32);
 }

lib/abi/EIP712StETH.json

@@ -1 +1 @@
-[{"inputs":[],"stateMutability":"nonpayable","type":"constructor"},{"inputs":[],"name":"domainSeparatorV4","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"bytes32","name":"_structHash","type":"bytes32"}],"name":"hashTypedDataV4","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"}]
+[{"inputs":[{"internalType":"address","name":"_stETH","type":"address"}],"stateMutability":"nonpayable","type":"constructor"},{"inputs":[],"name":"ZeroStETHAddress","type":"error"},{"inputs":[{"internalType":"address","name":"_stETH","type":"address"}],"name":"domainSeparatorV4","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"_stETH","type":"address"},{"internalType":"bytes32","name":"_structHash","type":"bytes32"}],"name":"hashTypedDataV4","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"}]

test/0.4.24/stethpermit.test.js

@@ -1,8 +1,7 @@
 const crypto = require('crypto')
 const { ACCOUNTS_AND_KEYS, MAX_UINT256, ZERO_ADDRESS } = require('./helpers/constants')
 const { bn } = require('@aragon/contract-helpers-test')
-const { assertBn, assertEvent } = require('@aragon/contract-helpers-test/src/asserts')
-const { assertRevert } = require('../helpers/assertThrow')
+const { assert } = require('../helpers/assert')
 const { signPermit, signTransferAuthorization, makeDomainSeparator } = require('./helpers/permit_helpers')
 const { hexStringFromBuffer } = require('./helpers/sign_utils')
 const { ETH } = require('../helpers/utils')
@@ -16,13 +15,13 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
   const snapshot = new EvmSnapshot(hre.ethers.provider)
 
   before('deploy mock token', async () => {
-    eip712StETH = await EIP712StETH.new({ from: deployer })
     stEthPermit = await StETHPermit.new({ from: deployer, value: ETH(1) })
+    eip712StETH = await EIP712StETH.new(stEthPermit.address, { from: deployer })
     await stEthPermit.initializeEIP712StETH(eip712StETH.address)
 
     chainId = await web3.eth.net.getId();
 
-    domainSeparator = makeDomainSeparator('Liquid staked Ether 2.0', '2', chainId, eip712StETH.address)
+    domainSeparator = makeDomainSeparator('Liquid staked Ether 2.0', '2', chainId, stEthPermit.address)
     await snapshot.make()
   })
 
@@ -50,8 +49,15 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       await stEthPermit.mintShares(permitParams.owner, initialBalance, { from: deployer })
     })
 
+    it('EIP-712 signature helper reverts when zero stETH address passed', async () => {
+      await assert.revertsWithCustomError(
+        EIP712StETH.new(ZERO_ADDRESS, { from: deployer }),
+        `ZeroStETHAddress()`
+      )
+    })
+
     it('EIP-712 signature helper contract matches the stored one', async () => {
-      assert.equal(await stEthPermit.getEIP712StETH(), eip712StETH.address)
+      assert.equals(await stEthPermit.getEIP712StETH(), eip712StETH.address)
     })
 
     it('grants allowance when a valid permit is given', async () => {
@@ -64,11 +70,11 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       let { v, r, s } = signPermit(owner, spender, value, nonce, deadline, domainSeparator, alice.key)
 
       // check that the allowance is initially zero
-      assertBn(await stEthPermit.allowance(owner, spender), bn(0))
+      assert.equals(await stEthPermit.allowance(owner, spender), bn(0))
       // check that the next nonce expected is zero
-      assertBn(await stEthPermit.nonces(owner), bn(0))
+      assert.equals(await stEthPermit.nonces(owner), bn(0))
       // check domain separator
-      assert.equal(
+      assert.equals(
         await stEthPermit.DOMAIN_SEPARATOR(),
         domainSeparator
       )
@@ -79,15 +85,15 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       )
 
       // check that allowance is updated
-      assertBn(await stEthPermit.allowance(owner, spender), bn(value))
+      assert.equals(await stEthPermit.allowance(owner, spender), bn(value))
 
-      assertEvent(
+      assert.emits(
         receipt,
         'Approval',
-        { expectedArgs: { owner: owner, spender: spender, value: bn(value) } }
+        { owner: owner, spender: spender, value: bn(value) }
       )
 
-      assertBn(await stEthPermit.nonces(owner), bn(1))
+      assert.equals(await stEthPermit.nonces(owner), bn(1))
 
       // increment nonce
       nonce = 1
@@ -99,15 +105,15 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const receipt2 = await stEthPermit.permit(owner, spender, value, deadline, v, r, s, { from: charlie })
 
       // check that allowance is updated
-      assertBn(await stEthPermit.allowance(owner, spender), bn(value))
+      assert.equals(await stEthPermit.allowance(owner, spender), bn(value))
 
-      assertEvent(
+      assert.emits(
         receipt2,
         'Approval',
-        { expectedArgs: { owner: owner, spender: spender, value: bn(value) } }
+        { owner: owner, spender: spender, value: bn(value) }
       )
 
-      assertBn(await stEthPermit.nonces(owner), bn(2))
+      assert.equals(await stEthPermit.nonces(owner), bn(2))
     })
 
     it('reverts if the signature does not match given parameters', async () => {
@@ -116,7 +122,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const { v, r, s } = signPermit(owner, spender, value, nonce, deadline, domainSeparator, alice.key)
 
       // try to cheat by claiming the approved amount + 1
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(
           owner,
           spender,
@@ -131,7 +137,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       )
 
       // check that msg is incorrect even if claim the approved amount - 1
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(
           owner,
           spender,
@@ -154,7 +160,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
 
       // try to cheat by submitting the permit that is signed by a
       // wrong person
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: charlie
         }),
@@ -166,7 +172,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       await web3.eth.sendTransaction({ to: bob.address, from: accounts[0], value: ETH(10) })
 
       // even Bob himself can't call permit with the invalid sig
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: bob.address
         }),
@@ -181,7 +187,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const { v, r, s } = signPermit(owner, spender, value, nonce, deadline, domainSeparator, alice.key)
 
       // try to submit the permit that is expired
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: charlie
         }),
@@ -194,11 +200,11 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
         const { v, r, s } = signPermit(owner, spender, value, nonce, deadline1min, domainSeparator, alice.key)
         const receipt = await stEthPermit.permit(owner, spender, value, deadline1min, v, r, s, { from: charlie })
 
-        assertBn(await stEthPermit.nonces(owner), bn(1))
-        assertEvent(
+        assert.equals(await stEthPermit.nonces(owner), bn(1))
+        assert.emits(
           receipt,
           'Approval',
-          { expectedArgs: { owner: owner, spender: spender, value: bn(value) } }
+          { owner: owner, spender: spender, value: bn(value) }
         )
       }
     })
@@ -209,10 +215,10 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       // create a signed permit
       const { v, r, s } = signPermit(owner, spender, value, nonce, deadline, domainSeparator, alice.key)
       // check that the next nonce expected is 0, not 1
-      assertBn(await stEthPermit.nonces(owner), bn(0))
+      assert.equals(await stEthPermit.nonces(owner), bn(0))
 
       // try to submit the permit
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: charlie
         }),
@@ -229,7 +235,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       await stEthPermit.permit(owner, spender, value, deadline, v, r, s, { from: charlie })
 
       // try to submit the permit again
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: charlie
         }),
@@ -241,7 +247,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       await web3.eth.sendTransaction({ to: alice.address, from: accounts[0], value: ETH(10) })
 
       // try to submit the permit again from Alice herself
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: alice.address
         }),
@@ -262,7 +268,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const permit2 = signPermit(owner, spender, 1e6, nonce, deadline, domainSeparator, alice.key)
 
       // try to submit the permit again
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, 1e6, deadline, permit2.v, permit2.r, permit2.s, { from: charlie }),
         'ERC20Permit: invalid signature'
       )
@@ -276,7 +282,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const { v, r, s } = signPermit(owner, spender, value, nonce, deadline, domainSeparator, alice.key)
 
       // try to submit the permit with invalid approval parameters
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(owner, spender, value, deadline, v, r, s, {
           from: charlie
         }),
@@ -292,7 +298,7 @@ contract('StETHPermit', ([deployer, ...accounts]) => {
       const { v, r, s } = signTransferAuthorization(from, to, value, validAfter, validBefore, nonce, domainSeparator, alice.key)
 
       // try to submit the transfer permit
-      await assertRevert(
+      await assert.reverts(
         stEthPermit.permit(from, to, value, validBefore, v, r, s, {
           from: charlie
         }),