Commit 7c299c81 authored by Kelvin Fichter's avatar Kelvin Fichter

Various fixes based on audit feedback

parent e108ab0e
...@@ -5,7 +5,6 @@ import * as mkdirp from 'mkdirp' ...@@ -5,7 +5,6 @@ import * as mkdirp from 'mkdirp'
/* Internal Imports */ /* Internal Imports */
import { makeStateDump } from '../src' import { makeStateDump } from '../src'
;(async () => { ;(async () => {
const outdir = path.resolve(__dirname, '../build/dumps') const outdir = path.resolve(__dirname, '../build/dumps')
const outfile = path.join(outdir, 'state-dump.latest.json') const outfile = path.join(outdir, 'state-dump.latest.json')
......
...@@ -4,11 +4,11 @@ pragma experimental ABIEncoderV2; ...@@ -4,11 +4,11 @@ pragma experimental ABIEncoderV2;
/* Interface Imports */ /* Interface Imports */
import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContractAccount.sol"; import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContractAccount.sol";
import { iOVM_ExecutionManager } from "../../iOVM/execution/iOVM_ExecutionManager.sol";
/* Library Imports */ /* Library Imports */
import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol"; import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol";
import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol"; import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol";
import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol";
/** /**
* @title OVM_ECDSAContractAccount * @title OVM_ECDSAContractAccount
...@@ -43,7 +43,7 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -43,7 +43,7 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
bytes memory _returndata bytes memory _returndata
) )
{ {
iOVM_ExecutionManager ovmExecutionManager = iOVM_ExecutionManager(msg.sender); address ovmExecutionManager = msg.sender;
// Address of this contract within the ovm (ovmADDRESS) should be the same as the // Address of this contract within the ovm (ovmADDRESS) should be the same as the
// recovered address of the user who signed this message. This is how we manage to shim // recovered address of the user who signed this message. This is how we manage to shim
...@@ -55,8 +55,8 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -55,8 +55,8 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
_v, _v,
_r, _r,
_s, _s,
ovmExecutionManager.ovmCHAINID() Lib_SafeExecutionManagerWrapper.safeCHAINID(ovmExecutionManager)
) == ovmExecutionManager.ovmADDRESS(), ) == Lib_SafeExecutionManagerWrapper.safeADDRESS(ovmExecutionManager),
"Signature provided for EOA transaction execution is invalid." "Signature provided for EOA transaction execution is invalid."
); );
...@@ -64,14 +64,15 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -64,14 +64,15 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// Need to make sure that the transaction nonce is right and bump it if so. // Need to make sure that the transaction nonce is right and bump it if so.
require( require(
decodedTx.nonce == ovmExecutionManager.ovmGETNONCE() + 1, decodedTx.nonce == Lib_SafeExecutionManagerWrapper.safeGETNONCE(ovmExecutionManager) + 1,
"Transaction nonce does not match the expected nonce." "Transaction nonce does not match the expected nonce."
); );
ovmExecutionManager.ovmSETNONCE(decodedTx.nonce);
// Contract creations are signalled by sending a transaction to the zero address. // Contract creations are signalled by sending a transaction to the zero address.
if (decodedTx.target == address(0)) { if (decodedTx.target == address(0)) {
address created = ovmExecutionManager.ovmCREATE{gas: decodedTx.gasLimit}( address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
ovmExecutionManager,
decodedTx.gasLimit,
decodedTx.data decodedTx.data
); );
...@@ -79,7 +80,13 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -79,7 +80,13 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// initialization. Always return `true` for our success value here. // initialization. Always return `true` for our success value here.
return (true, abi.encode(created)); return (true, abi.encode(created));
} else { } else {
return ovmExecutionManager.ovmCALL( // We only want to bump the nonce for `ovmCALL` because `ovmCREATE` automatically bumps
// the nonce of the calling account. Normally an EOA would bump the nonce for both
// cases, but since this is a contract we'd end up bumping the nonce twice.
Lib_SafeExecutionManagerWrapper.safeSETNONCE(ovmExecutionManager, decodedTx.nonce);
return Lib_SafeExecutionManagerWrapper.safeCALL(
ovmExecutionManager,
decodedTx.gasLimit, decodedTx.gasLimit,
decodedTx.target, decodedTx.target,
decodedTx.data decodedTx.data
......
...@@ -63,11 +63,15 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -63,11 +63,15 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
* @param _libAddressManager Address of the Address Manager. * @param _libAddressManager Address of the Address Manager.
*/ */
constructor( constructor(
address _libAddressManager address _libAddressManager,
GasMeterConfig memory _gasMeterConfig,
GlobalContext memory _globalContext
) )
Lib_AddressResolver(_libAddressManager) Lib_AddressResolver(_libAddressManager)
{ {
ovmSafetyChecker = iOVM_SafetyChecker(resolve("OVM_SafetyChecker")); ovmSafetyChecker = iOVM_SafetyChecker(resolve("OVM_SafetyChecker"));
gasMeterConfig = _gasMeterConfig;
globalContext = _globalContext;
} }
...@@ -818,7 +822,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -818,7 +822,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
) )
{ {
// We always update the nonce of the creating account, even if the creation fails. // We always update the nonce of the creating account, even if the creation fails.
_setAccountNonce(ovmADDRESS(), 1); _setAccountNonce(ovmADDRESS(), _getAccountNonce(ovmADDRESS()) + 1);
// We're stepping into a CREATE or CREATE2, so we need to update ADDRESS to point // We're stepping into a CREATE or CREATE2, so we need to update ADDRESS to point
// to the contract's associated address and CALLER to point to the previous ADDRESS. // to the contract's associated address and CALLER to point to the previous ADDRESS.
...@@ -1544,7 +1548,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -1544,7 +1548,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
_getGasMetadata(cumulativeGasKey) _getGasMetadata(cumulativeGasKey)
- _getGasMetadata(prevEpochGasKey) - _getGasMetadata(prevEpochGasKey)
+ _gasLimit + _gasLimit
) > gasMeterConfig.maxGasPerQueuePerEpoch ) < gasMeterConfig.maxGasPerQueuePerEpoch
); );
} }
...@@ -1666,6 +1670,8 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -1666,6 +1670,8 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
transactionContext.ovmL1QUEUEORIGIN = _transaction.l1QueueOrigin; transactionContext.ovmL1QUEUEORIGIN = _transaction.l1QueueOrigin;
transactionContext.ovmL1TXORIGIN = _transaction.l1Txorigin; transactionContext.ovmL1TXORIGIN = _transaction.l1Txorigin;
transactionContext.ovmGASLIMIT = gasMeterConfig.maxGasPerQueuePerEpoch; transactionContext.ovmGASLIMIT = gasMeterConfig.maxGasPerQueuePerEpoch;
messageRecord.nuisanceGasLeft = _getNuisanceGasLimit(_transaction.gasLimit);
} }
/** /**
......
...@@ -262,16 +262,27 @@ contract OVM_StateTransitioner is iOVM_StateTransitioner, Lib_AddressResolver { ...@@ -262,16 +262,27 @@ contract OVM_StateTransitioner is iOVM_StateTransitioner, Lib_AddressResolver {
"Contract must be verified before proving a storage slot." "Contract must be verified before proving a storage slot."
); );
require( (
Lib_SecureMerkleTrie.verifyInclusionProof( bool exists,
abi.encodePacked(_key), bytes memory value
abi.encodePacked(_value), ) = Lib_SecureMerkleTrie.get(
_storageTrieWitness, abi.encodePacked(_key),
ovmStateManager.getAccountStorageRoot(_ovmContractAddress) _storageTrieWitness,
), ovmStateManager.getAccountStorageRoot(_ovmContractAddress)
"Storage slot is invalid or invalid inclusion proof provided."
); );
if (exists == true) {
require(
keccak256(value) == keccak256(abi.encodePacked(_value)),
"Provided storage slot value is invalid."
);
} else {
require(
_value == bytes32(0),
"Provided storage slot value is invalid."
);
}
ovmStateManager.putContractStorage( ovmStateManager.putContractStorage(
_ovmContractAddress, _ovmContractAddress,
_key, _key,
......
...@@ -207,8 +207,8 @@ library Lib_OVMCodec { ...@@ -207,8 +207,8 @@ library Lib_OVMCodec {
// index-by-index circumvents this issue. // index-by-index circumvents this issue.
raw[0] = Lib_RLPWriter.writeUint(_account.nonce); raw[0] = Lib_RLPWriter.writeUint(_account.nonce);
raw[1] = Lib_RLPWriter.writeUint(_account.balance); raw[1] = Lib_RLPWriter.writeUint(_account.balance);
raw[2] = _account.storageRoot == 0 ? RLP_NULL_BYTES : Lib_RLPWriter.writeBytes(abi.encodePacked(_account.storageRoot)); raw[2] = Lib_RLPWriter.writeBytes(abi.encodePacked(_account.storageRoot));
raw[3] = _account.codeHash == 0 ? RLP_NULL_BYTES : Lib_RLPWriter.writeBytes(abi.encodePacked(_account.codeHash)); raw[3] = Lib_RLPWriter.writeBytes(abi.encodePacked(_account.codeHash));
return Lib_RLPWriter.writeList(raw); return Lib_RLPWriter.writeList(raw);
} }
......
...@@ -389,6 +389,10 @@ library Lib_RLPReader { ...@@ -389,6 +389,10 @@ library Lib_RLPReader {
address address
) )
{ {
if (_in.length == 1) {
return address(0);
}
require( require(
_in.length == 21, _in.length == 21,
"Invalid RLP address value." "Invalid RLP address value."
......
...@@ -6,8 +6,6 @@ import { Lib_BytesUtils } from "../utils/Lib_BytesUtils.sol"; ...@@ -6,8 +6,6 @@ import { Lib_BytesUtils } from "../utils/Lib_BytesUtils.sol";
import { Lib_RLPReader } from "../rlp/Lib_RLPReader.sol"; import { Lib_RLPReader } from "../rlp/Lib_RLPReader.sol";
import { Lib_RLPWriter } from "../rlp/Lib_RLPWriter.sol"; import { Lib_RLPWriter } from "../rlp/Lib_RLPWriter.sol";
import { console } from "@nomiclabs/buidler/console.sol";
/** /**
* @title Lib_MerkleTrie * @title Lib_MerkleTrie
*/ */
......
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.7.0;
/**
* @title Lib_SafeExecutionManagerWrapper
*/
library Lib_SafeExecutionManagerWrapper {
/**********************
* Internal Functions *
**********************/
/**
* Makes an ovmCALL and performs all the necessary safety checks.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @param _gasLimit Gas limit for the call.
* @param _target Address to call.
* @param _calldata Data to send to the call.
* @return _success Whether or not the call reverted.
* @return _returndata Data returned by the call.
*/
function safeCALL(
address _ovmExecutionManager,
uint256 _gasLimit,
address _target,
bytes memory _calldata
)
internal
returns (
bool _success,
bytes memory _returndata
)
{
bytes memory returndata = _safeExecutionManagerInteraction(
_ovmExecutionManager,
abi.encodeWithSignature(
"ovmCALL(uint256,address,bytes)",
_gasLimit,
_target,
_calldata
)
);
return abi.decode(returndata, (bool, bytes));
}
/**
* Performs an ovmCREATE and the necessary safety checks.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @param _gasLimit Gas limit for the creation.
* @param _bytecode Code for the new contract.
* @return _contract Address of the created contract.
*/
function safeCREATE(
address _ovmExecutionManager,
uint256 _gasLimit,
bytes memory _bytecode
)
internal
returns (
address _contract
)
{
bytes memory returndata = _safeExecutionManagerInteraction(
_ovmExecutionManager,
_gasLimit,
abi.encodeWithSignature(
"ovmCREATE(bytes)",
_bytecode
)
);
return abi.decode(returndata, (address));
}
/**
* Performs a safe ovmCHAINID call.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @return _CHAINID Result of calling ovmCHAINID.
*/
function safeCHAINID(
address _ovmExecutionManager
)
internal
returns (
uint256 _CHAINID
)
{
bytes memory returndata = _safeExecutionManagerInteraction(
_ovmExecutionManager,
abi.encodeWithSignature(
"ovmCHAINID()"
)
);
return abi.decode(returndata, (uint256));
}
/**
* Performs a safe ovmADDRESS call.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @return _ADDRESS Result of calling ovmADDRESS.
*/
function safeADDRESS(
address _ovmExecutionManager
)
internal
returns (
address _ADDRESS
)
{
bytes memory returndata = _safeExecutionManagerInteraction(
_ovmExecutionManager,
abi.encodeWithSignature(
"ovmADDRESS()"
)
);
return abi.decode(returndata, (address));
}
/**
* Performs a safe ovmGETNONCE call.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @return _nonce Result of calling ovmGETNONCE.
*/
function safeGETNONCE(
address _ovmExecutionManager
)
internal
returns (
uint256 _nonce
)
{
bytes memory returndata = _safeExecutionManagerInteraction(
_ovmExecutionManager,
abi.encodeWithSignature(
"ovmGETNONCE()"
)
);
return abi.decode(returndata, (uint256));
}
/**
* Performs a safe ovmSETNONCE call.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @param _nonce New account nonce.
*/
function safeSETNONCE(
address _ovmExecutionManager,
uint256 _nonce
)
internal
{
_safeExecutionManagerInteraction(
_ovmExecutionManager,
abi.encodeWithSignature(
"ovmSETNONCE(uint256)",
_nonce
)
);
}
/*********************
* Private Functions *
*********************/
/**
* Performs an ovm interaction and the necessary safety checks.
* @param _ovmExecutionManager Address of the OVM_ExecutionManager.
* @param _gasLimit Gas limit for the interaction.
* @param _calldata Data to send to the OVM_ExecutionManager (encoded with sighash).
* @return _returndata Data sent back by the OVM_ExecutionManager.
*/
function _safeExecutionManagerInteraction(
address _ovmExecutionManager,
uint256 _gasLimit,
bytes memory _calldata
)
private
returns (
bytes memory _returndata
)
{
(
bool success,
bytes memory returndata
) = _ovmExecutionManager.call{gas: _gasLimit}(_calldata);
if (success == false) {
assembly {
revert(add(returndata, 0x20), mload(returndata))
}
} else if (returndata.length == 1) {
assembly {
return(0, 1)
}
} else {
return returndata;
}
}
function _safeExecutionManagerInteraction(
address _ovmExecutionManager,
bytes memory _calldata
)
private
returns (
bytes memory _returndata
)
{
return _safeExecutionManagerInteraction(
_ovmExecutionManager,
gasleft(),
_calldata
);
}
}
...@@ -4,11 +4,11 @@ pragma experimental ABIEncoderV2; ...@@ -4,11 +4,11 @@ pragma experimental ABIEncoderV2;
/* Interface Imports */ /* Interface Imports */
import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContractAccount.sol"; import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContractAccount.sol";
import { iOVM_ExecutionManager } from "../../iOVM/execution/iOVM_ExecutionManager.sol";
/* Library Imports */ /* Library Imports */
import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol"; import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol";
import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol"; import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol";
import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol";
/** /**
* @title mockOVM_ECDSAContractAccount * @title mockOVM_ECDSAContractAccount
...@@ -43,22 +43,16 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -43,22 +43,16 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
bytes memory _returndata bytes memory _returndata
) )
{ {
iOVM_ExecutionManager ovmExecutionManager = iOVM_ExecutionManager(msg.sender); address ovmExecutionManager = msg.sender;
// Skip signature validation within the mock.
// Skip signature validation in this mock.
Lib_OVMCodec.EOATransaction memory decodedTx = Lib_OVMCodec.decodeEOATransaction(_transaction); Lib_OVMCodec.EOATransaction memory decodedTx = Lib_OVMCodec.decodeEOATransaction(_transaction);
// Need to make sure that the transaction nonce is right and bump it if so.
require(
decodedTx.nonce == ovmExecutionManager.ovmGETNONCE() + 1,
"Transaction nonce does not match the expected nonce."
);
ovmExecutionManager.ovmSETNONCE(decodedTx.nonce);
// Contract creations are signalled by sending a transaction to the zero address. // Contract creations are signalled by sending a transaction to the zero address.
if (decodedTx.target == address(0)) { if (decodedTx.target == address(0)) {
address created = ovmExecutionManager.ovmCREATE{gas: decodedTx.gasLimit}( address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
ovmExecutionManager,
decodedTx.gasLimit,
decodedTx.data decodedTx.data
); );
...@@ -66,7 +60,8 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -66,7 +60,8 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// initialization. Always return `true` for our success value here. // initialization. Always return `true` for our success value here.
return (true, abi.encode(created)); return (true, abi.encode(created));
} else { } else {
return ovmExecutionManager.ovmCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
ovmExecutionManager,
decodedTx.gasLimit, decodedTx.gasLimit,
decodedTx.target, decodedTx.target,
decodedTx.data decodedTx.data
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
], ],
"license": "MIT", "license": "MIT",
"scripts": { "scripts": {
"build": "yarn run build:contracts && yarn run build:typescript && yarn run build:copy", "build": "yarn run build:contracts && yarn run build:typescript && yarn run build:copy && yarn run build:dump",
"build:typescript": "tsc -p .", "build:typescript": "tsc -p .",
"build:contracts": "buidler compile", "build:contracts": "buidler compile",
"build:dump": "ts-node \"bin/take-dump.ts\"", "build:dump": "ts-node \"bin/take-dump.ts\"",
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
"ganache-core": "^2.12.1", "ganache-core": "^2.12.1",
"lodash": "^4.17.20", "lodash": "^4.17.20",
"merkle-patricia-tree": "git+https://github.com/kfichter/merkle-patricia-tree", "merkle-patricia-tree": "git+https://github.com/kfichter/merkle-patricia-tree",
"mkdirp": "^1.0.4",
"mocha": "^8.1.1", "mocha": "^8.1.1",
"prettier": "^2.1.2", "prettier": "^2.1.2",
"random-bytes-seed": "^1.0.3", "random-bytes-seed": "^1.0.3",
......
...@@ -12,6 +12,9 @@ export interface RollupDeployConfig { ...@@ -12,6 +12,9 @@ export interface RollupDeployConfig {
maxGasPerQueuePerEpoch: number maxGasPerQueuePerEpoch: number
secondsPerEpoch: number secondsPerEpoch: number
} }
ovmGlobalContext: {
ovmCHAINID: number
}
transactionChainConfig: { transactionChainConfig: {
sequencer: string | Signer sequencer: string | Signer
forceInclusionPeriodSeconds: number forceInclusionPeriodSeconds: number
...@@ -83,7 +86,11 @@ export const makeContractDeployConfig = async ( ...@@ -83,7 +86,11 @@ export const makeContractDeployConfig = async (
}, },
OVM_ExecutionManager: { OVM_ExecutionManager: {
factory: getContractFactory('OVM_ExecutionManager'), factory: getContractFactory('OVM_ExecutionManager'),
params: [AddressManager.address], params: [
AddressManager.address,
config.ovmGasMeteringConfig,
config.ovmGlobalContext
],
}, },
OVM_StateManager: { OVM_StateManager: {
factory: getContractFactory('OVM_StateManager'), factory: getContractFactory('OVM_StateManager'),
...@@ -108,5 +115,11 @@ export const makeContractDeployConfig = async ( ...@@ -108,5 +115,11 @@ export const makeContractDeployConfig = async (
OVM_StateTransitionerFactory: { OVM_StateTransitionerFactory: {
factory: getContractFactory('OVM_StateTransitionerFactory'), factory: getContractFactory('OVM_StateTransitionerFactory'),
}, },
OVM_ECDSAContractAccount: {
factory: getContractFactory('OVM_ECDSAContractAccount')
},
mockOVM_ECDSAContractAccount: {
factory: getContractFactory('mockOVM_ECDSAContractAccount')
}
} }
} }
...@@ -45,10 +45,12 @@ export const deploy = async ( ...@@ -45,10 +45,12 @@ export const deploy = async (
const SimpleProxy = await Factory__SimpleProxy.deploy() const SimpleProxy = await Factory__SimpleProxy.deploy()
await AddressManager.setAddress(name, SimpleProxy.address) await AddressManager.setAddress(name, SimpleProxy.address)
contracts[`Proxy__${name}`] = SimpleProxy
try { try {
contracts[name] = await contractDeployParameters.factory contracts[name] = await contractDeployParameters.factory
.connect(config.deploymentSigner) .connect(config.deploymentSigner)
.deploy(...contractDeployParameters.params) .deploy(...(contractDeployParameters.params || []))
await SimpleProxy.setTarget(contracts[name].address) await SimpleProxy.setTarget(contracts[name].address)
} catch (err) { } catch (err) {
failedDeployments.push(name) failedDeployments.push(name)
......
...@@ -7,6 +7,7 @@ import { keccak256 } from 'ethers/lib/utils' ...@@ -7,6 +7,7 @@ import { keccak256 } from 'ethers/lib/utils'
/* Internal Imports */ /* Internal Imports */
import { deploy, RollupDeployConfig } from './contract-deployment' import { deploy, RollupDeployConfig } from './contract-deployment'
import { fromHexString, toHexString, remove0x } from '../test/helpers/utils' import { fromHexString, toHexString, remove0x } from '../test/helpers/utils'
import { getContractDefinition } from './contract-defs'
interface StorageDump { interface StorageDump {
[key: string]: string [key: string]: string
...@@ -114,6 +115,9 @@ export const makeStateDump = async (): Promise<any> => { ...@@ -114,6 +115,9 @@ export const makeStateDump = async (): Promise<any> => {
maxGasPerQueuePerEpoch: 1_000_000_000_000, maxGasPerQueuePerEpoch: 1_000_000_000_000,
secondsPerEpoch: 600, secondsPerEpoch: 600,
}, },
ovmGlobalContext: {
ovmCHAINID: 420
},
transactionChainConfig: { transactionChainConfig: {
sequencer: signer, sequencer: signer,
forceInclusionPeriodSeconds: 600, forceInclusionPeriodSeconds: 600,
...@@ -131,19 +135,23 @@ export const makeStateDump = async (): Promise<any> => { ...@@ -131,19 +135,23 @@ export const makeStateDump = async (): Promise<any> => {
'OVM_SafetyChecker', 'OVM_SafetyChecker',
'OVM_ExecutionManager', 'OVM_ExecutionManager',
'OVM_StateManager', 'OVM_StateManager',
'OVM_ECDSAContractAccount' 'mockOVM_ECDSAContractAccount'
] ]
} }
const precompiles = { const precompiles = {
OVM_L2ToL1MessagePasser: '4200000000000000000000000000000000000000', OVM_L2ToL1MessagePasser: '0x4200000000000000000000000000000000000000',
OVM_L1MessageSender: '4200000000000000000000000000000000000001', OVM_L1MessageSender: '0x4200000000000000000000000000000000000001',
OVM_DeployerWhitelist: '4200000000000000000000000000000000000002' OVM_DeployerWhitelist: '0x4200000000000000000000000000000000000002'
} }
const deploymentResult = await deploy(config) const deploymentResult = await deploy(config)
deploymentResult.contracts['Lib_AddressManager'] = deploymentResult.AddressManager deploymentResult.contracts['Lib_AddressManager'] = deploymentResult.AddressManager
if (deploymentResult.failedDeployments.length > 0) {
throw new Error(`Could not generate state dump, deploy failed for: ${deploymentResult.failedDeployments}`)
}
const pStateManager = ganache.engine.manager.state.blockchain.vm.pStateManager const pStateManager = ganache.engine.manager.state.blockchain.vm.pStateManager
const cStateManager = pStateManager._wrapped const cStateManager = pStateManager._wrapped
...@@ -158,14 +166,14 @@ export const makeStateDump = async (): Promise<any> => { ...@@ -158,14 +166,14 @@ export const makeStateDump = async (): Promise<any> => {
const codeBuf = await pStateManager.getContractCode(fromHexString(contract.address)) const codeBuf = await pStateManager.getContractCode(fromHexString(contract.address))
const code = toHexString(codeBuf) const code = toHexString(codeBuf)
const deadAddress = precompiles[name] || `deaddeaddeaddeaddeaddeaddeaddeaddead${i.toString(16).padStart(4, '0')}` const deadAddress = precompiles[name] || `0xdeaddeaddeaddeaddeaddeaddeaddeaddead${i.toString(16).padStart(4, '0')}`
dump.accounts[name] = { dump.accounts[name] = {
address: deadAddress, address: deadAddress,
code, code,
codeHash: keccak256(code), codeHash: keccak256(code),
storage: await getStorageDump(cStateManager, contract.address), storage: await getStorageDump(cStateManager, contract.address),
abi: JSON.parse(contract.interface.format('json') as string), abi: getContractDefinition(name.replace('Proxy__', '')).abi
} }
} }
......
...@@ -15,10 +15,16 @@ import { ...@@ -15,10 +15,16 @@ import {
TrieTestGenerator, TrieTestGenerator,
ZERO_ADDRESS, ZERO_ADDRESS,
} from '../../../helpers' } from '../../../helpers'
import { MockContract, smockit } from '@eth-optimism/smock' import {
MockContract,
smockit,
ModifiableContract,
smoddit,
ModifiableContractFactory,
} from '@eth-optimism/smock'
import { keccak256 } from 'ethers/lib/utils' import { keccak256 } from 'ethers/lib/utils'
describe.skip('OVM_StateTransitioner', () => { describe('OVM_StateTransitioner', () => {
let AddressManager: Contract let AddressManager: Contract
before(async () => { before(async () => {
AddressManager = await makeAddressManager() AddressManager = await makeAddressManager()
...@@ -57,14 +63,12 @@ describe.skip('OVM_StateTransitioner', () => { ...@@ -57,14 +63,12 @@ describe.skip('OVM_StateTransitioner', () => {
Mock__OVM_StateManager.smocked.putAccount.will.return() Mock__OVM_StateManager.smocked.putAccount.will.return()
}) })
let Factory__OVM_StateTransitioner: ContractFactory let Factory__OVM_StateTransitioner: ModifiableContractFactory
before(async () => { before(async () => {
Factory__OVM_StateTransitioner = await ethers.getContractFactory( Factory__OVM_StateTransitioner = await smoddit('OVM_StateTransitioner')
'OVM_StateTransitioner'
)
}) })
let OVM_StateTransitioner: Contract let OVM_StateTransitioner: ModifiableContract
beforeEach(async () => { beforeEach(async () => {
OVM_StateTransitioner = await Factory__OVM_StateTransitioner.deploy( OVM_StateTransitioner = await Factory__OVM_StateTransitioner.deploy(
AddressManager.address, AddressManager.address,
...@@ -172,8 +176,8 @@ describe.skip('OVM_StateTransitioner', () => { ...@@ -172,8 +176,8 @@ describe.skip('OVM_StateTransitioner', () => {
BigNumber.from(account.balance), BigNumber.from(account.balance),
account.storageRoot, account.storageRoot,
account.codeHash, account.codeHash,
account.ethAddress, ethContractAddress,
account.isFresh, false,
], ],
]) ])
}) })
...@@ -207,52 +211,81 @@ describe.skip('OVM_StateTransitioner', () => { ...@@ -207,52 +211,81 @@ describe.skip('OVM_StateTransitioner', () => {
describe('when the corresponding account is proven', () => { describe('when the corresponding account is proven', () => {
beforeEach(() => { beforeEach(() => {
Mock__OVM_StateManager.smocked.hasAccount.will.return.with(false) Mock__OVM_StateManager.smocked.hasAccount.will.return.with(true)
}) })
describe('when provided an invalid slot inclusion proof', () => { describe('when provided an invalid slot inclusion proof', () => {
let account: any let key = keccak256('0x1234')
let proof: string let val = keccak256('0x5678')
let proof = '0x'
beforeEach(async () => { beforeEach(async () => {
account = { const generator = await TrieTestGenerator.fromNodes({
nonce: 0, nodes: [
balance: 0,
storageRoot: NULL_BYTES32,
codeHash: NULL_BYTES32,
}
const generator = await TrieTestGenerator.fromAccounts({
accounts: [
{ {
...account, key,
address: NON_ZERO_ADDRESS, val,
storage: [
{
key: keccak256('0x1234'),
val: keccak256('0x5678'),
},
],
}, },
], ],
secure: true, secure: true,
}) })
const test = await generator.makeAccountProofTest(NON_ZERO_ADDRESS) const test = await generator.makeInclusionProofTest(0)
proof = test.accountTrieWitness
OVM_StateTransitioner = await Factory__OVM_StateTransitioner.deploy( Mock__OVM_StateManager.smocked.getAccountStorageRoot.will.return.with(
AddressManager.address, test.root
0,
test.accountTrieRoot,
NULL_BYTES32
) )
}) })
it('should revert', async () => {}) it('should revert', async () => {
await expect(
OVM_StateTransitioner.proveStorageSlot(
ZERO_ADDRESS,
key,
val,
proof
)
).to.be.reverted
})
}) })
describe('when provided a valid slot inclusion proof', () => {}) describe('when provided a valid slot inclusion proof', () => {
let key = keccak256('0x1234')
let val = keccak256('0x5678')
let proof: string
beforeEach(async () => {
const generator = await TrieTestGenerator.fromNodes({
nodes: [
{
key,
val,
},
],
secure: true,
})
const test = await generator.makeInclusionProofTest(0)
proof = test.proof
Mock__OVM_StateManager.smocked.getAccountStorageRoot.will.return.with(
test.root
)
})
it('should insert the storage slot', async () => {
await expect(
OVM_StateTransitioner.proveStorageSlot(
ZERO_ADDRESS,
key,
val,
proof
)
).to.not.be.reverted
expect(
Mock__OVM_StateManager.smocked.putContractStorage.calls[0]
).to.deep.equal([ZERO_ADDRESS, key, val])
})
})
}) })
}) })
...@@ -261,46 +294,280 @@ describe.skip('OVM_StateTransitioner', () => { ...@@ -261,46 +294,280 @@ describe.skip('OVM_StateTransitioner', () => {
}) })
describe('commitContractState', () => { describe('commitContractState', () => {
describe('when the account was not changed', () => { beforeEach(async () => {
it('should revert', async () => {}) OVM_StateTransitioner.smodify.set({
phase: 1,
})
}) })
describe('when the account was changed', () => { let ovmContractAddress = NON_ZERO_ADDRESS
describe('when the account has not been committed', () => { let account: any
it('should commit the account and update the state', async () => {}) beforeEach(() => {
Mock__OVM_StateManager.smocked.hasAccount.will.return.with(false)
account = {
nonce: 0,
balance: 0,
storageRoot: NULL_BYTES32,
codeHash: NULL_BYTES32,
}
})
describe('when the account was not changed or has already been committed', () => {
before(() => {
Mock__OVM_StateManager.smocked.commitAccount.will.return.with(false)
})
it('should revert', async () => {
await expect(
OVM_StateTransitioner.commitContractState(
ovmContractAddress,
account,
'0x'
)
).to.be.revertedWith(
'Account was not changed or has already been committed.'
)
})
})
describe('when the account was changed or has not already been committed', () => {
before(() => {
Mock__OVM_StateManager.smocked.commitAccount.will.return.with(true)
}) })
describe('when the account was already committed', () => { describe('when given an valid update proof', () => {
it('should revert', () => {}) let proof: string
let postStateRoot: string
beforeEach(async () => {
const generator = await TrieTestGenerator.fromAccounts({
accounts: [
{
...account,
nonce: 10,
address: ovmContractAddress,
},
],
secure: true,
})
const test = await generator.makeAccountUpdateTest(
ovmContractAddress,
account
)
proof = test.accountTrieWitness
postStateRoot = test.newAccountTrieRoot
OVM_StateTransitioner.smodify.put({
postStateRoot: test.accountTrieRoot,
})
})
it('should update the post state root', async () => {
await expect(
OVM_StateTransitioner.commitContractState(
ovmContractAddress,
account,
proof
)
).to.not.be.reverted
expect(await OVM_StateTransitioner.getPostStateRoot()).to.equal(
postStateRoot
)
})
}) })
}) })
}) })
describe('commitStorageSlot', () => { describe('commitStorageSlot', () => {
describe('when the slot was not changed', () => { beforeEach(() => {
it('should revert', async () => {}) OVM_StateTransitioner.smodify.set({
phase: 1,
})
}) })
describe('when the slot was changed', () => { let ovmContractAddress = NON_ZERO_ADDRESS
describe('when the slot has not been committed', () => { let account: any
it('should commit the slot and update the state', async () => {}) let key = keccak256('0x1234')
let val = keccak256('0x5678')
let newVal = keccak256('0x4321')
beforeEach(() => {
account = {
nonce: 0,
balance: 0,
storageRoot: NULL_BYTES32,
codeHash: NULL_BYTES32,
}
Mock__OVM_StateManager.smocked.getAccount.will.return.with({
...account,
ethAddress: ZERO_ADDRESS,
isFresh: false,
}) })
})
describe('when the slot was already committed', () => { describe('when the slot was not changed or was already committed', () => {
it('should revert', () => {}) beforeEach(() => {
Mock__OVM_StateManager.smocked.commitContractStorage.will.return.with(
false
)
})
it('should revert', async () => {
await expect(
OVM_StateTransitioner.commitStorageSlot(
ovmContractAddress,
key,
val,
'0x',
'0x'
)
).to.be.revertedWith(
'Storage slot was not changed or has already been committed.'
)
})
})
describe('when the slot was changed or not already committed', () => {
beforeEach(() => {
Mock__OVM_StateManager.smocked.commitContractStorage.will.return.with(
true
)
})
describe('with a valid proof', () => {
let accountTrieProof: string
let storageTrieProof: string
let postStateRoot: string
beforeEach(async () => {
const storageGenerator = await TrieTestGenerator.fromNodes({
nodes: [
{
key,
val,
},
],
secure: true,
})
const storageTest = await storageGenerator.makeNodeUpdateTest(
key,
newVal
)
const generator = await TrieTestGenerator.fromAccounts({
accounts: [
{
...account,
storageRoot: storageTest.root,
address: ovmContractAddress,
},
],
secure: true,
})
const test = await generator.makeAccountUpdateTest(
ovmContractAddress,
{
...account,
storageRoot: storageTest.newRoot,
}
)
Mock__OVM_StateManager.smocked.getAccount.will.return.with({
...account,
storageRoot: storageTest.root,
ethAddress: ZERO_ADDRESS,
isFresh: false,
})
accountTrieProof = test.accountTrieWitness
storageTrieProof = storageTest.proof
postStateRoot = test.newAccountTrieRoot
OVM_StateTransitioner.smodify.put({
postStateRoot: test.accountTrieRoot,
})
})
it('should commit the slot and update the state', async () => {
await expect(
OVM_StateTransitioner.commitStorageSlot(
ovmContractAddress,
key,
newVal,
accountTrieProof,
storageTrieProof
)
).to.not.be.reverted
})
}) })
}) })
}) })
describe('completeTransition', () => { describe('completeTransition', () => {
beforeEach(() => {
OVM_StateTransitioner.smodify.set({
phase: 1,
})
})
describe('when there are uncommitted accounts', () => { describe('when there are uncommitted accounts', () => {
it('should revert', async () => {}) beforeEach(() => {
Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
1
)
Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
0
)
})
it('should revert', async () => {
await expect(
OVM_StateTransitioner.completeTransition()
).to.be.revertedWith(
'All accounts must be committed before completing a transition.'
)
})
}) })
describe('when there are uncommitted storage slots', () => { describe('when there are uncommitted storage slots', () => {
it('should revert', async () => {}) beforeEach(() => {
Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
0
)
Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
1
)
})
it('should revert', async () => {
await expect(
OVM_StateTransitioner.completeTransition()
).to.be.revertedWith(
'All storage must be committed before completing a transition.'
)
})
}) })
describe('when all state changes are committed', () => {}) describe('when all state changes are committed', () => {
beforeEach(() => {
Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
0
)
Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
0
)
})
it('should complete the transition', async () => {
await expect(OVM_StateTransitioner.completeTransition()).to.not.be
.reverted
expect(await OVM_StateTransitioner.isComplete()).to.equal(true)
})
})
}) })
}) })
...@@ -39,10 +39,10 @@ ...@@ -39,10 +39,10 @@
resolved "https://registry.yarnpkg.com/@ensdomains/resolver/-/resolver-0.2.4.tgz#c10fe28bf5efbf49bff4666d909aed0265efbc89" resolved "https://registry.yarnpkg.com/@ensdomains/resolver/-/resolver-0.2.4.tgz#c10fe28bf5efbf49bff4666d909aed0265efbc89"
integrity sha512-bvaTH34PMCbv6anRa9I/0zjLJgY4EuznbEMgbV77JBCQ9KNC46rzi0avuxpOfu+xDjPEtSFGqVEOr5GlUSGudA== integrity sha512-bvaTH34PMCbv6anRa9I/0zjLJgY4EuznbEMgbV77JBCQ9KNC46rzi0avuxpOfu+xDjPEtSFGqVEOr5GlUSGudA==
"@eth-optimism/smock@^0.0.1": "@eth-optimism/smock@^0.0.2":
version "0.0.1" version "0.0.2"
resolved "https://registry.yarnpkg.com/@eth-optimism/smock/-/smock-0.0.1.tgz#8ea7379072eccfe5cac327e7eb78384f0e38b18c" resolved "https://registry.yarnpkg.com/@eth-optimism/smock/-/smock-0.0.2.tgz#58b9b28885fef1b08c5b13cb31bf614635027d90"
integrity sha512-lJmdXaDkAQ7Y2T9GfEzrhF6lrJ3WiEb8HJyDBjr2r3Cd4/0b2RAQj4kqkKLFZyGDMnAZ0O+XnTTyaKhBXkDSXw== integrity sha512-6Wn4h8ZuDqZlNq2lF1Mov78b41S7g4xXWXBOqqS5T+d0yC2V6Ao9XIJBheBS6l1cCN8tFNR47zdlmOCRSDX7sw==
dependencies: dependencies:
ethereum-waffle "^3" ethereum-waffle "^3"
ethers "^5" ethers "^5"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment