Add erc1271 support to contract account (#1052)

* add ERC1271 support, failing unit tests

* add integration test for isValidSignature

* remove .only

* lint

* add changeset

* clean up 1271 tests and lint

* switch back to using waffle wallet

* lint

* fix import

.changeset/blue-cooks-join.md

+---
+'@eth-optimism/integration-tests': patch
+'@eth-optimism/contracts': patch
+---
+
+Adds ERC1271 support to default contract account

integration-tests/test/predeploys.spec.ts

+import { expect } from 'chai'
+import { ethers } from 'hardhat'
+
+/* Imports: External */
+import { Contract, Wallet } from 'ethers'
+import { OptimismEnv } from './shared/env'
+import { DEFAULT_TRANSACTION } from './shared/utils'
+import { getContractInterface } from '@eth-optimism/contracts'
+
+describe('ECDSAContractAccount', () => {
+  let l2Wallet: Wallet
+
+  before(async () => {
+    const env = await OptimismEnv.new()
+    l2Wallet = env.l2Wallet
+  })
+
+  let ProxyEOA: Contract
+  let messageHash: string
+  let signature: string
+
+  before(async () => {
+    // Send a transaction to ensure there is a ProxyEOA deployed at l2Wallet.address
+    const result = await l2Wallet.sendTransaction(DEFAULT_TRANSACTION)
+    await result.wait()
+    ProxyEOA = new Contract(
+      l2Wallet.address,
+      getContractInterface('OVM_ECDSAContractAccount'),
+      l2Wallet
+    )
+    const message = '0x42'
+    messageHash = ethers.utils.hashMessage(message)
+    signature = await l2Wallet.signMessage(message)
+  })
+
+  it('should correctly evaluate isValidSignature from this wallet', async () => {
+    const isValid = await ProxyEOA.isValidSignature(messageHash, signature)
+    expect(isValid).to.equal('0x1626ba7e')
+  })
+
+  it('should correctly evaluate isValidSignature from other wallet', async () => {
+    const otherWallet = Wallet.createRandom().connect(l2Wallet.provider)
+    const isValid = await ProxyEOA.connect(otherWallet).isValidSignature(
+      messageHash,
+      signature
+    )
+    expect(isValid).to.equal('0x1626ba7e')
+  })
+})

integration-tests/test/rpc.spec.ts

@@ -7,7 +7,12 @@ import {
 import { Wallet, BigNumber, Contract, ContractFactory } from 'ethers'
 import { ethers } from 'hardhat'
 import chai, { expect } from 'chai'
-import { sleep, l2Provider, l1Provider, fundUser } from './shared/utils'
+import {
+  sleep,
+  l2Provider,
+  DEFAULT_TRANSACTION,
+  fundUser,
+} from './shared/utils'
 import chaiAsPromised from 'chai-as-promised'
 import { OptimismEnv } from './shared/env'
 import {
@@ -22,14 +27,6 @@ describe('Basic RPC tests', () => {
   let env: OptimismEnv
   let wallet: Wallet
 
-  const DEFAULT_TRANSACTION = {
-    to: '0x' + '1234'.repeat(10),
-    gasLimit: 33600000000001,
-    gasPrice: 0,
-    data: '0x',
-    value: 0,
-  }
-
   const provider = injectL2Context(l2Provider)
 
   let Reverter: Contract

integration-tests/test/shared/utils.ts

@@ -113,3 +113,11 @@ const abiCoder = new utils.AbiCoder()
 export const encodeSolidityRevertMessage = (_reason: string): string => {
   return '0x08c379a0' + remove0x(abiCoder.encode(['string'], [_reason]))
 }
+
+export const DEFAULT_TRANSACTION = {
+  to: '0x' + '1234'.repeat(10),
+  gasLimit: 33600000000001,
+  gasPrice: 0,
+  data: '0x',
+  value: 0,
+}

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol

@@ -15,6 +15,7 @@ import { OVM_ETH } from "../predeploys/OVM_ETH.sol";
 
 /* External Imports */
 import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol";
+import { ECDSA } from "@openzeppelin/contracts/cryptography/ECDSA.sol";
 
 /**
  * @title OVM_ECDSAContractAccount
@@ -57,6 +58,26 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
         return;
     }
 
+   /**
+    * @dev Should return whether the signature provided is valid for the provided data
+    * @param hash      Hash of the data to be signed
+    * @param signature Signature byte array associated with _data
+    */
+    function isValidSignature(
+        bytes32 hash,
+        bytes memory signature
+    )
+        public
+        view
+        returns (
+            bytes4 magicValue
+        )
+    {
+        return ECDSA.recover(hash, signature) == address(this) ?
+            this.isValidSignature.selector :
+            bytes4(0);
+    }
+
     /**
      * Executes a signed transaction.
      * @param _transaction Signed EIP155 transaction.

packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts

@@ -2,7 +2,7 @@ import { expect } from '../../../setup'
 
 /* External Imports */
 import { ethers, waffle } from 'hardhat'
-import { ContractFactory, Contract, Wallet, BigNumber } from 'ethers'
+import { ContractFactory, Contract, Wallet, BigNumber, utils } from 'ethers'
 import { MockContract, smockit } from '@eth-optimism/smock'
 import { toPlainObject } from 'lodash'
 
@@ -187,7 +187,10 @@ describe('OVM_ECDSAContractAccount', () => {
     // NOTE: Upgrades are disabled for now but will be re-enabled at a later point in time. See
     // comment in OVM_ECDSAContractAccount.sol for additional information.
     it(`should revert if trying call itself`, async () => {
-      const transaction = { ...DEFAULT_EIP155_TX, to: wallet.address }
+      const transaction = {
+        ...DEFAULT_EIP155_TX,
+        to: wallet.address,
+      }
       const encodedTransaction = await wallet.signTransaction(transaction)
 
       await expect(
@@ -197,4 +200,25 @@ describe('OVM_ECDSAContractAccount', () => {
       )
     })
   })
+
+  describe('isValidSignature()', () => {
+    const message = '0x42'
+    const messageHash = ethers.utils.hashMessage(message)
+    it(`should revert for a malformed signature`, async () => {
+      await expect(
+        OVM_ECDSAContractAccount.isValidSignature(messageHash, '0xdeadbeef')
+      ).to.be.revertedWith('ECDSA: invalid signature length')
+    })
+
+    it(`should return 0 for an invalid signature`, async () => {
+      const signature = await wallet.signMessage(message)
+      const bytes = await OVM_ECDSAContractAccount.isValidSignature(
+        messageHash,
+        signature
+      )
+      expect(bytes).to.equal('0x00000000')
+    })
+    // NOTE: There is no good way to unit test verifying a valid signature
+    // An integration test exists testing this instead
+  })
 })