diff --git a/.changeset/sharp-roses-admire.md b/.changeset/sharp-roses-admire.md new file mode 100644 index 000000000000..4d3244390799 --- /dev/null +++ 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 index 0d5c5133741b..37d267f72ca3 100644 --- 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 index a8dbc6c38c0f..5965ab317a5f 100644 --- 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 index 9145d9e2b746..caaf33330025 100644 --- 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 index a5a6f35f736a..a80e97a48cdc 100644 --- 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(