Commit a4cbd12a authored by Kelvin Fichter's avatar Kelvin Fichter

Fixed isCreation bug

parent 7c299c81
...@@ -498,7 +498,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -498,7 +498,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
MessageContext memory nextMessageContext = messageContext; MessageContext memory nextMessageContext = messageContext;
nextMessageContext.ovmCALLER = nextMessageContext.ovmADDRESS; nextMessageContext.ovmCALLER = nextMessageContext.ovmADDRESS;
nextMessageContext.ovmADDRESS = _address; nextMessageContext.ovmADDRESS = _address;
nextMessageContext.isCreation = false;
bool isStaticEntrypoint = false; bool isStaticEntrypoint = false;
return _callContract( return _callContract(
...@@ -536,7 +535,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -536,7 +535,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
nextMessageContext.ovmCALLER = nextMessageContext.ovmADDRESS; nextMessageContext.ovmCALLER = nextMessageContext.ovmADDRESS;
nextMessageContext.ovmADDRESS = _address; nextMessageContext.ovmADDRESS = _address;
nextMessageContext.isStatic = true; nextMessageContext.isStatic = true;
nextMessageContext.isCreation = false;
bool isStaticEntrypoint = true; bool isStaticEntrypoint = true;
return _callContract( return _callContract(
...@@ -571,7 +569,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -571,7 +569,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
{ {
// DELEGATECALL does not change anything about the message context. // DELEGATECALL does not change anything about the message context.
MessageContext memory nextMessageContext = messageContext; MessageContext memory nextMessageContext = messageContext;
nextMessageContext.isCreation = false;
bool isStaticEntrypoint = false; bool isStaticEntrypoint = false;
return _callContract( return _callContract(
...@@ -757,19 +754,12 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -757,19 +754,12 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
// We always need to initialize the contract with the default account values. // We always need to initialize the contract with the default account values.
_initPendingAccount(_address); _initPendingAccount(_address);
// We're going into a contract creation, so we need to set this flag to get the correct
// revert behavior.
messageContext.isCreation = true;
// Actually deploy the contract and retrieve its address. This step is hiding a lot of // Actually deploy the contract and retrieve its address. This step is hiding a lot of
// complexity because we need to ensure that contract creation *never* reverts by itself. // complexity because we need to ensure that contract creation *never* reverts by itself.
// We cover this partially by storing a revert flag and returning (instead of reverting) // We cover this partially by storing a revert flag and returning (instead of reverting)
// when we know that we're inside a contract's creation code. // when we know that we're inside a contract's creation code.
address ethAddress = Lib_EthUtils.createContract(_bytecode); address ethAddress = Lib_EthUtils.createContract(_bytecode);
// Now reset this flag so we go back to normal revert behavior.
messageContext.isCreation = false;
// Contract creation returns the zero address when it fails, which should only be possible // Contract creation returns the zero address when it fails, which should only be possible
// if the user intentionally runs out of gas. However, we might still have a bit of gas // if the user intentionally runs out of gas. However, we might still have a bit of gas
// left over since contract calls can only be passed 63/64ths of total gas, so we need to // left over since contract calls can only be passed 63/64ths of total gas, so we need to
...@@ -1392,7 +1382,12 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -1392,7 +1382,12 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
// *single* byte, something the OVM_ExecutionManager will not return in any other case. // *single* byte, something the OVM_ExecutionManager will not return in any other case.
// We're thereby allowed to communicate failure without allowing contracts to trick us into // We're thereby allowed to communicate failure without allowing contracts to trick us into
// thinking there was a failure. // thinking there was a failure.
if (messageContext.isCreation) { bool isCreation;
assembly {
isCreation := eq(extcodesize(caller()), 0)
}
if (isCreation) {
messageRecord.revertFlag = _flag; messageRecord.revertFlag = _flag;
assembly { assembly {
...@@ -1648,11 +1643,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -1648,11 +1643,6 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
if (_prevMessageContext.isStatic != _nextMessageContext.isStatic) { if (_prevMessageContext.isStatic != _nextMessageContext.isStatic) {
messageContext.isStatic = _nextMessageContext.isStatic; messageContext.isStatic = _nextMessageContext.isStatic;
} }
// Avoid unnecessary the SSTORE.
if (_prevMessageContext.isCreation != _nextMessageContext.isCreation) {
messageContext.isCreation = _nextMessageContext.isCreation;
}
} }
/** /**
......
...@@ -62,7 +62,6 @@ interface iOVM_ExecutionManager { ...@@ -62,7 +62,6 @@ interface iOVM_ExecutionManager {
address ovmCALLER; address ovmCALLER;
address ovmADDRESS; address ovmADDRESS;
bool isStatic; bool isStatic;
bool isCreation;
} }
struct MessageRecord { struct MessageRecord {
......
...@@ -16,6 +16,8 @@ import { ...@@ -16,6 +16,8 @@ import {
const CREATED_CONTRACT_1 = '0x2bda4a99d5be88609d23b1e4ab5d1d34fb1c2feb' const CREATED_CONTRACT_1 = '0x2bda4a99d5be88609d23b1e4ab5d1d34fb1c2feb'
const CREATED_CONTRACT_2 = '0x2bda4a99d5be88609d23b1e4ab5d1d34fb1c2feb' const CREATED_CONTRACT_2 = '0x2bda4a99d5be88609d23b1e4ab5d1d34fb1c2feb'
const CREATED_CONTRACT_BY_2_1 = '0xe0d8be8101f36ebe6b01abacec884422c39a1f62'
const CREATED_CONTRACT_BY_2_2 = '0x15ac629e1a3866b17179ee4ae86de5cbda744335'
const NESTED_CREATED_CONTRACT = '0xcb964b3f4162a0d4f5c997b40e19da5a546bc36f' const NESTED_CREATED_CONTRACT = '0xcb964b3f4162a0d4f5c997b40e19da5a546bc36f'
const DUMMY_REVERT_DATA = const DUMMY_REVERT_DATA =
'0xdeadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420' '0xdeadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420'
...@@ -49,6 +51,14 @@ const test_ovmCREATE: TestDefinition = { ...@@ -49,6 +51,14 @@ const test_ovmCREATE: TestDefinition = {
codeHash: VERIFIED_EMPTY_CONTRACT_HASH, codeHash: VERIFIED_EMPTY_CONTRACT_HASH,
ethAddress: '0x' + '00'.repeat(20), ethAddress: '0x' + '00'.repeat(20),
}, },
[CREATED_CONTRACT_BY_2_1]: {
codeHash: VERIFIED_EMPTY_CONTRACT_HASH,
ethAddress: '0x' + '00'.repeat(20),
},
[CREATED_CONTRACT_BY_2_2]: {
codeHash: '0x' + '01'.repeat(32),
ethAddress: '0x' + '00'.repeat(20),
},
[NESTED_CREATED_CONTRACT]: { [NESTED_CREATED_CONTRACT]: {
codeHash: VERIFIED_EMPTY_CONTRACT_HASH, codeHash: VERIFIED_EMPTY_CONTRACT_HASH,
ethAddress: '0x' + '00'.repeat(20), ethAddress: '0x' + '00'.repeat(20),
...@@ -527,6 +537,38 @@ const test_ovmCREATE: TestDefinition = { ...@@ -527,6 +537,38 @@ const test_ovmCREATE: TestDefinition = {
}, },
], ],
}, },
{
name: 'ovmCREATE => ovmCREATE => ovmCALL(ADDRESS_NONEXIST)',
steps: [
{
functionName: 'ovmCALL',
functionParams: {
gasLimit: OVM_TX_GAS_LIMIT,
target: '$DUMMY_OVM_ADDRESS_2',
subSteps: [
{
functionName: 'ovmCREATE',
functionParams: {
subSteps: [
{
functionName: 'ovmCREATE',
functionParams: {
bytecode: '0x'
},
expectedReturnStatus: true,
expectedReturnValue: ZERO_ADDRESS,
},
],
},
expectedReturnStatus: true,
expectedReturnValue: CREATED_CONTRACT_BY_2_1
}
]
},
expectedReturnStatus: true
}
],
},
{ {
name: 'ovmCREATE => ovmCREATE => ovmCALL(ADDRESS_NONEXIST)', name: 'ovmCREATE => ovmCREATE => ovmCALL(ADDRESS_NONEXIST)',
expectInvalidStateAccess: true, expectInvalidStateAccess: true,
...@@ -563,6 +605,39 @@ const test_ovmCREATE: TestDefinition = { ...@@ -563,6 +605,39 @@ const test_ovmCREATE: TestDefinition = {
}, },
], ],
}, },
{
name: 'OZ-AUDIT: ovmCREATE => ((ovmCREATE => ovmADDRESS), ovmREVERT)',
steps: [
{
functionName: 'ovmCREATE',
functionParams: {
subSteps: [
{
functionName: 'ovmCREATE',
functionParams: {
subSteps: [
{
functionName: 'ovmADDRESS',
expectedReturnValue: NESTED_CREATED_CONTRACT
},
],
},
expectedReturnStatus: true,
expectedReturnValue: NESTED_CREATED_CONTRACT,
},
{
functionName: 'ovmREVERT',
revertData: DUMMY_REVERT_DATA,
expectedReturnStatus: true,
expectedReturnValue: '0x00',
}
],
},
expectedReturnStatus: true,
expectedReturnValue: ZERO_ADDRESS,
},
],
},
{ {
name: 'ovmCREATE => OUT_OF_GAS', name: 'ovmCREATE => OUT_OF_GAS',
steps: [ steps: [
...@@ -580,7 +655,7 @@ const test_ovmCREATE: TestDefinition = { ...@@ -580,7 +655,7 @@ const test_ovmCREATE: TestDefinition = {
}, },
], ],
}, },
], ]
} }
const runner = new ExecutionManagerTestRunner() const runner = new ExecutionManagerTestRunner()
......
...@@ -159,7 +159,18 @@ export class ExecutionManagerTestRunner { ...@@ -159,7 +159,18 @@ export class ExecutionManagerTestRunner {
this.contracts.OVM_ExecutionManager = await ( this.contracts.OVM_ExecutionManager = await (
await smoddit('OVM_ExecutionManager') await smoddit('OVM_ExecutionManager')
).deploy(AddressManager.address) ).deploy(
AddressManager.address,
{
minTransactionGasLimit: 0,
maxTransactionGasLimit: 1_000_000_000,
maxGasPerQueuePerEpoch: 1_000_000_000_000,
secondsPerEpoch: 600,
},
{
ovmCHAINID: 420
},
)
this.contracts.OVM_StateManager = await ( this.contracts.OVM_StateManager = await (
await smoddit('OVM_StateManager') await smoddit('OVM_StateManager')
......
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