Merge pull request #6956 from ethereum-optimism/refcell/call-resolve

fix(op-challenger): Only Resolve Won Dispute Games

op-challenger/fault/agent.go

@@ -13,7 +13,7 @@ import (
 // Responder takes a response action & executes.
 // For full op-challenger this means executing the transaction on chain.
 type Responder interface {
-	CanResolve(ctx context.Context) bool
+	CallResolve(ctx context.Context) (types.GameStatus, error)
 	Resolve(ctx context.Context) error
 	Respond(ctx context.Context, response types.Claim) error
 	Step(ctx context.Context, stepData types.StepCallData) error
@@ -65,10 +65,27 @@ func (a *Agent) Act(ctx context.Context) error {
 	return nil
 }
 
+// shouldResolve returns true if the agent should resolve the game.
+// This method will return false if the game is still in progress.
+func (a *Agent) shouldResolve(ctx context.Context, status types.GameStatus) bool {
+	expected := types.GameStatusDefenderWon
+	if a.agreeWithProposedOutput {
+		expected = types.GameStatusChallengerWon
+	}
+	if expected != status {
+		a.log.Warn("Game will be lost", "expected", expected, "actual", status)
+	}
+	return expected == status
+}
+
 // tryResolve resolves the game if it is in a terminal state
 // and returns true if the game resolves successfully.
 func (a *Agent) tryResolve(ctx context.Context) bool {
-	if !a.responder.CanResolve(ctx) {
+	status, err := a.responder.CallResolve(ctx)
+	if err != nil {
+		return false
+	}
+	if !a.shouldResolve(ctx, status) {
 		return false
 	}
 	a.log.Info("Resolving game")

op-challenger/fault/agent_test.go

+package fault
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/stretchr/testify/require"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/fault/types"
+	"github.com/ethereum-optimism/optimism/op-node/testlog"
+)
+
+// TestShouldResolve tests the resolution logic.
+func TestShouldResolve(t *testing.T) {
+	log := testlog.Logger(t, log.LvlCrit)
+
+	t.Run("AgreeWithProposedOutput", func(t *testing.T) {
+		agent := NewAgent(nil, 0, nil, nil, nil, true, log)
+		require.False(t, agent.shouldResolve(context.Background(), types.GameStatusDefenderWon))
+		require.True(t, agent.shouldResolve(context.Background(), types.GameStatusChallengerWon))
+		require.False(t, agent.shouldResolve(context.Background(), types.GameStatusInProgress))
+	})
+
+	t.Run("DisagreeWithProposedOutput", func(t *testing.T) {
+		agent := NewAgent(nil, 0, nil, nil, nil, false, log)
+		require.True(t, agent.shouldResolve(context.Background(), types.GameStatusDefenderWon))
+		require.False(t, agent.shouldResolve(context.Background(), types.GameStatusChallengerWon))
+		require.False(t, agent.shouldResolve(context.Background(), types.GameStatusInProgress))
+	})
+}

op-challenger/fault/responder.go

@@ -79,18 +79,25 @@ func (r *faultResponder) BuildTx(ctx context.Context, response types.Claim) ([]b
 	}
 }
 
-// CanResolve determines if the resolve function on the fault dispute game contract
-// would succeed. Returns true if the game can be resolved, otherwise false.
-func (r *faultResponder) CanResolve(ctx context.Context) bool {
+// CallResolve determines if the resolve function on the fault dispute game contract
+// would succeed. Returns the game status if the call would succeed, errors otherwise.
+func (r *faultResponder) CallResolve(ctx context.Context) (types.GameStatus, error) {
 	txData, err := r.buildResolveData()
 	if err != nil {
-		return false
+		return types.GameStatusInProgress, err
 	}
-	_, err = r.txMgr.Call(ctx, ethereum.CallMsg{
+	res, err := r.txMgr.Call(ctx, ethereum.CallMsg{
 		To:   &r.fdgAddr,
 		Data: txData,
 	}, nil)
-	return err == nil
+	if err != nil {
+		return types.GameStatusInProgress, err
+	}
+	var status uint8
+	if err = r.fdgAbi.UnpackIntoInterface(&status, "resolve", res); err != nil {
+		return types.GameStatusInProgress, err
+	}
+	return types.GameStatusFromUint8(status)
 }
 
 // Resolve executes a resolve transaction to resolve a fault dispute game.

op-challenger/fault/responder_test.go

@@ -25,18 +25,31 @@ var (
 	mockCallError  = errors.New("mock call error")
 )
 
-// TestCanResolve tests the [Responder.CanResolve].
-func TestCanResolve(t *testing.T) {
+// TestCallResolve tests the [Responder.CallResolve].
+func TestCallResolve(t *testing.T) {
 	t.Run("SendFails", func(t *testing.T) {
 		responder, mockTxMgr := newTestFaultResponder(t)
 		mockTxMgr.callFails = true
-		require.False(t, responder.CanResolve(context.Background()))
+		status, err := responder.CallResolve(context.Background())
+		require.ErrorIs(t, err, mockCallError)
+		require.Equal(t, types.GameStatusInProgress, status)
 		require.Equal(t, 0, mockTxMgr.calls)
 	})
 
+	t.Run("UnpackFails", func(t *testing.T) {
+		responder, mockTxMgr := newTestFaultResponder(t)
+		mockTxMgr.callBytes = []byte{0x00, 0x01}
+		status, err := responder.CallResolve(context.Background())
+		require.Error(t, err)
+		require.Equal(t, types.GameStatusInProgress, status)
+		require.Equal(t, 1, mockTxMgr.calls)
+	})
+
 	t.Run("Success", func(t *testing.T) {
 		responder, mockTxMgr := newTestFaultResponder(t)
-		require.True(t, responder.CanResolve(context.Background()))
+		status, err := responder.CallResolve(context.Background())
+		require.NoError(t, err)
+		require.Equal(t, types.GameStatusInProgress, status)
 		require.Equal(t, 1, mockTxMgr.calls)
 	})
 }

op-challenger/fault/types/types.go

@@ -3,6 +3,7 @@ package types
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -34,6 +35,14 @@ func (s GameStatus) String() string {
 	}
 }
 
+// GameStatusFromUint8 returns a game status from the uint8 representation.
+func GameStatusFromUint8(i uint8) (GameStatus, error) {
+	if i > 2 {
+		return GameStatus(i), fmt.Errorf("invalid game status: %d", i)
+	}
+	return GameStatus(i), nil
+}
+
 // PreimageOracleData encapsulates the preimage oracle data
 // to load into the onchain oracle.
 type PreimageOracleData struct {

op-challenger/fault/types/types_test.go

 package types
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
+var validGameStatuses = []GameStatus{
+	GameStatusInProgress,
+	GameStatusChallengerWon,
+	GameStatusDefenderWon,
+}
+
+func TestGameStatusFromUint8(t *testing.T) {
+	for _, status := range validGameStatuses {
+		t.Run(fmt.Sprintf("Valid Game Status %v", status), func(t *testing.T) {
+			parsed, err := GameStatusFromUint8(uint8(status))
+			require.NoError(t, err)
+			require.Equal(t, status, parsed)
+		})
+	}
+
+	t.Run("Invalid", func(t *testing.T) {
+		status, err := GameStatusFromUint8(3)
+		require.Error(t, err)
+		require.Equal(t, GameStatus(3), status)
+	})
+}
+
 func TestNewPreimageOracleData(t *testing.T) {
 	t.Run("LocalData", func(t *testing.T) {
 		data := NewPreimageOracleData([]byte{1, 2, 3}, []byte{4, 5, 6}, 7)