op-challenger: Stop validating preimage proposals after challenge period ends (#9372)

* op-challenger: Stop validating preimage proposals after challenge period ends

* op-challenger: Cache challenge period.

op-challenger/game/fault/contracts/oracle.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"math"
 	"math/big"
+	"sync/atomic"
 
 	"github.com/ethereum-optimism/optimism/op-bindings/bindings"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
@@ -48,6 +49,10 @@ type PreimageOracleContract struct {
 	addr        common.Address
 	multiCaller *batching.MultiCaller
 	contract    *batching.BoundContract
+
+	// challengePeriod caches the challenge period from the contract once it has been loaded.
+	// 0 indicates the period has not been loaded yet.
+	challengePeriod atomic.Uint64
 }
 
 // toPreimageOracleLeaf converts a Leaf to the contract [bindings.PreimageOracleLeaf] type.
@@ -114,11 +119,16 @@ func (c *PreimageOracleContract) MinLargePreimageSize(ctx context.Context) (uint
 
 // ChallengePeriod returns the challenge period for large preimages.
 func (c *PreimageOracleContract) ChallengePeriod(ctx context.Context) (uint64, error) {
+	if period := c.challengePeriod.Load(); period != 0 {
+		return period, nil
+	}
 	result, err := c.multiCaller.SingleCall(ctx, batching.BlockLatest, c.contract.Call(methodChallengePeriod))
 	if err != nil {
 		return 0, fmt.Errorf("failed to fetch challenge period: %w", err)
 	}
-	return result.GetBigInt(0).Uint64(), nil
+	period := result.GetBigInt(0).Uint64()
+	c.challengePeriod.Store(period)
+	return period, nil
 }
 
 func (c *PreimageOracleContract) CallSqueeze(

op-challenger/game/fault/contracts/oracle_test.go

@@ -62,6 +62,12 @@ func TestPreimageOracleContract_ChallengePeriod(t *testing.T) {
 	challengePeriod, err := oracle.ChallengePeriod(context.Background())
 	require.NoError(t, err)
 	require.Equal(t, uint64(123), challengePeriod)
+
+	// Should cache responses
+	stubRpc.ClearResponses(methodChallengePeriod)
+	challengePeriod, err = oracle.ChallengePeriod(context.Background())
+	require.NoError(t, err)
+	require.Equal(t, uint64(123), challengePeriod)
 }
 
 func TestPreimageOracleContract_MinLargePreimageSize(t *testing.T) {

op-challenger/game/keccak/scheduler.go

@@ -3,8 +3,11 @@ package keccak
 import (
 	"context"
 	"errors"
+	"fmt"
 	"sync"
+	"time"
 
+	faultTypes "github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
 	keccakTypes "github.com/ethereum-optimism/optimism/op-challenger/game/keccak/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
@@ -16,6 +19,7 @@ type Challenger interface {
 
 type LargePreimageScheduler struct {
 	log        log.Logger
+	cl         faultTypes.ClockReader
 	ch         chan common.Hash
 	oracles    []keccakTypes.LargePreimageOracle
 	challenger Challenger
@@ -23,9 +27,14 @@ type LargePreimageScheduler struct {
 	wg         sync.WaitGroup
 }
 
-func NewLargePreimageScheduler(logger log.Logger, oracles []keccakTypes.LargePreimageOracle, challenger Challenger) *LargePreimageScheduler {
+func NewLargePreimageScheduler(
+	logger log.Logger,
+	cl faultTypes.ClockReader,
+	oracles []keccakTypes.LargePreimageOracle,
+	challenger Challenger) *LargePreimageScheduler {
 	return &LargePreimageScheduler{
 		log:        logger,
+		cl:         cl,
 		ch:         make(chan common.Hash, 1),
 		oracles:    oracles,
 		challenger: challenger,
@@ -81,9 +90,13 @@ func (s *LargePreimageScheduler) verifyOraclePreimages(ctx context.Context, orac
 	if err != nil {
 		return err
 	}
+	period, err := oracle.ChallengePeriod(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to load challenge period: %w", err)
+	}
 	toVerify := make([]keccakTypes.LargePreimageMetaData, 0, len(preimages))
 	for _, preimage := range preimages {
-		if preimage.ShouldVerify() {
+		if preimage.ShouldVerify(s.cl.Now(), time.Duration(period)*time.Second) {
 			toVerify = append(toVerify, preimage)
 		}
 	}

op-challenger/game/keccak/scheduler_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	keccakTypes "github.com/ethereum-optimism/optimism/op-challenger/game/keccak/types"
+	"github.com/ethereum-optimism/optimism/op-service/clock"
 	"github.com/ethereum-optimism/optimism/op-service/sources/batching"
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
@@ -17,8 +18,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var stubChallengePeriod = uint64(3600)
+
 func TestScheduleNextCheck(t *testing.T) {
 	ctx := context.Background()
+	currentTimestamp := uint64(1240)
 	logger := testlog.Logger(t, log.LvlInfo)
 	preimage1 := keccakTypes.LargePreimageMetaData{ // Incomplete so won't be verified
 		LargePreimageIdent: keccakTypes.LargePreimageIdent{
@@ -31,7 +35,7 @@ func TestScheduleNextCheck(t *testing.T) {
 			Claimant: common.Address{0xab},
 			UUID:     big.NewInt(222),
 		},
-		Timestamp: 1234,
+		Timestamp: currentTimestamp - 10,
 		Countered: true,
 	}
 	preimage3 := keccakTypes.LargePreimageMetaData{
@@ -39,13 +43,14 @@ func TestScheduleNextCheck(t *testing.T) {
 			Claimant: common.Address{0xdd},
 			UUID:     big.NewInt(333),
 		},
-		Timestamp: 1234,
+		Timestamp: currentTimestamp - 10,
 	}
 	oracle := &stubOracle{
 		images: []keccakTypes.LargePreimageMetaData{preimage1, preimage2, preimage3},
 	}
+	cl := clock.NewDeterministicClock(time.Unix(int64(currentTimestamp), 0))
 	challenger := &stubChallenger{}
-	scheduler := NewLargePreimageScheduler(logger, []keccakTypes.LargePreimageOracle{oracle}, challenger)
+	scheduler := NewLargePreimageScheduler(logger, cl, []keccakTypes.LargePreimageOracle{oracle}, challenger)
 	scheduler.Start(ctx)
 	defer scheduler.Close()
 	err := scheduler.Schedule(common.Hash{0xaa}, 3)
@@ -68,6 +73,10 @@ type stubOracle struct {
 	treeRoots         map[keccakTypes.LargePreimageIdent]common.Hash
 }
 
+func (s *stubOracle) ChallengePeriod(_ context.Context) (uint64, error) {
+	return stubChallengePeriod, nil
+}
+
 func (s *stubOracle) GetInputDataBlocks(_ context.Context, _ batching.Block, _ keccakTypes.LargePreimageIdent) ([]uint64, error) {
 	panic("not supported")
 }

op-challenger/game/keccak/types/types.go

@@ -3,6 +3,7 @@ package types
 import (
 	"context"
 	"math/big"
+	"time"
 
 	"github.com/ethereum-optimism/optimism/op-challenger/game/keccak/merkle"
 	"github.com/ethereum-optimism/optimism/op-service/sources/batching"
@@ -65,10 +66,10 @@ type LargePreimageMetaData struct {
 	Countered       bool
 }
 
-// ShouldVerify returns true if the preimage upload is complete and has not yet been countered.
-// Note that the challenge period for the preimage may have expired but the image not yet been finalized.
-func (m LargePreimageMetaData) ShouldVerify() bool {
-	return m.Timestamp > 0 && !m.Countered
+// ShouldVerify returns true if the preimage upload is complete, has not yet been countered, and the
+// challenge period has not yet elapsed.
+func (m LargePreimageMetaData) ShouldVerify(now time.Time, ignoreAfter time.Duration) bool {
+	return m.Timestamp > 0 && !m.Countered && m.Timestamp+uint64(ignoreAfter.Seconds()) > uint64(now.Unix())
 }
 
 type StateSnapshot [25]uint64
@@ -102,4 +103,5 @@ type LargePreimageOracle interface {
 	GetProposalTreeRoot(ctx context.Context, block batching.Block, ident LargePreimageIdent) (common.Hash, error)
 	DecodeInputData(data []byte) (*big.Int, InputData, error)
 	ChallengeTx(ident LargePreimageIdent, challenge Challenge) (txmgr.TxCandidate, error)
+	ChallengePeriod(ctx context.Context) (uint64, error)
 }

op-challenger/game/keccak/types/types_test.go

+package types
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestShouldVerify(t *testing.T) {
+	tests := []struct {
+		name      string
+		timestamp uint64
+		countered bool
+		now       int64
+		expected  bool
+	}{
+		{
+			name:      "IgnoreNotFinalizedAndNotCountered",
+			timestamp: 0,
+			countered: false,
+			now:       100,
+			expected:  false,
+		},
+		{
+			name:      "VerifyFinalizedAndNotCountered",
+			timestamp: 50,
+			countered: false,
+			now:       100,
+			expected:  true,
+		},
+		{
+			name:      "IgnoreFinalizedAndCountered",
+			timestamp: 50,
+			countered: true,
+			now:       100,
+			expected:  false,
+		},
+		{
+			name:      "IgnoreNotFinalizedAndCountered",
+			timestamp: 0,
+			countered: true,
+			now:       100,
+			expected:  false,
+		},
+		{
+			name:      "IgnoreFinalizedBeforeTimeWindowAndNotCountered",
+			timestamp: 50,
+			countered: false,
+			now:       50 + int64((2 * time.Hour).Seconds()),
+			expected:  false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			metadata := LargePreimageMetaData{
+				Timestamp: test.timestamp,
+				Countered: test.countered,
+			}
+			require.Equal(t, test.expected, metadata.ShouldVerify(time.Unix(test.now, 0), 1*time.Hour))
+		})
+	}
+}

op-challenger/game/registry/registry_test.go

@@ -93,6 +93,10 @@ func TestBondContracts(t *testing.T) {
 
 type stubPreimageOracle common.Address
 
+func (s stubPreimageOracle) ChallengePeriod(_ context.Context) (uint64, error) {
+	panic("not supported")
+}
+
 func (s stubPreimageOracle) GetProposalTreeRoot(_ context.Context, _ batching.Block, _ keccakTypes.LargePreimageIdent) (common.Hash, error) {
 	panic("not supported")
 }

op-challenger/game/service.go

@@ -247,7 +247,7 @@ func (s *Service) initLargePreimages() error {
 	fetcher := fetcher.NewPreimageFetcher(s.logger, s.l1Client)
 	verifier := keccak.NewPreimageVerifier(s.logger, fetcher)
 	challenger := keccak.NewPreimageChallenger(s.logger, s.metrics, verifier, s.txSender)
-	s.preimages = keccak.NewLargePreimageScheduler(s.logger, s.registry.Oracles(), challenger)
+	s.preimages = keccak.NewLargePreimageScheduler(s.logger, s.cl, s.registry.Oracles(), challenger)
 	return nil
 }
 

op-service/sources/batching/test/stubs.go

@@ -77,6 +77,10 @@ func (l *AbiBasedRpc) SetResponse(to common.Address, method string, block batchi
 	})
 }
 
+func (l *AbiBasedRpc) ClearResponses(method string) {
+	delete(l.expectedCalls, method)
+}
+
 func (l *AbiBasedRpc) BatchCallContext(ctx context.Context, b []rpc.BatchElem) error {
 	var errs []error
 	for _, elem := range b {