op-service: Active L2/Rollup Providers: improve unit testing and logg…

…ing.

op-service/dial/active_l2_provider.go

@@ -77,10 +77,12 @@ func (p *ActiveL2EndpointProvider) EthClient(ctx context.Context) (EthClientInte
 		cctx, cancel := context.WithTimeout(ctx, p.networkTimeout)
 		defer cancel()
 		ep := p.ethUrls[p.currentIndex]
+		log.Info("sequencer changed, dialing new eth client", "new_index", p.currentIndex, "new_url", ep)
 		ethClient, err := p.ethDialer(cctx, p.networkTimeout, p.log, ep)
 		if err != nil {
 			return nil, fmt.Errorf("dialing eth client: %w", err)
 		}
+		p.currentEthClient.Close()
 		p.currentEthClient = ethClient
 	}
 	return p.currentEthClient, nil

op-service/dial/active_l2_provider_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestActiveSequencerFailoverBehavior_RollupProvider tests that the ActiveL2RollupProvider
+// TestActiveSequencerFailoverBehavior_RollupProviders_Inactive tests that the ActiveL2RollupProvider
 // will failover to the next provider if the current one is not active.
 func TestActiveSequencerFailoverBehavior_RollupProviders_Inactive(t *testing.T) {
 	// Create two mock rollup clients, one of which will declare itself inactive after first check.
@@ -51,7 +51,7 @@ func TestActiveSequencerFailoverBehavior_RollupProviders_Inactive(t *testing.T)
 	require.Same(t, secondarySequencer, secondSequencerUsed)
 }
 
-// TestActiveSequencerFailoverBehavior_L2Providers tests that the ActiveL2EndpointProvider
+// TestActiveSequencerFailoverBehavior_L2Providers_Inactive tests that the ActiveL2EndpointProvider
 // will failover to the next provider if the current one is not active.
 func TestActiveSequencerFailoverBehavior_L2Providers_Inactive(t *testing.T) {
 	// as TestActiveSequencerFailoverBehavior_RollupProviders,
@@ -97,14 +97,14 @@ func TestActiveSequencerFailoverBehavior_L2Providers_Inactive(t *testing.T) {
 	require.NoError(t, err)
 	firstClientUsed, err := endpointProvider.EthClient(context.Background())
 	require.NoError(t, err)
-	require.Same(t, primaryEthClient, firstClientUsed) // avoids copying the struct (and its mutex, etc.)
+	require.Same(t, primaryEthClient, firstClientUsed)
 	secondClientUsed, err := endpointProvider.EthClient(context.Background())
 	require.NoError(t, err)
 	require.Same(t, secondaryEthClient, secondClientUsed)
 }
 
-// TestActiveSequencerFailoverBehavior_RollupProvider tests that the ActiveL2RollupProvider
-// will failover to the next provider if the current one is not active.
+// TestActiveSequencerFailoverBehavior_RollupProviders_Errored is as the _Inactive test,
+// but with the first provider returning an error instead of declaring itself inactive.
 func TestActiveSequencerFailoverBehavior_RollupProviders_Errored(t *testing.T) {
 	// Create two mock rollup clients, one of which will error out after first check
 	primarySequencer := new(testutils.MockRollupClient)
@@ -141,3 +141,55 @@ func TestActiveSequencerFailoverBehavior_RollupProviders_Errored(t *testing.T) {
 	require.NoError(t, err)
 	require.Same(t, secondarySequencer, secondSequencerUsed)
 }
+
+// TestActiveSequencerFailoverBehavior_L2Providers_Errored is as the _Inactive test,
+// but with the first provider returning an error instead of declaring itself inactive.
+func TestActiveSequencerFailoverBehavior_L2Providers_Errored(t *testing.T) {
+	// as TestActiveSequencerFailoverBehavior_RollupProviders,
+	// but ensure the added `EthClient()` method also triggers the failover.
+	primarySequencer := new(testutils.MockRollupClient)
+	primarySequencer.ExpectSequencerActive(true, nil)
+	primarySequencer.ExpectSequencerActive(false, fmt.Errorf("a test error"))
+	primarySequencer.ExpectClose()
+	secondarySequencer := new(testutils.MockRollupClient)
+	secondarySequencer.ExpectSequencerActive(true, nil)
+
+	mockRollupDialer := func(ctx context.Context, timeout time.Duration, log log.Logger, url string) (RollupClientInterface, error) {
+		if url == "primaryRollup" {
+			return primarySequencer, nil
+		} else if url == "secondaryRollup" {
+			return secondarySequencer, nil
+		} else {
+			return nil, fmt.Errorf("unknown test url: %s", url)
+		}
+	}
+	primaryEthClient := new(testutils.MockEthClient)
+	primaryEthClient.ExpectClose()
+	secondaryEthClient := new(testutils.MockEthClient)
+	mockEthDialer := func(ctx context.Context, timeout time.Duration, log log.Logger, url string) (EthClientInterface, error) {
+		if url == "primaryEth" {
+			return primaryEthClient, nil
+		} else if url == "secondaryEth" {
+			return secondaryEthClient, nil
+		} else {
+			return nil, fmt.Errorf("unknown test url: %s", url)
+		}
+	}
+	endpointProvider, err := newActiveL2EndpointProvider(
+		context.Background(),
+		[]string{"primaryEth", "secondaryEth"},
+		[]string{"primaryRollup", "secondaryRollup"},
+		1*time.Microsecond,
+		1*time.Minute,
+		testlog.Logger(t, log.LvlDebug),
+		mockEthDialer,
+		mockRollupDialer,
+	)
+	require.NoError(t, err)
+	firstClientUsed, err := endpointProvider.EthClient(context.Background())
+	require.NoError(t, err)
+	require.Same(t, primaryEthClient, firstClientUsed)
+	secondClientUsed, err := endpointProvider.EthClient(context.Background())
+	require.NoError(t, err)
+	require.Same(t, secondaryEthClient, secondClientUsed)
+}

op-service/dial/active_rollup_provider.go

@@ -97,16 +97,17 @@ func (p *ActiveL2RollupProvider) shouldCheck() bool {
 }
 
 func (p *ActiveL2RollupProvider) findActiveEndpoints(ctx context.Context) error {
-	for attempt := range p.rollupUrls {
+	for index := range p.rollupUrls {
 		active, err := p.checkCurrentSequencer(ctx)
+		ep := p.rollupUrls[p.currentIndex]
 		if err != nil {
-			p.log.Warn("Error querying active sequencer, closing connection and trying next.", "err", err, "try", attempt)
+			p.log.Warn("Error querying active sequencer, closing connection and trying next.", "err", err, "index", index, "url", ep)
 			p.currentRollupClient.Close()
 		} else if active {
-			p.log.Debug("Current sequencer active.", "try", attempt)
+			p.log.Debug("Current sequencer active.", "index", index, "url", ep)
 			return nil
 		} else {
-			p.log.Info("Current sequencer inactive, closing connection and trying next.", "try", attempt)
+			p.log.Info("Current sequencer inactive, closing connection and trying next.", "index", index, "url", ep)
 			p.currentRollupClient.Close()
 		}
 		if err := p.dialNextSequencer(ctx); err != nil {
@@ -132,7 +133,7 @@ func (p *ActiveL2RollupProvider) dialNextSequencer(ctx context.Context) error {
 
 	p.currentIndex = (p.currentIndex + 1) % p.numEndpoints()
 	ep := p.rollupUrls[p.currentIndex]
-
+	p.log.Info("Dialing next sequencer.", "index", p.currentIndex, "url", ep)
 	rollupClient, err := p.rollupDialer(cctx, p.networkTimeout, p.log, ep)
 	if err != nil {
 		return fmt.Errorf("dialing rollup client: %w", err)