Reapply "challenger: Introduce StateConverter to abstract loading VM …

…states" (ethereum-optimism#11752)

With fix to set a state converter when creating trace providers for test.

op-challenger/game/fault/register_task.go

@@ -57,7 +57,7 @@ func NewCannonRegisterTask(gameType faultTypes.GameType, cfg *config.Config, m c
 			cfg.CannonAbsolutePreState,
 			filepath.Join(cfg.Datadir, "cannon-prestates"),
 			func(path string) faultTypes.PrestateProvider {
-				return cannon.NewPrestateProvider(path)
+				return vm.NewPrestateProvider(path, cannon.NewStateConverter())
 			}),
 		newTraceAccessor: func(
 			logger log.Logger,
@@ -71,7 +71,7 @@ func NewCannonRegisterTask(gameType faultTypes.GameType, cfg *config.Config, m c
 			splitDepth faultTypes.Depth,
 			prestateBlock uint64,
 			poststateBlock uint64) (*trace.Accessor, error) {
-			provider := vmPrestateProvider.(*cannon.CannonPrestateProvider)
+			provider := vmPrestateProvider.(*vm.PrestateProvider)
 			return outputs.NewOutputCannonTraceAccessor(logger, m, cfg.Cannon, serverExecutor, l2Client, prestateProvider, provider.PrestatePath(), rollupClient, dir, l1Head, splitDepth, prestateBlock, poststateBlock)
 		},
 	}
@@ -87,7 +87,7 @@ func NewAsteriscRegisterTask(gameType faultTypes.GameType, cfg *config.Config, m
 			cfg.AsteriscAbsolutePreState,
 			filepath.Join(cfg.Datadir, "asterisc-prestates"),
 			func(path string) faultTypes.PrestateProvider {
-				return asterisc.NewPrestateProvider(path)
+				return vm.NewPrestateProvider(path, asterisc.NewStateConverter())
 			}),
 		newTraceAccessor: func(
 			logger log.Logger,
@@ -101,7 +101,7 @@ func NewAsteriscRegisterTask(gameType faultTypes.GameType, cfg *config.Config, m
 			splitDepth faultTypes.Depth,
 			prestateBlock uint64,
 			poststateBlock uint64) (*trace.Accessor, error) {
-			provider := vmPrestateProvider.(*asterisc.AsteriscPreStateProvider)
+			provider := vmPrestateProvider.(*vm.PrestateProvider)
 			return outputs.NewOutputAsteriscTraceAccessor(logger, m, cfg.Asterisc, serverExecutor, l2Client, prestateProvider, provider.PrestatePath(), rollupClient, dir, l1Head, splitDepth, prestateBlock, poststateBlock)
 		},
 	}

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

@@ -27,6 +27,7 @@ type AsteriscTraceProvider struct {
 	generator      utils.ProofGenerator
 	gameDepth      types.Depth
 	preimageLoader *utils.PreimageLoader
+	stateConverter vm.StateConverter
 
 	types.PrestateProvider
 
@@ -46,6 +47,7 @@ func NewTraceProvider(logger log.Logger, m vm.Metricer, cfg vm.Config, vmCfg vm.
 			return kvstore.NewFileKV(vm.PreimageDir(dir))
 		}),
 		PrestateProvider: prestateProvider,
+		stateConverter:   NewStateConverter(),
 	}
 }
 
@@ -120,31 +122,23 @@ func (p *AsteriscTraceProvider) loadProof(ctx context.Context, i uint64) (*utils
 		file, err = ioutil.OpenDecompressed(path)
 		if errors.Is(err, os.ErrNotExist) {
 			// Expected proof wasn't generated, check if we reached the end of execution
-			state, err := p.finalState()
+			proof, step, exited, err := p.stateConverter.ConvertStateToProof(filepath.Join(p.dir, vm.FinalState))
 			if err != nil {
 				return nil, err
 			}
-			if state.Exited && state.Step <= i {
-				p.logger.Warn("Requested proof was after the program exited", "proof", i, "last", state.Step)
+			if exited && step <= i {
+				p.logger.Warn("Requested proof was after the program exited", "proof", i, "last", step)
 				// The final instruction has already been applied to this state, so the last step we can execute
 				// is one before its Step value.
-				p.lastStep = state.Step - 1
+				p.lastStep = step - 1
 				// Extend the trace out to the full length using a no-op instruction that doesn't change any state
 				// No execution is done, so no proof-data or oracle values are required.
-				proof := &utils.ProofData{
-					ClaimValue:   state.StateHash,
-					StateData:    state.Witness,
-					ProofData:    []byte{},
-					OracleKey:    nil,
-					OracleValue:  nil,
-					OracleOffset: 0,
-				}
 				if err := utils.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)
+				return nil, fmt.Errorf("expected proof not generated but final state was not exited, requested step %v, final state at step %v", i, step)
 			}
 		}
 	}
@@ -160,14 +154,6 @@ func (p *AsteriscTraceProvider) loadProof(ctx context.Context, i uint64) (*utils
 	return &proof, nil
 }
 
-func (c *AsteriscTraceProvider) finalState() (*VMState, error) {
-	state, err := parseState(filepath.Join(c.dir, vm.FinalState))
-	if err != nil {
-		return nil, fmt.Errorf("cannot read final state: %w", err)
-	}
-	return state, nil
-}
-
 // AsteriscTraceProviderForTest is a AsteriscTraceProvider that can find the step referencing the preimage read
 // Only to be used for testing
 type AsteriscTraceProviderForTest struct {
@@ -184,6 +170,7 @@ func NewTraceProviderForTest(logger log.Logger, m vm.Metricer, cfg *config.Confi
 		preimageLoader: utils.NewPreimageLoader(func() utils.PreimageSource {
 			return kvstore.NewFileKV(vm.PreimageDir(dir))
 		}),
+		stateConverter: NewStateConverter(),
 	}
 	return &AsteriscTraceProviderForTest{p}
 }
@@ -194,14 +181,14 @@ func (p *AsteriscTraceProviderForTest) FindStep(ctx context.Context, start uint6
 		return 0, fmt.Errorf("generate asterisc trace (until preimage read): %w", err)
 	}
 	// Load the step from the state asterisc finished with
-	state, err := p.finalState()
+	_, step, exited, err := p.stateConverter.ConvertStateToProof(filepath.Join(p.dir, vm.FinalState))
 	if err != nil {
 		return 0, fmt.Errorf("failed to load final state: %w", err)
 	}
 	// Check we didn't get to the end of the trace without finding the preimage read we were looking for
-	if state.Exited {
+	if exited {
 		return 0, fmt.Errorf("preimage read not found: %w", io.EOF)
 	}
 	// The state is the post-state so the step we want to execute to read the preimage is step - 1.
-	return state.Step - 1, nil
+	return step - 1, nil
 }

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

@@ -221,11 +221,12 @@ func setupTestData(t *testing.T) (string, string) {
 func setupWithTestData(t *testing.T, dataDir string, prestate string) (*AsteriscTraceProvider, *stubGenerator) {
 	generator := &stubGenerator{}
 	return &AsteriscTraceProvider{
-		logger:    testlog.Logger(t, log.LevelInfo),
-		dir:       dataDir,
-		generator: generator,
-		prestate:  filepath.Join(dataDir, prestate),
-		gameDepth: 63,
+		logger:         testlog.Logger(t, log.LevelInfo),
+		dir:            dataDir,
+		generator:      generator,
+		prestate:       filepath.Join(dataDir, prestate),
+		gameDepth:      63,
+		stateConverter: &StateConverter{},
 	}, generator
 }
 

op-challenger/game/fault/trace/asterisc/state.go → op-challenger/game/fault/trace/asterisc/state_converter.go

@@ -8,6 +8,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm"
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/utils"
 	"github.com/ethereum-optimism/optimism/op-service/ioutil"
 )
 
@@ -74,3 +75,27 @@ func parseStateFromReader(in io.ReadCloser) (*VMState, error) {
 	}
 	return &state, nil
 }
+
+type StateConverter struct {
+}
+
+func NewStateConverter() *StateConverter {
+	return &StateConverter{}
+}
+
+func (c *StateConverter) ConvertStateToProof(statePath string) (*utils.ProofData, uint64, bool, error) {
+	state, err := parseState(statePath)
+	if err != nil {
+		return nil, 0, false, fmt.Errorf("cannot read final state: %w", err)
+	}
+	// Extend the trace out to the full length using a no-op instruction that doesn't change any state
+	// No execution is done, so no proof-data or oracle values are required.
+	return &utils.ProofData{
+		ClaimValue:   state.StateHash,
+		StateData:    state.Witness,
+		ProofData:    []byte{},
+		OracleKey:    nil,
+		OracleValue:  nil,
+		OracleOffset: 0,
+	}, state.Step, state.Exited, nil
+}