Skip to content
This repository has been archived by the owner on Apr 12, 2021. It is now read-only.

[wip] OVM self-upgradability #357

Draft
wants to merge 38 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b6289b2
new create codepath working
ben-chain Mar 8, 2021
8b7aa9d
get calls ported over to new function
ben-chain Mar 8, 2021
c8b9a5c
remove from interface
ben-chain Mar 8, 2021
810b67a
Merge branch 'master' into feat/simplify-creation
ben-chain Mar 8, 2021
8ccd116
clean up, add comments
ben-chain Mar 8, 2021
6e71f6a
remove unused commented code
ben-chain Mar 8, 2021
222e345
fix nuisance gas
ben-chain Mar 9, 2021
42035c6
add return type to ovmCREATE
ben-chain Mar 9, 2021
a648449
focus on final failing test
ben-chain Mar 9, 2021
563853f
finish up tests with new contract account revert type
ben-chain Mar 9, 2021
bd7cc45
remove test focus
ben-chain Mar 9, 2021
477819b
linting
ben-chain Mar 9, 2021
9ed5fee
cleanup
ben-chain Mar 9, 2021
18734c7
add explicit unsafe bytecode test
ben-chain Mar 9, 2021
eb8e24d
linting
ben-chain Mar 9, 2021
06f3399
Merge branch 'master' into feat/simplify-creation
ben-chain Mar 9, 2021
d546cf0
remove pointless ternary
ben-chain Mar 9, 2021
99d96d6
fix docstring
ben-chain Mar 9, 2021
bbb4bc5
stub putcode
ben-chain Mar 9, 2021
7f17283
unauthenticated upgrader for easy testing
ben-chain Mar 10, 2021
ec4d11d
Merge branch 'master' into feat/763/ovm-self-upgrade
ben-chain Mar 10, 2021
31f87eb
Merge branch 'master' into feat/simplify-creation
ben-chain Mar 11, 2021
c64991d
fix eth_call for creates and fix contract account reversions
ben-chain Mar 11, 2021
4278b54
use if statement instead
ben-chain Mar 11, 2021
b8e3810
nits
ben-chain Mar 11, 2021
b8027b2
Update contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManag…
ben-chain Mar 11, 2021
250442d
Update contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManag…
ben-chain Mar 11, 2021
5856cf8
Update contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManag…
ben-chain Mar 11, 2021
821d27a
Update contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManag…
ben-chain Mar 11, 2021
014ed31
Update contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManag…
ben-chain Mar 11, 2021
4a0bdd4
Merge branch 'feat/simplify-creation' into feat/763/ovm-self-upgrade
ben-chain Mar 11, 2021
aff7395
auth skeleton
ben-chain Mar 29, 2021
d111f06
Merge branch 'master' into feat/763/ovm-self-upgrade
ben-chain Mar 29, 2021
bc52f97
add tests setstorage
ben-chain Mar 30, 2021
6bfa26a
update smock, get smoddit assertions passing
ben-chain Mar 30, 2021
c7240c4
finish unit tests
ben-chain Apr 2, 2021
843d684
clean up EM
ben-chain Apr 2, 2021
29c3d2a
address PR feedback
ben-chain Apr 5, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,18 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
_;
}

/**
* Modifies a function so that only a given address can call it.
*/
modifier onlyCallableBy(
address _allowed
) {
if (ovmADDRESS() != _allowed) {
_revertWithFlag(RevertFlag.CALLER_NOT_ALLOWED);
}
_;
}


/************************************
* Transaction Execution Entrypoint *
Expand Down Expand Up @@ -992,6 +1004,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
|| flag == RevertFlag.UNSAFE_BYTECODE
|| flag == RevertFlag.STATIC_VIOLATION
|| flag == RevertFlag.CREATOR_NOT_ALLOWED
|| flag == RevertFlag.CALLER_NOT_ALLOWED
) {
transactionRecord.ovmGasRefund = ovmGasRefund;
}
Expand Down Expand Up @@ -1815,6 +1828,52 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
messageRecord.nuisanceGasLeft = 0;
}


/*********************
* Upgrade Functions *
*********************/

/**
* Sets the code of an ovm contract.
* @param _address Address to update the code of.
* @param _code Bytecode to put into the ovm account.
*/
function ovmSETCODE(
address _address,
bytes memory _code
)
override
external
onlyCallableBy(resolve("OVM_Upgrader"))
{
_checkAccountLoad(_address);
ovmStateManager.putAccountCode(_address, _code);
}


/**
* Sets the storage slot of an OVM contract.
* @param _address OVM account to set storage of.
* @param _key Key to set set.
* @param _value Value to store at the given key.
*/
function ovmSETSTORAGE(
address _address,
bytes32 _key,
bytes32 _value
)
override
external
onlyCallableBy(resolve("OVM_Upgrader"))
{
_putContractStorage(
_address,
_key,
_value
);
}


/*****************************
* L2-only Helper Functions *
*****************************/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,23 @@ contract OVM_StateManager is iOVM_StateManager {
account.codeHash = EMPTY_ACCOUNT_CODE_HASH;
}

/**
* Inserts the given bytecode into the given OVM account.
* @param _address Address of the account to overwrite code onto.
* @param _code Bytecode to put at the address.
*/
function putAccountCode(
address _address,
bytes memory _code
)
override
public
authenticated
{
Lib_OVMCodec.Account storage account = accounts[_address];
account.codeHash = keccak256(_code);
}

/**
* Retrieves an account from the state.
* @param _address Address of the account to retrieve.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: MIT
pragma solidity >0.5.0 <0.8.0;

/* Library Imports */
import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol";

contract OVM_Upgrader {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs comments

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also although IMO the name OVM_Upgrader is fine, we should take a few mins to brainstorm names + make sure the name won't be confusing going forward. Since it's much easier to change the name now than in the future, as we're well aware...

// TODO: implement me
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ interface iOVM_ExecutionManager {
UNSAFE_BYTECODE,
CREATE_COLLISION,
STATIC_VIOLATION,
CREATOR_NOT_ALLOWED
CREATOR_NOT_ALLOWED,
CALLER_NOT_ALLOWED
}

enum GasMetadataKey {
Expand Down Expand Up @@ -153,4 +154,12 @@ interface iOVM_ExecutionManager {
***************************************/

function getMaxTransactionGasLimit() external view returns (uint _maxTransactionGasLimit);


/*********************
* Upgrade Functions *
*********************/

function ovmSETCODE(address _address, bytes memory _code) external;
function ovmSETSTORAGE(address _address, bytes32 _key, bytes32 _value) external;
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,15 @@ interface iOVM_StateManager {

function putAccount(address _address, Lib_OVMCodec.Account memory _account) external;
function putEmptyAccount(address _address) external;
function putAccountCode(address _address, bytes memory _code) external;
function getAccount(address _address) external view returns (Lib_OVMCodec.Account memory _account);
function hasAccount(address _address) external view returns (bool _exists);
function hasEmptyAccount(address _address) external view returns (bool _exists);
function setAccountNonce(address _address, uint256 _nonce) external;
function getAccountNonce(address _address) external view returns (uint256 _nonce);
function getAccountEthAddress(address _address) external view returns (address _ethAddress);
function getAccountStorageRoot(address _address) external view returns (bytes32 _storageRoot);
function initPendingAccount(address _address) external;
function initPendingAccount(address _address) external; // todo: deprecate/combine these two with this change?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder on this todo

function commitPendingAccount(address _address, address _ethAddress, bytes32 _codeHash) external;
function testAndSetAccountLoaded(address _address) external returns (bool _wasAccountAlreadyLoaded);
function testAndSetAccountChanged(address _address) external returns (bool _wasAccountAlreadyChanged);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,42 @@ library Lib_SafeExecutionManagerWrapper {
);
}

/**
* Performs a safe ovmSETCODE call.
*/
function safeSETCODE(
address _address,
bytes memory _code
)
internal
{
_safeExecutionManagerInteraction(
abi.encodeWithSignature(
"ovmSETCODE(address,bytes)",
_address,
_code
)
);
}

/**
* Performs a safe ovmSETSTORAGE call.
*/
function ovmSETSTORAGE(
address _address,
bytes memory _code
)
internal
{
_safeExecutionManagerInteraction(
abi.encodeWithSignature(
"ovmSETSTORAGE(address,bytes)",
_address,
_code
)
);
}

/*********************
* Private Functions *
*********************/
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
},
"devDependencies": {
"@eth-optimism/plugins": "^1.0.0-alpha.2",
"@eth-optimism/smock": "^1.0.0-alpha.3",
"@eth-optimism/smock": "1.0.0-alpha.4",
"@nomiclabs/hardhat-ethers": "^2.0.1",
"@nomiclabs/hardhat-waffle": "^2.0.1",
"@typechain/ethers-v5": "1.0.0",
Expand Down
4 changes: 4 additions & 0 deletions src/contract-deployment/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ export const makeContractDeployConfig = async (
'0x0000000000000000000000000000000000000000', // will be overridden by geth when state dump is ingested. Storage key: 0x0000000000000000000000000000000000000000000000000000000000000008
],
},
OVM_Upgrader: {
factory: getContractFactory('OVM_Upgrader'),
params: [],
},
'OVM_ChainStorageContainer:CTC:batches': {
factory: getContractFactory('OVM_ChainStorageContainer'),
params: [AddressManager.address, 'OVM_CanonicalTransactionChain'],
Expand Down
2 changes: 2 additions & 0 deletions src/contract-dumps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export const makeStateDump = async (cfg: RollupDeployConfig): Promise<any> => {
'OVM_ExecutionManager',
'OVM_StateManager',
'OVM_ETH',
'OVM_Upgrader',
'mockOVM_ECDSAContractAccount',
],
deployOverrides: {},
Expand All @@ -170,6 +171,7 @@ export const makeStateDump = async (cfg: RollupDeployConfig): Promise<any> => {
OVM_ETH: '0x4200000000000000000000000000000000000006',
OVM_L2CrossDomainMessenger: '0x4200000000000000000000000000000000000007',
Lib_AddressManager: '0x4200000000000000000000000000000000000008',
OVM_Upgrader: '0x4200000000000000000000000000000000000009',
ERC1820Registry: '0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24',
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ const test_nuisanceGas: TestDefinition = {
// This is because there is natural gas consumption between the ovmCALL(GAS/2) and ovmCREATE, which allots nuisance gas via _getNuisanceGasLimit.
// This means that the ovmCREATE exception, DOES consumes all nuisance gas allotted, but that allotment
// is less than the full OVM_TX_GAS_LIMIT / 2 which is alloted to the parent ovmCALL.
nuisanceGasLeft: 4531286,
nuisanceGasLeft: 4200163,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,24 @@ const test_ovmCREATE: TestDefinition = {
},
],
},
{
name: 'ovmCREATE(UNSAFE_CODE)',
steps: [
{
functionName: 'ovmCREATE',
functionParams: {
bytecode: UNSAFE_BYTECODE,
},
expectedReturnStatus: true,
expectedReturnValue: {
address: ethers.constants.AddressZero,
revertData: encodeSolidityError(
'Constructor attempted to deploy unsafe bytecode.'
),
},
},
],
},
],
subTests: [
{
Expand Down
Loading