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

YAS-335 Improvements to Ethereum event endpoints #116

Merged
merged 18 commits into from
Apr 29, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export const JSONRPC_ERRORS = {
code: -32603,
message: 'Internal error',
},
UNSUPPORTED_TOPICS_ERROR: {
code: -32604,
message: 'Unsupported filter parameters',
},
REVERT_ERROR: {
code: -32015,
message: 'revert: requested action reverted',
Expand Down
17 changes: 17 additions & 0 deletions packages/ovm/src/app/abi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* External Imports */
import { utils, ContractFactory } from 'ethers'

/* Internal Imports */
import * as ExecutionManager from '../../build/contracts/ExecutionManager.json'

const EMContract = new ContractFactory(
ExecutionManager.abi,
ExecutionManager.bytecode
)
const EMEvents = EMContract.interface.events
let topics = []
for (const eventKey of Object.keys(EMEvents)) {
topics.push(EMEvents[eventKey].topic)
}

export const ALL_EXECUTION_MANAGER_EVENT_TOPICS = topics
1 change: 1 addition & 0 deletions packages/ovm/src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from './constants'
export * from './ovm'
export * from './serialization'
export * from './utils'
export * from './abi'
59 changes: 47 additions & 12 deletions packages/ovm/src/app/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
ZERO_ADDRESS,
LOG_NEWLINE_STRING,
BloomFilter,
hexStrToNumber,
} from '@eth-optimism/core-utils'
import { ethers } from 'ethers'
import { LogDescription } from 'ethers/utils'
Expand Down Expand Up @@ -61,34 +62,64 @@ export interface OvmTransactionMetadata {
}

/**
* Convert internal logs into OVM logs. Or in other words, take the logs which
* Convert internal transaction logs into OVM logs. Or in other words, take the logs which
* are emitted by a normal Ganache or Geth node (this will include logs from the ExecutionManager),
* parse them, and then convert them into logs which look like they would if you were running this tx
* using an OVM backend.
*
* NOTE: The input logs MUST NOT be stripped of any Execution Manager events, or this function will break.
*
* @param logs an array of internal logs which we will parse and then convert.
* @param logs an array of internal transaction logs which we will parse and then convert.
* @return the converted logs
*/
export const convertInternalLogsToOvmLogs = (logs: Log[]): Log[] => {
export const convertInternalLogsToOvmLogs = (
logs: Log[],
executionManagerAddress: string
): Log[] => {
let activeContract = logs[0] ? logs[0].address : ZERO_ADDRESS
const loggerLogs = [`Parsing logs from contract ${activeContract}: `]
const ovmLogs = []
let cumulativeTxEMLogIndices = 0
let prevEMLogIndex = 0
logs.forEach((log) => {
const executionManagerLog = executionManagerInterface.parseLog(log)
if (executionManagerLog) {
if (executionManagerLog.name === 'ActiveContract') {
activeContract = executionManagerLog.values['_activeContract']
if (log.address.toUpperCase() === executionManagerAddress.toUpperCase()) {
const EMLogIndex = log.logIndex
if (EMLogIndex < prevEMLogIndex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should tihs be <=? E.g. if prevEMlogIndex is 0, and EMLogIndex is 0, it's still a new transaction, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice catch!

logger.debug(
`Detected raw EM log ${log} with lower logIndex than previously processed, must be from a new transaction. Resetting cumulative EM log indices for tx.`
)
cumulativeTxEMLogIndices = 0
}
cumulativeTxEMLogIndices++
prevEMLogIndex = EMLogIndex
const executionManagerLog = executionManagerInterface.parseLog(log)
if (!executionManagerLog) {
logger.debug(
`execution manager log ${log} was unrecognized by the interface parser--Definitely not an activeContract event, ignoring...`

Choose a reason for hiding this comment

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

I think ${log} will just print [object Object], but JSON.stringify(...) will result in circular error. Is there another property that would be useful to log here? log.name?

)
} else {
loggerLogs.push(
`${executionManagerLog.name}, values: ${JSON.stringify(
executionManagerLog.values
)}`
)} and cumulativeTxEMLogIndices: ${cumulativeTxEMLogIndices}`
)
if (executionManagerLog.name === 'ActiveContract') {
activeContract = executionManagerLog.values['_activeContract']
logger.debug(
`EM activeContract event detected, setting activeContract to ${activeContract}`
)
} else {
logger.debug(`EM-but-non-activeContract event detected, ignoring...`)
}
}
} else {
loggerLogs.push(`Non-EM log: ${JSON.stringify(log)}`)
ovmLogs.push({ ...log, address: activeContract })
const newIndex = log.logIndex - cumulativeTxEMLogIndices
Copy link
Contributor

Choose a reason for hiding this comment

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

👌👌👌👌👌

loggerLogs.push(
`Non-EM log: ${JSON.stringify(
log
)}. Using address of active contract ${activeContract} and log index ${newIndex}`
)
ovmLogs.push({ ...log, address: activeContract, logIndex: newIndex })
}
})
logger.debug(loggerLogs.join(LOG_NEWLINE_STRING))
Expand Down Expand Up @@ -178,17 +209,21 @@ export const getSuccessfulOvmTransactionMetadata = (
*/
export const internalTxReceiptToOvmTxReceipt = async (
internalTxReceipt: TransactionReceipt,
executionManagerAddress: string,
ovmTxHash?: string
): Promise<OvmTransactionReceipt> => {
const ovmTransactionMetadata = getSuccessfulOvmTransactionMetadata(
internalTxReceipt
)
// Construct a new receipt
//

// Start off with the internalTxReceipt
const ovmTxReceipt: OvmTransactionReceipt = internalTxReceipt
// Add the converted logs
ovmTxReceipt.logs = convertInternalLogsToOvmLogs(internalTxReceipt.logs)
ovmTxReceipt.logs = convertInternalLogsToOvmLogs(
internalTxReceipt.logs,
executionManagerAddress
)
// Update the to and from fields if necessary
if (ovmTransactionMetadata.ovmTo) {
ovmTxReceipt.to = ovmTransactionMetadata.ovmTo
Expand Down
15 changes: 8 additions & 7 deletions packages/ovm/test/app/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ describe('convertInternalLogsToOvmLogs', () => {
it('should replace the address of the event with the address of the last active contract event', async () => {
convertInternalLogsToOvmLogs(
[
[EXECUTION_MANAGER_ADDRESS, 'ActiveContract(address)', [ALICE]],
[EXECUTION_MANAGER_ADDRESS, 'EventFromAlice()', []],
[EXECUTION_MANAGER_ADDRESS, 'ActiveContract(address)', [BOB]],
[EXECUTION_MANAGER_ADDRESS, 'EventFromBob()', []],
].map((args) => buildLog.apply(null, args))
[EXECUTION_MANAGER_ADDRESS, 'ActiveContract(address)', [ALICE], 0],
[CODE_CONTRACT, 'EventFromAlice()', [], 1],
[EXECUTION_MANAGER_ADDRESS, 'ActiveContract(address)', [BOB], 2],
[CODE_CONTRACT, 'EventFromBob()', [], 3],
].map((args) => buildLog.apply(null, args)),
EXECUTION_MANAGER_ADDRESS
).should.deep.eq(
[
[ALICE, 'EventFromAlice()', []],
[BOB, 'EventFromBob()', []],
[ALICE, 'EventFromAlice()', [], 0],
[BOB, 'EventFromBob()', [], 1],
].map((args) => buildLog.apply(null, args))
)
})
Expand Down
6 changes: 4 additions & 2 deletions packages/ovm/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const manuallyDeployOvmContractReturnReceipt = async (
false
)

return internalTxReceiptToOvmTxReceipt(receipt)
return internalTxReceiptToOvmTxReceipt(receipt, executionManager.address)
}

/**
Expand Down Expand Up @@ -318,7 +318,8 @@ export const didCreateSucceed = async (
export const buildLog = (
address: string,
event: string,
data: string[]
data: string[],
logIndex: number
): Log => {
const types = event.match(/\((.+)\)/)
const encodedData = types ? abi.encode(types[1].split(','), data) : '0x'
Expand All @@ -327,6 +328,7 @@ export const buildLog = (
address,
topics: [add0x(keccak256(strToHexStr(event)))],
data: encodedData,
logIndex,
}
}

Expand Down
9 changes: 9 additions & 0 deletions packages/rollup-full-node/src/app/fullnode-rpc-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
RevertError,
TransactionLimitError,
UnsupportedMethodError,
UnsupportedFilterError,
} from '../types'

const log: Logger = getLogger('rollup-fullnode-rpc-server')
Expand Down Expand Up @@ -128,6 +129,14 @@ export class FullnodeRpcServer extends ExpressHttpServer {
)
return buildJsonRpcError('METHOD_NOT_FOUND', request.id)
}
if (err instanceof UnsupportedFilterError) {
log.debug(
`Received request with unsupported filter parameters: [${JSON.stringify(
request
)}]`
)
return buildJsonRpcError('UNSUPPORTED_TOPICS_ERROR', request.id)
}
if (err instanceof InvalidParametersError) {
log.debug(
`Received request with valid method but invalid parameters: [${JSON.stringify(
Expand Down
55 changes: 47 additions & 8 deletions packages/rollup-full-node/src/app/web3-rpc-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
internalTxReceiptToOvmTxReceipt,
l2ToL1MessagePasserInterface,
OvmTransactionReceipt,
ALL_EXECUTION_MANAGER_EVENT_TOPICS,
} from '@eth-optimism/ovm'

import AsyncLock from 'async-lock'
Expand All @@ -42,6 +43,7 @@ import {
Web3RpcTypes,
Web3RpcMethods,
RevertError,
UnsupportedFilterError,
} from '../types'
import { initializeL2Node, getCurrentTime, isErrorEVMRevert } from './util'
import { NoOpL2ToL1MessageSubmitter } from './message-submitter'
Expand Down Expand Up @@ -494,20 +496,55 @@ export class DefaultWeb3Handler
}

public async getLogs(ovmFilter: any): Promise<any[]> {
log.debug(`Requesting logs with filter [${JSON.stringify(ovmFilter)}].`)
const filter = JSON.parse(JSON.stringify(ovmFilter))
// We cannot filter out execution manager events or else convertInternalLogsToOvmLogs will break. So add EM address to address filter
if (filter['address']) {
const codeContractAddress = await this.context.executionManager.getCodeContractAddress(
filter.address
)
filter['address'] = codeContractAddress
if (!Array.isArray(filter['address'])) {
filter['address'] = [filter['address']]
}
const codeContractAddresses = []
for (const address of filter['address']) {
codeContractAddresses.push(
await this.context.executionManager.getCodeContractAddress(address)
)
}
filter['address'] = [
...codeContractAddresses,
this.context.executionManager.address,
]
}
// We cannot filter out execution manager events or else convertInternalLogsToOvmLogs will break. So add EM topics to topics filter
if (filter['topics']) {
if (filter['topics'].length > 1) {
// todo make this proper error
const msg = `The provided filter ${filter} has multiple levels of topic filter. Multi-level topic filters are currently unsupported by the OVM.`
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Choose a reason for hiding this comment

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

Should you log ${JSON.stringify(filter)}? It's probably just going to print [object Object] otherwise.

throw new UnsupportedFilterError(msg)
}
if (!Array.isArray(filter['topics'][0])) {
filter['topics'][0] = [JSON.parse(JSON.stringify(filter['topics'][0]))]
}
filter['topics'][0].push(...ALL_EXECUTION_MANAGER_EVENT_TOPICS)
}
log.debug(
`Converted ovm filter ${JSON.stringify(
ovmFilter
)} to internal filter ${JSON.stringify(filter)}`
)

const res = await this.context.provider.send(Web3RpcMethods.getLogs, [
filter,
])

let logs = JSON.parse(JSON.stringify(convertInternalLogsToOvmLogs(res)))
log.debug(`Log result: [${logs}], filter: [${JSON.stringify(filter)}].`)
let logs = JSON.parse(
JSON.stringify(
convertInternalLogsToOvmLogs(res, this.context.executionManager.address)
)
)
log.debug(
`Log result: [${JSON.stringify(logs)}], filter: [${JSON.stringify(
filter
)}].`
)
logs = await Promise.all(
logs.map(async (logItem, index) => {
logItem['logIndex'] = numberToHexString(index)
Expand Down Expand Up @@ -608,6 +645,7 @@ export class DefaultWeb3Handler
)
ovmTxReceipt = await internalTxReceiptToOvmTxReceipt(
internalTxReceipt,
this.context.executionManager.address,
ovmTxHash
)
} else {
Expand Down Expand Up @@ -807,7 +845,8 @@ export class DefaultWeb3Handler

try {
const ovmTxReceipt: OvmTransactionReceipt = await internalTxReceiptToOvmTxReceipt(
txReceipt
txReceipt,
this.context.executionManager.address
)
await this.processTransactionEvents(ovmTxReceipt)
} catch (e) {
Expand Down
6 changes: 6 additions & 0 deletions packages/rollup-full-node/src/types/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ export class InvalidParametersError extends Error {
}
}

export class UnsupportedFilterError extends Error {
constructor(message?: string) {
super(message || 'The provided filter is currently unsupported by the OVM')
}
}

export class RevertError extends Error {
constructor(message?: string) {
super(message || 'Revert: The provided transaction reverted.')
Expand Down
Loading