From 69dd39afbaa612943c701e2f809581df80312d58 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= <peter@scroll.io>
Date: Sun, 6 Aug 2023 14:22:02 +0200
Subject: [PATCH] fix: do not overwrite correct L1 index on signer startup
 (#443)

* fix signer worker

* add logs

* bump version

* Fix
---
 core/blockchain.go | 42 ++++++++++++++++++++++++++++++++++++++++--
 miner/worker.go    | 28 +++++++++++++++++++++++-----
 params/version.go  |  2 +-
 3 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/core/blockchain.go b/core/blockchain.go
index f6e1070788..68ef13c777 100644
--- a/core/blockchain.go
+++ b/core/blockchain.go
@@ -1186,9 +1186,28 @@ func (bc *BlockChain) writeBlockWithoutState(block *types.Block, td *big.Int) (e
 	// note: we can insert blocks with header-only ancestors here,
 	// so queueIndex might not yet be available in DB.
 	if queueIndex != nil {
+		numProcessed := uint64(block.NumL1MessagesProcessed(*queueIndex))
 		// do not overwrite the index written by the miner worker
 		if index := rawdb.ReadFirstQueueIndexNotInL2Block(bc.db, block.Hash()); index == nil {
-			rawdb.WriteFirstQueueIndexNotInL2Block(batch, block.Hash(), *queueIndex+uint64(block.NumL1MessagesProcessed(*queueIndex)))
+			newIndex := *queueIndex + numProcessed
+			log.Trace(
+				"Blockchain.writeBlockWithoutState WriteFirstQueueIndexNotInL2Block",
+				"number", block.Number(),
+				"hash", block.Hash().String(),
+				"queueIndex", *queueIndex,
+				"numProcessed", numProcessed,
+				"newIndex", newIndex,
+			)
+			rawdb.WriteFirstQueueIndexNotInL2Block(batch, block.Hash(), newIndex)
+		} else {
+			log.Trace(
+				"Blockchain.writeBlockWithoutState WriteFirstQueueIndexNotInL2Block: not overwriting existing index",
+				"number", block.Number(),
+				"hash", block.Hash().String(),
+				"queueIndex", *queueIndex,
+				"numProcessed", numProcessed,
+				"index", *index,
+			)
 		}
 	}
 
@@ -1253,9 +1272,28 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.
 		// so the parent will always be inserted first.
 		log.Crit("Queue index in DB is nil", "parent", block.ParentHash(), "hash", block.Hash())
 	}
+	numProcessed := uint64(block.NumL1MessagesProcessed(*queueIndex))
 	// do not overwrite the index written by the miner worker
 	if index := rawdb.ReadFirstQueueIndexNotInL2Block(bc.db, block.Hash()); index == nil {
-		rawdb.WriteFirstQueueIndexNotInL2Block(blockBatch, block.Hash(), *queueIndex+uint64(block.NumL1MessagesProcessed(*queueIndex)))
+		newIndex := *queueIndex + numProcessed
+		log.Trace(
+			"Blockchain.writeBlockWithState WriteFirstQueueIndexNotInL2Block",
+			"number", block.Number(),
+			"hash", block.Hash().String(),
+			"queueIndex", *queueIndex,
+			"numProcessed", numProcessed,
+			"newIndex", newIndex,
+		)
+		rawdb.WriteFirstQueueIndexNotInL2Block(blockBatch, block.Hash(), newIndex)
+	} else {
+		log.Trace(
+			"Blockchain.writeBlockWithState WriteFirstQueueIndexNotInL2Block: not overwriting existing index",
+			"number", block.Number(),
+			"hash", block.Hash().String(),
+			"queueIndex", *queueIndex,
+			"numProcessed", numProcessed,
+			"index", *index,
+		)
 	}
 
 	if err := blockBatch.Write(); err != nil {
diff --git a/miner/worker.go b/miner/worker.go
index f1161a59e6..e9cb1e9f98 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -698,11 +698,29 @@ func (w *worker) resultLoop() {
 				}
 				logs = append(logs, receipt.Logs...)
 			}
-			// Store first L1 queue index not processed by this block.
-			// Note: This accounts for both included and skipped messages. This
-			// way, if a block only skips messages, we won't reprocess the same
-			// messages from the next block.
-			rawdb.WriteFirstQueueIndexNotInL2Block(w.eth.ChainDb(), hash, task.nextL1MsgIndex)
+			// It's possible that we've stored L1 queue index for this block previously,
+			// in this case do not overwrite it.
+			if index := rawdb.ReadFirstQueueIndexNotInL2Block(w.eth.ChainDb(), hash); index == nil {
+				// Store first L1 queue index not processed by this block.
+				// Note: This accounts for both included and skipped messages. This
+				// way, if a block only skips messages, we won't reprocess the same
+				// messages from the next block.
+				log.Trace(
+					"Worker WriteFirstQueueIndexNotInL2Block",
+					"number", block.Number(),
+					"hash", hash.String(),
+					"task.nextL1MsgIndex", task.nextL1MsgIndex,
+				)
+				rawdb.WriteFirstQueueIndexNotInL2Block(w.eth.ChainDb(), hash, task.nextL1MsgIndex)
+			} else {
+				log.Trace(
+					"Worker WriteFirstQueueIndexNotInL2Block: not overwriting existing index",
+					"number", block.Number(),
+					"hash", hash.String(),
+					"index", *index,
+					"task.nextL1MsgIndex", task.nextL1MsgIndex,
+				)
+			}
 			// Store circuit row consumption.
 			log.Trace(
 				"Worker write block row consumption",
diff --git a/params/version.go b/params/version.go
index f54db44d96..6d75a0d05a 100644
--- a/params/version.go
+++ b/params/version.go
@@ -24,7 +24,7 @@ import (
 const (
 	VersionMajor = 4         // Major version component of the current release
 	VersionMinor = 3         // Minor version component of the current release
-	VersionPatch = 23        // Patch version component of the current release
+	VersionPatch = 24        // Patch version component of the current release
 	VersionMeta  = "sepolia" // Version metadata to append to the version string
 )