From 2a09c3e1a65bbc6c3b8319efaaccfd543dfd0cbb Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 8 Jul 2024 10:20:30 -0500 Subject: [PATCH] feedback --- system_tests/validation_mock_test.go | 12 +++- validator/server_arb/execution_run_test.go | 81 +++++++++++----------- 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/system_tests/validation_mock_test.go b/system_tests/validation_mock_test.go index e61fd407ec..1330f24882 100644 --- a/system_tests/validation_mock_test.go +++ b/system_tests/validation_mock_test.go @@ -127,7 +127,17 @@ func (r *mockExecRun) GetStepAt(position uint64) containers.PromiseInterface[*va } func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, maxIterations uint64) containers.PromiseInterface[[]common.Hash] { - return containers.NewReadyPromise[[]common.Hash](nil, nil) + ctx := context.Background() + hashes := make([]common.Hash, 0) + for i := uint64(0); i < maxIterations; i++ { + absoluteMachineIndex := machineStartIndex + stepSize*(i+1) + stepResult, err := r.GetStepAt(absoluteMachineIndex).Await(ctx) + if err != nil { + return containers.NewReadyPromise[[]common.Hash](nil, err) + } + hashes = append(hashes, stepResult.Hash) + } + return containers.NewReadyPromise[[]common.Hash](hashes, nil) } func (r *mockExecRun) GetLastStep() containers.PromiseInterface[*validator.MachineStepResult] { diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index bb148b5d98..bdc1eefc4d 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -4,7 +4,6 @@ import ( "context" "strings" "testing" - "time" "github.com/ethereum/go-ethereum/common" "github.com/offchainlabs/nitro/validator" @@ -64,40 +63,41 @@ func (m *mockMachine) Freeze() {} func (m *mockMachine) Destroy() {} func Test_machineHashesWithStep(t *testing.T) { - mm := &mockMachine{} - e := &executionRun{} - ctx := context.Background() - t.Run("basic argument checks", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + e := &executionRun{} machStartIndex := uint64(0) stepSize := uint64(0) maxIterations := uint64(0) _, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) - if !strings.Contains(err.Error(), "step size cannot be 0") { + if err == nil || !strings.Contains(err.Error(), "step size cannot be 0") { t.Error("Wrong error") } stepSize = uint64(1) _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) - if !strings.Contains(err.Error(), "number of iterations cannot be 0") { + if err == nil || !strings.Contains(err.Error(), "number of iterations cannot be 0") { t.Error("Wrong error") } }) t.Run("machine at start index 0 hash is the finished state hash", func(t *testing.T) { - mm.gs = validator.GoGlobalState{ - Batch: 1, + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + mm := &mockMachine{ + gs: validator.GoGlobalState{ + Batch: 1, + }, + totalSteps: 20, } machStartIndex := uint64(0) stepSize := uint64(1) maxIterations := uint64(1) - e.cache = &MachineCache{ - buildingLock: make(chan struct{}, 1), - machines: []MachineInterface{mm}, - finalMachine: mm, - } - go func() { - <-time.After(time.Millisecond * 50) - e.cache.buildingLock <- struct{}{} - }() + e := &executionRun{ + cache: NewMachineCache(ctx, func(_ context.Context) (MachineInterface, error) { + return mm, nil + }, &DefaultMachineCacheConfig), + } + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err) @@ -111,24 +111,24 @@ func Test_machineHashesWithStep(t *testing.T) { } }) t.Run("can step in step size increments and collect hashes", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() initialGs := validator.GoGlobalState{ Batch: 1, PosInBatch: 0, } - mm.gs = initialGs - mm.totalSteps = 20 + mm := &mockMachine{ + gs: initialGs, + totalSteps: 20, + } machStartIndex := uint64(0) stepSize := uint64(5) maxIterations := uint64(4) - e.cache = &MachineCache{ - buildingLock: make(chan struct{}, 1), - machines: []MachineInterface{mm}, - finalMachine: mm, - } - go func() { - <-time.After(time.Millisecond * 50) - e.cache.buildingLock <- struct{}{} - }() + e := &executionRun{ + cache: NewMachineCache(ctx, func(_ context.Context) (MachineInterface, error) { + return mm, nil + }, &DefaultMachineCacheConfig), + } hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err) @@ -155,24 +155,25 @@ func Test_machineHashesWithStep(t *testing.T) { } }) t.Run("if finishes execution early, can return a smaller number of hashes than the expected max iterations", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() initialGs := validator.GoGlobalState{ Batch: 1, PosInBatch: 0, } - mm.gs = initialGs - mm.totalSteps = 20 + mm := &mockMachine{ + gs: initialGs, + totalSteps: 20, + } machStartIndex := uint64(0) stepSize := uint64(5) maxIterations := uint64(10) - e.cache = &MachineCache{ - buildingLock: make(chan struct{}, 1), - machines: []MachineInterface{mm}, - finalMachine: mm, - } - go func() { - <-time.After(time.Millisecond * 50) - e.cache.buildingLock <- struct{}{} - }() + e := &executionRun{ + cache: NewMachineCache(ctx, func(_ context.Context) (MachineInterface, error) { + return mm, nil + }, &DefaultMachineCacheConfig), + } + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err)