cannon: Move CreateVM to FPVMState for simplicity

Test read/write/create for VersionedState

cannon/mipsevm/iface.go

@@ -1,13 +1,19 @@
 package mipsevm
 
 import (
+	"io"
+
+	"github.com/ethereum-optimism/optimism/cannon/serialize"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/memory"
 )
 
 type FPVMState interface {
+	serialize.Serializable
+
 	GetMemory() *memory.Memory
 
 	// GetHeap returns the current memory address at the top of the heap
@@ -49,6 +55,9 @@ type FPVMState interface {
 
 	// EncodeWitness returns the witness for the current state and the state hash
 	EncodeWitness() (witness []byte, hash common.Hash)
+
+	// CreateVM creates a FPVM that can operate on this state.
+	CreateVM(logger log.Logger, po PreimageOracle, stdOut, stdErr io.Writer) FPVM
 }
 
 type SymbolMatcher func(addr uint32) bool

cannon/mipsevm/multithreaded/state.go

@@ -9,6 +9,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/exec"
@@ -86,6 +87,11 @@ func CreateInitialState(pc, heapStart uint32) *State {
 	return state
 }
 
+func (s *State) CreateVM(logger log.Logger, po mipsevm.PreimageOracle, stdOut, stdErr io.Writer) mipsevm.FPVM {
+	logger.Info("Using cannon multithreaded VM")
+	return NewInstrumentedState(s, po, stdOut, stdErr, logger)
+}
+
 func (s *State) GetCurrentThread() *ThreadState {
 	activeStack := s.getActiveThreadStack()
 

cannon/mipsevm/singlethreaded/state.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/memory"
@@ -67,6 +68,11 @@ func CreateInitialState(pc, heapStart uint32) *State {
 	return state
 }
 
+func (s *State) CreateVM(logger log.Logger, po mipsevm.PreimageOracle, stdOut, stdErr io.Writer) mipsevm.FPVM {
+	logger.Info("Using cannon VM")
+	return NewInstrumentedState(s, po, stdOut, stdErr)
+}
+
 type stateMarshaling struct {
 	Memory         *memory.Memory `json:"memory"`
 	PreimageKey    common.Hash    `json:"preimageKey"`

cannon/mipsevm/versions/state.go

@@ -11,7 +11,6 @@ import (
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/singlethreaded"
 	"github.com/ethereum-optimism/optimism/cannon/serialize"
 	"github.com/ethereum-optimism/optimism/op-service/jsonutil"
-	"github.com/ethereum/go-ethereum/log"
 )
 
 type StateVersion uint8
@@ -22,16 +21,11 @@ const (
 )
 
 var (
-	ErrUnknownVersion = errors.New("unknown version")
+	ErrUnknownVersion   = errors.New("unknown version")
+	ErrJsonNotSupported = errors.New("json not supported")
 )
 
-type VersionedState interface {
-	mipsevm.FPVMState
-	serialize.Serializable
-	CreateVM(logger log.Logger, po mipsevm.PreimageOracle, stdOut, stdErr io.Writer) mipsevm.FPVM
-}
-
-func LoadStateFromFile(path string) (VersionedState, error) {
+func LoadStateFromFile(path string) (*VersionedState, error) {
 	if !serialize.IsBinaryFile(path) {
 		// Always use singlethreaded for JSON states
 		state, err := jsonutil.LoadJSON[singlethreaded.State](path)
@@ -40,74 +34,72 @@ func LoadStateFromFile(path string) (VersionedState, error) {
 		}
 		return NewFromState(state)
 	}
-	return serialize.LoadSerializedBinary[versionedState](path)
+	return serialize.LoadSerializedBinary[VersionedState](path)
 }
 
-func NewFromState(state mipsevm.FPVMState) (VersionedState, error) {
+func NewFromState(state mipsevm.FPVMState) (*VersionedState, error) {
 	switch state := state.(type) {
 	case *singlethreaded.State:
-		return &versionedState{
-			version:        VersionSingleThreaded,
-			VersionedState: &SingleThreadedState{state},
+		return &VersionedState{
+			Version:   VersionSingleThreaded,
+			FPVMState: state,
 		}, nil
 	case *multithreaded.State:
-		return &versionedState{
-			version:        VersionMultiThreaded,
-			VersionedState: &MultiThreadedState{state},
+		return &VersionedState{
+			Version:   VersionMultiThreaded,
+			FPVMState: state,
 		}, nil
 	default:
 		return nil, fmt.Errorf("%w: %T", ErrUnknownVersion, state)
 	}
 }
 
-// versionedState deserializes a FPVMState and implements VersionedState based on the version of that state.
+// VersionedState deserializes a FPVMState and implements VersionedState based on the version of that state.
 // It does this based on the version byte read in Deserialize
-type versionedState struct {
-	version StateVersion
-	VersionedState
+type VersionedState struct {
+	Version StateVersion
+	mipsevm.FPVMState
+}
+
+func (s *VersionedState) Serialize(w io.Writer) error {
+	bout := serialize.NewBinaryWriter(w)
+	if err := bout.WriteUInt(s.Version); err != nil {
+		return err
+	}
+	return s.FPVMState.Serialize(w)
 }
 
-func (s *versionedState) Deserialize(in io.Reader) error {
-	// Read the version byte and then create a multireader to allow the actual implementation to also read it
-	// Allows the Serialize and Deserialize methods of the states to be exact inverses of each other.
+func (s *VersionedState) Deserialize(in io.Reader) error {
 	bin := serialize.NewBinaryReader(in)
-	if err := bin.ReadUInt(&s.version); err != nil {
+	if err := bin.ReadUInt(&s.Version); err != nil {
 		return err
 	}
 
-	switch s.version {
+	switch s.Version {
 	case VersionSingleThreaded:
 		state := &singlethreaded.State{}
 		if err := state.Deserialize(in); err != nil {
 			return err
 		}
-		s.VersionedState = &SingleThreadedState{State: state}
+		s.FPVMState = state
 		return nil
 	case VersionMultiThreaded:
 		state := &multithreaded.State{}
 		if err := state.Deserialize(in); err != nil {
 			return err
 		}
-		s.VersionedState = &MultiThreadedState{State: state}
+		s.FPVMState = state
 		return nil
 	default:
-		return fmt.Errorf("%w: %d", ErrUnknownVersion, s.version)
-	}
-}
-
-func (s *versionedState) Serialize(w io.Writer) error {
-	bout := serialize.NewBinaryWriter(w)
-	if err := bout.WriteUInt(s.version); err != nil {
-		return err
+		return fmt.Errorf("%w: %d", ErrUnknownVersion, s.Version)
 	}
-	return s.VersionedState.Serialize(w)
 }
 
-// MarshalJSON marshalls the underlying state without adding version prefix.
+// MarshalJSON marshals the underlying state without adding version prefix.
 // JSON states are always assumed to be single threaded
-func (s *versionedState) MarshalJSON() ([]byte, error) {
-	if s.version != VersionSingleThreaded {
-		return nil, fmt.Errorf("attempting to JSON marshal state of type %T, only single threaded states support JSON", s.VersionedState)
+func (s *VersionedState) MarshalJSON() ([]byte, error) {
+	if s.Version != VersionSingleThreaded {
+		return nil, fmt.Errorf("%w for type %T", ErrJsonNotSupported, s.FPVMState)
 	}
-	return json.Marshal(s.VersionedState)
+	return json.Marshal(s.FPVMState)
 }

cannon/mipsevm/versions/state_test.go

@@ -0,0 +1,78 @@
+package versions
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/cannon/mipsevm/multithreaded"
+	"github.com/ethereum-optimism/optimism/cannon/mipsevm/singlethreaded"
+	"github.com/ethereum-optimism/optimism/cannon/serialize"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewFromState(t *testing.T) {
+	t.Run("singlethreaded", func(t *testing.T) {
+		rawState := singlethreaded.CreateEmptyState()
+		expected, err := NewFromState(rawState)
+		require.NoError(t, err)
+		require.IsType(t, &singlethreaded.State{}, expected.FPVMState)
+		require.Equal(t, VersionSingleThreaded, expected.Version)
+	})
+
+	t.Run("multithreaded", func(t *testing.T) {
+		rawState := multithreaded.CreateEmptyState()
+		expected, err := NewFromState(rawState)
+		require.NoError(t, err)
+		require.IsType(t, &multithreaded.State{}, expected.FPVMState)
+		require.Equal(t, VersionMultiThreaded, expected.Version)
+	})
+}
+
+func TestLoadStateFromFile(t *testing.T) {
+	t.Run("SinglethreadedFromJSON", func(t *testing.T) {
+		expected, err := NewFromState(singlethreaded.CreateEmptyState())
+		require.NoError(t, err)
+
+		path := writeToFile(t, "state.json", expected)
+		actual, err := LoadStateFromFile(path)
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
+
+	t.Run("SinglethreadedFromBinary", func(t *testing.T) {
+		expected, err := NewFromState(singlethreaded.CreateEmptyState())
+		require.NoError(t, err)
+
+		path := writeToFile(t, "state.bin.gz", expected)
+		actual, err := LoadStateFromFile(path)
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
+
+	t.Run("MultithreadedFromBinary", func(t *testing.T) {
+		expected, err := NewFromState(multithreaded.CreateEmptyState())
+		require.NoError(t, err)
+
+		path := writeToFile(t, "state.bin.gz", expected)
+		actual, err := LoadStateFromFile(path)
+		require.NoError(t, err)
+		require.Equal(t, expected, actual)
+	})
+}
+
+func TestMultithreadedDoesNotSupportJSON(t *testing.T) {
+	state, err := NewFromState(multithreaded.CreateEmptyState())
+	require.NoError(t, err)
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.json")
+	err = serialize.Write(path, state, 0o644)
+	require.ErrorIs(t, err, ErrJsonNotSupported)
+}
+
+func writeToFile(t *testing.T, filename string, data serialize.Serializable) string {
+	dir := t.TempDir()
+	path := filepath.Join(dir, filename)
+	require.NoError(t, serialize.Write(path, data, 0o644))
+	return path
+}