op-challenger: Don't progress completed games

op-challenger/fault/caller.go

@@ -8,7 +8,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/fault/types"
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/ethclient"
 )
 
 type FaultDisputeGameCaller interface {
@@ -26,7 +25,7 @@ func NewFaultCaller(caller FaultDisputeGameCaller) *FaultCaller {
 	}
 }
 
-func NewFaultCallerFromBindings(fdgAddr common.Address, client *ethclient.Client) (*FaultCaller, error) {
+func NewFaultCallerFromBindings(fdgAddr common.Address, client bind.ContractCaller) (*FaultCaller, error) {
 	caller, err := bindings.NewFaultDisputeGameCaller(fdgAddr, client)
 	if err != nil {
 		return nil, err

op-challenger/fault/player.go

@@ -10,8 +10,8 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/fault/cannon"
 	"github.com/ethereum-optimism/optimism/op-challenger/fault/types"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
+	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -38,7 +38,7 @@ func NewGamePlayer(
 	cfg *config.Config,
 	addr common.Address,
 	txMgr txmgr.TxManager,
-	client *ethclient.Client,
+	client bind.ContractCaller,
 ) (*GamePlayer, error) {
 	logger = logger.New("game", addr)
 	contract, err := bindings.NewFaultDisputeGameCaller(addr, client)
@@ -100,6 +100,15 @@ func NewGamePlayer(
 }
 
 func (g *GamePlayer) ProgressGame(ctx context.Context) bool {
+	status, err := g.caller.GetGameStatus(ctx)
+	if err != nil {
+		g.logger.Warn("Unable to retrieve game status", "err", err)
+		return false
+	}
+	if status != types.GameStatusInProgress {
+		// Game is already complete so don't try to perform further actions.
+		return true
+	}
 	g.logger.Trace("Checking if actions are required")
 	if err := g.agent.Act(ctx); err != nil {
 		g.logger.Error("Error when acting on game", "err", err)

op-challenger/fault/player_test.go

@@ -12,14 +12,14 @@ import (
 )
 
 func TestProgressGame_LogErrorFromAct(t *testing.T) {
-	handler, game, actor, _ := setupProgressGameTest(t, true)
-	actor.err = errors.New("boom")
+	handler, game, actor := setupProgressGameTest(t, true)
+	actor.actErr = errors.New("boom")
 	done := game.ProgressGame(context.Background())
 	require.False(t, done, "should not be done")
 	require.Equal(t, 1, actor.callCount, "should perform next actions")
 	errLog := handler.FindLog(log.LvlError, "Error when acting on game")
 	require.NotNil(t, errLog, "should log error")
-	require.Equal(t, actor.err, errLog.GetContextValue("err"))
+	require.Equal(t, actor.actErr, errLog.GetContextValue("err"))
 
 	// Should still log game status
 	msg := handler.FindLog(log.LvlInfo, "Game info")
@@ -92,11 +92,12 @@ func TestProgressGame_LogGameStatus(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			handler, game, actor, gameInfo := setupProgressGameTest(t, test.agreeWithOutput)
-			gameInfo.status = test.status
+			handler, game, gameState := setupProgressGameTest(t, test.agreeWithOutput)
+			gameState.preStatus = types.GameStatusInProgress
+			gameState.postStatus = test.status
 
 			done := game.ProgressGame(context.Background())
-			require.Equal(t, 1, actor.callCount, "should perform next actions")
+			require.Equal(t, 1, gameState.callCount, "should perform next actions")
 			require.Equal(t, test.status != types.GameStatusInProgress, done, "should be done when not in progress")
 			errLog := handler.FindLog(test.logLevel, test.logMsg)
 			require.NotNil(t, errLog, "should log game result")
@@ -105,43 +106,57 @@ func TestProgressGame_LogGameStatus(t *testing.T) {
 	}
 }
 
-func setupProgressGameTest(t *testing.T, agreeWithProposedRoot bool) (*testlog.CapturingHandler, *GamePlayer, *stubActor, *stubGameInfo) {
+func TestDoNotActOnCompleteGame(t *testing.T) {
+	for _, status := range []types.GameStatus{types.GameStatusChallengerWon, types.GameStatusDefenderWon} {
+		t.Run(status.String(), func(t *testing.T) {
+			handler, game, gameState := setupProgressGameTest(t, true)
+			gameState.preStatus = status
+
+			done := game.ProgressGame(context.Background())
+			require.Equal(t, 0, gameState.callCount, "should not perform actions")
+			require.True(t, done, "should be done")
+			require.Empty(t, handler.Logs, "should not log game status")
+		})
+	}
+}
+
+func setupProgressGameTest(t *testing.T, agreeWithProposedRoot bool) (*testlog.CapturingHandler, *GamePlayer, *stubGameState) {
 	logger := testlog.Logger(t, log.LvlDebug)
 	handler := &testlog.CapturingHandler{
 		Delegate: logger.GetHandler(),
 	}
 	logger.SetHandler(handler)
-	actor := &stubActor{}
-	gameInfo := &stubGameInfo{claimCount: 1}
+	gameState := &stubGameState{claimCount: 1}
 	game := &GamePlayer{
-		agent:                   actor,
+		agent:                   gameState,
 		agreeWithProposedOutput: agreeWithProposedRoot,
-		caller:                  gameInfo,
+		caller:                  gameState,
 		logger:                  logger,
 	}
-	return handler, game, actor, gameInfo
+	return handler, game, gameState
 }
 
-type stubActor struct {
-	callCount int
-	err       error
+type stubGameState struct {
+	preStatus  types.GameStatus
+	postStatus types.GameStatus
+	claimCount uint64
+	callCount  int
+	actErr     error
+	Err        error
 }
 
-func (a *stubActor) Act(ctx context.Context) error {
-	a.callCount++
-	return a.err
+func (s *stubGameState) Act(ctx context.Context) error {
+	s.callCount++
+	return s.actErr
 }
 
-type stubGameInfo struct {
-	status     types.GameStatus
-	claimCount uint64
-	err        error
-}
-
-func (s *stubGameInfo) GetGameStatus(ctx context.Context) (types.GameStatus, error) {
-	return s.status, s.err
+func (s *stubGameState) GetGameStatus(ctx context.Context) (types.GameStatus, error) {
+	if s.callCount == 0 {
+		return s.preStatus, nil
+	}
+	return s.postStatus, nil
 }
 
-func (s *stubGameInfo) GetClaimCount(ctx context.Context) (uint64, error) {
-	return s.claimCount, s.err
+func (s *stubGameState) GetClaimCount(ctx context.Context) (uint64, error) {
+	return s.claimCount, nil
 }