Merge pull request #6683 from ethereum-optimism/aj/cannon-fixes

op-challenger: Skip oracle updates when no data to publish

op-challenger/fault/agent.go

@@ -141,7 +141,7 @@ func (a *Agent) step(ctx context.Context, claim types.Claim, game types.Game) er
 	}
 
 	a.log.Info("Updating oracle data", "oracleKey", oracleData.OracleKey, "oracleData", oracleData.OracleData)
-	if err := a.updater.UpdateOracle(ctx, *oracleData); err != nil {
+	if err := a.updater.UpdateOracle(ctx, oracleData); err != nil {
 		return fmt.Errorf("failed to load oracle data: %w", err)
 	}
 

op-challenger/fault/alphabet/updater.go

@@ -21,7 +21,7 @@ func NewOracleUpdater(logger log.Logger) *alphabetUpdater {
 }
 
 // UpdateOracle updates the oracle with the given data.
-func (u *alphabetUpdater) UpdateOracle(ctx context.Context, data types.PreimageOracleData) error {
+func (u *alphabetUpdater) UpdateOracle(ctx context.Context, data *types.PreimageOracleData) error {
 	u.logger.Info("alphabet oracle updater called")
 	return nil
 }

op-challenger/fault/alphabet/updater_test.go

@@ -15,5 +15,5 @@ import (
 func TestAlphabetUpdater(t *testing.T) {
 	logger := testlog.Logger(t, log.LvlInfo)
 	updater := NewOracleUpdater(logger)
-	require.Nil(t, updater.UpdateOracle(context.Background(), types.PreimageOracleData{}))
+	require.Nil(t, updater.UpdateOracle(context.Background(), &types.PreimageOracleData{}))
 }

op-challenger/fault/cannon/provider.go

@@ -114,8 +114,7 @@ func (p *CannonTraceProvider) GetPreimage(ctx context.Context, i uint64) ([]byte
 }
 
 func (p *CannonTraceProvider) AbsolutePreState(ctx context.Context) ([]byte, error) {
-	path := filepath.Join(p.dir, p.prestate)
-	state, err := parseState(path)
+	state, err := parseState(p.prestate)
 	if err != nil {
 		return []byte{}, fmt.Errorf("cannot load absolute pre-state: %w", err)
 	}

op-challenger/fault/cannon/provider_test.go

@@ -240,7 +240,7 @@ func setupWithTestData(t *testing.T, dataDir string, prestate string) (*CannonTr
 		logger:    testlog.Logger(t, log.LvlInfo),
 		dir:       dataDir,
 		generator: generator,
-		prestate:  prestate,
+		prestate:  filepath.Join(dataDir, prestate),
 	}, generator
 }
 

op-challenger/fault/cannon/updater.go

@@ -86,7 +86,10 @@ func NewOracleUpdaterWithOracle(
 }
 
 // UpdateOracle updates the oracle with the given data.
-func (u *cannonUpdater) UpdateOracle(ctx context.Context, data types.PreimageOracleData) error {
+func (u *cannonUpdater) UpdateOracle(ctx context.Context, data *types.PreimageOracleData) error {
+	if len(data.OracleKey) == 0 {
+		return nil
+	}
 	if data.IsLocal {
 		return u.sendLocalOracleData(ctx, data)
 	}
@@ -94,7 +97,7 @@ func (u *cannonUpdater) UpdateOracle(ctx context.Context, data types.PreimageOra
 }
 
 // sendLocalOracleData sends the local oracle data to the [txmgr].
-func (u *cannonUpdater) sendLocalOracleData(ctx context.Context, data types.PreimageOracleData) error {
+func (u *cannonUpdater) sendLocalOracleData(ctx context.Context, data *types.PreimageOracleData) error {
 	txData, err := u.BuildLocalOracleData(data)
 	if err != nil {
 		return fmt.Errorf("local oracle tx data build: %w", err)
@@ -103,7 +106,7 @@ func (u *cannonUpdater) sendLocalOracleData(ctx context.Context, data types.Prei
 }
 
 // sendGlobalOracleData sends the global oracle data to the [txmgr].
-func (u *cannonUpdater) sendGlobalOracleData(ctx context.Context, data types.PreimageOracleData) error {
+func (u *cannonUpdater) sendGlobalOracleData(ctx context.Context, data *types.PreimageOracleData) error {
 	txData, err := u.BuildGlobalOracleData(data)
 	if err != nil {
 		return fmt.Errorf("global oracle tx data build: %w", err)
@@ -114,7 +117,7 @@ func (u *cannonUpdater) sendGlobalOracleData(ctx context.Context, data types.Pre
 // BuildLocalOracleData takes the local preimage key and data
 // and creates tx data to load the key, data pair into the
 // PreimageOracle contract from the FaultDisputeGame contract call.
-func (u *cannonUpdater) BuildLocalOracleData(data types.PreimageOracleData) ([]byte, error) {
+func (u *cannonUpdater) BuildLocalOracleData(data *types.PreimageOracleData) ([]byte, error) {
 	return u.fdgAbi.Pack(
 		"addLocalData",
 		data.GetIdent(),
@@ -125,7 +128,7 @@ func (u *cannonUpdater) BuildLocalOracleData(data types.PreimageOracleData) ([]b
 // BuildGlobalOracleData takes the global preimage key and data
 // and creates tx data to load the key, data pair into the
 // PreimageOracle contract.
-func (u *cannonUpdater) BuildGlobalOracleData(data types.PreimageOracleData) ([]byte, error) {
+func (u *cannonUpdater) BuildGlobalOracleData(data *types.PreimageOracleData) ([]byte, error) {
 	return u.preimageOracleAbi.Pack(
 		"loadKeccak256PreimagePart",
 		big.NewInt(int64(data.OracleOffset)),

op-challenger/fault/cannon/updater_test.go

@@ -74,7 +74,8 @@ func newTestCannonUpdater(t *testing.T, sendFails bool) (*cannonUpdater, *mockTx
 func TestCannonUpdater_UpdateOracle(t *testing.T) {
 	t.Run("succeeds", func(t *testing.T) {
 		updater, mockTxMgr := newTestCannonUpdater(t, false)
-		require.Nil(t, updater.UpdateOracle(context.Background(), types.PreimageOracleData{
+		require.NoError(t, updater.UpdateOracle(context.Background(), &types.PreimageOracleData{
+			OracleKey:  common.Hash{0xaa}.Bytes(),
 			OracleData: common.Hex2Bytes("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
 		}))
 		require.Equal(t, 1, mockTxMgr.sends)
@@ -82,18 +83,25 @@ func TestCannonUpdater_UpdateOracle(t *testing.T) {
 
 	t.Run("send fails", func(t *testing.T) {
 		updater, mockTxMgr := newTestCannonUpdater(t, true)
-		require.Error(t, updater.UpdateOracle(context.Background(), types.PreimageOracleData{
+		require.Error(t, updater.UpdateOracle(context.Background(), &types.PreimageOracleData{
+			OracleKey:  common.Hash{0xaa}.Bytes(),
 			OracleData: common.Hex2Bytes("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
 		}))
 		require.Equal(t, 1, mockTxMgr.failedSends)
 	})
+
+	t.Run("skip empty data", func(t *testing.T) {
+		updater, mockTxMgr := newTestCannonUpdater(t, true)
+		require.NoError(t, updater.UpdateOracle(context.Background(), &types.PreimageOracleData{}))
+		require.Equal(t, 0, mockTxMgr.sends)
+	})
 }
 
 // TestCannonUpdater_BuildLocalOracleData tests the [cannonUpdater]
 // builds a valid tx candidate for a local oracle update.
 func TestCannonUpdater_BuildLocalOracleData(t *testing.T) {
 	updater, _ := newTestCannonUpdater(t, false)
-	oracleData := types.PreimageOracleData{
+	oracleData := &types.PreimageOracleData{
 		OracleKey:    common.Hex2Bytes("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
 		OracleData:   common.Hex2Bytes("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
 		OracleOffset: 7,
@@ -117,7 +125,7 @@ func TestCannonUpdater_BuildLocalOracleData(t *testing.T) {
 // builds a valid tx candidate for a global oracle update.
 func TestCannonUpdater_BuildGlobalOracleData(t *testing.T) {
 	updater, _ := newTestCannonUpdater(t, false)
-	oracleData := types.PreimageOracleData{
+	oracleData := &types.PreimageOracleData{
 		OracleKey:    common.Hex2Bytes("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
 		OracleData:   common.Hex2Bytes("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"),
 		OracleOffset: 7,

op-challenger/fault/types/types.go

@@ -65,7 +65,7 @@ type StepCallData struct {
 // OracleUpdater is a generic interface for updating oracles.
 type OracleUpdater interface {
 	// UpdateOracle updates the oracle with the given data.
-	UpdateOracle(ctx context.Context, data PreimageOracleData) error
+	UpdateOracle(ctx context.Context, data *PreimageOracleData) error
 }
 
 // TraceProvider is a generic way to get a claim value at a specific step in the trace.