Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions .changeset/witty-buses-worry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@eth-optimism/batch-submitter": minor
"@eth-optimism/core-utils": patch
---

- Use raw transaction in batch submitter -- incompatible with L2Geth v0.1.2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

What is geth 0.1.2.1? Is it deployed / being used anywhere right now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah it is the version of Geth that we were running before our last regenesis ~2/3 weeks ago. This should not be breaking Kovan or Mainnet now that we've got rawTransaction in mainnet

- Pass through raw transaction in l2context
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { Promise as bPromise } from 'bluebird'
import { Contract, Signer, providers } from 'ethers'
import { TransactionReceipt } from '@ethersproject/abstract-provider'
import { getContractFactory } from 'old-contracts'
import { Logger, Bytes32, remove0x } from '@eth-optimism/core-utils'
import {
Logger,
Bytes32,
remove0x,
toRpcHexString,
} from '@eth-optimism/core-utils'

/* Internal Imports */
import { L2Block } from '..'
Expand Down
132 changes: 19 additions & 113 deletions packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
/* External Imports */
import { Promise as bPromise } from 'bluebird'
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of removing bluebird as a dependency in the future? Concurrent async calls are easy with Promise.all

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same thought when I read that code before, here's a post providing good reasoning on "why bluebird" https://blog.kuzzle.io/bluebird-native-async_await-javascript-promises-performances-2020

import { BigNumber, Signer, ethers, Wallet, Contract, providers } from 'ethers'
import { Signer, ethers, Contract, providers } from 'ethers'
import {
TransactionResponse,
TransactionReceipt,
} from '@ethersproject/abstract-provider'
import { getContractInterface, getContractFactory } from 'old-contracts'
import { getContractInterface as getNewContractInterface } from '@eth-optimism/contracts'
import {
Logger,
EIP155TxData,
TxType,
ctcCoder,
EthSignTxData,
txTypePlainText,
} from '@eth-optimism/core-utils'
import { Logger, ctcCoder } from '@eth-optimism/core-utils'

/* Internal Imports */
import {
Expand All @@ -24,13 +17,7 @@ import {
AppendSequencerBatchParams,
} from '../transaction-chain-contract'

import {
L2Block,
BatchElement,
Batch,
QueueOrigin,
queueOriginPlainText,
} from '..'
import { L2Block, BatchElement, Batch, QueueOrigin } from '..'
Copy link
Contributor

Choose a reason for hiding this comment

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

These types could all go in core-utils, not opinionated on if its as part of this PR or not

Copy link
Contributor Author

@karlfloersch karlfloersch Apr 22, 2021

Choose a reason for hiding this comment

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

Agree they should definitely go into core-utils. That said there's a bunch of type & utility cleanup that we could do and so I'd lean against doing a potentially big refactor in the same PR that we're adding this new feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

^happy to help with the utility cleanup!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙏🙏🙏

import { RollupInfo, Range, BatchSubmitter, BLOCK_OFFSET } from '.'

export interface AutoFixBatchOptions {
Expand Down Expand Up @@ -602,41 +589,11 @@ export class TransactionBatchSubmitter extends BatchSubmitter {
queueElementBlockNumber: queueElement.blockNumber,
}
)
// This implies that we've double played a deposit.
// We can correct this by instead submitting a dummy sequencer tx
const wallet = Wallet.createRandom()
const gasLimit = 8_000_000
const gasPrice = 0
const chainId = 10
const nonce = 0
const rawTx = await wallet.signTransaction({
gasLimit,
gasPrice,
chainId,
nonce,
to: '0x1111111111111111111111111111111111111111',
data: '0x1234',
})
// tx: [0nonce, 1gasprice, 2startgas, 3to, 4value, 5data, 6v, 7r, 8s]
const tx = ethers.utils.RLP.decode(rawTx)
const dummyTx: EIP155TxData = {
sig: {
v: tx[6],
r: tx[7],
s: tx[8],
},
gasLimit,
gasPrice,
nonce,
target: tx[3],
data: tx[5],
type: TxType.EIP155,
}
const dummyTx: string = '0x1234'
return {
stateRoot: queueElement.stateRoot,
isSequencerTx: true,
sequencerTxType: TxType.EIP155,
txData: dummyTx,
rawTransaction: dummyTx,
timestamp: queueElement.timestamp,
blockNumber: queueElement.blockNumber,
}
Expand Down Expand Up @@ -715,17 +672,7 @@ export class TransactionBatchSubmitter extends BatchSubmitter {
if (!block.isSequencerTx) {
continue
}
let encoding: string
if (block.sequencerTxType === TxType.EIP155) {
encoding = ctcCoder.eip155TxData.encode(block.txData as EIP155TxData)
} else if (block.sequencerTxType === TxType.EthSign) {
encoding = ctcCoder.ethSignTxData.encode(block.txData as EthSignTxData)
} else {
throw new Error(
`Trying to build batch with unknown type ${block.sequencerTxType}`
)
}
transactions.push(encoding)
transactions.push(block.rawTransaction)
}

return {
Expand All @@ -742,67 +689,26 @@ export class TransactionBatchSubmitter extends BatchSubmitter {
this.log.debug('Fetched L2 block', {
block,
})
const txType = block.transactions[0].txType

if (this._isSequencerTx(block)) {
if (txType === TxType.EIP155 || txType === TxType.EthSign) {
return this._getDefaultEcdsaTxBatchElement(block)
} else {
throw new Error('Unsupported Tx Type!')
}
} else {
return {
stateRoot: block.stateRoot,
isSequencerTx: false,
sequencerTxType: undefined,
txData: undefined,
timestamp: block.timestamp,
blockNumber: block.transactions[0].l1BlockNumber,
}
const batchElement = {
stateRoot: block.stateRoot,
timestamp: block.timestamp,
blockNumber: block.transactions[0].l1BlockNumber,
isSequencerTx: false,
rawTransaction: undefined,
}
}

private async _getBlock(blockNumber: number): Promise<L2Block> {
const block = (await this.l2Provider.getBlockWithTransactions(
blockNumber
)) as L2Block
// Convert the tx type to a number
block.transactions[0].txType = txTypePlainText[block.transactions[0].txType]
block.transactions[0].queueOrigin =
queueOriginPlainText[block.transactions[0].queueOrigin]
if (!block.transactions[0].l1BlockNumber) {
this.log.error('Current block missing l1BlockNumber', {
blockNumber,
block,
})
if (this._isSequencerTx(block)) {
batchElement.isSequencerTx = true
batchElement.rawTransaction = block.transactions[0].rawTransaction
}

return block
return batchElement
}

private _getDefaultEcdsaTxBatchElement(block: L2Block): BatchElement {
const tx: TransactionResponse = block.transactions[0]
const txData: EIP155TxData = {
sig: {
v: tx.v - this.l2ChainId * 2 - 8 - 27,
r: tx.r,
s: tx.s,
},
gasLimit: BigNumber.from(tx.gasLimit).toNumber(),
gasPrice: BigNumber.from(tx.gasPrice).toNumber(),
nonce: tx.nonce,
target: tx.to ? tx.to : '00'.repeat(20),
data: tx.data,
type: block.transactions[0].txType,
}
return {
stateRoot: block.stateRoot,
isSequencerTx: true,
sequencerTxType: block.transactions[0].txType,
txData,
timestamp: block.timestamp,
blockNumber: block.transactions[0].l1BlockNumber,
}
private async _getBlock(blockNumber: number): Promise<L2Block> {
const p = this.l2Provider.getBlockWithTransactions(blockNumber)
return p as Promise<L2Block>
}

private _isSequencerTx(block: L2Block): boolean {
Expand Down
1 change: 1 addition & 0 deletions packages/batch-submitter/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './batch-submitter'
export * from './transaction-chain-contract'
export * from './types'
export * from './utils'
22 changes: 5 additions & 17 deletions packages/batch-submitter/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,12 @@
/* External Imports */
import {
BlockWithTransactions,
Provider,
TransactionResponse,
} from '@ethersproject/abstract-provider'

/* Internal Imports */
import { EIP155TxData, TxType } from '@eth-optimism/core-utils'

export enum QueueOrigin {
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially out of scope of this PR but this enum should be moved to core-utils

Copy link
Contributor

Choose a reason for hiding this comment

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

I notice there is another change to core-utils as part of this PR

Sequencer = 0,
L1ToL2 = 1,
}

export const queueOriginPlainText = {
0: QueueOrigin.Sequencer,
1: QueueOrigin.L1ToL2,
sequencer: QueueOrigin.Sequencer,
l1ToL2: QueueOrigin.L1ToL2,
Sequencer = 'sequencer',
L1ToL2 = 'l1',
}

/**
Expand All @@ -27,8 +16,8 @@ export const queueOriginPlainText = {
export interface L2Transaction extends TransactionResponse {
l1BlockNumber: number
l1TxOrigin: string
txType: number
queueOrigin: number
queueOrigin: string
rawTransaction: string
}

export interface L2Block extends BlockWithTransactions {
Expand All @@ -43,8 +32,7 @@ export interface L2Block extends BlockWithTransactions {
export interface BatchElement {
stateRoot: string
isSequencerTx: boolean
sequencerTxType: undefined | TxType
txData: undefined | EIP155TxData
rawTransaction: undefined | string
timestamp: number
blockNumber: number
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,18 +247,9 @@ describe('BatchSubmitter', () => {
const nextQueueElement = await getQueueElement(
OVM_CanonicalTransactionChain
)
const data = ctcCoder.eip155TxData.encode({
sig: DUMMY_SIG,
gasLimit: 0,
gasPrice: 0,
nonce: 0,
target: DUMMY_ADDRESS,
data: '0x',
type: TxType.EIP155,
})
l2Provider.setL2BlockData(
{
data,
rawTransaction: '0x1234',
l1BlockNumber: nextQueueElement.blockNumber - 1,
txType: TxType.EIP155,
queueOrigin: QueueOrigin.Sequencer,
Expand Down Expand Up @@ -305,18 +296,9 @@ describe('BatchSubmitter', () => {
OVM_CanonicalTransactionChain,
2
)
const data = ctcCoder.ethSignTxData.encode({
sig: DUMMY_SIG,
gasLimit: 0,
gasPrice: 0,
nonce: 0,
target: DUMMY_ADDRESS,
data: '0x',
type: TxType.EthSign,
})
l2Provider.setL2BlockData(
{
data,
rawTransaction: '0x1234',
l1BlockNumber: nextQueueElement.blockNumber - 1,
txType: TxType.EthSign,
queueOrigin: QueueOrigin.Sequencer,
Expand Down Expand Up @@ -418,18 +400,9 @@ describe('BatchSubmitter', () => {
const nextQueueElement = await getQueueElement(
OVM_CanonicalTransactionChain
)
const data = ctcCoder.eip155TxData.encode({
sig: DUMMY_SIG,
gasLimit: 0,
gasPrice: 0,
nonce: 0,
target: DUMMY_ADDRESS,
data: '0x',
type: TxType.EIP155,
})
l2Provider.setL2BlockData(
{
data,
rawTransaction: '0x1234',
l1BlockNumber: nextQueueElement.blockNumber - 1,
txType: TxType.EIP155,
queueOrigin: QueueOrigin.Sequencer,
Expand Down
Loading