From 4c656b3a1638076aeaf3ee999f6e03dd796ed385 Mon Sep 17 00:00:00 2001
From: Matthew Slipper <me@matthewslipper.com>
Date: Fri, 8 Nov 2024 05:19:35 -0700
Subject: [PATCH] op-e2e: Minor test updates (#12877)

Attempt at fixing the ongoing test flakiness with the batcher tests. Makes two changes:

- Waits for the L1 to be up for all end-to-end tests to mitigate the I/O and context timeouts we've been seeing.
- Update the multi batcher test to use an algorithm that's more tolerant of when the L1 doesn't immediately include the transaction.
---
 op-e2e/e2eutils/wait/waits.go  | 26 +++++++++++++++++++++
 op-e2e/system/da/multi_test.go | 42 +++++++++++++++++++++-------------
 op-e2e/system/e2esys/setup.go  | 10 ++++++++
 3 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/op-e2e/e2eutils/wait/waits.go b/op-e2e/e2eutils/wait/waits.go
index 3c0e29466cea..9b03e0bba7f9 100644
--- a/op-e2e/e2eutils/wait/waits.go
+++ b/op-e2e/e2eutils/wait/waits.go
@@ -9,6 +9,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/ethereum/go-ethereum/log"
+
 	"github.com/ethereum/go-ethereum"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -164,3 +166,27 @@ func AndGet[T interface{}](ctx context.Context, pollRate time.Duration, get func
 		}
 	}
 }
+
+func ForNodeUp(ctx context.Context, client *ethclient.Client, lgr log.Logger) error {
+	for {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		default:
+			// Create a new context deliberately. The shorter timeout is used to detect
+			// potential I/O timeouts on the RPC so we can retry.
+			callCtx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
+			_, err := client.BlockNumber(callCtx)
+			cancel()
+			if err == nil {
+				lgr.Info("node is up")
+				return nil
+			}
+			if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, os.ErrDeadlineExceeded) {
+				lgr.Info("timeout waiting for node come up, trying again")
+				continue
+			}
+			return err
+		}
+	}
+}
diff --git a/op-e2e/system/da/multi_test.go b/op-e2e/system/da/multi_test.go
index 3d150010a0f3..461270282008 100644
--- a/op-e2e/system/da/multi_test.go
+++ b/op-e2e/system/da/multi_test.go
@@ -32,10 +32,8 @@ func TestBatcherMultiTx(t *testing.T) {
 	_, err = geth.WaitForBlock(big.NewInt(10), l2Seq)
 	require.NoError(t, err, "Waiting for L2 blocks")
 
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
-	l1Number, err := l1Client.BlockNumber(ctx)
-	require.NoError(t, err)
 
 	// start batch submission
 	driver := sys.BatchSubmitter.TestDriver()
@@ -43,21 +41,33 @@ func TestBatcherMultiTx(t *testing.T) {
 	require.NoError(t, err)
 
 	totalBatcherTxsCount := int64(0)
-	// wait for up to 5 L1 blocks, usually only 3 is required, but it's
-	// possible additional L1 blocks will be created before the batcher starts,
-	// so we wait additional blocks.
-	for i := int64(0); i < 5; i++ {
-		block, err := geth.WaitForBlock(big.NewInt(int64(l1Number)+i), l1Client)
-		require.NoError(t, err, "Waiting for l1 blocks")
-		// there are possibly other services (proposer/challenger) in the background sending txs
-		// so we only count the batcher txs
-		batcherTxCount, err := transactions.TransactionsBySender(block, cfg.DeployConfig.BatchSenderAddress)
-		require.NoError(t, err)
-		totalBatcherTxsCount += int64(batcherTxCount)
 
-		if totalBatcherTxsCount >= 10 {
-			return
+	headNum, err := l1Client.BlockNumber(ctx)
+	require.NoError(t, err)
+	stopNum := headNum + 10
+	startBlock := uint64(1)
+
+	for {
+		for i := startBlock; i <= headNum; i++ {
+			block, err := l1Client.BlockByNumber(ctx, big.NewInt(int64(i)))
+			require.NoError(t, err)
+
+			batcherTxCount, err := transactions.TransactionsBySender(block, cfg.DeployConfig.BatchSenderAddress)
+			require.NoError(t, err)
+			totalBatcherTxsCount += batcherTxCount
+
+			if totalBatcherTxsCount >= 10 {
+				return
+			}
+		}
+
+		headNum++
+		if headNum > stopNum {
+			break
 		}
+		startBlock = headNum
+		_, err = geth.WaitForBlock(big.NewInt(int64(headNum)), l1Client)
+		require.NoError(t, err)
 	}
 
 	t.Fatal("Expected at least 10 transactions from the batcher")
diff --git a/op-e2e/system/e2esys/setup.go b/op-e2e/system/e2esys/setup.go
index 9ab6d2de14f6..a54a46d1e5db 100644
--- a/op-e2e/system/e2esys/setup.go
+++ b/op-e2e/system/e2esys/setup.go
@@ -16,6 +16,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/wait"
+
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/maps"
 
@@ -671,6 +673,14 @@ func (cfg SystemConfig) Start(t *testing.T, startOpts ...StartOption) (*System,
 		return nil, err
 	}
 
+	sysLogger := testlog.Logger(t, log.LevelInfo).New("role", "system")
+
+	l1UpCtx, l1UpCancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer l1UpCancel()
+	if err := wait.ForNodeUp(l1UpCtx, sys.NodeClient(RoleL1), sysLogger); err != nil {
+		return nil, fmt.Errorf("l1 never came up: %w", err)
+	}
+
 	// Ordered such that the Sequencer is initialized first. Setup this way so that
 	// the `RollupSequencerHTTP` GethOption can be supplied to any sentry nodes.
 	l2Nodes := []string{RoleSeq}