From 5b54682afcc2a36f7ef39a16375c4c680e62fcbf Mon Sep 17 00:00:00 2001 From: Mason Fischer Date: Sun, 26 Apr 2020 16:59:17 -0400 Subject: [PATCH] Fix eth_getLogs bugs (#106) * Fix eth_getLogs bugs Note: Logs need to be converted to objects via `JSON.parse(JSON.stringify(x))` because the Log object's logIndex type is a `number` where here we need to set it to a `string` (the hex encoded number). Fixes: * address * logIndex * transactionHash * Lint * Remove only * Lint * Lint * add transaction to, dont alter params object directly * convert numbers to hex in responses * lint * fix getTransactionReceipt * lint Co-authored-by: Kevin Ho Co-authored-by: Karl Floersch --- packages/ovm/src/app/utils.ts | 9 +++-- .../src/app/web3-rpc-handler.ts | 39 +++++++++++++------ .../src/types/web3-rpc-handler.ts | 2 +- .../test/app/web-rpc-handler.spec.ts | 19 ++++----- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/packages/ovm/src/app/utils.ts b/packages/ovm/src/app/utils.ts index c000794aa409..19cbe2b25998 100644 --- a/packages/ovm/src/app/utils.ts +++ b/packages/ovm/src/app/utils.ts @@ -5,6 +5,7 @@ import { getLogger, hexStrToBuf, bufToHexString, + numberToHexString, logError, remove0x, ZERO_ADDRESS, @@ -69,7 +70,7 @@ export interface OvmTransactionMetadata { * @return the converted logs */ export const convertInternalLogsToOvmLogs = (logs: Log[]): Log[] => { - let activeContract = ZERO_ADDRESS + let activeContract = logs[0] ? logs[0].address : ZERO_ADDRESS const ovmLogs = [] logs.forEach((log) => { const executionManagerLog = executionManagerInterface.parseLog(log) @@ -189,8 +190,6 @@ export const internalTxReceiptToOvmTxReceipt = async ( if (ovmTransactionMetadata.ovmTo) { ovmTxReceipt.to = ovmTransactionMetadata.ovmTo } - // TODO: Update this to use some default account abstraction library potentially. - ovmTxReceipt.from = ovmTransactionMetadata.ovmFrom // Also update the contractAddress in case we deployed a new contract ovmTxReceipt.contractAddress = !!ovmTransactionMetadata.ovmCreatedContractAddress ? ovmTransactionMetadata.ovmCreatedContractAddress @@ -208,9 +207,11 @@ export const internalTxReceiptToOvmTxReceipt = async ( logger.debug('Ovm parsed logs:', ovmTxReceipt.logs) const logsBloom = new BloomFilter() - ovmTxReceipt.logs.forEach((log) => { + ovmTxReceipt.logs.forEach((log, index) => { logsBloom.add(hexStrToBuf(log.address)) log.topics.forEach((topic) => logsBloom.add(hexStrToBuf(topic))) + log.transactionHash = ovmTxReceipt.transactionHash + log.logIndex = numberToHexString(index) as any }) ovmTxReceipt.logsBloom = bufToHexString(logsBloom.bitvector) diff --git a/packages/rollup-full-node/src/app/web3-rpc-handler.ts b/packages/rollup-full-node/src/app/web3-rpc-handler.ts index e325e904a391..d2a753c4e7fa 100644 --- a/packages/rollup-full-node/src/app/web3-rpc-handler.ts +++ b/packages/rollup-full-node/src/app/web3-rpc-handler.ts @@ -465,9 +465,9 @@ export class DefaultWeb3Handler return this.context.executionManager.address } - public async getLogs(filter: any): Promise { - log.debug(`Requesting logs with filter [${JSON.stringify(filter)}].`) - + public async getLogs(ovmFilter: any): Promise { + log.debug(`Requesting logs with filter [${JSON.stringify(ovmFilter)}].`) + const filter = JSON.parse(JSON.stringify(ovmFilter)) if (filter['address']) { const codeContractAddress = await this.context.executionManager.getCodeContractAddress( filter.address @@ -477,8 +477,28 @@ export class DefaultWeb3Handler const res = await this.context.provider.send(Web3RpcMethods.getLogs, [ filter, ]) - const logs = convertInternalLogsToOvmLogs(res) + + let logs = JSON.parse(JSON.stringify(convertInternalLogsToOvmLogs(res))) log.debug(`Log result: [${logs}], filter: [${JSON.stringify(filter)}].`) + logs = await Promise.all( + logs.map(async (logItem, index) => { + logItem['logIndex'] = numberToHexString(index) + logItem['transactionHash'] = await this.getOvmTxHash( + logItem['transactionHash'] + ) + const transaction = await this.getTransactionByHash( + logItem['transactionHash'] + ) + if (transaction['to'] === null) { + const receipt = await this.getTransactionReceipt(transaction.hash) + transaction['to'] = receipt.contractAddress + } + logItem['address'] = transaction['to'] + + return logItem + }) + ) + return logs } @@ -566,18 +586,15 @@ export class DefaultWeb3Handler log.debug( `Internal tx previously failed for this OVM tx, creating receipt from the OVM tx itself.` ) - const rawOvmTx = await this.getOvmTransactionByHash(ovmTxHash) - const ovmTx = utils.parseTransaction(rawOvmTx) - // for a failing tx, everything is identical between the internal and external receipts, except to and from - ovmTxReceipt = internalTxReceipt - ovmTxReceipt.from = ovmTx.from - ovmTxReceipt.to = ovmTx.to } + const ovmTx = await this.getTransactionByHash(ovmTxReceipt.transactionHash) + log.debug(`got OVM tx from hash: [${JSON.stringify(ovmTx)}]`) + ovmTxReceipt.to = ovmTx.to ? ovmTx.to : ovmTxReceipt.to + ovmTxReceipt.from = ovmTx.from if (ovmTxReceipt.revertMessage !== undefined && !includeRevertMessage) { delete ovmTxReceipt.revertMessage } - if (typeof ovmTxReceipt.status === 'number') { ovmTxReceipt.status = numberToHexString(ovmTxReceipt.status) } diff --git a/packages/rollup-full-node/src/types/web3-rpc-handler.ts b/packages/rollup-full-node/src/types/web3-rpc-handler.ts index 0dde45c1c019..40a66c70faad 100644 --- a/packages/rollup-full-node/src/types/web3-rpc-handler.ts +++ b/packages/rollup-full-node/src/types/web3-rpc-handler.ts @@ -17,7 +17,7 @@ export interface Web3Handler { getBlockByHash(blockHash: string, fullObjects: boolean): Promise getCode(address: Address, defaultBlock: string): Promise getExecutionManagerAddress() - getLogs(filter: any): Promise + getLogs(ovmFilter: any): Promise getTransactionByHash(transactionHash: string): Promise getTransactionCount(address: Address, defaultBlock: string): Promise getTransactionReceipt(txHash: string): Promise diff --git a/packages/rollup-full-node/test/app/web-rpc-handler.spec.ts b/packages/rollup-full-node/test/app/web-rpc-handler.spec.ts index 8be7f46d3303..9d832c84dca2 100644 --- a/packages/rollup-full-node/test/app/web-rpc-handler.spec.ts +++ b/packages/rollup-full-node/test/app/web-rpc-handler.spec.ts @@ -327,7 +327,7 @@ describe('Web3Handler', () => { hexStrToBuf(block.transactions[0]).length.should.eq(32) }) - it('should return a block with the correct logsBloom', async () => { + it.only('should return a block with the correct logsBloom', async () => { const executionManagerAddress = await httpProvider.send( 'ovm_getExecutionManagerAddress', [] @@ -344,7 +344,7 @@ describe('Web3Handler', () => { eventEmitter.deployTransaction.hash ) const tx = await eventEmitter.emitEvent(executionManagerAddress) - + await wallet.provider.getTransactionReceipt(tx.hash) const block = await httpProvider.send('eth_getBlockByNumber', [ 'latest', true, @@ -469,13 +469,14 @@ describe('Web3Handler', () => { ) const tx = await eventEmitter.emitEvent(executionManagerAddress) - const logs = ( - await httpProvider.getLogs({ - address: eventEmitter.address, - }) - ).map((x) => factory.interface.parseLog(x)) - logs.length.should.eq(1) - logs[0].name.should.eq('Event') + const logs = await httpProvider.getLogs({ + address: eventEmitter.address, + }) + logs[0].address.should.eq(eventEmitter.address) + logs[0].logIndex.should.eq(0) + const parsedLogs = logs.map((x) => factory.interface.parseLog(x)) + parsedLogs.length.should.eq(1) + parsedLogs[0].name.should.eq('Event') }) })