Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require timestamp is set on execution #61

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions packages/core-utils/src/app/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,12 @@ export const runInDomain = async (
const domainToUse: domain.Domain = !!d ? d : domain.create()
return domainToUse.run(func)
}

/**
* Gets the current number of seconds since the epoch.
*
* @returns The seconds since epoch.
*/
export const getCurrentTime = (): number => {
return Math.round(Date.now() / 1000)
}
4 changes: 1 addition & 3 deletions packages/ovm/src/contracts/ExecutionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ contract ExecutionManager is FullStateManager {
address _l1MsgSenderAddress,
bool _allowRevert
) public {
require(_timestamp > 0, "Timestamp must be greater than 0");
uint _nonce = getOvmContractNonce(_fromAddress);
// Initialize our context
initializeContext(_timestamp, _queueOrigin, _fromAddress, _l1MsgSenderAddress);
Expand Down Expand Up @@ -397,9 +398,6 @@ contract ExecutionManager is FullStateManager {
* returndata: uint256 representing the current timestamp.
*/
function ovmTIMESTAMP() public view {
// First make sure the timestamp was set
require(executionContext.timestamp != 0, "Error: attempting to access non-existent timestamp.");

uint t = executionContext.timestamp;

assembly {
Expand Down
24 changes: 12 additions & 12 deletions packages/ovm/test/contracts/execution-manager.call-opcodes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import '../setup'

/* External Imports */
import { Address } from '@eth-optimism/rollup-core'
import { getLogger, remove0x, add0x, TestUtils } from '@eth-optimism/core-utils'
import { getLogger, remove0x, add0x, TestUtils, getCurrentTime } from '@eth-optimism/core-utils'

import { Contract, ContractFactory, ethers } from 'ethers'
import { createMockProvider, deployContract, getWallets } from 'ethereum-waffle'
Expand Down Expand Up @@ -132,7 +132,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
encodeMethodId('executeCall') +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeCall,
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeDelegateCall,
Expand Down Expand Up @@ -253,7 +253,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeDelegateCall,
Expand Down Expand Up @@ -355,7 +355,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCallThenCall,
Expand All @@ -374,7 +374,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand All @@ -396,7 +396,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand All @@ -420,7 +420,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand All @@ -447,7 +447,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand All @@ -470,7 +470,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand Down Expand Up @@ -498,7 +498,7 @@ describe('Execution Manager -- Call opcodes', () => {
const data: string =
methodIds.executeCall +
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeStaticCall,
Expand All @@ -522,7 +522,7 @@ describe('Execution Manager -- Call opcodes', () => {
const executeCall = (args: any[]): Promise<string> => {
return executeOVMCall(executionManager, 'executeCall', [
encodeRawArguments([
0,
getCurrentTime(),
0,
addressToBytes32Address(callContractAddress),
methodIds.makeCall,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
hexStrToNumber,
remove0x,
TestUtils,
getCurrentTime,
} from '@eth-optimism/core-utils'

import { Contract, ContractFactory, ethers } from 'ethers'
Expand Down Expand Up @@ -152,18 +153,7 @@ describe('Execution Manager -- Context opcodes', () => {
})

describe('ovmTIMESTAMP', async () => {
it('reverts when TIMESTAMP is not set', async () => {
await TestUtils.assertThrowsAsync(async () => {
await executeCall([
contractAddress32,
methodIds.callThroughExecutionManager,
contract2Address32,
methodIds.getTIMESTAMP,
])
})
})

it('properly retrieves TIMESTAMP when timestamp is set', async () => {
it('properly retrieves TIMESTAMP', async () => {
const timestamp: number = 1582890922
const result = await executeOVMCall(executionManager, 'executeCall', [
timestamp,
Expand Down Expand Up @@ -201,7 +191,7 @@ describe('Execution Manager -- Context opcodes', () => {
it('gets Queue Origin when it is 0', async () => {
const queueOrigin: string = '00'.repeat(32)
const result = await executeOVMCall(executionManager, 'executeCall', [
0,
getCurrentTime(),
queueOrigin,
contractAddress32,
methodIds.callThroughExecutionManager,
Expand All @@ -218,7 +208,7 @@ describe('Execution Manager -- Context opcodes', () => {
it('properly retrieves Queue Origin when queue origin is set', async () => {
const queueOrigin: string = '00'.repeat(30) + '1111'
const result = await executeOVMCall(executionManager, 'executeCall', [
0,
getCurrentTime(),
queueOrigin,
contractAddress32,
methodIds.callThroughExecutionManager,
Expand All @@ -235,7 +225,7 @@ describe('Execution Manager -- Context opcodes', () => {

const executeCall = (args: any[]): Promise<string> => {
return executeOVMCall(executionManager, 'executeCall', [
encodeRawArguments([0, 0, ...args]),
encodeRawArguments([getCurrentTime(), 0, ...args]),
])
}
})
48 changes: 43 additions & 5 deletions packages/ovm/test/contracts/execution-manager.executeCall.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import '../setup'

/* External Imports */
import { Address } from '@eth-optimism/rollup-core'
import { getLogger, padToLength, ZERO_ADDRESS } from '@eth-optimism/core-utils'
import { getLogger, padToLength, ZERO_ADDRESS, TestUtils, getCurrentTime } from '@eth-optimism/core-utils'

import { Contract, ContractFactory, ethers } from 'ethers'
import { createMockProvider, deployContract, getWallets } from 'ethereum-waffle'
Expand Down Expand Up @@ -75,6 +75,44 @@ describe('Execution Manager -- Call opcodes', () => {
})

describe('executeNonEOACall', async () => {
it('fails if the provided timestamp is 0', async () => {
// Create the variables we will use for setStorage
const intParam = 0
const bytesParam = '0xdeadbeef'
// Generate our tx calldata
const calldata = getUnsignedTransactionCalldata(
dummyContract,
'dummyFunction',
[intParam, bytesParam]
)
const nonce = await executionManager.getOvmContractNonce(wallet.address)
const transaction = {
nonce,
gasLimit: GAS_LIMIT,
gasPrice: 0,
to: dummyContractAddress,
value: 0,
data: calldata,
chainId: CHAIN_ID,
}
const signedMessage = await wallet.sign(transaction)
const [v, r, s] = ethers.utils.RLP.decode(signedMessage).slice(-3)

await TestUtils.assertThrowsAsync(async () => {

Choose a reason for hiding this comment

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

This is good. Is there a good check that we can do to prevent false-positives? A brittle check might be checking for revert or something, but false-negatives may be better than false-positives. Also fine to say no and just merge

Copy link
Author

Choose a reason for hiding this comment

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

// Call using Ethers
const tx = await executionManager.executeEOACall(
0,
0,
transaction.nonce,
transaction.to,
transaction.data,
padToLength(v, 4),
padToLength(r, 64),
padToLength(s, 64)
)
await provider.waitForTransaction(tx.hash)
})
})
it('properly executes a raw call -- 0 param', async () => {
// Create the variables we will use for setStorage
const intParam = 0
Expand All @@ -98,7 +136,7 @@ describe('Execution Manager -- Call opcodes', () => {

// Call using Ethers
const tx = await executionManager.executeUnsignedEOACall(
0,
getCurrentTime(),
0,
transaction.to,
transaction.data,
Expand Down Expand Up @@ -136,7 +174,7 @@ describe('Execution Manager -- Call opcodes', () => {

// Call using Ethers
const tx = await executionManager.executeEOACall(
0,
getCurrentTime(),
0,
transaction.nonce,
transaction.to,
Expand Down Expand Up @@ -173,7 +211,7 @@ describe('Execution Manager -- Call opcodes', () => {

// Call using Ethers
const tx = await executionManager.executeEOACall(
0,
getCurrentTime(),
0,
transaction.nonce,
transaction.to,
Expand Down Expand Up @@ -213,7 +251,7 @@ describe('Execution Manager -- Call opcodes', () => {

// Call using Ethers
const tx = await executionManager.executeEOACall(
0,
getCurrentTime(),
0,
transaction.nonce,
transaction.to,
Expand Down
4 changes: 2 additions & 2 deletions packages/ovm/test/contracts/simple-storage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import '../setup'
/* External Imports */
import { Address } from '@eth-optimism/rollup-core'
import { createMockProvider, deployContract, getWallets } from 'ethereum-waffle'
import { getLogger, add0x } from '@eth-optimism/core-utils'
import { getLogger, add0x, getCurrentTime } from '@eth-optimism/core-utils'
import { Contract, ContractFactory, ethers } from 'ethers'
import { TransactionReceipt } from 'ethers/providers'
import * as ethereumjsAbi from 'ethereumjs-abi'
Expand Down Expand Up @@ -118,7 +118,7 @@ describe('SimpleStorage', () => {
const callData = getUnsignedTransactionCalldata(
executionManager,
'executeEOACall',
[0, 0, transaction.nonce, transaction.to, transaction.data, v, r, s]
[getCurrentTime(), 0, transaction.nonce, transaction.to, transaction.data, v, r, s]
)

const result = await executionManager.provider.call({
Expand Down
4 changes: 2 additions & 2 deletions packages/ovm/test/contracts/tx-origin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import '../setup'
/* External Imports */
import { Address } from '@eth-optimism/rollup-core'
import { createMockProvider, deployContract, getWallets } from 'ethereum-waffle'
import { getLogger, add0x } from '@eth-optimism/core-utils'
import { getLogger, add0x, getCurrentTime } from '@eth-optimism/core-utils'
import { Contract, ContractFactory } from 'ethers'
import * as ethereumjsAbi from 'ethereumjs-abi'

Expand Down Expand Up @@ -85,7 +85,7 @@ describe('SimpleTxOrigin', () => {
const callData = getUnsignedTransactionCalldata(
executionManager,
'executeEOACall',
[0, 0, transaction.nonce, transaction.to, transaction.data, v, r, s]
[getCurrentTime(), 0, transaction.nonce, transaction.to, transaction.data, v, r, s]
)

const result = await executionManager.provider.call({
Expand Down
3 changes: 2 additions & 1 deletion packages/ovm/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getLogger,
add0x,
abi,
getCurrentTime,
keccak256,
strToHexStr,
remove0x,
Expand Down Expand Up @@ -119,7 +120,7 @@ export const executeUnsignedEOACall = async (

// Actually make the call
const tx = await executionManager.executeUnsignedEOACall(
0,
getCurrentTime(),
0,
ovmTo,
data,
Expand Down