Commit e04de624 authored by ben-chain's avatar ben-chain Committed by GitHub

feat(contracts, l2geth): native ETH value support for ovmCALL (#1038)

* feat(contracts): add ovmCALL-types with native value

* add ovmCALLVALUE context

* add ovmBALANCE

* test success and revert cases

* test empty contract case

* chore: lint

* test(integration-tests): ovmCALL-types with value (compiler and wrapper)

* fix ovmDELEGATECALL type, update tests

* add ovmSELFBALANCE

* fix ovmDELEGATECALL jumping to CALL

* chore: lint

* fix(contracts): account for intrinsic gas of OVM_ETH sends

* fix(contracts): merge conflict bug

* fix(contracts): update gas benchmark

* feat(contracts, integration-tests): use new value-compatible compiler

* feat(contracts,l2geth): support value calls in OVM_ECDSAContractAccount

* fix(contracts): ovmDELEGATECALL does not change message context

* feat(contracts): sending value between EOAs

* test(integration-tests): ovmDELEGATECALL preserves ovmCALLVALUE

* test(integration-tests): assert ovmSELFBALANCEs correct

* test(integration-tests): intrinsic gas for eth value calls

* test(integration-tests): update gas values

* chore(contracts): lint

* feat(contracts, l2geth): eth_calls with nonzero value

* chore: minor fixups and comments based on PR feedback

* test(integration-tests): add requested tests from PR reviews

* test(integration-tests): ovmSELFBALANCE is preserved in ovmDELEGATECALLs

* fix(contracts): fix bug where ovmDELEGATECALL could fail if balance was lower than the ovmCALLVALUE

* chore: add changeset

* fix(contracts): update intrinsic gas for worst-case value sends

* chore: address final PR nits/improvements
Co-authored-by: default avatarKelvin Fichter <kelvinfichter@gmail.com>
parent c2a04893
---
'@eth-optimism/integration-tests': minor
'@eth-optimism/l2geth': minor
'@eth-optimism/contracts': minor
---
Add support for ovmCALL with nonzero ETH value
...@@ -2,10 +2,16 @@ ...@@ -2,10 +2,16 @@
pragma solidity >=0.7.0; pragma solidity >=0.7.0;
contract TestOOG { contract TestOOG {
constructor() { function runOutOfGas() public {
bytes32 h; bytes32 h;
for (uint256 i = 0; i < 100000; i++) { for (uint256 i = 0; i < 100000; i++) {
h = keccak256(abi.encodePacked(h)); h = keccak256(abi.encodePacked(h));
} }
} }
} }
contract TestOOGInConstructor is TestOOG {
constructor() {
runOutOfGas();
}
}
// SPDX-License-Identifier: MIT
pragma solidity >=0.7.0;
contract ValueContext {
function getSelfBalance() external payable returns(uint256) {
uint selfBalance;
assembly {
selfBalance := selfbalance()
}
return selfBalance;
}
function getAddressThisBalance() external view returns(uint256) {
return address(this).balance;
}
function getBalance(
address _address
) external payable returns(uint256) {
return _address.balance;
}
function getCallValue() public payable returns(uint256) {
return msg.value;
}
}
contract ValueCalls is ValueContext {
receive() external payable {}
function nonPayable() external {}
function simpleSend(
address _address,
uint _value
) external payable returns (bool, bytes memory) {
return sendWithData(_address, _value, hex"");
}
function sendWithDataAndGas(
address _address,
uint _value,
uint _gasLimit,
bytes memory _calldata
) public returns (bool, bytes memory) {
return _address.call{value: _value, gas: _gasLimit}(_calldata);
}
function sendWithData(
address _address,
uint _value,
bytes memory _calldata
) public returns (bool, bytes memory) {
return _address.call{value: _value}(_calldata);
}
function verifyCallValueAndRevert(
uint256 _expectedValue
) external payable {
bool correct = _checkCallValue(_expectedValue);
// do the opposite of expected if the value is wrong.
if (correct) {
revert("expected revert");
} else {
return;
}
}
function verifyCallValueAndReturn(
uint256 _expectedValue
) external payable {
bool correct = _checkCallValue(_expectedValue);
// do the opposite of expected if the value is wrong.
if (correct) {
return;
} else {
revert("unexpected revert");
}
}
function delegateCallToCallValue(
address _valueContext
) public payable returns(bool, bytes memory) {
bytes memory data = abi.encodeWithSelector(ValueContext.getCallValue.selector);
return _valueContext.delegatecall(data);
}
function delegateCallToAddressThisBalance(
address _valueContext
) public payable returns(bool, bytes memory) {
bytes memory data = abi.encodeWithSelector(ValueContext.getAddressThisBalance.selector);
return _valueContext.delegatecall(data);
}
function _checkCallValue(
uint256 _expectedValue
) internal returns(bool) {
return getCallValue() == _expectedValue;
}
}
contract ValueGasMeasurer {
function measureGasOfTransferingEthViaCall(
address target,
uint256 value,
uint256 gasLimit
) public returns(uint256) {
uint256 gasBefore = gasleft();
assembly {
pop(call(gasLimit, target, value, 0, 0, 0, 0))
}
return gasBefore - gasleft();
}
}
contract PayableConstant {
function returnValue() external payable returns(uint256) {
return 42;
}
}
contract SendETHAwayAndDelegateCall {
function emptySelfAndDelegateCall(
address _delegateTo,
bytes memory _data
) public payable returns (bool, bytes memory) {
address(0).call{value: address(this).balance}(_data);
return _delegateTo.delegatecall(_data);
}
}
...@@ -20,7 +20,7 @@ const config: HardhatUserConfig = { ...@@ -20,7 +20,7 @@ const config: HardhatUserConfig = {
}, },
solidity: '0.7.6', solidity: '0.7.6',
ovm: { ovm: {
solcVersion: '0.7.6', solcVersion: '0.7.6+commit.3b061308',
}, },
gasReporter: { gasReporter: {
enabled: enableGasReport, enabled: enableGasReport,
......
This diff is collapsed.
...@@ -5,8 +5,8 @@ import { Direction } from './shared/watcher-utils' ...@@ -5,8 +5,8 @@ import { Direction } from './shared/watcher-utils'
import { PROXY_SEQUENCER_ENTRYPOINT_ADDRESS } from './shared/utils' import { PROXY_SEQUENCER_ENTRYPOINT_ADDRESS } from './shared/utils'
import { OptimismEnv } from './shared/env' import { OptimismEnv } from './shared/env'
const DEFAULT_TEST_GAS_L1 = 230_000 const DEFAULT_TEST_GAS_L1 = 330_000
const DEFAULT_TEST_GAS_L2 = 825_000 const DEFAULT_TEST_GAS_L2 = 1_000_000
// TX size enforced by CTC: // TX size enforced by CTC:
const MAX_ROLLUP_TX_SIZE = 50_000 const MAX_ROLLUP_TX_SIZE = 50_000
...@@ -50,13 +50,13 @@ describe('Native ETH Integration Tests', async () => { ...@@ -50,13 +50,13 @@ describe('Native ETH Integration Tests', async () => {
const amount = utils.parseEther('0.5') const amount = utils.parseEther('0.5')
const addr = '0x' + '1234'.repeat(10) const addr = '0x' + '1234'.repeat(10)
const gas = await env.ovmEth.estimateGas.transfer(addr, amount) const gas = await env.ovmEth.estimateGas.transfer(addr, amount)
expect(gas).to.be.deep.eq(BigNumber.from(6430020)) expect(gas).to.be.deep.eq(BigNumber.from(6430021))
}) })
it('Should estimate gas for ETH withdraw', async () => { it('Should estimate gas for ETH withdraw', async () => {
const amount = utils.parseEther('0.5') const amount = utils.parseEther('0.5')
const gas = await env.ovmEth.estimateGas.withdraw(amount, 0, '0xFFFF') const gas = await env.ovmEth.estimateGas.withdraw(amount, 0, '0xFFFF')
expect(gas).to.be.deep.eq(BigNumber.from(6580050)) expect(gas).to.be.deep.eq(BigNumber.from(6580054))
}) })
}) })
......
...@@ -4,10 +4,10 @@ import { ...@@ -4,10 +4,10 @@ import {
TxGasPrice, TxGasPrice,
toRpcHexString, toRpcHexString,
} from '@eth-optimism/core-utils' } from '@eth-optimism/core-utils'
import { Wallet, BigNumber, Contract } from 'ethers' import { Wallet, BigNumber, Contract, ContractFactory } from 'ethers'
import { ethers } from 'hardhat' import { ethers } from 'hardhat'
import chai, { expect } from 'chai' import chai, { expect } from 'chai'
import { sleep, l2Provider, l1Provider } from './shared/utils' import { sleep, l2Provider, l1Provider, fundUser } from './shared/utils'
import chaiAsPromised from 'chai-as-promised' import chaiAsPromised from 'chai-as-promised'
import { OptimismEnv } from './shared/env' import { OptimismEnv } from './shared/env'
import { import {
...@@ -154,7 +154,7 @@ describe('Basic RPC tests', () => { ...@@ -154,7 +154,7 @@ describe('Basic RPC tests', () => {
}) })
it('should correctly report OOG for contract creations', async () => { it('should correctly report OOG for contract creations', async () => {
const factory = await ethers.getContractFactory('TestOOG') const factory = await ethers.getContractFactory('TestOOGInConstructor')
await expect(factory.connect(wallet).deploy()).to.be.rejectedWith( await expect(factory.connect(wallet).deploy()).to.be.rejectedWith(
'gas required exceeds allowance' 'gas required exceeds allowance'
...@@ -207,6 +207,32 @@ describe('Basic RPC tests', () => { ...@@ -207,6 +207,32 @@ describe('Basic RPC tests', () => {
'Contract creation code contains unsafe opcodes. Did you use the right compiler or pass an unsafe constructor argument?' 'Contract creation code contains unsafe opcodes. Did you use the right compiler or pass an unsafe constructor argument?'
) )
}) })
it('should allow eth_calls with nonzero value', async () => {
// Deploy a contract to check msg.value of the call
const Factory__ValueContext: ContractFactory = await ethers.getContractFactory(
'ValueContext',
wallet
)
const ValueContext: Contract = await Factory__ValueContext.deploy()
await ValueContext.deployTransaction.wait()
// Fund account to call from
const from = wallet.address
const value = 15
await fundUser(env.watcher, env.gateway, value, from)
// Do the call and check msg.value
const data = ValueContext.interface.encodeFunctionData('getCallValue')
const res = await provider.call({
to: ValueContext.address,
from,
data,
value,
})
expect(res).to.eq(BigNumber.from(value))
})
}) })
describe('eth_getTransactionReceipt', () => { describe('eth_getTransactionReceipt', () => {
...@@ -236,7 +262,7 @@ describe('Basic RPC tests', () => { ...@@ -236,7 +262,7 @@ describe('Basic RPC tests', () => {
it('correctly exposes revert data for contract creations', async () => { it('correctly exposes revert data for contract creations', async () => {
const req: TransactionRequest = { const req: TransactionRequest = {
...revertingDeployTx, ...revertingDeployTx,
gasLimit: 17700899, // override gas estimation gasLimit: 27700899, // override gas estimation
} }
const tx = await wallet.sendTransaction(req) const tx = await wallet.sendTransaction(req)
...@@ -353,7 +379,7 @@ describe('Basic RPC tests', () => { ...@@ -353,7 +379,7 @@ describe('Basic RPC tests', () => {
to: DEFAULT_TRANSACTION.to, to: DEFAULT_TRANSACTION.to,
value: 0, value: 0,
}) })
expect(estimate).to.be.eq(5920012) expect(estimate).to.be.eq(5920013)
}) })
it('should return a gas estimate that grows with the size of data', async () => { it('should return a gas estimate that grows with the size of data', async () => {
......
...@@ -100,6 +100,11 @@ func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, execut ...@@ -100,6 +100,11 @@ func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, execut
to = &common.Address{0} to = &common.Address{0}
} }
value := msg.Value()
if value == nil {
value = common.Big0
}
tx := ovmTransaction{ tx := ovmTransaction{
timestamp, timestamp,
blockNumber, blockNumber,
...@@ -114,6 +119,7 @@ func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, execut ...@@ -114,6 +119,7 @@ func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, execut
var args = []interface{}{ var args = []interface{}{
tx, tx,
from, from,
value,
stateManager.Address, stateManager.Address,
} }
......
...@@ -321,14 +321,6 @@ func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction) ...@@ -321,14 +321,6 @@ func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction)
if len(signedTx.Data()) > b.MaxCallDataSize { if len(signedTx.Data()) > b.MaxCallDataSize {
return fmt.Errorf("Calldata cannot be larger than %d, sent %d", b.MaxCallDataSize, len(signedTx.Data())) return fmt.Errorf("Calldata cannot be larger than %d, sent %d", b.MaxCallDataSize, len(signedTx.Data()))
} }
// If there is a value field set then reject transactions that
// contain calldata. The feature of sending transactions with value
// and calldata will be added in the future.
if signedTx.Value().Cmp(common.Big0) != 0 {
if len(signedTx.Data()) > 0 {
return errors.New("Cannot send transactions with value and calldata")
}
}
} }
return b.eth.syncService.ValidateAndApplySequencerTransaction(signedTx) return b.eth.syncService.ValidateAndApplySequencerTransaction(signedTx)
} }
......
...@@ -47,6 +47,16 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -47,6 +47,16 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
* Public Functions * * Public Functions *
********************/ ********************/
/**
* No-op fallback mirrors behavior of calling an EOA on L1.
*/
fallback()
external
payable
{
return;
}
/** /**
* Executes a signed transaction. * Executes a signed transaction.
* @param _transaction Signed EIP155 transaction. * @param _transaction Signed EIP155 transaction.
...@@ -121,34 +131,15 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -121,34 +131,15 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// cases, but since this is a contract we'd end up bumping the nonce twice. // cases, but since this is a contract we'd end up bumping the nonce twice.
Lib_ExecutionManagerWrapper.ovmINCREMENTNONCE(); Lib_ExecutionManagerWrapper.ovmINCREMENTNONCE();
// Value transfer currently only supported for CALL but not for CREATE. // NOTE: Upgrades are temporarily disabled because users can, in theory, modify their EOA
if (_transaction.value > 0) { // so that they don't have to pay any fees to the sequencer. Function will remain disabled
// TEMPORARY: Block value transfer if the transaction has input data. // until a robust solution is in place.
require( require(
_transaction.data.length == 0, _transaction.to != Lib_ExecutionManagerWrapper.ovmADDRESS(),
"Value is nonzero but input data was provided." "Calls to self are disabled until upgradability is re-enabled."
); );
require( return _transaction.to.call{value: _transaction.value}(_transaction.data);
OVM_ETH(Lib_PredeployAddresses.OVM_ETH).transfer(
_transaction.to,
_transaction.value
),
"Value could not be transferred to recipient."
);
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);
}
} }
} }
} }
...@@ -39,6 +39,7 @@ contract OVM_ProxyEOA { ...@@ -39,6 +39,7 @@ contract OVM_ProxyEOA {
fallback() fallback()
external external
payable
{ {
(bool success, bytes memory returndata) = getImplementation().delegatecall(msg.data); (bool success, bytes memory returndata) = getImplementation().delegatecall(msg.data);
......
...@@ -22,6 +22,7 @@ contract OVM_ExecutionManagerWrapper { ...@@ -22,6 +22,7 @@ contract OVM_ExecutionManagerWrapper {
fallback() fallback()
external external
payable
{ {
bytes memory data = msg.data; bytes memory data = msg.data;
assembly { assembly {
......
...@@ -29,6 +29,14 @@ interface iOVM_ExecutionManager { ...@@ -29,6 +29,14 @@ interface iOVM_ExecutionManager {
PREV_EPOCH_L1TOL2_QUEUE_GAS PREV_EPOCH_L1TOL2_QUEUE_GAS
} }
enum MessageType {
ovmCALL,
ovmSTATICCALL,
ovmDELEGATECALL,
ovmCREATE,
ovmCREATE2
}
/*********** /***********
* Structs * * Structs *
***********/ ***********/
...@@ -60,6 +68,7 @@ interface iOVM_ExecutionManager { ...@@ -60,6 +68,7 @@ interface iOVM_ExecutionManager {
struct MessageContext { struct MessageContext {
address ovmCALLER; address ovmCALLER;
address ovmADDRESS; address ovmADDRESS;
uint256 ovmCALLVALUE;
bool isStatic; bool isStatic;
} }
...@@ -84,6 +93,7 @@ interface iOVM_ExecutionManager { ...@@ -84,6 +93,7 @@ interface iOVM_ExecutionManager {
function ovmCALLER() external view returns (address _caller); function ovmCALLER() external view returns (address _caller);
function ovmADDRESS() external view returns (address _address); function ovmADDRESS() external view returns (address _address);
function ovmCALLVALUE() external view returns (uint _callValue);
function ovmTIMESTAMP() external view returns (uint256 _timestamp); function ovmTIMESTAMP() external view returns (uint256 _timestamp);
function ovmNUMBER() external view returns (uint256 _number); function ovmNUMBER() external view returns (uint256 _number);
function ovmGASLIMIT() external view returns (uint256 _gasLimit); function ovmGASLIMIT() external view returns (uint256 _gasLimit);
...@@ -126,7 +136,9 @@ interface iOVM_ExecutionManager { ...@@ -126,7 +136,9 @@ interface iOVM_ExecutionManager {
* Contract Calling Opcodes * * Contract Calling Opcodes *
****************************/ ****************************/
// Valueless ovmCALL for maintaining backwards compatibility with legacy OVM bytecode.
function ovmCALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata); function ovmCALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata);
function ovmCALL(uint256 _gasLimit, address _address, uint256 _value, bytes memory _calldata) external returns (bool _success, bytes memory _returndata);
function ovmSTATICCALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata); function ovmSTATICCALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata);
function ovmDELEGATECALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata); function ovmDELEGATECALL(uint256 _gasLimit, address _address, bytes memory _calldata) external returns (bool _success, bytes memory _returndata);
...@@ -148,6 +160,14 @@ interface iOVM_ExecutionManager { ...@@ -148,6 +160,14 @@ interface iOVM_ExecutionManager {
function ovmEXTCODEHASH(address _contract) external returns (bytes32 _hash); function ovmEXTCODEHASH(address _contract) external returns (bytes32 _hash);
/*********************
* ETH Value Opcodes *
*********************/
function ovmBALANCE(address _contract) external returns (uint256 _balance);
function ovmSELFBALANCE() external returns (uint256 _balance);
/*************************************** /***************************************
* Public Functions: Execution Context * * Public Functions: Execution Context *
***************************************/ ***************************************/
......
...@@ -158,6 +158,82 @@ library Lib_ExecutionManagerWrapper { ...@@ -158,6 +158,82 @@ library Lib_ExecutionManagerWrapper {
return abi.decode(returndata, (address)); return abi.decode(returndata, (address));
} }
/**
* Calls the value-enabled ovmCALL opcode.
* @param _gasLimit Amount of gas to be passed into this call.
* @param _address Address of the contract to call.
* @param _value ETH value to pass with the call.
* @param _calldata Data to send along with the call.
* @return _success Whether or not the call returned (rather than reverted).
* @return _returndata Data returned by the call.
*/
function ovmCALL(
uint256 _gasLimit,
address _address,
uint256 _value,
bytes memory _calldata
)
internal
returns (
bool,
bytes memory
)
{
bytes memory returndata = _callWrapperContract(
abi.encodeWithSignature(
"ovmCALL(uint256,address,uint256,bytes)",
_gasLimit,
_address,
_value,
_calldata
)
);
return abi.decode(returndata, (bool, bytes));
}
/**
* Calls the ovmBALANCE opcode.
* @param _address OVM account to query the balance of.
* @return Balance of the account.
*/
function ovmBALANCE(
address _address
)
internal
returns (
uint256
)
{
bytes memory returndata = _callWrapperContract(
abi.encodeWithSignature(
"ovmBALANCE(address)",
_address
)
);
return abi.decode(returndata, (uint256));
}
/**
* Calls the ovmCALLVALUE opcode.
* @return Value of the current call frame.
*/
function ovmCALLVALUE()
internal
returns (
uint256
)
{
bytes memory returndata = _callWrapperContract(
abi.encodeWithSignature(
"ovmCALLVALUE()"
)
);
return abi.decode(returndata, (uint256));
}
/********************* /*********************
* Private Functions * * Private Functions *
......
...@@ -53,7 +53,7 @@ const config: HardhatUserConfig = { ...@@ -53,7 +53,7 @@ const config: HardhatUserConfig = {
}, },
}, },
ovm: { ovm: {
solcVersion: '0.7.6', solcVersion: '0.7.6+commit.3b061308',
}, },
typechain: { typechain: {
outDir: 'dist/types', outDir: 'dist/types',
......
...@@ -49,7 +49,18 @@ describe('OVM_ECDSAContractAccount', () => { ...@@ -49,7 +49,18 @@ describe('OVM_ECDSAContractAccount', () => {
Mock__OVM_ETH.smocked.transfer.will.return.with(true) Mock__OVM_ETH.smocked.transfer.will.return.with(true)
}) })
describe('fallback()', () => { describe('fallback', async () => {
it('should successfully accept value sent to it', async () => {
await expect(
wallet.sendTransaction({
to: OVM_ECDSAContractAccount.address,
value: 1,
})
).to.not.be.reverted
})
})
describe('execute()', () => {
it(`should successfully execute an EIP155Transaction`, async () => { it(`should successfully execute an EIP155Transaction`, async () => {
const transaction = DEFAULT_EIP155_TX const transaction = DEFAULT_EIP155_TX
const encodedTransaction = await wallet.signTransaction(transaction) const encodedTransaction = await wallet.signTransaction(transaction)
...@@ -132,37 +143,36 @@ describe('OVM_ECDSAContractAccount', () => { ...@@ -132,37 +143,36 @@ describe('OVM_ECDSAContractAccount', () => {
}) })
it(`should transfer value if value is greater than 0`, async () => { it(`should transfer value if value is greater than 0`, async () => {
const transaction = { ...DEFAULT_EIP155_TX, value: 1234, data: '0x' } const value = 100
const valueRecipient = '0x' + '34'.repeat(20)
const transaction = {
...DEFAULT_EIP155_TX,
to: valueRecipient,
value,
data: '0x',
}
const encodedTransaction = await wallet.signTransaction(transaction) const encodedTransaction = await wallet.signTransaction(transaction)
// fund the contract account
await wallet.sendTransaction({
to: OVM_ECDSAContractAccount.address,
value: value * 10,
gasLimit: 1_000_000,
})
const receipientBalanceBefore = await wallet.provider.getBalance(
valueRecipient
)
await OVM_ECDSAContractAccount.execute( await OVM_ECDSAContractAccount.execute(
LibEIP155TxStruct(encodedTransaction) LibEIP155TxStruct(encodedTransaction)
) )
const recipientBalanceAfter = await wallet.provider.getBalance(
valueRecipient
)
// First call transfers fee, second transfers value (since value > 0).
expect( expect(
toPlainObject(Mock__OVM_ETH.smocked.transfer.calls[1]) recipientBalanceAfter.sub(receipientBalanceBefore).toNumber()
).to.deep.include({ ).to.eq(value)
to: transaction.to,
value: BigNumber.from(transaction.value),
})
})
it(`should revert if the value is not transferred to the recipient`, async () => {
const transaction = { ...DEFAULT_EIP155_TX, value: 1234, data: '0x' }
const encodedTransaction = await wallet.signTransaction(transaction)
Mock__OVM_ETH.smocked.transfer.will.return.with((to, value) => {
if (to === transaction.to) {
return false
} else {
return true
}
})
await expect(
OVM_ECDSAContractAccount.execute(LibEIP155TxStruct(encodedTransaction))
).to.be.revertedWith('Value could not be transferred to recipient.')
}) })
it(`should revert if trying to send value with a contract creation`, async () => { it(`should revert if trying to send value with a contract creation`, async () => {
...@@ -174,15 +184,6 @@ describe('OVM_ECDSAContractAccount', () => { ...@@ -174,15 +184,6 @@ describe('OVM_ECDSAContractAccount', () => {
).to.be.revertedWith('Value transfer in contract creation not supported.') ).to.be.revertedWith('Value transfer in contract creation not supported.')
}) })
it(`should revert if trying to send value with non-empty transaction data`, async () => {
const transaction = { ...DEFAULT_EIP155_TX, value: 1234, data: '0x1234' }
const encodedTransaction = await wallet.signTransaction(transaction)
await expect(
OVM_ECDSAContractAccount.execute(LibEIP155TxStruct(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 // 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. // comment in OVM_ECDSAContractAccount.sol for additional information.
it(`should revert if trying call itself`, async () => { it(`should revert if trying call itself`, async () => {
......
...@@ -7,7 +7,7 @@ import { MockContract, smockit } from '@eth-optimism/smock' ...@@ -7,7 +7,7 @@ import { MockContract, smockit } from '@eth-optimism/smock'
import { toPlainObject } from 'lodash' import { toPlainObject } from 'lodash'
/* Internal Imports */ /* Internal Imports */
import { getContractInterface, predeploys } from '../../../../src' import { predeploys } from '../../../../src'
import { DEFAULT_EIP155_TX, LibEIP155TxStruct } from '../../../helpers' import { DEFAULT_EIP155_TX, LibEIP155TxStruct } from '../../../helpers'
describe('OVM_ProxyEOA', () => { describe('OVM_ProxyEOA', () => {
......
...@@ -110,7 +110,7 @@ describe('OVM_ExecutionManager gas consumption', () => { ...@@ -110,7 +110,7 @@ describe('OVM_ExecutionManager gas consumption', () => {
) )
console.log(`calculated gas cost of ${gasCost}`) console.log(`calculated gas cost of ${gasCost}`)
const benchmark: number = 106_000 const benchmark: number = 110_000
expect(gasCost).to.be.lte(benchmark) expect(gasCost).to.be.lte(benchmark)
expect(gasCost).to.be.gte( expect(gasCost).to.be.gte(
benchmark - 1_000, benchmark - 1_000,
......
...@@ -12,12 +12,12 @@ import { ...@@ -12,12 +12,12 @@ import {
ParsedTestStep, ParsedTestStep,
TestParameter, TestParameter,
TestStep, TestStep,
TestStep_CALL, TestStep_CALLType,
TestStep_Run, TestStep_Run,
isRevertFlagError, isRevertFlagError,
isTestStep_SSTORE, isTestStep_SSTORE,
isTestStep_SLOAD, isTestStep_SLOAD,
isTestStep_CALL, isTestStep_CALLType,
isTestStep_CREATE, isTestStep_CREATE,
isTestStep_CREATE2, isTestStep_CREATE2,
isTestStep_CREATEEOA, isTestStep_CREATEEOA,
...@@ -27,7 +27,9 @@ import { ...@@ -27,7 +27,9 @@ import {
isTestStep_EXTCODESIZE, isTestStep_EXTCODESIZE,
isTestStep_EXTCODEHASH, isTestStep_EXTCODEHASH,
isTestStep_EXTCODECOPY, isTestStep_EXTCODECOPY,
isTestStep_BALANCE,
isTestStep_REVERT, isTestStep_REVERT,
isTestStep_CALL,
} from './test.types' } from './test.types'
import { encodeRevertData, REVERT_FLAGS } from '../codec' import { encodeRevertData, REVERT_FLAGS } from '../codec'
import { import {
...@@ -49,6 +51,7 @@ export class ExecutionManagerTestRunner { ...@@ -49,6 +51,7 @@ export class ExecutionManagerTestRunner {
Factory__Helper_TestRunner_CREATE: ContractFactory Factory__Helper_TestRunner_CREATE: ContractFactory
OVM_DeployerWhitelist: Contract OVM_DeployerWhitelist: Contract
OVM_ProxyEOA: Contract OVM_ProxyEOA: Contract
OVM_ETH: Contract
} = { } = {
OVM_SafetyChecker: undefined, OVM_SafetyChecker: undefined,
OVM_StateManager: undefined, OVM_StateManager: undefined,
...@@ -57,6 +60,7 @@ export class ExecutionManagerTestRunner { ...@@ -57,6 +60,7 @@ export class ExecutionManagerTestRunner {
Factory__Helper_TestRunner_CREATE: undefined, Factory__Helper_TestRunner_CREATE: undefined,
OVM_DeployerWhitelist: undefined, OVM_DeployerWhitelist: undefined,
OVM_ProxyEOA: undefined, OVM_ProxyEOA: undefined,
OVM_ETH: undefined,
} }
// Default pre-state with contract deployer whitelist NOT initialized. // Default pre-state with contract deployer whitelist NOT initialized.
...@@ -68,6 +72,10 @@ export class ExecutionManagerTestRunner { ...@@ -68,6 +72,10 @@ export class ExecutionManagerTestRunner {
codeHash: NON_NULL_BYTES32, codeHash: NON_NULL_BYTES32,
ethAddress: '$OVM_DEPLOYER_WHITELIST', ethAddress: '$OVM_DEPLOYER_WHITELIST',
}, },
[predeploys.OVM_ETH]: {
codeHash: NON_NULL_BYTES32,
ethAddress: '$OVM_ETH',
},
[predeploys.OVM_ProxyEOA]: { [predeploys.OVM_ProxyEOA]: {
codeHash: NON_NULL_BYTES32, codeHash: NON_NULL_BYTES32,
ethAddress: '$OVM_PROXY_EOA', ethAddress: '$OVM_PROXY_EOA',
...@@ -227,6 +235,14 @@ export class ExecutionManagerTestRunner { ...@@ -227,6 +235,14 @@ export class ExecutionManagerTestRunner {
this.contracts.OVM_DeployerWhitelist = DeployerWhitelist this.contracts.OVM_DeployerWhitelist = DeployerWhitelist
const OvmEth = await getContractFactory(
'OVM_ETH',
AddressManager.signer,
true
).deploy(ethers.constants.AddressZero, ethers.constants.AddressZero)
this.contracts.OVM_ETH = OvmEth
this.contracts.OVM_ProxyEOA = await getContractFactory( this.contracts.OVM_ProxyEOA = await getContractFactory(
'OVM_ProxyEOA', 'OVM_ProxyEOA',
AddressManager.signer, AddressManager.signer,
...@@ -282,6 +298,8 @@ export class ExecutionManagerTestRunner { ...@@ -282,6 +298,8 @@ export class ExecutionManagerTestRunner {
return this.contracts.Helper_TestRunner.address return this.contracts.Helper_TestRunner.address
} else if (kv === '$OVM_DEPLOYER_WHITELIST') { } else if (kv === '$OVM_DEPLOYER_WHITELIST') {
return this.contracts.OVM_DeployerWhitelist.address return this.contracts.OVM_DeployerWhitelist.address
} else if (kv === '$OVM_ETH') {
return this.contracts.OVM_ETH.address
} else if (kv === '$OVM_PROXY_EOA') { } else if (kv === '$OVM_PROXY_EOA') {
return this.contracts.OVM_ProxyEOA.address return this.contracts.OVM_ProxyEOA.address
} else if (kv.startsWith('$DUMMY_OVM_ADDRESS_')) { } else if (kv.startsWith('$DUMMY_OVM_ADDRESS_')) {
...@@ -328,7 +346,7 @@ export class ExecutionManagerTestRunner { ...@@ -328,7 +346,7 @@ export class ExecutionManagerTestRunner {
if (step.functionParams.data) { if (step.functionParams.data) {
calldata = step.functionParams.data calldata = step.functionParams.data
} else { } else {
const runStep: TestStep_CALL = { const runStep: TestStep_CALLType = {
functionName: 'ovmCALL', functionName: 'ovmCALL',
functionParams: { functionParams: {
gasLimit: OVM_TX_GAS_LIMIT, gasLimit: OVM_TX_GAS_LIMIT,
...@@ -362,9 +380,12 @@ export class ExecutionManagerTestRunner { ...@@ -362,9 +380,12 @@ export class ExecutionManagerTestRunner {
await toRun await toRun
} }
} else { } else {
await this.contracts.OVM_ExecutionManager.ovmCALL( await this.contracts.OVM_ExecutionManager[
'ovmCALL(uint256,address,uint256,bytes)'
](
OVM_TX_GAS_LIMIT, OVM_TX_GAS_LIMIT,
ExecutionManagerTestRunner.getDummyAddress('$DUMMY_OVM_ADDRESS_1'), ExecutionManagerTestRunner.getDummyAddress('$DUMMY_OVM_ADDRESS_1'),
0,
this.contracts.Helper_TestRunner.interface.encodeFunctionData( this.contracts.Helper_TestRunner.interface.encodeFunctionData(
'runSingleTestStep', 'runSingleTestStep',
[this.parseTestStep(step)] [this.parseTestStep(step)]
...@@ -398,7 +419,7 @@ export class ExecutionManagerTestRunner { ...@@ -398,7 +419,7 @@ export class ExecutionManagerTestRunner {
return false return false
} else if (isTestStep_Context(step)) { } else if (isTestStep_Context(step)) {
return true return true
} else if (isTestStep_CALL(step)) { } else if (isTestStep_CALLType(step)) {
if ( if (
isRevertFlagError(step.expectedReturnValue) && isRevertFlagError(step.expectedReturnValue) &&
(step.expectedReturnValue.flag === REVERT_FLAGS.INVALID_STATE_ACCESS || (step.expectedReturnValue.flag === REVERT_FLAGS.INVALID_STATE_ACCESS ||
...@@ -435,23 +456,36 @@ export class ExecutionManagerTestRunner { ...@@ -435,23 +456,36 @@ export class ExecutionManagerTestRunner {
isTestStep_EXTCODESIZE(step) || isTestStep_EXTCODESIZE(step) ||
isTestStep_EXTCODEHASH(step) || isTestStep_EXTCODEHASH(step) ||
isTestStep_EXTCODECOPY(step) || isTestStep_EXTCODECOPY(step) ||
isTestStep_BALANCE(step) ||
isTestStep_CREATEEOA(step) isTestStep_CREATEEOA(step)
) { ) {
functionParams = Object.values(step.functionParams) functionParams = Object.values(step.functionParams)
} else if (isTestStep_CALL(step)) { } else if (isTestStep_CALLType(step)) {
functionParams = [ const innnerCalldata =
step.functionParams.gasLimit,
step.functionParams.target,
step.functionParams.calldata || step.functionParams.calldata ||
this.contracts.Helper_TestRunner.interface.encodeFunctionData( this.contracts.Helper_TestRunner.interface.encodeFunctionData(
'runMultipleTestSteps', 'runMultipleTestSteps',
[ [
step.functionParams.subSteps.map((subStep) => { step.functionParams.subSteps.map((subStep) => {
return this.parseTestStep(subStep) return this.parseTestStep(subStep)
}), }),
] ]
), )
] // only ovmCALL accepts a value parameter.
if (isTestStep_CALL(step)) {
functionParams = [
step.functionParams.gasLimit,
step.functionParams.target,
step.functionParams.value || 0,
innnerCalldata,
]
} else {
functionParams = [
step.functionParams.gasLimit,
step.functionParams.target,
innnerCalldata,
]
}
} else if (isTestStep_CREATE(step)) { } else if (isTestStep_CREATE(step)) {
functionParams = [ functionParams = [
this.contracts.Factory__Helper_TestRunner_CREATE.getDeployTransaction( this.contracts.Factory__Helper_TestRunner_CREATE.getDeployTransaction(
...@@ -475,8 +509,16 @@ export class ExecutionManagerTestRunner { ...@@ -475,8 +509,16 @@ export class ExecutionManagerTestRunner {
functionParams = [step.revertData || '0x'] functionParams = [step.revertData || '0x']
} }
// legacy ovmCALL causes multiple matching functions without the full signature
let functionName
if (step.functionName === 'ovmCALL') {
functionName = 'ovmCALL(uint256,address,uint256,bytes)'
} else {
functionName = step.functionName
}
return this.contracts.OVM_ExecutionManager.interface.encodeFunctionData( return this.contracts.OVM_ExecutionManager.interface.encodeFunctionData(
step.functionName, functionName,
functionParams functionParams
) )
} }
...@@ -500,7 +542,7 @@ export class ExecutionManagerTestRunner { ...@@ -500,7 +542,7 @@ export class ExecutionManagerTestRunner {
} }
let returnData: any[] = [] let returnData: any[] = []
if (isTestStep_CALL(step)) { if (isTestStep_CALLType(step)) {
if (step.expectedReturnValue === '0x00') { if (step.expectedReturnValue === '0x00') {
return step.expectedReturnValue return step.expectedReturnValue
} else if ( } else if (
...@@ -540,8 +582,16 @@ export class ExecutionManagerTestRunner { ...@@ -540,8 +582,16 @@ export class ExecutionManagerTestRunner {
} }
} }
// legacy ovmCALL causes multiple matching functions without the full signature
let functionName
if (step.functionName === 'ovmCALL') {
functionName = 'ovmCALL(uint256,address,uint256,bytes)'
} else {
functionName = step.functionName
}
return this.contracts.OVM_ExecutionManager.interface.encodeFunctionResult( return this.contracts.OVM_ExecutionManager.interface.encodeFunctionResult(
step.functionName, functionName,
returnData returnData
) )
} }
......
...@@ -11,6 +11,7 @@ export type ContextOpcode = ...@@ -11,6 +11,7 @@ export type ContextOpcode =
| 'ovmGASLIMIT' | 'ovmGASLIMIT'
| 'ovmCHAINID' | 'ovmCHAINID'
| 'ovmGETNONCE' | 'ovmGETNONCE'
| 'ovmCALLVALUE'
type CallOpcode = 'ovmCALL' | 'ovmSTATICCALL' | 'ovmDELEGATECALL' type CallOpcode = 'ovmCALL' | 'ovmSTATICCALL' | 'ovmDELEGATECALL'
...@@ -68,6 +69,15 @@ interface TestStep_EXTCODECOPY { ...@@ -68,6 +69,15 @@ interface TestStep_EXTCODECOPY {
expectedReturnValue: string | RevertFlagError expectedReturnValue: string | RevertFlagError
} }
interface TestStep_BALANCE {
functionName: 'ovmBALANCE'
functionParams: {
address: string
}
expectedReturnStatus: boolean
expectedReturnValue: number | RevertFlagError
}
interface TestStep_SSTORE { interface TestStep_SSTORE {
functionName: 'ovmSSTORE' functionName: 'ovmSSTORE'
functionParams: { functionParams: {
...@@ -93,11 +103,12 @@ interface TestStep_INCREMENTNONCE { ...@@ -93,11 +103,12 @@ interface TestStep_INCREMENTNONCE {
expectedReturnValue?: RevertFlagError expectedReturnValue?: RevertFlagError
} }
export interface TestStep_CALL { export interface TestStep_CALLType {
functionName: CallOpcode functionName: CallOpcode
functionParams: { functionParams: {
gasLimit: number | BigNumber gasLimit: number | BigNumber
target: string target: string
value?: number | BigNumber
calldata?: string calldata?: string
subSteps?: TestStep[] subSteps?: TestStep[]
} }
...@@ -174,13 +185,14 @@ export type TestStep = ...@@ -174,13 +185,14 @@ export type TestStep =
| TestStep_SSTORE | TestStep_SSTORE
| TestStep_SLOAD | TestStep_SLOAD
| TestStep_INCREMENTNONCE | TestStep_INCREMENTNONCE
| TestStep_CALL | TestStep_CALLType
| TestStep_CREATE | TestStep_CREATE
| TestStep_CREATE2 | TestStep_CREATE2
| TestStep_CREATEEOA | TestStep_CREATEEOA
| TestStep_EXTCODESIZE | TestStep_EXTCODESIZE
| TestStep_EXTCODEHASH | TestStep_EXTCODEHASH
| TestStep_EXTCODECOPY | TestStep_EXTCODECOPY
| TestStep_BALANCE
| TestStep_REVERT | TestStep_REVERT
| TestStep_evm | TestStep_evm
...@@ -220,6 +232,7 @@ export const isTestStep_Context = ( ...@@ -220,6 +232,7 @@ export const isTestStep_Context = (
'ovmCHAINID', 'ovmCHAINID',
'ovmL1QUEUEORIGIN', 'ovmL1QUEUEORIGIN',
'ovmGETNONCE', 'ovmGETNONCE',
'ovmCALLVALUE',
].includes(step.functionName) ].includes(step.functionName)
} }
...@@ -255,16 +268,28 @@ export const isTestStep_EXTCODECOPY = ( ...@@ -255,16 +268,28 @@ export const isTestStep_EXTCODECOPY = (
return step.functionName === 'ovmEXTCODECOPY' return step.functionName === 'ovmEXTCODECOPY'
} }
export const isTestStep_BALANCE = (
step: TestStep
): step is TestStep_BALANCE => {
return step.functionName === 'ovmBALANCE'
}
export const isTestStep_REVERT = (step: TestStep): step is TestStep_REVERT => { export const isTestStep_REVERT = (step: TestStep): step is TestStep_REVERT => {
return step.functionName === 'ovmREVERT' return step.functionName === 'ovmREVERT'
} }
export const isTestStep_CALL = (step: TestStep): step is TestStep_CALL => { export const isTestStep_CALLType = (
step: TestStep
): step is TestStep_CALLType => {
return ['ovmCALL', 'ovmSTATICCALL', 'ovmDELEGATECALL'].includes( return ['ovmCALL', 'ovmSTATICCALL', 'ovmDELEGATECALL'].includes(
step.functionName step.functionName
) )
} }
export const isTestStep_CALL = (step: TestStep): boolean => {
return step.functionName === 'ovmCALL'
}
export const isTestStep_CREATE = (step: TestStep): step is TestStep_CREATE => { export const isTestStep_CREATE = (step: TestStep): step is TestStep_CREATE => {
return step.functionName === 'ovmCREATE' return step.functionName === 'ovmCREATE'
} }
......
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