Revert "challenger: Introduce StateConverter to abstract loading VM states"

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 vm.NewPrestateProvider(path, cannon.NewStateConverter())
+				return cannon.NewPrestateProvider(path)
 			}),
 		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.(*vm.PrestateProvider)
+			provider := vmPrestateProvider.(*cannon.CannonPrestateProvider)
 			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 vm.NewPrestateProvider(path, asterisc.NewStateConverter())
+				return asterisc.NewPrestateProvider(path)
 			}),
 		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.(*vm.PrestateProvider)
+			provider := vmPrestateProvider.(*asterisc.AsteriscPreStateProvider)
 			return outputs.NewOutputAsteriscTraceAccessor(logger, m, cfg.Asterisc, serverExecutor, l2Client, prestateProvider, provider.PrestatePath(), rollupClient, dir, l1Head, splitDepth, prestateBlock, poststateBlock)
 		},
 	}

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

@@ -0,0 +1,45 @@
+package asterisc
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum/go-ethereum/common"
+)
+
+var _ types.PrestateProvider = (*AsteriscPreStateProvider)(nil)
+
+type AsteriscPreStateProvider struct {
+	prestate string
+
+	prestateCommitment common.Hash
+}
+
+func NewPrestateProvider(prestate string) *AsteriscPreStateProvider {
+	return &AsteriscPreStateProvider{prestate: prestate}
+}
+
+func (p *AsteriscPreStateProvider) absolutePreState() (*VMState, error) {
+	state, err := parseState(p.prestate)
+	if err != nil {
+		return nil, fmt.Errorf("cannot load absolute pre-state: %w", err)
+	}
+	return state, nil
+}
+
+func (p *AsteriscPreStateProvider) AbsolutePreStateCommitment(_ context.Context) (common.Hash, error) {
+	if p.prestateCommitment != (common.Hash{}) {
+		return p.prestateCommitment, nil
+	}
+	state, err := p.absolutePreState()
+	if err != nil {
+		return common.Hash{}, fmt.Errorf("cannot load absolute pre-state: %w", err)
+	}
+	p.prestateCommitment = state.StateHash
+	return state.StateHash, nil
+}
+
+func (p *AsteriscPreStateProvider) PrestatePath() string {
+	return p.prestate
+}

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

@@ -0,0 +1,59 @@
+package asterisc
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func newAsteriscPrestateProvider(dataDir string, prestate string) *AsteriscPreStateProvider {
+	return &AsteriscPreStateProvider{
+		prestate: filepath.Join(dataDir, prestate),
+	}
+}
+
+func TestAbsolutePreStateCommitment(t *testing.T) {
+	dataDir := t.TempDir()
+
+	prestate := "state.json"
+
+	t.Run("StateUnavailable", func(t *testing.T) {
+		provider := newAsteriscPrestateProvider("/dir/does/not/exist", prestate)
+		_, err := provider.AbsolutePreStateCommitment(context.Background())
+		require.ErrorIs(t, err, os.ErrNotExist)
+	})
+
+	t.Run("InvalidStateFile", func(t *testing.T) {
+		setupPreState(t, dataDir, "invalid.json")
+		provider := newAsteriscPrestateProvider(dataDir, prestate)
+		_, err := provider.AbsolutePreStateCommitment(context.Background())
+		require.ErrorContains(t, err, "invalid asterisc VM state")
+	})
+
+	t.Run("CacheAbsolutePreState", func(t *testing.T) {
+		setupPreState(t, dataDir, prestate)
+		provider := newAsteriscPrestateProvider(dataDir, prestate)
+		first, err := provider.AbsolutePreStateCommitment(context.Background())
+		require.NoError(t, err)
+
+		// Remove the prestate from disk
+		require.NoError(t, os.Remove(provider.prestate))
+
+		// Value should still be available from cache
+		cached, err := provider.AbsolutePreStateCommitment(context.Background())
+		require.NoError(t, err)
+		require.Equal(t, first, cached)
+	})
+}
+
+func setupPreState(t *testing.T, dataDir string, filename string) {
+	srcDir := filepath.Join("test_data")
+	path := filepath.Join(srcDir, filename)
+	file, err := testData.ReadFile(path)
+	require.NoErrorf(t, err, "reading %v", path)
+	err = os.WriteFile(filepath.Join(dataDir, "state.json"), file, 0o644)
+	require.NoErrorf(t, err, "writing %v", path)
+}

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

@@ -27,7 +27,6 @@ type AsteriscTraceProvider struct {
 	generator      utils.ProofGenerator
 	gameDepth      types.Depth
 	preimageLoader *utils.PreimageLoader
-	stateConverter vm.StateConverter
 
 	types.PrestateProvider
 
@@ -47,7 +46,6 @@ func NewTraceProvider(logger log.Logger, m vm.Metricer, cfg vm.Config, vmCfg vm.
 			return kvstore.NewFileKV(vm.PreimageDir(dir))
 		}),
 		PrestateProvider: prestateProvider,
-		stateConverter:   NewStateConverter(),
 	}
 }
 
@@ -122,23 +120,31 @@ 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
-			proof, step, exited, err := p.stateConverter.ConvertStateToProof(filepath.Join(p.dir, vm.FinalState))
+			state, err := p.finalState()
 			if err != nil {
 				return nil, err
 			}
-			if exited && step <= i {
-				p.logger.Warn("Requested proof was after the program exited", "proof", i, "last", step)
+			if state.Exited && state.Step <= i {
+				p.logger.Warn("Requested proof was after the program exited", "proof", i, "last", state.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 = step - 1
+				p.lastStep = state.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, step)
+				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)
 			}
 		}
 	}
@@ -154,6 +160,14 @@ 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 {
@@ -180,14 +194,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
-	_, step, exited, err := p.stateConverter.ConvertStateToProof(filepath.Join(p.dir, vm.FinalState))
+	state, err := p.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 exited {
+	if state.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 step - 1, nil
+	return state.Step - 1, nil
 }

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

@@ -221,12 +221,11 @@ 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,
-		stateConverter: &StateConverter{},
+		logger:    testlog.Logger(t, log.LevelInfo),
+		dir:       dataDir,
+		generator: generator,
+		prestate:  filepath.Join(dataDir, prestate),
+		gameDepth: 63,
 	}, generator
 }
 

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

@@ -8,7 +8,6 @@ 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"
 )
 
@@ -75,27 +74,3 @@ 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
-}

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

@@ -0,0 +1,47 @@
+package cannon
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/ethereum/go-ethereum/common"
+
+	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+)
+
+var _ types.PrestateProvider = (*CannonPrestateProvider)(nil)
+
+type CannonPrestateProvider struct {
+	prestate string
+
+	prestateCommitment common.Hash
+}
+
+func NewPrestateProvider(prestate string) *CannonPrestateProvider {
+	return &CannonPrestateProvider{prestate: prestate}
+}
+
+func (p *CannonPrestateProvider) absolutePreState() ([]byte, common.Hash, error) {
+	state, err := parseState(p.prestate)
+	if err != nil {
+		return nil, common.Hash{}, fmt.Errorf("cannot load absolute pre-state: %w", err)
+	}
+	witness, hash := state.EncodeWitness()
+	return witness, hash, nil
+}
+
+func (p *CannonPrestateProvider) AbsolutePreStateCommitment(_ context.Context) (common.Hash, error) {
+	if p.prestateCommitment != (common.Hash{}) {
+		return p.prestateCommitment, nil
+	}
+	_, hash, err := p.absolutePreState()
+	if err != nil {
+		return common.Hash{}, fmt.Errorf("cannot load absolute pre-state: %w", err)
+	}
+	p.prestateCommitment = hash
+	return hash, nil
+}
+
+func (p *CannonPrestateProvider) PrestatePath() string {
+	return p.prestate
+}