From 7973aafa46f7eef051eb8fbac77a1bb0ed6405df Mon Sep 17 00:00:00 2001 From: pcw109550 Date: Wed, 22 May 2024 02:59:29 +0900 Subject: [PATCH 1/2] Fix inverted hint writing conditional check --- rvgo/fast/state.go | 2 +- rvgo/fast/vm.go | 2 +- rvgo/test/syscall_test.go | 206 +++++++++++++++++++++++++++++++++++++- 3 files changed, 204 insertions(+), 6 deletions(-) diff --git a/rvgo/fast/state.go b/rvgo/fast/state.go index 7a02036d..70c24a90 100644 --- a/rvgo/fast/state.go +++ b/rvgo/fast/state.go @@ -48,7 +48,7 @@ type VMState struct { // to make sure pre-image requests can be served. // The first 4 bytes are a uin32 length prefix. // Warning: the hint MAY NOT BE COMPLETE. I.e. this is buffered, - // and should only be read when len(LastHint) > 4 && uint32(LastHint[:4]) >= len(LastHint[4:]) + // and should only be read when len(LastHint) > 4 && uint32(LastHint[:4]) <= len(LastHint[4:]) LastHint hexutil.Bytes `json:"lastHint,omitempty"` // VMState must hold these values because if not, we must ask FPVM again to diff --git a/rvgo/fast/vm.go b/rvgo/fast/vm.go index 91b1de58..d4591493 100644 --- a/rvgo/fast/vm.go +++ b/rvgo/fast/vm.go @@ -461,7 +461,7 @@ func (inst *InstrumentedState) riscvStep() (outErr error) { s.LastHint = append(inst.state.LastHint, hintData...) for len(s.LastHint) >= 4 { // process while there is enough data to check if there are any hints hintLen := binary.BigEndian.Uint32(s.LastHint[:4]) - if hintLen >= uint32(len(s.LastHint[4:])) { + if hintLen <= uint32(len(s.LastHint[4:])) { hint := s.LastHint[4 : 4+hintLen] // without the length prefix s.LastHint = s.LastHint[4+hintLen:] inst.preimageOracle.Hint(hint) diff --git a/rvgo/test/syscall_test.go b/rvgo/test/syscall_test.go index 962ce559..6aec4121 100644 --- a/rvgo/test/syscall_test.go +++ b/rvgo/test/syscall_test.go @@ -1,8 +1,10 @@ package test import ( + "bytes" "encoding/binary" "fmt" + "math/rand" "os" "testing" @@ -30,6 +32,18 @@ func staticOracle(t *testing.T, preimageData []byte) *testOracle { } } +type hintTrackingOracle struct { + hints [][]byte +} + +func (t *hintTrackingOracle) Hint(v []byte) { + t.hints = append(t.hints, v) +} + +func (t *hintTrackingOracle) GetPreimage(k [32]byte) []byte { + return nil +} + func runEVM(t *testing.T, contracts *Contracts, addrs *Addresses, stepWitness *fast.StepWitness, fastPost fast.StateWitness, revertCode []byte) { env := newEVMEnv(t, contracts, addrs) evmPost, _, _ := stepEVM(t, env, stepWitness, addrs, 0, revertCode) @@ -93,6 +107,176 @@ func TestStateSyscallUnsupported(t *testing.T) { } } +func TestEVMSysWriteHint(t *testing.T) { + contracts := testContracts(t) + addrs := testAddrs + + cases := []struct { + name string + memOffset int // Where the hint data is stored in memory + hintData []byte // Hint data stored in memory at memOffset + bytesToWrite int // How many bytes of hintData to write + lastHint []byte // The buffer that stores lastHint in the state + expectedHints [][]byte // The hints we expect to be processed + }{ + { + name: "write 1 full hint at beginning of page", + memOffset: 4096, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 10, + lastHint: nil, + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB}, + }, + }, + { + name: "write 1 full hint across page boundary", + memOffset: 4092, + hintData: []byte{ + 0, 0, 0, 8, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 12, + lastHint: nil, + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB}, + }, + }, + { + name: "write 2 full hints", + memOffset: 5012, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + 0, 0, 0, 8, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 22, + lastHint: nil, + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB}, + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB}, + }, + }, + { + name: "write a single partial hint", + memOffset: 4092, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 8, + lastHint: nil, + expectedHints: nil, + }, + { + name: "write 1 full, 1 partial hint", + memOffset: 5012, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + 0, 0, 0, 8, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 16, + lastHint: nil, + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB}, + }, + }, + { + name: "write a single partial hint to large capacity lastHint buffer", + memOffset: 4092, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 8, + lastHint: make([]byte, 0, 4096), + expectedHints: nil, + }, + { + name: "write full hint to large capacity lastHint buffer", + memOffset: 5012, + hintData: []byte{ + 0, 0, 0, 6, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 10, + lastHint: make([]byte, 0, 4096), + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB}, + }, + }, + { + name: "write multiple hints to large capacity lastHint buffer", + memOffset: 4092, + hintData: []byte{ + 0, 0, 0, 8, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xCC, 0xCC, // Hint data + 0, 0, 0, 8, // Length prefix + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB, // Hint data + }, + bytesToWrite: 24, + lastHint: make([]byte, 0, 4096), + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xCC, 0xCC}, + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xBB, 0xBB}, + }, + }, + { + name: "write remaining hint data to non-empty lastHint buffer", + memOffset: 4092, + hintData: []byte{ + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xCC, 0xCC, // Hint data + }, + bytesToWrite: 8, + lastHint: []byte{0, 0, 0, 8}, + expectedHints: [][]byte{ + {0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xCC, 0xCC}, + }, + }, + { + name: "write partial hint data to non-empty lastHint buffer", + memOffset: 4092, + hintData: []byte{ + 0xAA, 0xAA, 0xAA, 0xAA, 0xBB, 0xBB, 0xCC, 0xCC, // Hint data + }, + bytesToWrite: 4, + lastHint: []byte{0, 0, 0, 8}, + expectedHints: nil, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + oracle := hintTrackingOracle{} + + state := &fast.VMState{ + PC: 0, + Memory: fast.NewMemory(), + Registers: [32]uint64{17: riscv.SysWrite, 10: riscv.FdHintWrite, 11: uint64(tt.memOffset), 12: uint64(tt.bytesToWrite)}, + LastHint: tt.lastHint, + } + + err := state.Memory.SetMemoryRange(uint64(tt.memOffset), bytes.NewReader(tt.hintData)) + require.NoError(t, err) + state.Memory.SetUnaligned(0, syscallInsn) + + fastState := fast.NewInstrumentedState(state, &oracle, os.Stdout, os.Stderr) + stepWitness, err := fastState.Step(true) + require.NoError(t, err) + require.Equal(t, tt.expectedHints, oracle.hints) + + fastPost := state.EncodeWitness() + runEVM(t, contracts, addrs, stepWitness, fastPost, nil) + }) + } +} + func FuzzStateSyscallExit(f *testing.F) { contracts := testContracts(f) addrs := testAddrs @@ -817,7 +1001,7 @@ func FuzzStateHintWrite(f *testing.F) { contracts := testContracts(f) addrs := testAddrs - f.Fuzz(func(t *testing.T, addr uint64, count uint64, preimageOffset uint64, pc uint64, step uint64) { + f.Fuzz(func(t *testing.T, addr uint64, count uint64, preimageOffset uint64, pc uint64, step uint64, randSeed int64) { pc = pc & 0xFF_FF_FF_FF_FF_FF_FF_FC // align PC preimageData := []byte("hello world") if preimageOffset >= uint64(len(preimageData)) { @@ -835,10 +1019,15 @@ func FuzzStateHintWrite(f *testing.F) { PreimageKey: preimage.Keccak256Key(crypto.Keccak256Hash(preimageData)).PreimageKey(), PreimageOffset: preimageOffset, - // This is only used by fast/vm.go. The reads a zeroed page-sized buffer when reading hint data from memory. - // We pre-allocate a buffer for the read hint data to be copied into. - LastHint: make(hexutil.Bytes, fast.PageSize), + LastHint: nil, } + // Set random data at the target memory range + randBytes, err := randomBytes(randSeed, count) + require.NoError(t, err) + err = state.Memory.SetMemoryRange(addr, bytes.NewReader(randBytes)) + require.NoError(t, err) + + // Set syscall instruction state.Memory.SetUnaligned(pc, syscallInsn) preStatePreimageKey := state.PreimageKey preStateRoot := state.Memory.MerkleRoot() @@ -940,3 +1129,12 @@ func FuzzStatePreimageWrite(f *testing.F) { runSlow(t, stepWitness, fastPost, oracle, nil) }) } + +func randomBytes(seed int64, length uint64) ([]byte, error) { + r := rand.New(rand.NewSource(seed)) + randBytes := make([]byte, length) + if _, err := r.Read(randBytes); err != nil { + return nil, err + } + return randBytes, nil +} From 33afd2638f9a4c8667bf16633317f76ae797d149 Mon Sep 17 00:00:00 2001 From: pcw109550 Date: Wed, 22 May 2024 03:35:01 +0900 Subject: [PATCH 2/2] Always write 4 bytes for syscall instr --- rvgo/test/syscall_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rvgo/test/syscall_test.go b/rvgo/test/syscall_test.go index 6aec4121..6b952f8c 100644 --- a/rvgo/test/syscall_test.go +++ b/rvgo/test/syscall_test.go @@ -18,7 +18,7 @@ import ( "github.com/ethereum-optimism/asterisc/rvgo/slow" ) -var syscallInsn = []byte{0x73} +var syscallInsn = []byte{0x73, 0x00, 0x00, 0x00} func staticOracle(t *testing.T, preimageData []byte) *testOracle { return &testOracle{