fix: disable upgradability from ECDSA Account (#885)

ethereum-optimism · May 21, 2021 · b9c5e81 · b9c5e81
1 parent 8d477be
commit b9c5e81
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 13 deletions.
diff --git a/.changeset/sharp-roses-admire.md b/.changeset/sharp-roses-admire.md
@@ -0,0 +1,5 @@
+---
+'@eth-optimism/contracts': patch
+---
+
+Disable upgradability from the ECDSA account instead of the EOA proxy.
diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol
@@ -140,6 +140,14 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
 
                 return (true, bytes(""));
             } else {
+                // NOTE: Upgrades are temporarily disabled because users can, in theory, modify their EOA
+                // so that they don't have to pay any fees to the sequencer. Function will remain disabled
+                // until a robust solution is in place.
+                require(
+                    transaction.to != Lib_ExecutionManagerWrapper.ovmADDRESS(),
+                    "Calls to self are disabled until upgradability is re-enabled."
+                );
+
                 return transaction.to.call(transaction.data);
             }
         }

diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol
@@ -71,17 +71,13 @@ contract OVM_ProxyEOA {
     )
         external
     {
-        // NOTE: Upgrades are temporarily disabled because users can, in theory, modify their EOA
-        // so that they don't have to pay any fees to the sequencer. Function will remain disabled
-        // until a robust solution is in place.
+        require(
+            msg.sender == Lib_ExecutionManagerWrapper.ovmADDRESS(),
+            "EOAs can only upgrade their own EOA implementation."
+        );
 
-        // require(
-        //     msg.sender == Lib_ExecutionManagerWrapper.ovmADDRESS(),
-        //     "EOAs can only upgrade their own EOA implementation"
-        // );
-
-        // _setImplementation(_implementation);
-        // emit Upgraded(_implementation);
+        _setImplementation(_implementation);
+        emit Upgraded(_implementation);
     }
 
     /**

diff --git a/packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts b/packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts
@@ -176,5 +176,18 @@ describe('OVM_ECDSAContractAccount', () => {
         OVM_ECDSAContractAccount.execute(encodedTransaction)
       ).to.be.revertedWith('Value is nonzero but input data was provided.')
     })
+
+    // 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 encodedTransaction = await wallet.signTransaction(transaction)
+
+      await expect(
+        OVM_ECDSAContractAccount.execute(encodedTransaction)
+      ).to.be.revertedWith(
+        'Calls to self are disabled until upgradability is re-enabled.'
+      )
+    })
   })
 })
diff --git a/packages/contracts/test/contracts/OVM/accounts/OVM_ProxyEOA.spec.ts b/packages/contracts/test/contracts/OVM/accounts/OVM_ProxyEOA.spec.ts
@@ -44,9 +44,7 @@ describe('OVM_ProxyEOA', () => {
     })
   })
 
-  // NOTE: Upgrades are disabled for now but will be re-enabled at a later point in time. See
-  // comment in OVM_ProxyEOA.sol for additional information.
-  describe.skip('upgrade()', () => {
+  describe('upgrade()', () => {
     it(`should upgrade the proxy implementation`, async () => {
       const newImpl = `0x${'81'.repeat(20)}`
       Mock__OVM_ExecutionManager.smocked.ovmADDRESS.will.return.with(