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 <[email protected]> Co-authored-by: Karl Floersch <[email protected]>
ethereum-optimism · Apr 26, 2020 · 5b54682 · 5b54682
1 parent 9d67507
commit 5b54682
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 25 deletions.
diff --git 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
@@ -465,9 +465,9 @@ export class DefaultWeb3Handler
     return this.context.executionManager.address
   }
 
-  public async getLogs(filter: any): Promise<any[]> {
-    log.debug(`Requesting logs with filter [${JSON.stringify(filter)}].`)
-
+  public async getLogs(ovmFilter: any): Promise<any[]> {
+    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
@@ -17,7 +17,7 @@ export interface Web3Handler {
   getBlockByHash(blockHash: string, fullObjects: boolean): Promise<any>
   getCode(address: Address, defaultBlock: string): Promise<string>
   getExecutionManagerAddress()
-  getLogs(filter: any): Promise<any[]>
+  getLogs(ovmFilter: any): Promise<any[]>
   getTransactionByHash(transactionHash: string): Promise<any>
   getTransactionCount(address: Address, defaultBlock: string): Promise<string>
   getTransactionReceipt(txHash: string): Promise<string>

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
@@ -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')
       })
     })