dtl: fix replica tx indexing (#872)

* dtl: update constant to 9 million gaslimit

* dtl: parse as rlp without top level type

* chore: add changeset

* test: delete dead code

* lint: fix

* deps: add etherproject transactions

* dtl: better handler for more number types

* tests: fix chainid

.changeset/kind-cows-draw.md

+---
+'@eth-optimism/data-transport-layer': patch
+---
+
+Fixes a bug where L2 synced transactions were not RLP encoded

packages/data-transport-layer/package.json

@@ -25,6 +25,7 @@
     "@eth-optimism/contracts": "^0.3.0",
     "@eth-optimism/core-utils": "^0.4.2",
     "@ethersproject/providers": "^5.0.21",
+    "@ethersproject/transactions": "^5.0.21",
     "@sentry/node": "^6.3.1",
     "@sentry/tracing": "^6.3.1",
     "@types/express": "^4.17.11",

packages/data-transport-layer/src/services/l1-ingestion/handlers/sequencer-batch-appended.ts

@@ -104,7 +104,7 @@ export const handleEventsSequencerBatchAppended: EventHandlerSet<
           nextTxPointer
         )
 
-        const { decoded, type } = maybeDecodeSequencerBatchTransaction(
+        const decoded = maybeDecodeSequencerBatchTransaction(
           sequencerTransaction
         )
 
@@ -120,7 +120,6 @@ export const handleEventsSequencerBatchAppended: EventHandlerSet<
           origin: null,
           data: toHexString(sequencerTransaction),
           queueOrigin: 'sequencer',
-          type,
           value: decoded ? decoded.value : '0x0',
           queueIndex: null,
           decoded,
@@ -153,7 +152,6 @@ export const handleEventsSequencerBatchAppended: EventHandlerSet<
           origin: constants.AddressZero,
           data: '0x',
           queueOrigin: 'l1',
-          type: 'EIP155',
           value: '0x0',
           queueIndex: queueIndex.toNumber(),
           decoded: null,
@@ -239,52 +237,24 @@ const parseSequencerBatchTransaction = (
 
 const maybeDecodeSequencerBatchTransaction = (
   transaction: Buffer
-): {
-  decoded: DecodedSequencerBatchTransaction | null
-  type: 'EIP155' | 'ETH_SIGN' | null
-} => {
+): DecodedSequencerBatchTransaction | null => {
   try {
     const decodedTx = ethers.utils.parseTransaction(transaction)
 
     return {
-      type: 'EIP155',
-      decoded: {
-        nonce: BigNumber.from(decodedTx.nonce).toNumber(),
-        gasPrice: BigNumber.from(decodedTx.gasPrice).toNumber(),
-        gasLimit: BigNumber.from(decodedTx.gasLimit).toNumber(),
-        value: toRpcHexString(decodedTx.value),
-        target: toHexString(decodedTx.to), // Maybe null this out for creations?
-        data: toHexString(decodedTx.data),
-        sig: {
-          v: BigNumber.from(decodedTx.v).toNumber(),
-          r: toHexString(decodedTx.r),
-          s: toHexString(decodedTx.s),
-        },
-        type: 0, // EIP155 legacy holdover.
+      nonce: BigNumber.from(decodedTx.nonce).toNumber(),
+      gasPrice: BigNumber.from(decodedTx.gasPrice).toNumber(),
+      gasLimit: BigNumber.from(decodedTx.gasLimit).toNumber(),
+      value: toRpcHexString(decodedTx.value),
+      target: toHexString(decodedTx.to), // Maybe null this out for creations?
+      data: toHexString(decodedTx.data),
+      sig: {
+        v: BigNumber.from(decodedTx.v).toNumber(),
+        r: toHexString(decodedTx.r),
+        s: toHexString(decodedTx.s),
       },
     }
   } catch (err) {
-    return {
-      decoded: null,
-      type: null,
-    }
-  }
-}
-
-export function validateBatchTransaction(
-  type: string | null,
-  decoded: DecodedSequencerBatchTransaction | null
-): boolean {
-  // Unknown types are considered invalid
-  if (type === null) {
-    return false
-  }
-  if (type === 'EIP155' || type === 'ETH_SIGN') {
-    if (decoded.sig.v !== 1 && decoded.sig.v !== 0) {
-      return false
-    }
-    return true
+    return null
   }
-  // Allow soft forks
-  return false
 }

packages/data-transport-layer/src/services/l2-ingestion/handlers/transaction.ts

 /* Imports: External */
-import { ctcCoder } from '@eth-optimism/core-utils'
 import { BigNumber, constants, ethers } from 'ethers'
+import { serialize } from '@ethersproject/transactions'
 
 /* Imports: Internal */
 import { TransportDB } from '../../../db/transport-db'
@@ -37,7 +37,6 @@ export const handleSequencerBlock = {
       blockNumber: BigNumber.from(transaction.l1BlockNumber).toNumber(),
       timestamp: BigNumber.from(transaction.l1Timestamp).toNumber(),
       queueOrigin: transaction.queueOrigin,
-      type: parseTxType(transaction.txType),
       confirmed: false,
     }
 
@@ -54,7 +53,6 @@ export const handleSequencerBlock = {
         nonce: BigNumber.from(transaction.nonce).toNumber(),
         target: transaction.to || constants.AddressZero, // ?
         data: transaction.input,
-        type: transaction.txType,
       }
 
       transactionEntry = {
@@ -62,9 +60,21 @@ export const handleSequencerBlock = {
         gasLimit: SEQUENCER_GAS_LIMIT, // ?
         target: SEQUENCER_ENTRYPOINT_ADDRESS,
         origin: null,
-        data: maybeEncodeSequencerBatchTransaction(
-          decodedTransaction,
-          transaction.txType
+        data: serialize(
+          {
+            value: transaction.value,
+            gasLimit: transaction.gas,
+            gasPrice: transaction.gasPrice,
+            nonce: transaction.nonce,
+            to: transaction.to || constants.AddressZero,
+            data: transaction.input,
+            chainId,
+          },
+          {
+            v: BigNumber.from(transaction.v).toNumber(),
+            r: padHexString(transaction.r, 32),
+            s: padHexString(transaction.s, 32),
+          }
         ),
         decoded: decodedTransaction,
         queueIndex: null,
@@ -111,37 +121,3 @@ export const handleSequencerBlock = {
     await db.putUnconfirmedStateRootEntries([entry.stateRootEntry])
   },
 }
-
-/**
- * Attempts to encode a sequencer batch transaction.
- * @param transaction Transaction to encode.
- * @param type Transaction type.
- */
-const maybeEncodeSequencerBatchTransaction = (
-  transaction: DecodedSequencerBatchTransaction,
-  type: 'EIP155' | 'EthSign' | null
-): string => {
-  if (type === 'EIP155') {
-    return ctcCoder.eip155TxData.encode(transaction).toLowerCase()
-  } else if (type === 'EthSign') {
-    return ctcCoder.ethSignTxData.encode(transaction).toLowerCase()
-  } else {
-    // Throw?
-    return
-  }
-}
-
-/**
- * Handles differences between the sequencer's enum strings and our own.
- * Will probably want to move this into core-utils eventually.
- * @param type Sequencer transaction type to parse.
- */
-const parseTxType = (
-  type: 'EIP155' | 'EthSign' | null
-): 'EIP155' | 'ETH_SIGN' | null => {
-  if (type === 'EthSign') {
-    return 'ETH_SIGN'
-  } else {
-    return type
-  }
-}

packages/data-transport-layer/src/types/database-types.ts

@@ -10,7 +10,6 @@ export interface DecodedSequencerBatchTransaction {
   nonce: number
   target: string
   data: string
-  type: number
 }
 
 export interface EnqueueEntry {
@@ -35,7 +34,6 @@ export interface TransactionEntry {
   value: string
   queueOrigin: 'sequencer' | 'l1'
   queueIndex: number | null
-  type: 'EIP155' | 'ETH_SIGN' | null
   decoded: DecodedSequencerBatchTransaction | null
   confirmed: boolean
 }

packages/data-transport-layer/src/utils/constants.ts

-export const SEQUENCER_GAS_LIMIT = 8_000_000 // TODO: Remove and use value from event.
+export const SEQUENCER_GAS_LIMIT = 9_000_000 // TODO: Remove and use value from event.
 export const SEQUENCER_ENTRYPOINT_ADDRESS =
   '0x4200000000000000000000000000000000000005'

packages/data-transport-layer/test/unit-tests/services/l1-ingestion/handlers/sequencer-batch-appended.spec.ts

 import { BigNumber, ethers } from 'ethers'
 import { expect } from '../../../../setup'
-import {
-  validateBatchTransaction,
-  handleEventsSequencerBatchAppended,
-} from '../../../../../src/services/l1-ingestion/handlers/sequencer-batch-appended'
+import { handleEventsSequencerBatchAppended } from '../../../../../src/services/l1-ingestion/handlers/sequencer-batch-appended'
 import { SequencerBatchAppendedExtraData } from '../../../../../src/types'
 import { l1TransactionData } from '../../../examples/l1-data'
 import { blocksOnL2 } from '../../../examples/l2-data'
 
 describe('Event Handlers: OVM_CanonicalTransactionChain.SequencerBatchAppended', () => {
-  describe('validateBatchTransaction', () => {
-    it('should mark a transaction as invalid if the type is null', () => {
-      const input1: [any, any] = [null, null]
-
-      const output1 = validateBatchTransaction(...input1)
-
-      const expected1 = false
-
-      expect(output1).to.equal(expected1)
-    })
-
-    it('should mark a transaction as invalid if the type is not EIP155 or ETH_SIGN', () => {
-      const input1: [any, any] = ['SOME_RANDOM_TYPE', null]
-
-      const output1 = validateBatchTransaction(...input1)
-
-      const expected1 = false
-
-      expect(output1).to.equal(expected1)
-    })
-
-    describe('when the transaction type is EIP155 or ETH_SIGN', () => {
-      it('should mark a transaction as valid if the `v` parameter is 0', () => {
-        // CTC index 23159
-        const input1: [any, any] = [
-          'EIP155',
-          {
-            sig: {
-              v: 0,
-            },
-          },
-        ]
-
-        const output1 = validateBatchTransaction(...input1)
-
-        const expected1 = true
-
-        expect(output1).to.equal(expected1)
-      })
-
-      it('should mark a transaction as valid if the `v` parameter is 1', () => {
-        // CTC index 23159
-        const input1: [any, any] = [
-          'EIP155',
-          {
-            sig: {
-              v: 1,
-            },
-          },
-        ]
-
-        const output1 = validateBatchTransaction(...input1)
-
-        const expected1 = true
-
-        expect(output1).to.equal(expected1)
-      })
-
-      it('should mark a transaction as invalid if the `v` parameter is greater than 1', () => {
-        // CTC index 23159
-        const input1: [any, any] = [
-          'EIP155',
-          {
-            sig: {
-              v: 2,
-            },
-          },
-        ]
-
-        const output1 = validateBatchTransaction(...input1)
-
-        const expected1 = false
-
-        expect(output1).to.equal(expected1)
-      })
-    })
-
-    describe('regressions', () => {
-      it('should catch the invalid transaction', () => {
-        // CTC index 23159
-        const input1: [any, any] = [
-          'EIP155',
-          {
-            sig: {
-              r:
-                '0x0fbef2080fadc4198ee0d6027e2eb70799d3418574cc085c34a14dcefe14d5d3',
-              s:
-                '0x3bf394a7cb2aca6790e67382f782a406aefce7553212db52b54a4e087c2195ad',
-              v: 56,
-            },
-            gasLimit: 8000000,
-            gasPrice: 0,
-            nonce: 0,
-            target: '0x1111111111111111111111111111111111111111',
-            data: '0x1234',
-          },
-        ]
-
-        const output1 = validateBatchTransaction(...input1)
-
-        const expected1 = false
-
-        expect(output1).to.equal(expected1)
-      })
-    })
-  })
-
   describe('handleEventsSequencerBatchAppended.parseEvent', () => {
     // This tests the behavior of parsing a real mainnet transaction,
     // so it will break if the encoding scheme changes.

packages/data-transport-layer/test/unit-tests/services/l2-ingestion/handlers/transaction.spec.ts

@@ -5,7 +5,7 @@ import { handleSequencerBlock } from '../../../../../src/services/l2-ingestion/h
 describe('Handlers: handleSequencerBlock', () => {
   describe('parseBlock', () => {
     it('should correctly extract key fields from an L2 mainnet transaction', async () => {
-      const input1: [any, number] = [l2Block, 420]
+      const input1: [any, number] = [l2Block, 10]
 
       const output1 = await handleSequencerBlock.parseBlock(...input1)