fix(tx-submitter): finalize lastFinalized+1 instead of skipping via p…

tx-submitter/services/pendingtx.go

@@ -210,7 +210,7 @@ func (pt *PendingTxs) GetPFinalize() uint64 {
 	return atomic.LoadUint64(&pt.pfinalize)
 }
 
-// ExistedIndex checks if a batch index exists
+// ExistedIndex checks if a commit-like tx for the batch index is already pending.
 func (pt *PendingTxs) ExistedIndex(index uint64) bool {
 	txs := pt.GetAll() // snapshot taken under RLock inside GetAll; caller does not hold the mutex
 	abi, err := bindings.RollupMetaData.GetAbi()
@@ -231,6 +231,26 @@ func (pt *PendingTxs) ExistedIndex(index uint64) bool {
 	return false
 }
 
+// ExistedFinalizeIndex reports whether a finalizeBatch tx for index is already pending.
+func (pt *PendingTxs) ExistedFinalizeIndex(index uint64) bool {
+	txs := pt.GetAll()
+	abi, err := bindings.RollupMetaData.GetAbi()
+	if err != nil {
+		log.Error("Failed to get ABI", "err", err)
+		return false
+	}
+
+	for i := len(txs) - 1; i >= 0; i-- {
+		tx := txs[i].Tx
+		if utils.ParseMethod(tx, abi) == constants.MethodFinalizeBatch {
+			if utils.ParseFBatchIndex(tx.Data()) == index {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // Recover recovers transactions from the journal
 func (pt *PendingTxs) Recover(txs []*ethtypes.Transaction, abi *abi.ABI) error {
 	if len(txs) == 0 {

tx-submitter/services/rollup.go

@@ -764,16 +764,29 @@ func (r *Rollup) finalize() error {
 		return fmt.Errorf("get last committed error:%v", err)
 	}
 
-	target := big.NewInt(int64(r.pendingTxs.GetPFinalize() + 1))
-	if target.Cmp(lastFinalized) <= 0 {
-		target = new(big.Int).Add(lastFinalized, big.NewInt(1))
-	}
+	// Contract requires consecutive finalize (lastFinalized+1). Do not advance
+	// from pfinalize: a dropped/failed finalize tx leaves pfinalize ahead and
+	// would skip the next required batch (incorrect previous state root).
+	target := new(big.Int).Add(lastFinalized, big.NewInt(1))
 
 	if target.Cmp(lastCommitted) > 0 {
 		log.Info("no need to finalize", "last_finalized", lastFinalized.Uint64(), "last_committed", lastCommitted.Uint64())
 		return nil
 	}
 
+	if pf := r.pendingTxs.GetPFinalize(); pf > lastFinalized.Uint64() {
+		log.Warn("pfinalize ahead of lastFinalized, targeting lastFinalized+1",
+			"pfinalize", pf,
+			"last_finalized", lastFinalized,
+			"finalize_index", target,
+		)
+	}
+
+	if r.pendingTxs.ExistedFinalizeIndex(target.Uint64()) {
+		log.Info("finalize tx already pending", "batch_index", target)
+		return nil
+	}
+
 	log.Info("finalize info",
 		"last_finalized", lastFinalized,
 		"last_committed", lastCommitted,

tx-submitter/services/rollup_submitter_activity_test.go

@@ -1,6 +1,7 @@
 package services
 
 import (
+	"encoding/binary"
 	"errors"
 	"math/big"
 	"testing"
@@ -10,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"morph-l2/tx-submitter/mock"
+	"morph-l2/tx-submitter/utils"
 )
 
 // A submitter can be removed, slashed, priced out by a raised minimum stake, or
@@ -69,3 +71,37 @@ func TestFinalizeDoesNotRoughEstimatePastRevert(t *testing.T) {
 	require.NoError(t, r.finalize())
 	require.Equal(t, 1, r.pendingTxs.Len())
 }
+
+func headerWithBatchIndex(index uint64) hexutil.Bytes {
+	h := make([]byte, 9)
+	h[0] = 1
+	binary.BigEndian.PutUint64(h[1:9], index)
+	return h
+}
+
+// A dropped finalize tx can leave pfinalize ahead of lastFinalized. The contract
+// still requires lastFinalized+1; skipping that batch reverts with
+// "incorrect previous state root".
+func TestFinalizeDoesNotSkipAheadOfLastFinalized(t *testing.T) {
+	r, l1Mock, _, rollupContract := setupTestRollup(t)
+	r.cfg.RoughEstimateGas = true
+	l1Mock.EstimateGasErr = errors.New("connection refused")
+
+	lastFinalized := uint64(27036)
+	next := lastFinalized + 1
+	rollupContract.SetLastFinalizedBatchIndex(new(big.Int).SetUint64(lastFinalized))
+	rollupContract.SetLastCommittedBatchIndex(big.NewInt(27076))
+	rollupContract.SetBatchExists(true)
+	rollupContract.SetBatchInsideChallengeWindow(false)
+	r.pendingTxs.SetPFinalize(next) // stale: previous finalize(27037) was sent then dropped
+	r.batchCacheLegacy.Set(next+1, &eth.RPCRollupBatch{
+		ParentBatchHeader: headerWithBatchIndex(next),
+	})
+
+	require.NoError(t, r.finalize())
+	require.Equal(t, 1, r.pendingTxs.Len())
+	require.Equal(t, next, utils.ParseFBatchIndex(r.pendingTxs.GetAll()[0].Tx.Data()))
+
+	require.NoError(t, r.finalize())
+	require.Equal(t, 1, r.pendingTxs.Len(), "must not send a second finalize while one is pending")
+}