From 9fc3a21cc5e093f8af4c9f857f7c3f0a48d9c2af Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Sun, 25 Oct 2020 12:17:02 -0400 Subject: [PATCH 1/5] Remove broken blockCache & improve tests --- .../batch-submitter/src/batch-submitter.ts | 23 ++--- .../batch-submitter/batch-submitter.spec.ts | 95 +++++++++++++++++-- .../batch-submitter/mockchain-provider.ts | 4 +- 3 files changed, 98 insertions(+), 24 deletions(-) diff --git a/packages/batch-submitter/src/batch-submitter.ts b/packages/batch-submitter/src/batch-submitter.ts index 4e5b59c65594..d080b33cea58 100644 --- a/packages/batch-submitter/src/batch-submitter.ts +++ b/packages/batch-submitter/src/batch-submitter.ts @@ -26,24 +26,20 @@ import { import { L2Block, BatchElement, Batch, QueueOrigin } from '.' export class BatchSubmitter { - public blockCache: { - [blockNumber: number]: BatchElement - } = {} - constructor( readonly txChain: CanonicalTransactionChainContract, readonly signer: Signer, readonly l2Provider: OptimismProvider, readonly l2ChainId: number, readonly maxTxSize: number, - readonly defaultBatchSize: number, + readonly maxBatchSize: number, readonly numConfirmations: number ) {} public async submitNextBatch(): Promise { const startBlock = parseInt(await this.txChain.getTotalElements(), 16) + 1 const endBlock = Math.min( - startBlock + this.defaultBatchSize, + startBlock + this.maxBatchSize, await this.l2Provider.getBlockNumber() ) log.info( @@ -69,24 +65,21 @@ export class BatchSubmitter { startBlock: number, endBlock: number ): Promise { - // Get all L2Blocks between the given range - const blocks: Batch = [] + // Get all L2 BatchElements for the given range + const batch: Batch = [] for (let i = startBlock; i < endBlock; i++) { - if (!this.blockCache.hasOwnProperty(i)) { - this.blockCache[i] = await this._getL2BatchElement(i) - } - blocks.push(this.blockCache[i]) + batch.push(await this._getL2BatchElement(i)) } let sequencerBatchParams = await this._getSequencerBatchParams( startBlock, - blocks + batch ) let encoded = encodeAppendSequencerBatch(sequencerBatchParams) while (encoded.length / 2 > this.maxTxSize) { - blocks.splice(Math.ceil((blocks.length * 2) / 3)) // Delete 1/3rd of all of the blocks + batch.splice(Math.ceil((batch.length * 2) / 3)) // Delete 1/3rd of all of the batch elements sequencerBatchParams = await this._getSequencerBatchParams( startBlock, - blocks + batch ) encoded = encodeAppendSequencerBatch(sequencerBatchParams) } diff --git a/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts b/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts index d902c2c31798..c8042333cd39 100644 --- a/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts +++ b/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts @@ -34,10 +34,13 @@ interface QueueElement { timestamp: number blockNumber: number } -const getNextQueueElement = async ( - ctcContract: Contract +const getQueueElement = async ( + ctcContract: Contract, + nextQueueIndex?: number ): Promise => { - const nextQueueIndex = await ctcContract.getNextQueueIndex() + if (!nextQueueIndex) { + nextQueueIndex = await ctcContract.getNextQueueIndex() + } const nextQueueElement = await ctcContract.getQueueElement(nextQueueIndex) return nextQueueElement } @@ -50,10 +53,8 @@ const DUMMY_SIG: Signature = { describe('BatchSubmitter', () => { let signer: Signer let sequencer: Signer - let l2Provider: MockchainProvider before(async () => { ;[signer, sequencer] = await ethers.getSigners() - l2Provider = new MockchainProvider() }) let AddressManager: Contract @@ -104,6 +105,7 @@ describe('BatchSubmitter', () => { }) let OVM_CanonicalTransactionChain: CanonicalTransactionChainContract + let l2Provider: MockchainProvider beforeEach(async () => { const unwrapped_OVM_CanonicalTransactionChain = await Factory__OVM_CanonicalTransactionChain.deploy( AddressManager.address, @@ -114,6 +116,7 @@ describe('BatchSubmitter', () => { getContractInterface('OVM_CanonicalTransactionChain'), sequencer ) + l2Provider = new MockchainProvider() }) describe('Submit', () => { @@ -135,7 +138,7 @@ describe('BatchSubmitter', () => { } }) - it('should execute without reverting', async () => { + it('should submit a sequencer batch correctly', async () => { const batchSubmitter = new BatchSubmitter( OVM_CanonicalTransactionChain, sequencer, @@ -146,7 +149,7 @@ describe('BatchSubmitter', () => { 1 ) l2Provider.setNumBlocksToReturn(5) - const nextQueueElement = await getNextQueueElement( + const nextQueueElement = await getQueueElement( OVM_CanonicalTransactionChain ) const data = ctcCoder.createEOATxData.encode({ @@ -176,5 +179,83 @@ describe('BatchSubmitter', () => { expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(0) // _numQueueElements expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(10) // _totalElements }) + + it('should submit a queue batch correctly', async () => { + const batchSubmitter = new BatchSubmitter( + OVM_CanonicalTransactionChain, + sequencer, + l2Provider as any, + l2Provider.chainId(), + MAX_TX_SIZE, + 10, + 1 + ) + l2Provider.setNumBlocksToReturn(5) + l2Provider.setL2BlockData( + { + meta: { + queueOrigin: QueueOrigin.L1ToL2, + } + } as any, + ) + let receipt = await batchSubmitter.submitNextBatch() + let logData = remove0x(receipt.logs[0].data) + expect(parseInt(logData.slice(64 * 0, 64 * 1), 16)).to.equal(0) // _startingQueueIndex + expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(5) // _numQueueElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(5) // _totalElements + receipt = await batchSubmitter.submitNextBatch() + logData = remove0x(receipt.logs[0].data) + expect(parseInt(logData.slice(64 * 0, 64 * 1), 16)).to.equal(5) // _startingQueueIndex + expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(5) // _numQueueElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(10) // _totalElements + }) + + it('should submit a batch with both queue and sequencer chain elements', async () => { + const batchSubmitter = new BatchSubmitter( + OVM_CanonicalTransactionChain, + sequencer, + l2Provider as any, + l2Provider.chainId(), + MAX_TX_SIZE, + 10, + 1 + ) + l2Provider.setNumBlocksToReturn(10) // For this batch we'll return 10 elements! + l2Provider.setL2BlockData( + { + meta: { + queueOrigin: QueueOrigin.L1ToL2, + } + } as any, + ) + // Turn blocks 3-5 into sequencer txs + const nextQueueElement = await getQueueElement( + OVM_CanonicalTransactionChain, + 2 + ) + const data = ctcCoder.createEOATxData.encode({ + sig: DUMMY_SIG, + messageHash: '66'.repeat(32), + }) + l2Provider.setL2BlockData( + { + data, + meta: { + l1BlockNumber: nextQueueElement.blockNumber - 1, + txType: TxType.createEOA, + queueOrigin: QueueOrigin.Sequencer, + l1TxOrigin: '0x' + '12'.repeat(20), + }, + } as any, + nextQueueElement.timestamp - 1, + 3, + 6 + ) + let receipt = await batchSubmitter.submitNextBatch() + let logData = remove0x(receipt.logs[0].data) + expect(parseInt(logData.slice(64 * 0, 64 * 1), 16)).to.equal(0) // _startingQueueIndex + expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(7) // _numQueueElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(10) // _totalElements + }) }) }) diff --git a/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts b/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts index 1365d608a1a9..ae33aed07802 100644 --- a/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts +++ b/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts @@ -59,12 +59,12 @@ export class MockchainProvider extends OptimismProvider { public setL2BlockData( tx: L2Transaction, - timestamp: number, + timestamp?: number, start: number = 0, end: number = this.mockBlocks.length ) { for (let i = start; i < end; i++) { - this.mockBlocks[i].timestamp = timestamp + this.mockBlocks[i].timestamp = (timestamp) ? timestamp : this.mockBlocks[i].timestamp this.mockBlocks[i].transactions[0] = { ...this.mockBlocks[i].transactions[0], ...tx, From 713f6d5247f2ba55ca286a040f6cfaf7250b788a Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Sun, 25 Oct 2020 12:34:59 -0400 Subject: [PATCH 2/5] Add tx response to debug logs --- packages/batch-submitter/src/batch-submitter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/batch-submitter/src/batch-submitter.ts b/packages/batch-submitter/src/batch-submitter.ts index d080b33cea58..0f44c0baa678 100644 --- a/packages/batch-submitter/src/batch-submitter.ts +++ b/packages/batch-submitter/src/batch-submitter.ts @@ -57,7 +57,8 @@ export class BatchSubmitter { const txRes = await this.txChain.appendSequencerBatch(batchParams) const receipt = await txRes.wait(this.numConfirmations) log.info('Submitted batch!') - log.debug('Tx receipt:', receipt) + log.debug('Transaction Response:', txRes) + log.debug('Transaction receipt:', receipt) return receipt } From 1f9d13adef4f057ef1247d3ca34e97e1f695c669 Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Sun, 25 Oct 2020 12:44:21 -0400 Subject: [PATCH 3/5] Add batch submitter CI --- .../build-test-lint-batch-submitter.yml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 .github/workflows/build-test-lint-batch-submitter.yml diff --git a/.github/workflows/build-test-lint-batch-submitter.yml b/.github/workflows/build-test-lint-batch-submitter.yml new file mode 100644 index 000000000000..8f8b3426b972 --- /dev/null +++ b/.github/workflows/build-test-lint-batch-submitter.yml @@ -0,0 +1,63 @@ +name: CI - batch-submitter + +on: + push: + branches: + - master + pull_request: + branches: + - master + +jobs: + build-test-lint: + name: Run batch-submitter Test Suite on Node ${{matrix.node}} + runs-on: ubuntu-latest + + strategy: + matrix: + node: [ '10' ] + + steps: + - uses: actions/checkout@v2 + + - name: Setup node ${{ matrix.node }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node }} + + # START DEPENDENCY CACHING + - name: Cache root deps + uses: actions/cache@v1 + id: cache_base + with: + path: node_modules + key: ${{ runner.os }}-${{ matrix.node }}-${{ hashFiles('package.json') }} + + - name: Cache batch-submitter deps + uses: actions/cache@v1 + id: cache_batch-submitter + with: + path: packages/batch-submitter/node_modules + key: ${{ runner.os }}-${{ matrix.node }}-${{ hashFiles('packages/batch-submitter/package.json') }} + + # END DEPENDENCY CACHING + + - name: Install Dependencies + run: yarn install + + - name: Lint + run: yarn lint + env: + PKGS: "batch-submitter" + + - name: Build + run: | + yarn clean + yarn build + env: + PKGS: "batch-submitter" + + - name: Test + run: yarn test + env: + PKGS: "batch-submitter" From 38a2d24d58c857222bd54a767b67d50dc99f8995 Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Sun, 25 Oct 2020 12:45:29 -0400 Subject: [PATCH 4/5] Fix lint errors --- .../batch-submitter/batch-submitter.spec.ts | 28 ++++++++----------- .../batch-submitter/mockchain-provider.ts | 4 ++- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts b/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts index c8042333cd39..7597376a3ade 100644 --- a/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts +++ b/packages/batch-submitter/test/batch-submitter/batch-submitter.spec.ts @@ -191,13 +191,11 @@ describe('BatchSubmitter', () => { 1 ) l2Provider.setNumBlocksToReturn(5) - l2Provider.setL2BlockData( - { - meta: { - queueOrigin: QueueOrigin.L1ToL2, - } - } as any, - ) + l2Provider.setL2BlockData({ + meta: { + queueOrigin: QueueOrigin.L1ToL2, + }, + } as any) let receipt = await batchSubmitter.submitNextBatch() let logData = remove0x(receipt.logs[0].data) expect(parseInt(logData.slice(64 * 0, 64 * 1), 16)).to.equal(0) // _startingQueueIndex @@ -221,13 +219,11 @@ describe('BatchSubmitter', () => { 1 ) l2Provider.setNumBlocksToReturn(10) // For this batch we'll return 10 elements! - l2Provider.setL2BlockData( - { - meta: { - queueOrigin: QueueOrigin.L1ToL2, - } - } as any, - ) + l2Provider.setL2BlockData({ + meta: { + queueOrigin: QueueOrigin.L1ToL2, + }, + } as any) // Turn blocks 3-5 into sequencer txs const nextQueueElement = await getQueueElement( OVM_CanonicalTransactionChain, @@ -251,8 +247,8 @@ describe('BatchSubmitter', () => { 3, 6 ) - let receipt = await batchSubmitter.submitNextBatch() - let logData = remove0x(receipt.logs[0].data) + const receipt = await batchSubmitter.submitNextBatch() + const logData = remove0x(receipt.logs[0].data) expect(parseInt(logData.slice(64 * 0, 64 * 1), 16)).to.equal(0) // _startingQueueIndex expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(7) // _numQueueElements expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(10) // _totalElements diff --git a/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts b/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts index ae33aed07802..2e3029300751 100644 --- a/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts +++ b/packages/batch-submitter/test/batch-submitter/mockchain-provider.ts @@ -64,7 +64,9 @@ export class MockchainProvider extends OptimismProvider { end: number = this.mockBlocks.length ) { for (let i = start; i < end; i++) { - this.mockBlocks[i].timestamp = (timestamp) ? timestamp : this.mockBlocks[i].timestamp + this.mockBlocks[i].timestamp = timestamp + ? timestamp + : this.mockBlocks[i].timestamp this.mockBlocks[i].transactions[0] = { ...this.mockBlocks[i].transactions[0], ...tx, From 23e5c398b73cff5e9891bd535ce8c22a9f1d0e43 Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Sun, 25 Oct 2020 13:07:52 -0400 Subject: [PATCH 5/5] Update eth-optimism/contracts --- packages/batch-submitter/package.json | 2 +- yarn.lock | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/batch-submitter/package.json b/packages/batch-submitter/package.json index 973942a3034f..d2f8d85690c3 100644 --- a/packages/batch-submitter/package.json +++ b/packages/batch-submitter/package.json @@ -31,7 +31,7 @@ "url": "https://github.com/ethereum-optimism/optimism-monorepo.git" }, "dependencies": { - "@eth-optimism/contracts": "^0.0.2-alpha.2", + "@eth-optimism/contracts": "^0.0.2-alpha.3", "@eth-optimism/core-utils": "^0.0.1-alpha.30", "@eth-optimism/provider": "^0.0.1-alpha.6", "@ethersproject/abstract-provider": "^5.0.5", diff --git a/yarn.lock b/yarn.lock index 254d7ecbe66a..7b0f42f4b092 100644 --- a/yarn.lock +++ b/yarn.lock @@ -39,13 +39,20 @@ resolved "https://registry.yarnpkg.com/@ensdomains/resolver/-/resolver-0.2.4.tgz#c10fe28bf5efbf49bff4666d909aed0265efbc89" integrity sha512-bvaTH34PMCbv6anRa9I/0zjLJgY4EuznbEMgbV77JBCQ9KNC46rzi0avuxpOfu+xDjPEtSFGqVEOr5GlUSGudA== -"@eth-optimism/contracts@^0.0.2-alpha.1", "@eth-optimism/contracts@^0.0.2-alpha.2": +"@eth-optimism/contracts@^0.0.2-alpha.1": version "0.0.2-alpha.2" resolved "https://registry.yarnpkg.com/@eth-optimism/contracts/-/contracts-0.0.2-alpha.2.tgz#3b69c4d4055d508f13c7c30c930a514a36215ce4" integrity sha512-5L+owINbxyDIQUCZo+TSFriD4MjXNI6aniUS3TV2PkE4svPesCdKJiz2cV0P28+AIrMxQ+wa+KofTywyo+ZihA== dependencies: ethers "5.0.0" +"@eth-optimism/contracts@^0.0.2-alpha.3": + version "0.0.2-alpha.3" + resolved "https://registry.yarnpkg.com/@eth-optimism/contracts/-/contracts-0.0.2-alpha.3.tgz#01cf0fe95389b84f98a8e57cc0b89097c8823241" + integrity sha512-ek7gOFX43monrv7GV4G61HXJ+GnjAWYdY4nHkUhPVid/2mlAVJ6fDJ4fP7XQdLJ4bqUGIBc34lAX2O5eDLbO+A== + dependencies: + ethers "5.0.0" + "@eth-optimism/ethereumjs-vm@4.2.0-alpha.0": version "4.2.0-alpha.0" resolved "https://registry.yarnpkg.com/@eth-optimism/ethereumjs-vm/-/ethereumjs-vm-4.2.0-alpha.0.tgz#58c8b84732568b73a7ba8105c30dc1513b331324"