From b59c747f3b00f1f984c21be4f45b7d98e8bf95fc Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Thu, 5 Nov 2020 14:12:03 -0500 Subject: [PATCH 1/3] Fix bug where we dont submit the last block --- .../batch-submitter/src/batch-submitter/tx-batch-submitter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts index e8530b8de0b..47b8f694825 100644 --- a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts +++ b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts @@ -97,7 +97,7 @@ export class TransactionBatchSubmitter extends BatchSubmitter { const endBlock = Math.min( startBlock + this.maxBatchSize, await this.l2Provider.getBlockNumber() - ) + ) + 1 // +1 because the `endBlock` is *exclusive* if (startBlock >= endBlock) { if (startBlock > endBlock) { this.log From b32c90192938dfdf9ca8468d34562a570d3c79fe Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Thu, 5 Nov 2020 14:34:30 -0500 Subject: [PATCH 2/3] Temp fix lastL1BlockNum & small off by 1 The off by 1 error was simply that we weren't pulling the *last* block because I forgot to add +1 to the `end` value because it's exclusive. I was treating it as inclusive. --- .../src/batch-submitter/tx-batch-submitter.ts | 39 ++++++++++++++----- .../transaction-batch-submitter.spec.ts | 16 ++++---- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts index 47b8f694825..75bccdc72b1 100644 --- a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts +++ b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts @@ -37,6 +37,7 @@ export class TransactionBatchSubmitter extends BatchSubmitter { protected chainContract: CanonicalTransactionChainContract protected l2ChainId: number protected syncing: boolean + protected lastL1BlockNumber: number /***************************** * Batch Submitter Overrides * @@ -77,6 +78,8 @@ export class TransactionBatchSubmitter extends BatchSubmitter { public async _onSync(): Promise { const pendingQueueElements = await this.chainContract.getNumPendingQueueElements() + this._updateLastL1BlockNumber(pendingQueueElements) + if (pendingQueueElements !== 0) { this.log.info( `Syncing mode enabled! Skipping batch submission and clearing ${pendingQueueElements} queue elements` @@ -91,13 +94,35 @@ export class TransactionBatchSubmitter extends BatchSubmitter { return } + // TODO: Remove this function and use geth for lastL1BlockNumber! + private async _updateLastL1BlockNumber(pendingQueueElements: number) { + if (pendingQueueElements !== 0) { + const nextQueueIndex = await this.chainContract.getNextQueueIndex() + this.lastL1BlockNumber = await this.chainContract.getQueueElement( + nextQueueIndex + ).blockNumber + } else { + const curBlockNum = await this.chainContract.provider.getBlockNumber() + if (!this.lastL1BlockNumber) { + // Set the block number to the current l1BlockNumber + this.lastL1BlockNumber = curBlockNum + } else { + this.lastL1BlockNumber = + curBlockNum - this.lastL1BlockNumber > 30 + ? curBlockNum - 10 + : this.lastL1BlockNumber + } + } + } + public async _getBatchStartAndEnd(): Promise { const startBlock = parseInt(await this.chainContract.getTotalElements(), 16) + 1 // +1 to skip L2 genesis block - const endBlock = Math.min( - startBlock + this.maxBatchSize, - await this.l2Provider.getBlockNumber() - ) + 1 // +1 because the `endBlock` is *exclusive* + const endBlock = + Math.min( + startBlock + this.maxBatchSize, + await this.l2Provider.getBlockNumber() + ) + 1 // +1 because the `endBlock` is *exclusive* if (startBlock >= endBlock) { if (startBlock > endBlock) { this.log @@ -261,12 +286,8 @@ export class TransactionBatchSubmitter extends BatchSubmitter { block.transactions[0].queueOrigin = queueOriginPlainText[block.transactions[0].queueOrigin] // For now just set the l1BlockNumber based on the current l1 block number - const _getMockedL1BlockNumber = async (): Promise => { - const curBlockNum = await this.chainContract.signer.provider.getBlockNumber() - return curBlockNum - 1 - } if (!block.transactions[0].l1BlockNumber) { - block.transactions[0].l1BlockNumber = await _getMockedL1BlockNumber() + block.transactions[0].l1BlockNumber = this.lastL1BlockNumber } return block } diff --git a/packages/batch-submitter/test/batch-submitter/transaction-batch-submitter.spec.ts b/packages/batch-submitter/test/batch-submitter/transaction-batch-submitter.spec.ts index ab1badff24e..f6d5383aaaf 100644 --- a/packages/batch-submitter/test/batch-submitter/transaction-batch-submitter.spec.ts +++ b/packages/batch-submitter/test/batch-submitter/transaction-batch-submitter.spec.ts @@ -177,12 +177,12 @@ describe('TransactionBatchSubmitter', () => { 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(0) // _numQueueElements - expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(5) // _totalElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(6) // _totalElements receipt = await batchSubmitter.submitNextBatch() 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(0) // _numQueueElements - expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(10) // _totalElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(11) // _totalElements }) it('should submit a queue batch correctly', async () => { @@ -193,13 +193,13 @@ describe('TransactionBatchSubmitter', () => { 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 + expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(6) // _numQueueElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(6) // _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 * 0, 64 * 1), 16)).to.equal(6) // _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 + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(11) // _totalElements }) it('should submit a batch with both queue and sequencer chain elements', async () => { @@ -231,8 +231,8 @@ describe('TransactionBatchSubmitter', () => { 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 + expect(parseInt(logData.slice(64 * 1, 64 * 2), 16)).to.equal(8) // _numQueueElements + expect(parseInt(logData.slice(64 * 2, 64 * 3), 16)).to.equal(11) // _totalElements }) }) }) From cf9542c3d553b4e99618361d8014e5e960b2d68d Mon Sep 17 00:00:00 2001 From: Karl Floersch Date: Thu, 5 Nov 2020 15:07:39 -0500 Subject: [PATCH 3/3] Improve readability --- .../src/batch-submitter/tx-batch-submitter.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts index 75bccdc72b1..a5f6d0f59fc 100644 --- a/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts +++ b/packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts @@ -107,10 +107,11 @@ export class TransactionBatchSubmitter extends BatchSubmitter { // Set the block number to the current l1BlockNumber this.lastL1BlockNumber = curBlockNum } else { - this.lastL1BlockNumber = - curBlockNum - this.lastL1BlockNumber > 30 - ? curBlockNum - 10 - : this.lastL1BlockNumber + if (curBlockNum - this.lastL1BlockNumber > 30) { + // If the lastL1BlockNumber is too old, then set it to a recent + // block number. (10 blocks ago to prevent reorgs) + this.lastL1BlockNumber = curBlockNum - 10 + } } } }