Merge pull request #7255 from ethereum-optimism/aj/fix-last-step

op-challenger: Fix recording of last step index

op-challenger/game/fault/trace/cannon/provider.go

@@ -53,8 +53,6 @@ type CannonTraceProvider struct {
 	// lastStep stores the last step in the actual trace if known. 0 indicates unknown.
 	// Cached as an optimisation to avoid repeatedly attempting to execute beyond the end of the trace.
 	lastStep uint64
-	// lastProof stores the proof data to use for all steps extended beyond lastStep
-	lastProof *proofData
 }
 
 func NewTraceProvider(ctx context.Context, logger log.Logger, m CannonMetricer, cfg *config.Config, l1Client bind.ContractCaller, dir string, gameAddr common.Address) (*CannonTraceProvider, error) {
@@ -139,24 +137,19 @@ func (p *CannonTraceProvider) AbsolutePreStateCommitment(ctx context.Context) (c
 // loadProof will attempt to load or generate the proof data at the specified index
 // If the requested index is beyond the end of the actual trace it is extended with no-op instructions.
 func (p *CannonTraceProvider) loadProof(ctx context.Context, i uint64) (*proofData, error) {
-	if p.lastProof != nil && i > p.lastStep {
-		// If the requested index is after the last step in the actual trace, extend the final no-op step
-		return p.lastProof, nil
-	}
 	// Attempt to read the last step from disk cache
-	if p.lastProof == nil && p.lastStep == 0 {
-		step, err := ReadLastStep(p.dir)
+	if p.lastStep == 0 {
+		step, err := readLastStep(p.dir)
 		if err != nil {
 			p.logger.Warn("Failed to read last step from disk cache", "err", err)
 		} else {
 			p.lastStep = step
-			// If the last step is tracked, set i to the last step
-			// to read the correct proof from disk.
-			if i > p.lastStep {
-				i = step
-			}
 		}
 	}
+	// If the last step is tracked, set i to the last step to generate or load the final proof
+	if p.lastStep != 0 && i > p.lastStep {
+		i = p.lastStep
+	}
 	path := filepath.Join(p.dir, proofsDir, fmt.Sprintf("%d.json.gz", i))
 	file, err := ioutil.OpenDecompressed(path)
 	if errors.Is(err, os.ErrNotExist) {
@@ -183,9 +176,6 @@ func (p *CannonTraceProvider) loadProof(ctx context.Context, i uint64) (*proofDa
 				if err != nil {
 					return nil, fmt.Errorf("cannot hash witness: %w", err)
 				}
-				if err := WriteLastStep(p.dir, state.Step); err != nil {
-					p.logger.Warn("Failed to write last step to disk cache", "step", p.lastStep)
-				}
 				proof := &proofData{
 					ClaimValue:   witnessHash,
 					StateData:    hexutil.Bytes(witness),
@@ -194,7 +184,9 @@ func (p *CannonTraceProvider) loadProof(ctx context.Context, i uint64) (*proofDa
 					OracleValue:  nil,
 					OracleOffset: 0,
 				}
-				p.lastProof = proof
+				if err := writeLastStep(p.dir, proof, p.lastStep); err != nil {
+					p.logger.Warn("Failed to write last step to disk cache", "step", p.lastStep)
+				}
 				return proof, nil
 			} else {
 				return nil, fmt.Errorf("expected proof not generated but final state was not exited, requested step %v, final state at step %v", i, state.Step)
@@ -217,8 +209,8 @@ type diskStateCacheObj struct {
 	Step uint64 `json:"step"`
 }
 
-// ReadLastStep reads the tracked last step from disk.
-func ReadLastStep(dir string) (uint64, error) {
+// readLastStep reads the tracked last step from disk.
+func readLastStep(dir string) (uint64, error) {
 	state := diskStateCacheObj{}
 	file, err := ioutil.OpenDecompressed(filepath.Join(dir, diskStateCache))
 	if err != nil {
@@ -232,8 +224,15 @@ func ReadLastStep(dir string) (uint64, error) {
 	return state.Step, nil
 }
 
-// WriteLastStep writes the last step to disk as a persistent cache.
-func WriteLastStep(dir string, step uint64) error {
+// writeLastStep writes the last step and proof to disk as a persistent cache.
+func writeLastStep(dir string, proof *proofData, step uint64) error {
 	state := diskStateCacheObj{Step: step}
-	return ioutil.WriteCompressedJson(filepath.Join(dir, diskStateCache), state)
+	lastStepFile := filepath.Join(dir, diskStateCache)
+	if err := ioutil.WriteCompressedJson(lastStepFile, state); err != nil {
+		return fmt.Errorf("failed to write last step to %v: %w", lastStepFile, err)
+	}
+	if err := ioutil.WriteCompressedJson(filepath.Join(dir, proofsDir, fmt.Sprintf("%d.json.gz", step)), proof); err != nil {
+		return fmt.Errorf("failed to write proof: %w", err)
+	}
+	return nil
 }

op-challenger/game/fault/trace/cannon/provider_test.go

@@ -158,20 +158,16 @@ func TestGetStepData(t *testing.T) {
 			Exited: true,
 		}
 		generator.proof = &proofData{
-			ClaimValue:   common.Hash{0xaa},
-			StateData:    []byte{0xbb},
-			ProofData:    []byte{0xcc},
-			OracleKey:    common.Hash{0xdd}.Bytes(),
-			OracleValue:  []byte{0xdd},
-			OracleOffset: 10,
+			ClaimValue: common.Hash{0xaa},
+			StateData:  []byte{0xbb},
+			ProofData:  []byte{0xcc},
 		}
 		preimage, proof, data, err := provider.GetStepData(context.Background(), 7000)
 		require.NoError(t, err)
-		require.Contains(t, generator.generated, 10, "should have tried to generate the proof")
+		require.Empty(t, generator.generated, "should not have to generate the proof again")
 
-		witness := generator.finalState.EncodeWitness()
-		require.EqualValues(t, witness, preimage)
-		require.Equal(t, []byte{}, proof)
+		require.EqualValues(t, initGenerator.finalState.EncodeWitness(), preimage)
+		require.Empty(t, proof)
 		require.Nil(t, data)
 	})