op-challenger: Cache created trace providers

op-challenger/game/fault/register.go

@@ -65,7 +65,7 @@ func registerOutputCannon(
 		if err != nil {
 			return nil, nil, err
 		}
-		accessor, err := outputs.NewOutputCannonTraceAccessor(ctx, logger, cfg.RollupRpc, gameDepth, agreed.L2BlockNumber.Uint64(), disputed.L2BlockNumber.Uint64())
+		accessor, err := outputs.NewOutputCannonTraceAccessor(ctx, logger, m, cfg.RollupRpc, gameDepth, agreed.L2BlockNumber.Uint64(), disputed.L2BlockNumber.Uint64())
 		if err != nil {
 			return nil, nil, err
 		}

op-challenger/game/fault/trace/outputs/output_cannon.go

@@ -8,11 +8,20 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/split"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum-optimism/optimism/op-challenger/metrics"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
 )
 
-func NewOutputCannonTraceAccessor(ctx context.Context, logger log.Logger, rollupRpc string, gameDepth uint64, prestateBlock uint64, poststateBlock uint64) (*trace.Accessor, error) {
+func NewOutputCannonTraceAccessor(
+	ctx context.Context,
+	logger log.Logger,
+	m metrics.Metricer,
+	rollupRpc string,
+	gameDepth uint64,
+	prestateBlock uint64,
+	poststateBlock uint64,
+) (*trace.Accessor, error) {
 	topDepth := gameDepth / 2 // TODO(client-pod#43): Load this from the contract
 	outputProvider, err := NewTraceProvider(ctx, logger, rollupRpc, topDepth, prestateBlock, poststateBlock)
 	if err != nil {
@@ -24,6 +33,7 @@ func NewOutputCannonTraceAccessor(ctx context.Context, logger log.Logger, rollup
 		return nil, errors.New("not implemented")
 	}
 
-	selector := split.NewSplitProviderSelector(outputProvider, int(topDepth), OutputRootSplitAdapter(outputProvider, cannonCreator))
+	cache := NewProviderCache(m, "output_cannon_provider", cannonCreator)
+	selector := split.NewSplitProviderSelector(outputProvider, int(topDepth), OutputRootSplitAdapter(outputProvider, cache.GetOrCreate))
 	return trace.NewAccessor(selector), nil
 }

op-challenger/game/fault/trace/outputs/provider_cache.go

+package outputs
+
+import (
+	"context"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum-optimism/optimism/op-service/sources/caching"
+	"github.com/ethereum/go-ethereum/common"
+)
+
+type ProviderCache struct {
+	cache   *caching.LRUCache[common.Hash, types.TraceProvider]
+	creator ProposalTraceProviderCreator
+}
+
+func (c *ProviderCache) GetOrCreate(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
+	provider, ok := c.cache.Get(localContext)
+	if ok {
+		return provider, nil
+	}
+	provider, err := c.creator(ctx, localContext, agreed, claimed)
+	if err != nil {
+		return nil, err
+	}
+	c.cache.Add(localContext, provider)
+	return provider, nil
+}
+
+func NewProviderCache(m caching.Metrics, metricsLabel string, creator ProposalTraceProviderCreator) *ProviderCache {
+	cache := caching.NewLRUCache[common.Hash, types.TraceProvider](m, metricsLabel, 100)
+	return &ProviderCache{
+		cache:   cache,
+		creator: creator,
+	}
+}

op-challenger/game/fault/trace/outputs/provider_cache_test.go

+package outputs
+
+import (
+	"context"
+	"errors"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/alphabet"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum-optimism/optimism/op-challenger/metrics"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+)
+
+func TestProviderCache(t *testing.T) {
+	agreed := contracts.Proposal{
+		L2BlockNumber: big.NewInt(34),
+		OutputRoot:    common.Hash{0xaa},
+	}
+	claimed := contracts.Proposal{
+		L2BlockNumber: big.NewInt(35),
+		OutputRoot:    common.Hash{0xcc},
+	}
+	var createdProvider types.TraceProvider
+	creator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
+		createdProvider = alphabet.NewTraceProvider("abcdef", 6)
+		return createdProvider, nil
+	}
+	localContext1 := common.Hash{0xdd}
+	localContext2 := common.Hash{0xee}
+
+	cache := NewProviderCache(metrics.NoopMetrics, "test", creator)
+
+	// Create on first call
+	provider1, err := cache.GetOrCreate(context.Background(), localContext1, agreed, claimed)
+	require.NoError(t, err)
+	require.Same(t, createdProvider, provider1, "should return created trace provider")
+
+	// Return the cached provider on subsequent calls.
+	createdProvider = nil
+	cached, err := cache.GetOrCreate(context.Background(), localContext1, agreed, claimed)
+	require.NoError(t, err)
+	require.Same(t, provider1, cached, "should return exactly the same instance from cache")
+	require.Nil(t, createdProvider)
+
+	// Create a new provider when the local context is different
+	createdProvider = nil
+	otherProvider, err := cache.GetOrCreate(context.Background(), localContext2, agreed, claimed)
+	require.NoError(t, err)
+	require.Same(t, otherProvider, createdProvider, "should return newly created trace provider")
+	require.NotSame(t, otherProvider, provider1, "should not use cached provider for different local context")
+}
+
+func TestProviderCache_DoNotCacheErrors(t *testing.T) {
+	callCount := 0
+	providerErr := errors.New("boom")
+	creator := func(ctx context.Context, localContext common.Hash, agreed contracts.Proposal, claimed contracts.Proposal) (types.TraceProvider, error) {
+		callCount++
+		return nil, providerErr
+	}
+	localContext1 := common.Hash{0xdd}
+
+	cache := NewProviderCache(metrics.NoopMetrics, "test", creator)
+	provider, err := cache.GetOrCreate(context.Background(), localContext1, contracts.Proposal{}, contracts.Proposal{})
+	require.Nil(t, provider)
+	require.ErrorIs(t, err, providerErr)
+	require.Equal(t, 1, callCount)
+
+	// Should call the creator again on the second attempt
+	provider, err = cache.GetOrCreate(context.Background(), localContext1, contracts.Proposal{}, contracts.Proposal{})
+	require.Nil(t, provider)
+	require.ErrorIs(t, err, providerErr)
+	require.Equal(t, 2, callCount)
+}

op-challenger/metrics/metrics.go

@@ -3,6 +3,7 @@ package metrics
 import (
 	"context"
 
+	"github.com/ethereum-optimism/optimism/op-service/sources/caching"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/log"
@@ -22,6 +23,9 @@ type Metricer interface {
 	// Record Tx metrics
 	txmetrics.TxMetricer
 
+	// Record cache metrics
+	caching.Metrics
+
 	RecordGameStep()
 	RecordGameMove()
 	RecordCannonExecutionTime(t float64)
@@ -44,6 +48,8 @@ type Metrics struct {
 
 	txmetrics.TxMetrics
 
+	*opmetrics.CacheMetrics
+
 	info prometheus.GaugeVec
 	up   prometheus.Gauge
 
@@ -71,6 +77,8 @@ func NewMetrics() *Metrics {
 
 		TxMetrics: txmetrics.MakeTxMetrics(Namespace, factory),
 
+		CacheMetrics: opmetrics.NewCacheMetrics(factory, Namespace, "provider_cache", "Provider cache"),
+
 		info: *factory.NewGaugeVec(prometheus.GaugeOpts{
 			Namespace: Namespace,
 			Name:      "info",

op-challenger/metrics/noop.go

@@ -27,3 +27,6 @@ func (*NoopMetricsImpl) IncActiveExecutors() {}
 func (*NoopMetricsImpl) DecActiveExecutors() {}
 func (*NoopMetricsImpl) IncIdleExecutors()   {}
 func (*NoopMetricsImpl) DecIdleExecutors()   {}
+
+func (*NoopMetricsImpl) CacheAdd(_ string, _ int, _ bool) {}
+func (*NoopMetricsImpl) CacheGet(_ string, _ bool)        {}