From 9c8b14646b026317b3fc3f339011768f3c261771 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Thu, 13 Jun 2024 16:24:36 -0500 Subject: [PATCH 01/29] get machine hashes with step size --- system_tests/validation_mock_test.go | 5 ++ validator/client/validation_client.go | 11 ++++ validator/interface.go | 1 + validator/server_arb/execution_run.go | 80 +++++++++++++++++++++++++++ validator/valnode/validation_api.go | 13 +++++ 5 files changed, 110 insertions(+) diff --git a/system_tests/validation_mock_test.go b/system_tests/validation_mock_test.go index fb4f868571..38beee021b 100644 --- a/system_tests/validation_mock_test.go +++ b/system_tests/validation_mock_test.go @@ -126,6 +126,11 @@ func (r *mockExecRun) GetStepAt(position uint64) containers.PromiseInterface[*va }, nil) } +func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numDesiredLeaves, fromBatch uint64) containers.PromiseInterface[[]common.Hash] { + // TODO: Add mock implementation for GetMachineHashesWithStepSize + return containers.NewReadyPromise[[]common.Hash](nil, nil) +} + func (r *mockExecRun) GetLastStep() containers.PromiseInterface[*validator.MachineStepResult] { return r.GetStepAt(mockExecLastPos) } diff --git a/validator/client/validation_client.go b/validator/client/validation_client.go index fa6b9000f2..c454cefa3c 100644 --- a/validator/client/validation_client.go +++ b/validator/client/validation_client.go @@ -197,6 +197,17 @@ func (r *ExecutionClientRun) GetStepAt(pos uint64) containers.PromiseInterface[* }) } +func (r *ExecutionClientRun) GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] { + return stopwaiter.LaunchPromiseThread[[]common.Hash](r, func(ctx context.Context) ([]common.Hash, error) { + var resJson []common.Hash + err := r.client.client.CallContext(ctx, &resJson, server_api.Namespace+"_getMachineHashesWithStepSize", r.id, fromBatch, machineStartIndex, stepSize, numDesiredLeaves) + if err != nil { + return nil, err + } + return resJson, err + }) +} + func (r *ExecutionClientRun) GetProofAt(pos uint64) containers.PromiseInterface[[]byte] { return stopwaiter.LaunchPromiseThread[[]byte](r, func(ctx context.Context) ([]byte, error) { var resString string diff --git a/validator/interface.go b/validator/interface.go index 0324b996ed..1ae0b2ac69 100644 --- a/validator/interface.go +++ b/validator/interface.go @@ -30,6 +30,7 @@ type ExecutionSpawner interface { type ExecutionRun interface { GetStepAt(uint64) containers.PromiseInterface[*MachineStepResult] + GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] GetLastStep() containers.PromiseInterface[*MachineStepResult] GetProofAt(uint64) containers.PromiseInterface[[]byte] PrepareRange(uint64, uint64) containers.PromiseInterface[struct{}] diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 255d42ab16..8893d0b3da 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -7,7 +7,10 @@ import ( "context" "fmt" "sync" + "time" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/log" "github.com/offchainlabs/nitro/util/containers" "github.com/offchainlabs/nitro/util/stopwaiter" "github.com/offchainlabs/nitro/validator" @@ -79,6 +82,83 @@ func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*v }) } +func (e *executionRun) GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] { + return stopwaiter.LaunchPromiseThread[[]common.Hash](e, func(ctx context.Context) ([]common.Hash, error) { + machine, err := e.cache.GetMachineAt(ctx, machineStartIndex) + if err != nil { + return nil, err + } + log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", machineStartIndex)) + // If the machine is starting at index 0, we always want to start at the "Machine finished" global state status + // to align with the machine hashes that the inbox machine will produce. + var machineHashes []common.Hash + + if machineStartIndex == 0 { + gs := machine.GetGlobalState() + log.Debug(fmt.Sprintf("Start global state for machine index 0: %+v", gs), "fromBatch", fromBatch) + machineHashes = append(machineHashes, machineFinishedHash(gs)) + } else { + // Otherwise, we simply append the machine hash at the specified start index. + machineHashes = append(machineHashes, machine.Hash()) + } + startHash := machineHashes[0] + + // If we only want 1 hash, we can return early. + if numDesiredLeaves == 1 { + return machineHashes, nil + } + + logInterval := numDesiredLeaves / 20 // Log every 5% progress + if logInterval == 0 { + logInterval = 1 + } + + start := time.Now() + for numIterations := uint64(0); numIterations < numDesiredLeaves; numIterations++ { + // The absolute opcode position the machine should be in after stepping. + position := machineStartIndex + stepSize*(numIterations+1) + + // Advance the machine in step size increments. + if err := machine.Step(ctx, stepSize); err != nil { + return nil, fmt.Errorf("failed to step machine to position %d: %w", position, err) + } + if numIterations%logInterval == 0 || numIterations == numDesiredLeaves-1 { + progressPercent := (float64(numIterations+1) / float64(numDesiredLeaves)) * 100 + log.Info( + fmt.Sprintf( + "Computing subchallenge progress: %.2f%% - %d of %d hashes needed", + progressPercent, + numIterations+1, + numDesiredLeaves, + ), + "fromBatch", fromBatch, + "machinePosition", numIterations*stepSize+machineStartIndex, + "timeSinceStart", time.Since(start), + "stepSize", stepSize, + "startHash", startHash, + "machineStartIndex", machineStartIndex, + "numDesiredLeaves", numDesiredLeaves, + ) + } + machineHashes = append(machineHashes, machine.Hash()) + if len(machineHashes) == int(numDesiredLeaves) { + break + } + } + log.Info( + "Successfully finished computing the data needed for opening a subchallenge", + "fromBatch", fromBatch, + "stepSize", stepSize, + "startHash", startHash, + "machineStartIndex", machineStartIndex, + "numDesiredLeaves", numDesiredLeaves, + "finishedHash", machineHashes[len(machineHashes)-1], + "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), + ) + return machineHashes, nil + }) +} + func (e *executionRun) GetProofAt(position uint64) containers.PromiseInterface[[]byte] { return stopwaiter.LaunchPromiseThread[[]byte](e, func(ctx context.Context) ([]byte, error) { machine, err := e.cache.GetMachineAt(ctx, position) diff --git a/validator/valnode/validation_api.go b/validator/valnode/validation_api.go index a67299b1a1..075ab44397 100644 --- a/validator/valnode/validation_api.go +++ b/validator/valnode/validation_api.go @@ -148,6 +148,19 @@ func (a *ExecServerAPI) GetStepAt(ctx context.Context, execid uint64, position u return server_api.MachineStepResultToJson(res), nil } +func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid, fromBatch, fromStep, stepSize, numDesiredLeaves uint64) ([]common.Hash, error) { + run, err := a.getRun(execid) + if err != nil { + return nil, err + } + leavesInRange := run.GetMachineHashesWithStepSize(fromBatch, fromStep, stepSize, numDesiredLeaves) + res, err := leavesInRange.Await(ctx) + if err != nil { + return nil, err + } + return res, nil +} + func (a *ExecServerAPI) GetProofAt(ctx context.Context, execid uint64, position uint64) (string, error) { run, err := a.getRun(execid) if err != nil { From 09c8b034d4add4386024de02597594fa71fced2f Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Thu, 13 Jun 2024 16:51:18 -0500 Subject: [PATCH 02/29] tests --- validator/server_arb/execution_run.go | 246 +++++++++++++-------- validator/server_arb/execution_run_test.go | 167 ++++++++++++++ 2 files changed, 317 insertions(+), 96 deletions(-) create mode 100644 validator/server_arb/execution_run_test.go diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 8893d0b3da..059957fedf 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -10,6 +10,8 @@ import ( "time" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/log" "github.com/offchainlabs/nitro/util/containers" "github.com/offchainlabs/nitro/util/stopwaiter" @@ -18,8 +20,10 @@ import ( type executionRun struct { stopwaiter.StopWaiter - cache *MachineCache - close sync.Once + cache *MachineCache + initialMachineGetter func(context.Context) (MachineInterface, error) + config *MachineCacheConfig + close sync.Once } // NewExecutionChallengeBackend creates a backend with the given arguments. @@ -31,6 +35,8 @@ func NewExecutionRun( ) (*executionRun, error) { exec := &executionRun{} exec.Start(ctxIn, exec) + exec.initialMachineGetter = initialMachineGetter + exec.config = config exec.cache = NewMachineCache(exec.GetContext(), initialMachineGetter, config) return exec, nil } @@ -53,110 +59,150 @@ func (e *executionRun) PrepareRange(start uint64, end uint64) containers.Promise func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*validator.MachineStepResult] { return stopwaiter.LaunchPromiseThread[*validator.MachineStepResult](e, func(ctx context.Context) (*validator.MachineStepResult, error) { - var machine MachineInterface - var err error - if position == ^uint64(0) { - machine, err = e.cache.GetFinalMachine(ctx) - } else { - // todo cache last machine - machine, err = e.cache.GetMachineAt(ctx, position) - } - if err != nil { - return nil, err - } - machineStep := machine.GetStepCount() - if position != machineStep { - machineRunning := machine.IsRunning() - if machineRunning || machineStep > position { - return nil, fmt.Errorf("machine is in wrong position want: %d, got: %d", position, machine.GetStepCount()) - } - - } - result := &validator.MachineStepResult{ - Position: machineStep, - Status: validator.MachineStatus(machine.Status()), - GlobalState: machine.GetGlobalState(), - Hash: machine.Hash(), - } - return result, nil + return e.intermediateGetStepAt(ctx, position) }) } func (e *executionRun) GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] { - return stopwaiter.LaunchPromiseThread[[]common.Hash](e, func(ctx context.Context) ([]common.Hash, error) { - machine, err := e.cache.GetMachineAt(ctx, machineStartIndex) - if err != nil { - return nil, err + return stopwaiter.LaunchPromiseThread(e, func(ctx context.Context) ([]common.Hash, error) { + return e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + startIndex: machineStartIndex, + fromBatch: fromBatch, + stepSize: stepSize, + requiredNumHashes: numDesiredLeaves, + }) + }) +} + +type GlobalStateGetter interface { + GetGlobalState() validator.GoGlobalState + HashStepper +} + +type HashStepper interface { + Step(ctx context.Context, stepCount uint64) error + Hash() common.Hash +} + +type machineHashesWithStepSizeArgs struct { + startIndex uint64 + fromBatch uint64 + stepSize uint64 + requiredNumHashes uint64 + getMachineAtIndex func(context.Context, uint64) (GlobalStateGetter, error) +} + +func (e *executionRun) machineHashesWithStepSize( + ctx context.Context, + args machineHashesWithStepSizeArgs, +) ([]common.Hash, error) { + if args.stepSize == 0 { + return nil, fmt.Errorf("step size cannot be 0") + } + if args.requiredNumHashes == 0 { + return nil, fmt.Errorf("required number of hashes cannot be 0") + } + machine, err := args.getMachineAtIndex(ctx, args.startIndex) + if err != nil { + return nil, err + } + log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", args.startIndex)) + + // If the machine is starting at index 0, we always want to start at the "Machine finished" global state status + // to align with the machine hashes that the inbox machine will produce. + var machineHashes []common.Hash + if args.startIndex == 0 { + gs := machine.GetGlobalState() + log.Debug(fmt.Sprintf("Start global state for machine index 0: %+v", gs), "fromBatch", args.fromBatch) + machineHashes = append(machineHashes, machineFinishedHash(gs)) + } else { + // Otherwise, we simply append the machine hash at the specified start index. + machineHashes = append(machineHashes, machine.Hash()) + } + startHash := machineHashes[0] + + // If we only want 1 hash, we can return early. + if args.requiredNumHashes == 1 { + return machineHashes, nil + } + + logInterval := args.requiredNumHashes / 20 // Log every 5% progress + if logInterval == 0 { + logInterval = 1 + } + + start := time.Now() + for numIterations := uint64(0); numIterations < args.requiredNumHashes; numIterations++ { + // The absolute program counter the machine should be in after stepping. + absoluteMachineIndex := args.startIndex + args.stepSize*(numIterations+1) + + // Advance the machine in step size increments. + if err := machine.Step(ctx, args.stepSize); err != nil { + return nil, fmt.Errorf("failed to step machine to position %d: %w", absoluteMachineIndex, err) } - log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", machineStartIndex)) - // If the machine is starting at index 0, we always want to start at the "Machine finished" global state status - // to align with the machine hashes that the inbox machine will produce. - var machineHashes []common.Hash - - if machineStartIndex == 0 { - gs := machine.GetGlobalState() - log.Debug(fmt.Sprintf("Start global state for machine index 0: %+v", gs), "fromBatch", fromBatch) - machineHashes = append(machineHashes, machineFinishedHash(gs)) - } else { - // Otherwise, we simply append the machine hash at the specified start index. - machineHashes = append(machineHashes, machine.Hash()) + if numIterations%logInterval == 0 || numIterations == args.requiredNumHashes-1 { + progressPercent := (float64(numIterations+1) / float64(args.requiredNumHashes)) * 100 + log.Info( + fmt.Sprintf( + "Computing BOLD subchallenge progress: %.2f%% - %d of %d hashes needed", + progressPercent, + numIterations+1, + args.requiredNumHashes, + ), + "fromBatch", args.fromBatch, + "machinePosition", numIterations*args.stepSize+args.startIndex, + "timeSinceStart", time.Since(start), + "stepSize", args.stepSize, + "startHash", startHash, + "machineStartIndex", args.startIndex, + "numDesiredLeaves", args.requiredNumHashes, + ) } - startHash := machineHashes[0] - - // If we only want 1 hash, we can return early. - if numDesiredLeaves == 1 { - return machineHashes, nil + machineHashes = append(machineHashes, machine.Hash()) + if uint64(len(machineHashes)) == args.requiredNumHashes { + break } + } + log.Info( + "Successfully finished computing the data needed for opening a subchallenge", + "fromBatch", args.fromBatch, + "stepSize", args.stepSize, + "startHash", startHash, + "machineStartIndex", args.startIndex, + "numDesiredLeaves", args.requiredNumHashes, + "finishedHash", machineHashes[len(machineHashes)-1], + "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), + ) + return machineHashes, nil +} - logInterval := numDesiredLeaves / 20 // Log every 5% progress - if logInterval == 0 { - logInterval = 1 +func (e *executionRun) intermediateGetStepAt(ctx context.Context, position uint64) (*validator.MachineStepResult, error) { + var machine MachineInterface + var err error + if position == ^uint64(0) { + machine, err = e.cache.GetFinalMachine(ctx) + } else { + // TODO(rauljordan): Cache last machine. + machine, err = e.cache.GetMachineAt(ctx, position) + } + if err != nil { + return nil, err + } + machineStep := machine.GetStepCount() + if position != machineStep { + machineRunning := machine.IsRunning() + if machineRunning || machineStep > position { + return nil, fmt.Errorf("machine is in wrong position want: %d, got: %d", position, machine.GetStepCount()) } - start := time.Now() - for numIterations := uint64(0); numIterations < numDesiredLeaves; numIterations++ { - // The absolute opcode position the machine should be in after stepping. - position := machineStartIndex + stepSize*(numIterations+1) - - // Advance the machine in step size increments. - if err := machine.Step(ctx, stepSize); err != nil { - return nil, fmt.Errorf("failed to step machine to position %d: %w", position, err) - } - if numIterations%logInterval == 0 || numIterations == numDesiredLeaves-1 { - progressPercent := (float64(numIterations+1) / float64(numDesiredLeaves)) * 100 - log.Info( - fmt.Sprintf( - "Computing subchallenge progress: %.2f%% - %d of %d hashes needed", - progressPercent, - numIterations+1, - numDesiredLeaves, - ), - "fromBatch", fromBatch, - "machinePosition", numIterations*stepSize+machineStartIndex, - "timeSinceStart", time.Since(start), - "stepSize", stepSize, - "startHash", startHash, - "machineStartIndex", machineStartIndex, - "numDesiredLeaves", numDesiredLeaves, - ) - } - machineHashes = append(machineHashes, machine.Hash()) - if len(machineHashes) == int(numDesiredLeaves) { - break - } - } - log.Info( - "Successfully finished computing the data needed for opening a subchallenge", - "fromBatch", fromBatch, - "stepSize", stepSize, - "startHash", startHash, - "machineStartIndex", machineStartIndex, - "numDesiredLeaves", numDesiredLeaves, - "finishedHash", machineHashes[len(machineHashes)-1], - "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), - ) - return machineHashes, nil - }) + } + result := &validator.MachineStepResult{ + Position: machineStep, + Status: validator.MachineStatus(machine.Status()), + GlobalState: machine.GetGlobalState(), + Hash: machine.Hash(), + } + return result, nil } func (e *executionRun) GetProofAt(position uint64) containers.PromiseInterface[[]byte] { @@ -172,3 +218,11 @@ func (e *executionRun) GetProofAt(position uint64) containers.PromiseInterface[[ func (e *executionRun) GetLastStep() containers.PromiseInterface[*validator.MachineStepResult] { return e.GetStepAt(^uint64(0)) } + +func (e *executionRun) CheckAlive(ctx context.Context) error { + return nil +} + +func machineFinishedHash(gs validator.GoGlobalState) common.Hash { + return crypto.Keccak256Hash([]byte("Machine finished:"), gs.Hash().Bytes()) +} diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go new file mode 100644 index 0000000000..c1f0e1ef50 --- /dev/null +++ b/validator/server_arb/execution_run_test.go @@ -0,0 +1,167 @@ +package server_arb + +import ( + "context" + "strings" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/offchainlabs/nitro/validator" +) + +var _ GlobalStateGetter = (*mockMachine)(nil) + +type mockMachine struct { + gs validator.GoGlobalState + totalSteps uint64 +} + +func (m *mockMachine) Hash() common.Hash { + if m.gs.PosInBatch == m.totalSteps-1 { + return machineFinishedHash(m.gs) + } + return m.gs.Hash() +} + +func (m *mockMachine) GetGlobalState() validator.GoGlobalState { + return m.gs +} + +func (m *mockMachine) Step(ctx context.Context, stepSize uint64) error { + for i := uint64(0); i < stepSize; i++ { + if m.gs.PosInBatch == m.totalSteps-1 { + return nil + } + m.gs.PosInBatch += 1 + } + return nil +} + +func Test_machineHashesWithStep(t *testing.T) { + mm := &mockMachine{} + e := &executionRun{} + ctx := context.Background() + + machGetter := func(ctx context.Context, index uint64) (GlobalStateGetter, error) { + return mm, nil + } + t.Run("basic argument checks", func(t *testing.T) { + _, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + stepSize: 0, + }) + if !strings.Contains(err.Error(), "step size cannot be 0") { + t.Fatal("Wrong error") + } + _, err = e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + stepSize: 1, + requiredNumHashes: 0, + }) + if !strings.Contains(err.Error(), "required number of hashes cannot be 0") { + t.Fatal("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, + } + hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + fromBatch: 0, + stepSize: 1, + requiredNumHashes: 1, + startIndex: 0, + getMachineAtIndex: machGetter, + }) + if err != nil { + t.Fatal(err) + } + expected := machineFinishedHash(mm.gs) + if len(hashes) != 1 { + t.Fatal("Wanted one hash") + } + if expected != hashes[0] { + t.Fatalf("Wanted %#x, got %#x", expected, hashes[0]) + } + }) + t.Run("can step in step size increments and collect hashes", func(t *testing.T) { + initialGs := validator.GoGlobalState{ + Batch: 1, + PosInBatch: 0, + } + mm.gs = initialGs + mm.totalSteps = 20 + stepSize := uint64(5) + hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + fromBatch: 1, + stepSize: stepSize, + requiredNumHashes: 4, + startIndex: 0, + getMachineAtIndex: machGetter, + }) + if err != nil { + t.Fatal(err) + } + expectedHashes := make([]common.Hash, 0) + for i := uint64(0); i < 4; i++ { + if i == 0 { + expectedHashes = append(expectedHashes, machineFinishedHash(initialGs)) + continue + } + gs := validator.GoGlobalState{ + Batch: 1, + PosInBatch: uint64(i * stepSize), + } + expectedHashes = append(expectedHashes, gs.Hash()) + } + if len(hashes) != len(expectedHashes) { + t.Fatal("Wanted one hash") + } + for i := range hashes { + if expectedHashes[i] != hashes[i] { + t.Fatalf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) + } + } + }) + t.Run("if finishes execution early, simply pads the remaining desired hashes with the machine finished hash", func(t *testing.T) { + initialGs := validator.GoGlobalState{ + Batch: 1, + PosInBatch: 0, + } + mm.gs = initialGs + mm.totalSteps = 20 + stepSize := uint64(5) + hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ + fromBatch: 1, + stepSize: stepSize, + requiredNumHashes: 10, + startIndex: 0, + getMachineAtIndex: machGetter, + }) + if err != nil { + t.Fatal(err) + } + expectedHashes := make([]common.Hash, 0) + for i := uint64(0); i < 4; i++ { + if i == 0 { + expectedHashes = append(expectedHashes, machineFinishedHash(initialGs)) + continue + } + gs := validator.GoGlobalState{ + Batch: 1, + PosInBatch: uint64(i * stepSize), + } + expectedHashes = append(expectedHashes, gs.Hash()) + } + // The rest of the expected hashes should be the machine finished hash repeated. + for i := uint64(4); i < 10; i++ { + expectedHashes = append(expectedHashes, machineFinishedHash(mm.gs)) + } + if len(hashes) != len(expectedHashes) { + t.Fatal("Wanted one hash") + } + for i := range hashes { + if expectedHashes[i] != hashes[i] { + t.Fatalf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) + } + } + }) +} From f9470c03bd2e9d8be1ca78f37618f5fda27ad719 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Thu, 13 Jun 2024 16:55:40 -0500 Subject: [PATCH 03/29] rem old --- validator/server_arb/execution_run.go | 68 +++++++++++---------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 059957fedf..c3f78e8963 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -20,10 +20,8 @@ import ( type executionRun struct { stopwaiter.StopWaiter - cache *MachineCache - initialMachineGetter func(context.Context) (MachineInterface, error) - config *MachineCacheConfig - close sync.Once + cache *MachineCache + close sync.Once } // NewExecutionChallengeBackend creates a backend with the given arguments. @@ -35,8 +33,6 @@ func NewExecutionRun( ) (*executionRun, error) { exec := &executionRun{} exec.Start(ctxIn, exec) - exec.initialMachineGetter = initialMachineGetter - exec.config = config exec.cache = NewMachineCache(exec.GetContext(), initialMachineGetter, config) return exec, nil } @@ -59,7 +55,32 @@ func (e *executionRun) PrepareRange(start uint64, end uint64) containers.Promise func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*validator.MachineStepResult] { return stopwaiter.LaunchPromiseThread[*validator.MachineStepResult](e, func(ctx context.Context) (*validator.MachineStepResult, error) { - return e.intermediateGetStepAt(ctx, position) + var machine MachineInterface + var err error + if position == ^uint64(0) { + machine, err = e.cache.GetFinalMachine(ctx) + } else { + // TODO(rauljordan): Cache last machine. + machine, err = e.cache.GetMachineAt(ctx, position) + } + if err != nil { + return nil, err + } + machineStep := machine.GetStepCount() + if position != machineStep { + machineRunning := machine.IsRunning() + if machineRunning || machineStep > position { + return nil, fmt.Errorf("machine is in wrong position want: %d, got: %d", position, machine.GetStepCount()) + } + + } + result := &validator.MachineStepResult{ + Position: machineStep, + Status: validator.MachineStatus(machine.Status()), + GlobalState: machine.GetGlobalState(), + Hash: machine.Hash(), + } + return result, nil }) } @@ -176,35 +197,6 @@ func (e *executionRun) machineHashesWithStepSize( return machineHashes, nil } -func (e *executionRun) intermediateGetStepAt(ctx context.Context, position uint64) (*validator.MachineStepResult, error) { - var machine MachineInterface - var err error - if position == ^uint64(0) { - machine, err = e.cache.GetFinalMachine(ctx) - } else { - // TODO(rauljordan): Cache last machine. - machine, err = e.cache.GetMachineAt(ctx, position) - } - if err != nil { - return nil, err - } - machineStep := machine.GetStepCount() - if position != machineStep { - machineRunning := machine.IsRunning() - if machineRunning || machineStep > position { - return nil, fmt.Errorf("machine is in wrong position want: %d, got: %d", position, machine.GetStepCount()) - } - - } - result := &validator.MachineStepResult{ - Position: machineStep, - Status: validator.MachineStatus(machine.Status()), - GlobalState: machine.GetGlobalState(), - Hash: machine.Hash(), - } - return result, nil -} - func (e *executionRun) GetProofAt(position uint64) containers.PromiseInterface[[]byte] { return stopwaiter.LaunchPromiseThread[[]byte](e, func(ctx context.Context) ([]byte, error) { machine, err := e.cache.GetMachineAt(ctx, position) @@ -219,10 +211,6 @@ func (e *executionRun) GetLastStep() containers.PromiseInterface[*validator.Mach return e.GetStepAt(^uint64(0)) } -func (e *executionRun) CheckAlive(ctx context.Context) error { - return nil -} - func machineFinishedHash(gs validator.GoGlobalState) common.Hash { return crypto.Keccak256Hash([]byte("Machine finished:"), gs.Hash().Bytes()) } From 18a2510b23d8832d5dc3f9cca649bf20373fd5cb Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Thu, 13 Jun 2024 16:59:14 -0500 Subject: [PATCH 04/29] done --- system_tests/validation_mock_test.go | 1 - validator/server_arb/execution_run.go | 1 - 2 files changed, 2 deletions(-) diff --git a/system_tests/validation_mock_test.go b/system_tests/validation_mock_test.go index 38beee021b..0731c6564b 100644 --- a/system_tests/validation_mock_test.go +++ b/system_tests/validation_mock_test.go @@ -127,7 +127,6 @@ func (r *mockExecRun) GetStepAt(position uint64) containers.PromiseInterface[*va } func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numDesiredLeaves, fromBatch uint64) containers.PromiseInterface[[]common.Hash] { - // TODO: Add mock implementation for GetMachineHashesWithStepSize return containers.NewReadyPromise[[]common.Hash](nil, nil) } diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index c3f78e8963..de60d987f2 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -60,7 +60,6 @@ func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*v if position == ^uint64(0) { machine, err = e.cache.GetFinalMachine(ctx) } else { - // TODO(rauljordan): Cache last machine. machine, err = e.cache.GetMachineAt(ctx, position) } if err != nil { From 71c0a3d57697a03ad004b27e7559ecc2b8cf5a9e Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 17 Jun 2024 11:03:02 -0500 Subject: [PATCH 05/29] tsahi feedback --- system_tests/validation_mock_test.go | 2 +- validator/client/validation_client.go | 4 +- validator/interface.go | 2 +- validator/server_arb/execution_run.go | 77 ++++++---------- validator/server_arb/execution_run_test.go | 101 ++++++++++++++------- validator/server_arb/machine_cache.go | 1 + validator/valnode/validation_api.go | 4 +- 7 files changed, 102 insertions(+), 89 deletions(-) diff --git a/system_tests/validation_mock_test.go b/system_tests/validation_mock_test.go index 0731c6564b..d80a2041ec 100644 --- a/system_tests/validation_mock_test.go +++ b/system_tests/validation_mock_test.go @@ -126,7 +126,7 @@ func (r *mockExecRun) GetStepAt(position uint64) containers.PromiseInterface[*va }, nil) } -func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numDesiredLeaves, fromBatch uint64) containers.PromiseInterface[[]common.Hash] { +func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] { return containers.NewReadyPromise[[]common.Hash](nil, nil) } diff --git a/validator/client/validation_client.go b/validator/client/validation_client.go index c454cefa3c..0f40ef0387 100644 --- a/validator/client/validation_client.go +++ b/validator/client/validation_client.go @@ -197,10 +197,10 @@ func (r *ExecutionClientRun) GetStepAt(pos uint64) containers.PromiseInterface[* }) } -func (r *ExecutionClientRun) GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] { +func (r *ExecutionClientRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] { return stopwaiter.LaunchPromiseThread[[]common.Hash](r, func(ctx context.Context) ([]common.Hash, error) { var resJson []common.Hash - err := r.client.client.CallContext(ctx, &resJson, server_api.Namespace+"_getMachineHashesWithStepSize", r.id, fromBatch, machineStartIndex, stepSize, numDesiredLeaves) + err := r.client.client.CallContext(ctx, &resJson, server_api.Namespace+"_getMachineHashesWithStepSize", r.id, machineStartIndex, stepSize, numRequiredHashes) if err != nil { return nil, err } diff --git a/validator/interface.go b/validator/interface.go index 1ae0b2ac69..58ad841ff5 100644 --- a/validator/interface.go +++ b/validator/interface.go @@ -30,7 +30,7 @@ type ExecutionSpawner interface { type ExecutionRun interface { GetStepAt(uint64) containers.PromiseInterface[*MachineStepResult] - GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] + GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] GetLastStep() containers.PromiseInterface[*MachineStepResult] GetProofAt(uint64) containers.PromiseInterface[[]byte] PrepareRange(uint64, uint64) containers.PromiseInterface[struct{}] diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index de60d987f2..1d765688d5 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -83,57 +83,36 @@ func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*v }) } -func (e *executionRun) GetMachineHashesWithStepSize(fromBatch, machineStartIndex, stepSize, numDesiredLeaves uint64) containers.PromiseInterface[[]common.Hash] { +func (e *executionRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, requiredNumHashes uint64) containers.PromiseInterface[[]common.Hash] { return stopwaiter.LaunchPromiseThread(e, func(ctx context.Context) ([]common.Hash, error) { - return e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - startIndex: machineStartIndex, - fromBatch: fromBatch, - stepSize: stepSize, - requiredNumHashes: numDesiredLeaves, - }) + return e.machineHashesWithStepSize(ctx, machineStartIndex, stepSize, requiredNumHashes) }) } -type GlobalStateGetter interface { - GetGlobalState() validator.GoGlobalState - HashStepper -} - -type HashStepper interface { - Step(ctx context.Context, stepCount uint64) error - Hash() common.Hash -} - -type machineHashesWithStepSizeArgs struct { - startIndex uint64 - fromBatch uint64 - stepSize uint64 - requiredNumHashes uint64 - getMachineAtIndex func(context.Context, uint64) (GlobalStateGetter, error) -} - func (e *executionRun) machineHashesWithStepSize( ctx context.Context, - args machineHashesWithStepSizeArgs, + machineStartIndex, + stepSize, + requiredNumHashes uint64, ) ([]common.Hash, error) { - if args.stepSize == 0 { + if stepSize == 0 { return nil, fmt.Errorf("step size cannot be 0") } - if args.requiredNumHashes == 0 { + if requiredNumHashes == 0 { return nil, fmt.Errorf("required number of hashes cannot be 0") } - machine, err := args.getMachineAtIndex(ctx, args.startIndex) + machine, err := e.cache.GetMachineAt(ctx, machineStartIndex) if err != nil { return nil, err } - log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", args.startIndex)) + log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", machineStartIndex)) // If the machine is starting at index 0, we always want to start at the "Machine finished" global state status // to align with the machine hashes that the inbox machine will produce. var machineHashes []common.Hash - if args.startIndex == 0 { + if machineStartIndex == 0 { gs := machine.GetGlobalState() - log.Debug(fmt.Sprintf("Start global state for machine index 0: %+v", gs), "fromBatch", args.fromBatch) + log.Debug(fmt.Sprintf("Start global state for machine index 0: %+v", gs)) machineHashes = append(machineHashes, machineFinishedHash(gs)) } else { // Otherwise, we simply append the machine hash at the specified start index. @@ -142,54 +121,52 @@ func (e *executionRun) machineHashesWithStepSize( startHash := machineHashes[0] // If we only want 1 hash, we can return early. - if args.requiredNumHashes == 1 { + if requiredNumHashes == 1 { return machineHashes, nil } - logInterval := args.requiredNumHashes / 20 // Log every 5% progress + logInterval := requiredNumHashes / 20 // Log every 5% progress if logInterval == 0 { logInterval = 1 } start := time.Now() - for numIterations := uint64(0); numIterations < args.requiredNumHashes; numIterations++ { + for numIterations := uint64(0); numIterations < requiredNumHashes; numIterations++ { // The absolute program counter the machine should be in after stepping. - absoluteMachineIndex := args.startIndex + args.stepSize*(numIterations+1) + absoluteMachineIndex := machineStartIndex + stepSize*(numIterations+1) // Advance the machine in step size increments. - if err := machine.Step(ctx, args.stepSize); err != nil { + if err := machine.Step(ctx, stepSize); err != nil { return nil, fmt.Errorf("failed to step machine to position %d: %w", absoluteMachineIndex, err) } - if numIterations%logInterval == 0 || numIterations == args.requiredNumHashes-1 { - progressPercent := (float64(numIterations+1) / float64(args.requiredNumHashes)) * 100 + if numIterations%logInterval == 0 || numIterations == requiredNumHashes-1 { + progressPercent := (float64(numIterations+1) / float64(requiredNumHashes)) * 100 log.Info( fmt.Sprintf( "Computing BOLD subchallenge progress: %.2f%% - %d of %d hashes needed", progressPercent, numIterations+1, - args.requiredNumHashes, + requiredNumHashes, ), - "fromBatch", args.fromBatch, - "machinePosition", numIterations*args.stepSize+args.startIndex, + "machinePosition", numIterations*stepSize+machineStartIndex, "timeSinceStart", time.Since(start), - "stepSize", args.stepSize, + "stepSize", stepSize, "startHash", startHash, - "machineStartIndex", args.startIndex, - "numDesiredLeaves", args.requiredNumHashes, + "machineStartIndex", machine, + "numDesiredLeaves", requiredNumHashes, ) } machineHashes = append(machineHashes, machine.Hash()) - if uint64(len(machineHashes)) == args.requiredNumHashes { + if uint64(len(machineHashes)) == requiredNumHashes { break } } log.Info( "Successfully finished computing the data needed for opening a subchallenge", - "fromBatch", args.fromBatch, - "stepSize", args.stepSize, + "stepSize", stepSize, "startHash", startHash, - "machineStartIndex", args.startIndex, - "numDesiredLeaves", args.requiredNumHashes, + "machineStartIndex", machineStartIndex, + "numDesiredLeaves", requiredNumHashes, "finishedHash", machineHashes[len(machineHashes)-1], "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), ) diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index c1f0e1ef50..24e401f351 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -4,13 +4,12 @@ import ( "context" "strings" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/offchainlabs/nitro/validator" ) -var _ GlobalStateGetter = (*mockMachine)(nil) - type mockMachine struct { gs validator.GoGlobalState totalSteps uint64 @@ -37,25 +36,45 @@ func (m *mockMachine) Step(ctx context.Context, stepSize uint64) error { return nil } +func (m *mockMachine) CloneMachineInterface() MachineInterface { + return m +} +func (m *mockMachine) GetStepCount() uint64 { + return m.totalSteps +} +func (m *mockMachine) IsRunning() bool { + return m.gs.PosInBatch < m.totalSteps-1 +} +func (m *mockMachine) ValidForStep(uint64) bool { + return true +} +func (m *mockMachine) Status() uint8 { + if m.gs.PosInBatch == m.totalSteps-1 { + return uint8(validator.MachineStatusFinished) + } + return uint8(validator.MachineStatusRunning) +} +func (m *mockMachine) ProveNextStep() []byte { + return nil +} +func (m *mockMachine) Freeze() {} +func (m *mockMachine) Destroy() {} + func Test_machineHashesWithStep(t *testing.T) { mm := &mockMachine{} e := &executionRun{} ctx := context.Background() - machGetter := func(ctx context.Context, index uint64) (GlobalStateGetter, error) { - return mm, nil - } t.Run("basic argument checks", func(t *testing.T) { - _, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - stepSize: 0, - }) + machStartIndex := uint64(0) + stepSize := uint64(0) + numRequiredHashes := uint64(0) + _, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if !strings.Contains(err.Error(), "step size cannot be 0") { t.Fatal("Wrong error") } - _, err = e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - stepSize: 1, - requiredNumHashes: 0, - }) + stepSize = uint64(1) + _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if !strings.Contains(err.Error(), "required number of hashes cannot be 0") { t.Fatal("Wrong error") } @@ -64,13 +83,19 @@ func Test_machineHashesWithStep(t *testing.T) { mm.gs = validator.GoGlobalState{ Batch: 1, } - hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - fromBatch: 0, - stepSize: 1, - requiredNumHashes: 1, - startIndex: 0, - getMachineAtIndex: machGetter, - }) + machStartIndex := uint64(0) + stepSize := uint64(1) + numRequiredHashes := 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{}{} + }() + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if err != nil { t.Fatal(err) } @@ -89,14 +114,19 @@ func Test_machineHashesWithStep(t *testing.T) { } mm.gs = initialGs mm.totalSteps = 20 + machStartIndex := uint64(0) stepSize := uint64(5) - hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - fromBatch: 1, - stepSize: stepSize, - requiredNumHashes: 4, - startIndex: 0, - getMachineAtIndex: machGetter, - }) + numRequiredHashes := 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{}{} + }() + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if err != nil { t.Fatal(err) } @@ -128,14 +158,19 @@ func Test_machineHashesWithStep(t *testing.T) { } mm.gs = initialGs mm.totalSteps = 20 + machStartIndex := uint64(0) stepSize := uint64(5) - hashes, err := e.machineHashesWithStepSize(ctx, machineHashesWithStepSizeArgs{ - fromBatch: 1, - stepSize: stepSize, - requiredNumHashes: 10, - startIndex: 0, - getMachineAtIndex: machGetter, - }) + numRequiredHashes := 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{}{} + }() + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if err != nil { t.Fatal(err) } diff --git a/validator/server_arb/machine_cache.go b/validator/server_arb/machine_cache.go index 23fcdef6d6..b656f1d0ee 100644 --- a/validator/server_arb/machine_cache.go +++ b/validator/server_arb/machine_cache.go @@ -124,6 +124,7 @@ func (c *MachineCache) lockBuild(ctx context.Context) error { return ctx.Err() case <-c.buildingLock: } + fmt.Println("Got past the lock") err := c.err if err != nil { c.buildingLock <- struct{}{} diff --git a/validator/valnode/validation_api.go b/validator/valnode/validation_api.go index 075ab44397..a2ca8cfc70 100644 --- a/validator/valnode/validation_api.go +++ b/validator/valnode/validation_api.go @@ -148,12 +148,12 @@ func (a *ExecServerAPI) GetStepAt(ctx context.Context, execid uint64, position u return server_api.MachineStepResultToJson(res), nil } -func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid, fromBatch, fromStep, stepSize, numDesiredLeaves uint64) ([]common.Hash, error) { +func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid, fromStep, stepSize, numRequiredHashes uint64) ([]common.Hash, error) { run, err := a.getRun(execid) if err != nil { return nil, err } - leavesInRange := run.GetMachineHashesWithStepSize(fromBatch, fromStep, stepSize, numDesiredLeaves) + leavesInRange := run.GetMachineHashesWithStepSize(fromStep, stepSize, numRequiredHashes) res, err := leavesInRange.Await(ctx) if err != nil { return nil, err From 65e1b573ebee8452b9850aa23948a15688106c66 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 17 Jun 2024 11:21:02 -0500 Subject: [PATCH 06/29] tests passing --- validator/server_arb/execution_run_test.go | 14 ++++++++++---- validator/server_arb/machine_cache.go | 1 - 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index 24e401f351..0f047db39e 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -37,10 +37,13 @@ func (m *mockMachine) Step(ctx context.Context, stepSize uint64) error { } func (m *mockMachine) CloneMachineInterface() MachineInterface { - return m + return &mockMachine{ + gs: validator.GoGlobalState{Batch: m.gs.Batch, PosInBatch: m.gs.PosInBatch}, + totalSteps: m.totalSteps, + } } func (m *mockMachine) GetStepCount() uint64 { - return m.totalSteps + return 0 } func (m *mockMachine) IsRunning() bool { return m.gs.PosInBatch < m.totalSteps-1 @@ -187,8 +190,11 @@ func Test_machineHashesWithStep(t *testing.T) { expectedHashes = append(expectedHashes, gs.Hash()) } // The rest of the expected hashes should be the machine finished hash repeated. - for i := uint64(4); i < 10; i++ { - expectedHashes = append(expectedHashes, machineFinishedHash(mm.gs)) + for len(expectedHashes) < 10 { + expectedHashes = append(expectedHashes, machineFinishedHash(validator.GoGlobalState{ + Batch: 1, + PosInBatch: mm.totalSteps - 1, + })) } if len(hashes) != len(expectedHashes) { t.Fatal("Wanted one hash") diff --git a/validator/server_arb/machine_cache.go b/validator/server_arb/machine_cache.go index b656f1d0ee..23fcdef6d6 100644 --- a/validator/server_arb/machine_cache.go +++ b/validator/server_arb/machine_cache.go @@ -124,7 +124,6 @@ func (c *MachineCache) lockBuild(ctx context.Context) error { return ctx.Err() case <-c.buildingLock: } - fmt.Println("Got past the lock") err := c.err if err != nil { c.buildingLock <- struct{}{} From f9484da8cf59cb54a9476018141e677f0ef1815a Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 17 Jun 2024 12:24:08 -0500 Subject: [PATCH 07/29] edit --- validator/server_arb/execution_run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 1d765688d5..50c9b5608e 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -152,7 +152,7 @@ func (e *executionRun) machineHashesWithStepSize( "timeSinceStart", time.Since(start), "stepSize", stepSize, "startHash", startHash, - "machineStartIndex", machine, + "machineStartIndex", machineStartIndex, "numDesiredLeaves", requiredNumHashes, ) } From 5ccda16f22be321356cb9d8f04825640e40ff579 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Mon, 17 Jun 2024 19:15:25 -0700 Subject: [PATCH 08/29] Deprecate local-db-storage, add migration opt The local DB storage backend, based on badgerdb, is being deprecated due to its unfavorable memory performance when compacting. local-file-storage is now the recommended local storage instead. To migrate to it, use --data-availability.migrate-local-db-to-file-storage and enable the local-file-storage service. Migration must be enabled manually using this flag since the operator may need to set up a new storage directory and backing volume. In-process migration is only provided from local-db-storage to local-file-storage since it is the only other local storage backend we current provide. To migrate to other backends, start a new daserver instance and use the rest-aggregator.sync-to-storage options. In the case where both S3 and DB backends were being used, it is recommended to migrate to the file backend and retain S3 instead of just dropping the DB backend configuration, otherwise there would no longer be a local store. ERROR and WARN messages on startup will inform operators that they should migrate (but can continue anyway), if migration failed, or if migration had previously succeeded and config/storage cleanup is recommended. These messages will be made fatal in the future before the local-db-storage backend code is removed entirely for the full deprecation. --- das/das.go | 3 ++ das/db_storage_service.go | 96 ++++++++++++++++++++++++++++++++++++++- das/factory.go | 36 ++++++++++----- 3 files changed, 122 insertions(+), 13 deletions(-) diff --git a/das/das.go b/das/das.go index fea1e6c6a2..658a5178c2 100644 --- a/das/das.go +++ b/das/das.go @@ -45,6 +45,8 @@ type DataAvailabilityConfig struct { LocalFileStorage LocalFileStorageConfig `koanf:"local-file-storage"` S3Storage S3StorageServiceConfig `koanf:"s3-storage"` + MigrateLocalDBToFileStorage bool `koanf:"migrate-local-db-to-file-storage"` + Key KeyConfig `koanf:"key"` RPCAggregator AggregatorConfig `koanf:"rpc-aggregator"` @@ -112,6 +114,7 @@ func dataAvailabilityConfigAddOptions(prefix string, f *flag.FlagSet, r role) { LocalDBStorageConfigAddOptions(prefix+".local-db-storage", f) LocalFileStorageConfigAddOptions(prefix+".local-file-storage", f) S3ConfigAddOptions(prefix+".s3-storage", f) + f.Bool(prefix+".migrate-local-db-to-file-storage", DefaultDataAvailabilityConfig.MigrateLocalDBToFileStorage, "daserver will migrate all data on startup from local-db-storage to local-file-storage, then mark local-db-storage as unusable") // Key config for storage KeyConfigAddOptions(prefix+".key", f) diff --git a/das/db_storage_service.go b/das/db_storage_service.go index 0fbe1c2723..e3b6183c37 100644 --- a/das/db_storage_service.go +++ b/das/db_storage_service.go @@ -7,6 +7,9 @@ import ( "bytes" "context" "errors" + "fmt" + "os" + "path/filepath" "time" badger "github.com/dgraph-io/badger/v4" @@ -35,6 +38,8 @@ type LocalDBStorageConfig struct { var badgerDefaultOptions = badger.DefaultOptions("") +const migratedMarker = "MIGRATED" + var DefaultLocalDBStorageConfig = LocalDBStorageConfig{ Enable: false, DataDir: "", @@ -49,7 +54,7 @@ var DefaultLocalDBStorageConfig = LocalDBStorageConfig{ } func LocalDBStorageConfigAddOptions(prefix string, f *flag.FlagSet) { - f.Bool(prefix+".enable", DefaultLocalDBStorageConfig.Enable, "enable storage/retrieval of sequencer batch data from a database on the local filesystem") + f.Bool(prefix+".enable", DefaultLocalDBStorageConfig.Enable, "!!!DEPRECATED, USE local-file-storage!!! enable storage/retrieval of sequencer batch data from a database on the local filesystem") f.String(prefix+".data-dir", DefaultLocalDBStorageConfig.DataDir, "directory in which to store the database") f.Bool(prefix+".discard-after-timeout", DefaultLocalDBStorageConfig.DiscardAfterTimeout, "discard data after its expiry timeout") @@ -69,7 +74,17 @@ type DBStorageService struct { stopWaiter stopwaiter.StopWaiterSafe } -func NewDBStorageService(ctx context.Context, config *LocalDBStorageConfig) (StorageService, error) { +// The DBStorageService is deprecated. This function will migrate data to the target +// LocalFileStorageService if it is provided and migration hasn't already happened. +func NewDBStorageService(ctx context.Context, config *LocalDBStorageConfig, target *LocalFileStorageService) (*DBStorageService, error) { + if alreadyMigrated(config.DataDir) { + log.Warn("local-db-storage already migrated, please remove it from the daserver configuration and restart. data-dir can be cleaned up manually now") + return nil, nil + } + if target == nil { + log.Error("local-db-storage is DEPRECATED, please use use the local-file-storage and migrate-local-db-to-file-storage options. This error will be made fatal in future, continuing for now...") + } + options := badger.DefaultOptions(config.DataDir). WithNumMemtables(config.NumMemtables). WithNumLevelZeroTables(config.NumLevelZeroTables). @@ -87,9 +102,21 @@ func NewDBStorageService(ctx context.Context, config *LocalDBStorageConfig) (Sto discardAfterTimeout: config.DiscardAfterTimeout, dirPath: config.DataDir, } + + if target != nil { + if err = ret.migrateTo(ctx, target); err != nil { + return nil, fmt.Errorf("error migrating local-db-storage to %s: %w", target, err) + } + if err = ret.setMigrated(); err != nil { + return nil, fmt.Errorf("error finalizing migration of local-db-storage to %s: %w", target, err) + } + return nil, nil + } + if err := ret.stopWaiter.Start(ctx, ret); err != nil { return nil, err } + err = ret.stopWaiter.LaunchThreadSafe(func(myCtx context.Context) { ticker := time.NewTicker(5 * time.Minute) defer ticker.Stop() @@ -152,6 +179,48 @@ func (dbs *DBStorageService) Put(ctx context.Context, data []byte, timeout uint6 }) } +func (dbs *DBStorageService) migrateTo(ctx context.Context, s StorageService) error { + originExpirationPolicy, err := dbs.ExpirationPolicy(ctx) + if err != nil { + return err + } + targetExpirationPolicy, err := s.ExpirationPolicy(ctx) + if err != nil { + return err + } + + if originExpirationPolicy == daprovider.KeepForever && targetExpirationPolicy == daprovider.DiscardAfterDataTimeout { + return errors.New("can't migrate from DBStorageService to target, incompatible expiration policies - can't migrate from non-expiring to expiring since non-expiring DB lacks expiry time metadata") + } + + return dbs.db.View(func(txn *badger.Txn) error { + opts := badger.DefaultIteratorOptions + it := txn.NewIterator(opts) + defer it.Close() + log.Info("Migrating from DBStorageService", "target", s) + migrationStart := time.Now() + count := 0 + for it.Rewind(); it.Valid(); it.Next() { + if count%1000 == 0 { + log.Info("Migration in progress", "migrated", count) + } + item := it.Item() + k := item.Key() + expiry := item.ExpiresAt() + err := item.Value(func(v []byte) error { + log.Trace("migrated", "key", pretty.FirstFewBytes(k), "value", pretty.FirstFewBytes(v), "expiry", expiry) + return s.Put(ctx, v, expiry) + }) + if err != nil { + return err + } + count++ + } + log.Info("Migration from DBStorageService complete", "target", s, "migrated", count, "duration", time.Since(migrationStart)) + return nil + }) +} + func (dbs *DBStorageService) Sync(ctx context.Context) error { return dbs.db.Sync() } @@ -160,6 +229,29 @@ func (dbs *DBStorageService) Close(ctx context.Context) error { return dbs.stopWaiter.StopAndWait() } +func alreadyMigrated(dirPath string) bool { + migratedMarkerFile := filepath.Join(dirPath, migratedMarker) + _, err := os.Stat(migratedMarkerFile) + if os.IsNotExist(err) { + return false + } + if err != nil { + log.Error("error checking if local-db-storage is already migrated", "err", err) + return false + } + return true +} + +func (dbs *DBStorageService) setMigrated() error { + migratedMarkerFile := filepath.Join(dbs.dirPath, migratedMarker) + file, err := os.OpenFile(migratedMarkerFile, os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return err + } + file.Close() + return nil +} + func (dbs *DBStorageService) ExpirationPolicy(ctx context.Context) (daprovider.ExpirationPolicy, error) { if dbs.discardAfterTimeout { return daprovider.DiscardAfterDataTimeout, nil diff --git a/das/factory.go b/das/factory.go index 8f6432234d..d640cee9fb 100644 --- a/das/factory.go +++ b/das/factory.go @@ -25,26 +25,36 @@ func CreatePersistentStorageService( ) (StorageService, *LifecycleManager, error) { storageServices := make([]StorageService, 0, 10) var lifecycleManager LifecycleManager - if config.LocalDBStorage.Enable { - s, err := NewDBStorageService(ctx, &config.LocalDBStorage) + var err error + + var fs *LocalFileStorageService + if config.LocalFileStorage.Enable { + fs, err = NewLocalFileStorageService(config.LocalFileStorage) if err != nil { return nil, nil, err } - lifecycleManager.Register(s) - storageServices = append(storageServices, s) - } - - if config.LocalFileStorage.Enable { - s, err := NewLocalFileStorageService(config.LocalFileStorage) + err = fs.start(ctx) if err != nil { return nil, nil, err } - err = s.start(ctx) + lifecycleManager.Register(fs) + storageServices = append(storageServices, fs) + } + + if config.LocalDBStorage.Enable { + var s *DBStorageService + if config.MigrateLocalDBToFileStorage { + s, err = NewDBStorageService(ctx, &config.LocalDBStorage, fs) + } else { + s, err = NewDBStorageService(ctx, &config.LocalDBStorage, nil) + } if err != nil { return nil, nil, err } - lifecycleManager.Register(s) - storageServices = append(storageServices, s) + if s != nil { + lifecycleManager.Register(s) + storageServices = append(storageServices, s) + } } if config.S3Storage.Enable { @@ -67,6 +77,10 @@ func CreatePersistentStorageService( if len(storageServices) == 1 { return storageServices[0], &lifecycleManager, nil } + if len(storageServices) == 0 { + return nil, nil, errors.New("No data-availability storage backend has been configured") + } + return nil, &lifecycleManager, nil } From f551f1f7c9360f85ab1e958be567c125e58fd7c6 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Tue, 18 Jun 2024 17:22:13 -0500 Subject: [PATCH 09/29] Don't post a batch that would cause a reorg due to being near the layer 1 minimum block or timestamp bounds --- arbnode/batch_poster.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 60693689fe..5c35c25e2d 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -162,6 +162,7 @@ type BatchPosterConfig struct { UseAccessLists bool `koanf:"use-access-lists" reload:"hot"` GasEstimateBaseFeeMultipleBips arbmath.Bips `koanf:"gas-estimate-base-fee-multiple-bips"` Dangerous BatchPosterDangerousConfig `koanf:"dangerous"` + ReorgResistanceMargin time.Duration `koanf:"reorg-resistance-margin" reload:"hot"` gasRefunder common.Address l1BlockBound l1BlockBound @@ -213,6 +214,7 @@ func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Duration(prefix+".l1-block-bound-bypass", DefaultBatchPosterConfig.L1BlockBoundBypass, "post batches even if not within the layer 1 future bounds if we're within this margin of the max delay") f.Bool(prefix+".use-access-lists", DefaultBatchPosterConfig.UseAccessLists, "post batches with access lists to reduce gas usage (disabled for L3s)") f.Uint64(prefix+".gas-estimate-base-fee-multiple-bips", uint64(DefaultBatchPosterConfig.GasEstimateBaseFeeMultipleBips), "for gas estimation, use this multiple of the basefee (measured in basis points) as the max fee per gas") + f.Duration(prefix+".reorg-resistance-margin", DefaultBatchPosterConfig.ReorgResistanceMargin, "do not post batch if its over the layer 1 minimum bounds but within this duration from them") redislock.AddConfigOptions(prefix+".redis-lock", f) dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f, dataposter.DefaultDataPosterConfig) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultBatchPosterConfig.ParentChainWallet.Pathname) @@ -242,6 +244,7 @@ var DefaultBatchPosterConfig = BatchPosterConfig{ UseAccessLists: true, RedisLock: redislock.DefaultCfg, GasEstimateBaseFeeMultipleBips: arbmath.OneInBips * 3 / 2, + ReorgResistanceMargin: 10 * time.Minute, } var DefaultBatchPosterL1WalletConfig = genericconf.WalletConfig{ @@ -1238,7 +1241,24 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) b.building.msgCount++ } - if !forcePostBatch || !b.building.haveUsefulMessage { + var disablePosting bool + firstMsgTimeStamp := firstMsg.Message.Header.Timestamp + firstMsgBlockNumber := firstMsg.Message.Header.BlockNumber + batchNearL1BoundMinTimestamp := firstMsgTimeStamp >= l1BoundMinTimestamp && firstMsgTimeStamp <= l1BoundMinTimestamp+uint64(config.ReorgResistanceMargin/time.Second) + batchNearL1BoundMinBlockNumber := firstMsgBlockNumber >= l1BoundMinBlockNumber && firstMsgBlockNumber <= l1BoundMinBlockNumber+uint64(config.ReorgResistanceMargin/ethPosBlockTime) + if config.ReorgResistanceMargin > 0 && (batchNearL1BoundMinTimestamp || batchNearL1BoundMinBlockNumber) { + log.Error( + "Disabling batch posting due to batch being within reorg resistance margin from layer 1 minimum block or timestamp bounds", + "reorgResistanceMargin", config.ReorgResistanceMargin, + "firstMsgTimeStamp", firstMsgTimeStamp, + "l1BoundMinTimestamp", l1BoundMinTimestamp, + "firstMsgBlockNumber", firstMsgBlockNumber, + "l1BoundMinBlockNumber", l1BoundMinBlockNumber, + ) + disablePosting = true + } + + if disablePosting || !forcePostBatch || !b.building.haveUsefulMessage { // the batch isn't full yet and we've posted a batch recently // don't post anything for now return false, nil From d13e42ba1f3e4a3e9fe29737272ccb3564e8a345 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 20 Jun 2024 10:26:25 -0500 Subject: [PATCH 10/29] address PR comments --- arbnode/batch_poster.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 5c35c25e2d..779958865d 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -1241,11 +1241,10 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) b.building.msgCount++ } - var disablePosting bool firstMsgTimeStamp := firstMsg.Message.Header.Timestamp firstMsgBlockNumber := firstMsg.Message.Header.BlockNumber - batchNearL1BoundMinTimestamp := firstMsgTimeStamp >= l1BoundMinTimestamp && firstMsgTimeStamp <= l1BoundMinTimestamp+uint64(config.ReorgResistanceMargin/time.Second) - batchNearL1BoundMinBlockNumber := firstMsgBlockNumber >= l1BoundMinBlockNumber && firstMsgBlockNumber <= l1BoundMinBlockNumber+uint64(config.ReorgResistanceMargin/ethPosBlockTime) + batchNearL1BoundMinTimestamp := firstMsgTimeStamp <= l1BoundMinTimestamp+uint64(config.ReorgResistanceMargin/time.Second) + batchNearL1BoundMinBlockNumber := firstMsgBlockNumber <= l1BoundMinBlockNumber+uint64(config.ReorgResistanceMargin/ethPosBlockTime) if config.ReorgResistanceMargin > 0 && (batchNearL1BoundMinTimestamp || batchNearL1BoundMinBlockNumber) { log.Error( "Disabling batch posting due to batch being within reorg resistance margin from layer 1 minimum block or timestamp bounds", @@ -1255,10 +1254,10 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) "firstMsgBlockNumber", firstMsgBlockNumber, "l1BoundMinBlockNumber", l1BoundMinBlockNumber, ) - disablePosting = true + return false, errors.New("batch is within reorg resistance margin from layer 1 minimum block or timestamp bounds") } - if disablePosting || !forcePostBatch || !b.building.haveUsefulMessage { + if !forcePostBatch || !b.building.haveUsefulMessage { // the batch isn't full yet and we've posted a batch recently // don't post anything for now return false, nil From ae1dbcc22e9e57c772889a6eae12a864b46ce782 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Thu, 20 Jun 2024 10:29:37 -0500 Subject: [PATCH 11/29] fix flag description --- arbnode/batch_poster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 779958865d..2d788ac686 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -214,7 +214,7 @@ func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Duration(prefix+".l1-block-bound-bypass", DefaultBatchPosterConfig.L1BlockBoundBypass, "post batches even if not within the layer 1 future bounds if we're within this margin of the max delay") f.Bool(prefix+".use-access-lists", DefaultBatchPosterConfig.UseAccessLists, "post batches with access lists to reduce gas usage (disabled for L3s)") f.Uint64(prefix+".gas-estimate-base-fee-multiple-bips", uint64(DefaultBatchPosterConfig.GasEstimateBaseFeeMultipleBips), "for gas estimation, use this multiple of the basefee (measured in basis points) as the max fee per gas") - f.Duration(prefix+".reorg-resistance-margin", DefaultBatchPosterConfig.ReorgResistanceMargin, "do not post batch if its over the layer 1 minimum bounds but within this duration from them") + f.Duration(prefix+".reorg-resistance-margin", DefaultBatchPosterConfig.ReorgResistanceMargin, "do not post batch if its within this duration from layer 1 minimum bounds ") redislock.AddConfigOptions(prefix+".redis-lock", f) dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f, dataposter.DefaultDataPosterConfig) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultBatchPosterConfig.ParentChainWallet.Pathname) From 5ed59b6ddd6350f7f3a9ff5d8d1b5306abab0666 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 09:07:33 -0500 Subject: [PATCH 12/29] feedback --- validator/server_arb/execution_run_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index 0f047db39e..f5620b7aa1 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -74,12 +74,12 @@ func Test_machineHashesWithStep(t *testing.T) { numRequiredHashes := uint64(0) _, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if !strings.Contains(err.Error(), "step size cannot be 0") { - t.Fatal("Wrong error") + t.Error("Wrong error") } stepSize = uint64(1) _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) if !strings.Contains(err.Error(), "required number of hashes cannot be 0") { - t.Fatal("Wrong error") + t.Error("Wrong error") } }) t.Run("machine at start index 0 hash is the finished state hash", func(t *testing.T) { @@ -104,10 +104,10 @@ func Test_machineHashesWithStep(t *testing.T) { } expected := machineFinishedHash(mm.gs) if len(hashes) != 1 { - t.Fatal("Wanted one hash") + t.Error("Wanted one hash") } if expected != hashes[0] { - t.Fatalf("Wanted %#x, got %#x", expected, hashes[0]) + t.Errorf("Wanted %#x, got %#x", expected, hashes[0]) } }) t.Run("can step in step size increments and collect hashes", func(t *testing.T) { @@ -150,7 +150,7 @@ func Test_machineHashesWithStep(t *testing.T) { } for i := range hashes { if expectedHashes[i] != hashes[i] { - t.Fatalf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) + t.Errorf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) } } }) @@ -201,7 +201,7 @@ func Test_machineHashesWithStep(t *testing.T) { } for i := range hashes { if expectedHashes[i] != hashes[i] { - t.Fatalf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) + t.Errorf("Wanted at index %d, %#x, got %#x", i, expectedHashes[i], hashes[i]) } } }) From f7ed4f0cdd06c31daadbfc40c08aa01868dfaa21 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 09:26:53 -0500 Subject: [PATCH 13/29] feedback --- validator/server_arb/execution_run.go | 39 +++++++++++++-------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 50c9b5608e..89aed3fcf4 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -83,9 +83,9 @@ func (e *executionRun) GetStepAt(position uint64) containers.PromiseInterface[*v }) } -func (e *executionRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, requiredNumHashes uint64) containers.PromiseInterface[[]common.Hash] { +func (e *executionRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, maxIterations uint64) containers.PromiseInterface[[]common.Hash] { return stopwaiter.LaunchPromiseThread(e, func(ctx context.Context) ([]common.Hash, error) { - return e.machineHashesWithStepSize(ctx, machineStartIndex, stepSize, requiredNumHashes) + return e.machineHashesWithStepSize(ctx, machineStartIndex, stepSize, maxIterations) }) } @@ -93,13 +93,13 @@ func (e *executionRun) machineHashesWithStepSize( ctx context.Context, machineStartIndex, stepSize, - requiredNumHashes uint64, + maxIterations uint64, ) ([]common.Hash, error) { if stepSize == 0 { return nil, fmt.Errorf("step size cannot be 0") } - if requiredNumHashes == 0 { - return nil, fmt.Errorf("required number of hashes cannot be 0") + if maxIterations == 0 { + return nil, fmt.Errorf("max number of iterations cannot be 0") } machine, err := e.cache.GetMachineAt(ctx, machineStartIndex) if err != nil { @@ -107,8 +107,7 @@ func (e *executionRun) machineHashesWithStepSize( } log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", machineStartIndex)) - // If the machine is starting at index 0, we always want to start at the "Machine finished" global state status - // to align with the machine hashes that the inbox machine will produce. + // A var machineHashes []common.Hash if machineStartIndex == 0 { gs := machine.GetGlobalState() @@ -121,43 +120,43 @@ func (e *executionRun) machineHashesWithStepSize( startHash := machineHashes[0] // If we only want 1 hash, we can return early. - if requiredNumHashes == 1 { + if maxIterations == 1 { return machineHashes, nil } - logInterval := requiredNumHashes / 20 // Log every 5% progress + logInterval := maxIterations / 20 // Log every 5% progress if logInterval == 0 { logInterval = 1 } start := time.Now() - for numIterations := uint64(0); numIterations < requiredNumHashes; numIterations++ { + for i := uint64(0); i < maxIterations; i++ { // The absolute program counter the machine should be in after stepping. - absoluteMachineIndex := machineStartIndex + stepSize*(numIterations+1) + absoluteMachineIndex := machineStartIndex + stepSize*(i+1) // Advance the machine in step size increments. if err := machine.Step(ctx, stepSize); err != nil { return nil, fmt.Errorf("failed to step machine to position %d: %w", absoluteMachineIndex, err) } - if numIterations%logInterval == 0 || numIterations == requiredNumHashes-1 { - progressPercent := (float64(numIterations+1) / float64(requiredNumHashes)) * 100 + if i%logInterval == 0 || i == maxIterations-1 { + progressPercent := (float64(i+1) / float64(maxIterations)) * 100 log.Info( fmt.Sprintf( - "Computing BOLD subchallenge progress: %.2f%% - %d of %d hashes needed", + "Computing BOLD subchallenge progress: %.2f%% - %d of %d hashes", progressPercent, - numIterations+1, - requiredNumHashes, + i+1, + maxIterations, ), - "machinePosition", numIterations*stepSize+machineStartIndex, + "machinePosition", i*stepSize+machineStartIndex, "timeSinceStart", time.Since(start), "stepSize", stepSize, "startHash", startHash, "machineStartIndex", machineStartIndex, - "numDesiredLeaves", requiredNumHashes, + "maxIterations", maxIterations, ) } machineHashes = append(machineHashes, machine.Hash()) - if uint64(len(machineHashes)) == requiredNumHashes { + if uint64(len(machineHashes)) == maxIterations { break } } @@ -166,7 +165,7 @@ func (e *executionRun) machineHashesWithStepSize( "stepSize", stepSize, "startHash", startHash, "machineStartIndex", machineStartIndex, - "numDesiredLeaves", requiredNumHashes, + "maxIterations", maxIterations, "finishedHash", machineHashes[len(machineHashes)-1], "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), ) From 85d0e8de64e54d3004cd1aa9f76f73c6f435761d Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 09:58:02 -0500 Subject: [PATCH 14/29] commentary --- validator/server_arb/execution_run.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/validator/server_arb/execution_run.go b/validator/server_arb/execution_run.go index 89aed3fcf4..8bdce145a2 100644 --- a/validator/server_arb/execution_run.go +++ b/validator/server_arb/execution_run.go @@ -107,7 +107,10 @@ func (e *executionRun) machineHashesWithStepSize( } log.Debug(fmt.Sprintf("Advanced machine to index %d, beginning hash computation", machineStartIndex)) - // A + // In BOLD, the hash of a machine at index 0 is a special hash that is computed as the + // `machineFinishedHash(gs)` where `gs` is the global state of the machine at index 0. + // This is so that the hash aligns with the start state of the claimed challenge edge + // at the level above, as required by the BOLD protocol. var machineHashes []common.Hash if machineStartIndex == 0 { gs := machine.GetGlobalState() @@ -157,6 +160,11 @@ func (e *executionRun) machineHashesWithStepSize( } machineHashes = append(machineHashes, machine.Hash()) if uint64(len(machineHashes)) == maxIterations { + log.Info("Reached the max number of iterations for the hashes needed to open a subchallenge") + break + } + if !machine.IsRunning() { + log.Info("Machine no longer running, exiting early from hash computation loop") break } } @@ -165,6 +173,7 @@ func (e *executionRun) machineHashesWithStepSize( "stepSize", stepSize, "startHash", startHash, "machineStartIndex", machineStartIndex, + "numberOfHashesComputed", len(machineHashes), "maxIterations", maxIterations, "finishedHash", machineHashes[len(machineHashes)-1], "finishedGlobalState", fmt.Sprintf("%+v", machine.GetGlobalState()), From bf60a37dde49cb2fbe3d0493c3fe72ac1f7848d4 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 10:14:50 -0500 Subject: [PATCH 15/29] rename --- validator/server_arb/execution_run_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index f5620b7aa1..3188c84a32 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -154,7 +154,7 @@ func Test_machineHashesWithStep(t *testing.T) { } } }) - t.Run("if finishes execution early, simply pads the remaining desired hashes with the machine finished hash", func(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) { initialGs := validator.GoGlobalState{ Batch: 1, PosInBatch: 0, @@ -189,15 +189,8 @@ func Test_machineHashesWithStep(t *testing.T) { } expectedHashes = append(expectedHashes, gs.Hash()) } - // The rest of the expected hashes should be the machine finished hash repeated. - for len(expectedHashes) < 10 { - expectedHashes = append(expectedHashes, machineFinishedHash(validator.GoGlobalState{ - Batch: 1, - PosInBatch: mm.totalSteps - 1, - })) - } - if len(hashes) != len(expectedHashes) { - t.Fatal("Wanted one hash") + if len(hashes) >= int(numRequiredHashes) { + t.Fatal("Wanted fewer hashes than the max iterations") } for i := range hashes { if expectedHashes[i] != hashes[i] { From 27e4a826816db2ae59f3eb1f1e0cc84034504d68 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 10:46:40 -0500 Subject: [PATCH 16/29] commentary --- validator/server_arb/execution_run_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index 3188c84a32..b5332e108e 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -78,7 +78,7 @@ func Test_machineHashesWithStep(t *testing.T) { } stepSize = uint64(1) _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) - if !strings.Contains(err.Error(), "required number of hashes cannot be 0") { + if !strings.Contains(err.Error(), "number of iterations cannot be 0") { t.Error("Wrong error") } }) @@ -189,6 +189,10 @@ func Test_machineHashesWithStep(t *testing.T) { } expectedHashes = append(expectedHashes, gs.Hash()) } + expectedHashes = append(expectedHashes, machineFinishedHash(validator.GoGlobalState{ + Batch: 1, + PosInBatch: mm.totalSteps - 1, + })) if len(hashes) >= int(numRequiredHashes) { t.Fatal("Wanted fewer hashes than the max iterations") } From 1eff6fb335d8122383e21ae36c4ba36d2b5a11cb Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 10:47:29 -0500 Subject: [PATCH 17/29] replace --- system_tests/validation_mock_test.go | 2 +- validator/client/validation_client.go | 4 ++-- validator/interface.go | 2 +- validator/server_arb/execution_run_test.go | 20 ++++++++++---------- validator/valnode/validation_api.go | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/system_tests/validation_mock_test.go b/system_tests/validation_mock_test.go index d80a2041ec..e61fd407ec 100644 --- a/system_tests/validation_mock_test.go +++ b/system_tests/validation_mock_test.go @@ -126,7 +126,7 @@ func (r *mockExecRun) GetStepAt(position uint64) containers.PromiseInterface[*va }, nil) } -func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] { +func (r *mockExecRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, maxIterations uint64) containers.PromiseInterface[[]common.Hash] { return containers.NewReadyPromise[[]common.Hash](nil, nil) } diff --git a/validator/client/validation_client.go b/validator/client/validation_client.go index 0f40ef0387..949260002d 100644 --- a/validator/client/validation_client.go +++ b/validator/client/validation_client.go @@ -197,10 +197,10 @@ func (r *ExecutionClientRun) GetStepAt(pos uint64) containers.PromiseInterface[* }) } -func (r *ExecutionClientRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] { +func (r *ExecutionClientRun) GetMachineHashesWithStepSize(machineStartIndex, stepSize, maxIterations uint64) containers.PromiseInterface[[]common.Hash] { return stopwaiter.LaunchPromiseThread[[]common.Hash](r, func(ctx context.Context) ([]common.Hash, error) { var resJson []common.Hash - err := r.client.client.CallContext(ctx, &resJson, server_api.Namespace+"_getMachineHashesWithStepSize", r.id, machineStartIndex, stepSize, numRequiredHashes) + err := r.client.client.CallContext(ctx, &resJson, server_api.Namespace+"_getMachineHashesWithStepSize", r.id, machineStartIndex, stepSize, maxIterations) if err != nil { return nil, err } diff --git a/validator/interface.go b/validator/interface.go index 58ad841ff5..91668a3771 100644 --- a/validator/interface.go +++ b/validator/interface.go @@ -30,7 +30,7 @@ type ExecutionSpawner interface { type ExecutionRun interface { GetStepAt(uint64) containers.PromiseInterface[*MachineStepResult] - GetMachineHashesWithStepSize(machineStartIndex, stepSize, numRequiredHashes uint64) containers.PromiseInterface[[]common.Hash] + GetMachineHashesWithStepSize(machineStartIndex, stepSize, maxIterations uint64) containers.PromiseInterface[[]common.Hash] GetLastStep() containers.PromiseInterface[*MachineStepResult] GetProofAt(uint64) containers.PromiseInterface[[]byte] PrepareRange(uint64, uint64) containers.PromiseInterface[struct{}] diff --git a/validator/server_arb/execution_run_test.go b/validator/server_arb/execution_run_test.go index b5332e108e..bb148b5d98 100644 --- a/validator/server_arb/execution_run_test.go +++ b/validator/server_arb/execution_run_test.go @@ -71,13 +71,13 @@ func Test_machineHashesWithStep(t *testing.T) { t.Run("basic argument checks", func(t *testing.T) { machStartIndex := uint64(0) stepSize := uint64(0) - numRequiredHashes := uint64(0) - _, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) + maxIterations := uint64(0) + _, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if !strings.Contains(err.Error(), "step size cannot be 0") { t.Error("Wrong error") } stepSize = uint64(1) - _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) + _, err = e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if !strings.Contains(err.Error(), "number of iterations cannot be 0") { t.Error("Wrong error") } @@ -88,7 +88,7 @@ func Test_machineHashesWithStep(t *testing.T) { } machStartIndex := uint64(0) stepSize := uint64(1) - numRequiredHashes := uint64(1) + maxIterations := uint64(1) e.cache = &MachineCache{ buildingLock: make(chan struct{}, 1), machines: []MachineInterface{mm}, @@ -98,7 +98,7 @@ func Test_machineHashesWithStep(t *testing.T) { <-time.After(time.Millisecond * 50) e.cache.buildingLock <- struct{}{} }() - hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err) } @@ -119,7 +119,7 @@ func Test_machineHashesWithStep(t *testing.T) { mm.totalSteps = 20 machStartIndex := uint64(0) stepSize := uint64(5) - numRequiredHashes := uint64(4) + maxIterations := uint64(4) e.cache = &MachineCache{ buildingLock: make(chan struct{}, 1), machines: []MachineInterface{mm}, @@ -129,7 +129,7 @@ func Test_machineHashesWithStep(t *testing.T) { <-time.After(time.Millisecond * 50) e.cache.buildingLock <- struct{}{} }() - hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err) } @@ -163,7 +163,7 @@ func Test_machineHashesWithStep(t *testing.T) { mm.totalSteps = 20 machStartIndex := uint64(0) stepSize := uint64(5) - numRequiredHashes := uint64(10) + maxIterations := uint64(10) e.cache = &MachineCache{ buildingLock: make(chan struct{}, 1), machines: []MachineInterface{mm}, @@ -173,7 +173,7 @@ func Test_machineHashesWithStep(t *testing.T) { <-time.After(time.Millisecond * 50) e.cache.buildingLock <- struct{}{} }() - hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, numRequiredHashes) + hashes, err := e.machineHashesWithStepSize(ctx, machStartIndex, stepSize, maxIterations) if err != nil { t.Fatal(err) } @@ -193,7 +193,7 @@ func Test_machineHashesWithStep(t *testing.T) { Batch: 1, PosInBatch: mm.totalSteps - 1, })) - if len(hashes) >= int(numRequiredHashes) { + if len(hashes) >= int(maxIterations) { t.Fatal("Wanted fewer hashes than the max iterations") } for i := range hashes { diff --git a/validator/valnode/validation_api.go b/validator/valnode/validation_api.go index a2ca8cfc70..d9711f9cb8 100644 --- a/validator/valnode/validation_api.go +++ b/validator/valnode/validation_api.go @@ -148,12 +148,12 @@ func (a *ExecServerAPI) GetStepAt(ctx context.Context, execid uint64, position u return server_api.MachineStepResultToJson(res), nil } -func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid, fromStep, stepSize, numRequiredHashes uint64) ([]common.Hash, error) { +func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid, fromStep, stepSize, maxIterations uint64) ([]common.Hash, error) { run, err := a.getRun(execid) if err != nil { return nil, err } - leavesInRange := run.GetMachineHashesWithStepSize(fromStep, stepSize, numRequiredHashes) + leavesInRange := run.GetMachineHashesWithStepSize(fromStep, stepSize, maxIterations) res, err := leavesInRange.Await(ctx) if err != nil { return nil, err From e853efffd328cbe07d25113506d6cd112b1c0688 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 24 Jun 2024 11:00:20 -0500 Subject: [PATCH 18/29] rename --- validator/valnode/validation_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator/valnode/validation_api.go b/validator/valnode/validation_api.go index d9711f9cb8..3299366821 100644 --- a/validator/valnode/validation_api.go +++ b/validator/valnode/validation_api.go @@ -153,8 +153,8 @@ func (a *ExecServerAPI) GetMachineHashesWithStepSize(ctx context.Context, execid if err != nil { return nil, err } - leavesInRange := run.GetMachineHashesWithStepSize(fromStep, stepSize, maxIterations) - res, err := leavesInRange.Await(ctx) + hashesInRange := run.GetMachineHashesWithStepSize(fromStep, stepSize, maxIterations) + res, err := hashesInRange.Await(ctx) if err != nil { return nil, err } From 31a168d8aecccf81bcfa35af7548f9219f85f3b1 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Tue, 2 Jul 2024 17:29:30 +0530 Subject: [PATCH 19/29] Add test for gas estimation with RPC gas limit --- system_tests/estimation_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/system_tests/estimation_test.go b/system_tests/estimation_test.go index e7f00ca94e..0e0b329708 100644 --- a/system_tests/estimation_test.go +++ b/system_tests/estimation_test.go @@ -324,3 +324,33 @@ func TestDisableL1Charging(t *testing.T) { _, err = builder.L2.Client.CallContract(ctx, ethereum.CallMsg{To: &addr, Gas: gasWithoutL1Charging, SkipL1Charging: true}, nil) Require(t, err) } + +func TestGasEstimationWithRPCGasLimit(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + builder := NewNodeBuilder(ctx).DefaultConfig(t, true) + cleanup := builder.Build(t) + defer cleanup() + + execConfigA := builder.execConfig + execConfigA.RPC.RPCGasCap = params.TxGas + testClientA, cleanupA := builder.Build2ndNode(t, &SecondNodeParams{execConfig: execConfigA}) + defer cleanupA() + addr := common.HexToAddress("0x12345678") + estimateGas, err := testClientA.Client.EstimateGas(ctx, ethereum.CallMsg{To: &addr}) + Require(t, err) + if estimateGas <= params.TxGas { + Fatal(t, "Incorrect gas estimate") + } + + _, err = testClientA.Client.CallContract(ctx, ethereum.CallMsg{To: &addr}, nil) + Require(t, err) + + execConfigB := builder.execConfig + execConfigB.RPC.RPCGasCap = params.TxGas - 1 + testClientB, cleanupB := builder.Build2ndNode(t, &SecondNodeParams{execConfig: execConfigB}) + defer cleanupB() + _, err = testClientB.Client.EstimateGas(ctx, ethereum.CallMsg{To: &addr}) + Require(t, err) +} From 66e732e8b0a2bfe2062fc875810f64d36ba29215 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Tue, 2 Jul 2024 16:39:15 -0500 Subject: [PATCH 20/29] address PR comments --- arbnode/batch_poster.go | 57 +++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index a1bae7b6da..b6020c6dac 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -1143,6 +1143,8 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) var l1BoundMaxTimestamp uint64 = math.MaxUint64 var l1BoundMinBlockNumber uint64 var l1BoundMinTimestamp uint64 + var l1BoundMinBlockNumberWithBypass uint64 + var l1BoundMinTimestampWithBypass uint64 hasL1Bound := config.l1BlockBound != l1BlockBoundIgnore if hasL1Bound { var l1Bound *types.Header @@ -1187,17 +1189,19 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) l1BoundMaxBlockNumber = arbmath.SaturatingUAdd(l1BoundBlockNumber, arbmath.BigToUintSaturating(maxTimeVariationFutureBlocks)) l1BoundMaxTimestamp = arbmath.SaturatingUAdd(l1Bound.Time, arbmath.BigToUintSaturating(maxTimeVariationFutureSeconds)) + latestHeader, err := b.l1Reader.LastHeader(ctx) + if err != nil { + return false, err + } + latestBlockNumber := arbutil.ParentHeaderToL1BlockNumber(latestHeader) + l1BoundMinBlockNumber = arbmath.SaturatingUSub(latestBlockNumber, arbmath.BigToUintSaturating(maxTimeVariationDelayBlocks)) + l1BoundMinTimestamp = arbmath.SaturatingUSub(latestHeader.Time, arbmath.BigToUintSaturating(maxTimeVariationDelaySeconds)) + if config.L1BlockBoundBypass > 0 { - latestHeader, err := b.l1Reader.LastHeader(ctx) - if err != nil { - return false, err - } - latestBlockNumber := arbutil.ParentHeaderToL1BlockNumber(latestHeader) blockNumberWithPadding := arbmath.SaturatingUAdd(latestBlockNumber, uint64(config.L1BlockBoundBypass/ethPosBlockTime)) timestampWithPadding := arbmath.SaturatingUAdd(latestHeader.Time, uint64(config.L1BlockBoundBypass/time.Second)) - - l1BoundMinBlockNumber = arbmath.SaturatingUSub(blockNumberWithPadding, arbmath.BigToUintSaturating(maxTimeVariationDelayBlocks)) - l1BoundMinTimestamp = arbmath.SaturatingUSub(timestampWithPadding, arbmath.BigToUintSaturating(maxTimeVariationDelaySeconds)) + l1BoundMinBlockNumberWithBypass = arbmath.SaturatingUSub(blockNumberWithPadding, arbmath.BigToUintSaturating(maxTimeVariationDelayBlocks)) + l1BoundMinTimestampWithBypass = arbmath.SaturatingUSub(timestampWithPadding, arbmath.BigToUintSaturating(maxTimeVariationDelaySeconds)) } } @@ -1207,13 +1211,14 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) log.Error("error getting message from streamer", "error", err) break } - if msg.Message.Header.BlockNumber < l1BoundMinBlockNumber || msg.Message.Header.Timestamp < l1BoundMinTimestamp { + if msg.Message.Header.BlockNumber < l1BoundMinBlockNumberWithBypass || msg.Message.Header.Timestamp < l1BoundMinTimestampWithBypass { log.Error( "disabling L1 bound as batch posting message is close to the maximum delay", "blockNumber", msg.Message.Header.BlockNumber, - "l1BoundMinBlockNumber", l1BoundMinBlockNumber, + "l1BoundMinBlockNumberWithBypass", l1BoundMinBlockNumberWithBypass, "timestamp", msg.Message.Header.Timestamp, - "l1BoundMinTimestamp", l1BoundMinTimestamp, + "l1BoundMinTimestampWithBypass", l1BoundMinTimestampWithBypass, + "l1BlockBoundBypass", config.L1BlockBoundBypass, ) l1BoundMaxBlockNumber = math.MaxUint64 l1BoundMaxTimestamp = math.MaxUint64 @@ -1249,20 +1254,22 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) b.building.msgCount++ } - firstMsgTimeStamp := firstMsg.Message.Header.Timestamp - firstMsgBlockNumber := firstMsg.Message.Header.BlockNumber - batchNearL1BoundMinTimestamp := firstMsgTimeStamp <= l1BoundMinTimestamp+uint64(config.ReorgResistanceMargin/time.Second) - batchNearL1BoundMinBlockNumber := firstMsgBlockNumber <= l1BoundMinBlockNumber+uint64(config.ReorgResistanceMargin/ethPosBlockTime) - if config.ReorgResistanceMargin > 0 && (batchNearL1BoundMinTimestamp || batchNearL1BoundMinBlockNumber) { - log.Error( - "Disabling batch posting due to batch being within reorg resistance margin from layer 1 minimum block or timestamp bounds", - "reorgResistanceMargin", config.ReorgResistanceMargin, - "firstMsgTimeStamp", firstMsgTimeStamp, - "l1BoundMinTimestamp", l1BoundMinTimestamp, - "firstMsgBlockNumber", firstMsgBlockNumber, - "l1BoundMinBlockNumber", l1BoundMinBlockNumber, - ) - return false, errors.New("batch is within reorg resistance margin from layer 1 minimum block or timestamp bounds") + if hasL1Bound && config.ReorgResistanceMargin > 0 { + firstMsgBlockNumber := firstMsg.Message.Header.BlockNumber + firstMsgTimeStamp := firstMsg.Message.Header.Timestamp + batchNearL1BoundMinBlockNumber := firstMsgBlockNumber <= arbmath.SaturatingUAdd(l1BoundMinBlockNumber, uint64(config.ReorgResistanceMargin/ethPosBlockTime)) + batchNearL1BoundMinTimestamp := firstMsgTimeStamp <= arbmath.SaturatingUAdd(l1BoundMinTimestamp, uint64(config.ReorgResistanceMargin/time.Second)) + if batchNearL1BoundMinTimestamp || batchNearL1BoundMinBlockNumber { + log.Error( + "Disabling batch posting due to batch being within reorg resistance margin from layer 1 minimum block or timestamp bounds", + "reorgResistanceMargin", config.ReorgResistanceMargin, + "firstMsgTimeStamp", firstMsgTimeStamp, + "l1BoundMinTimestamp", l1BoundMinTimestamp, + "firstMsgBlockNumber", firstMsgBlockNumber, + "l1BoundMinBlockNumber", l1BoundMinBlockNumber, + ) + return false, errors.New("batch is within reorg resistance margin from layer 1 minimum block or timestamp bounds") + } } if !forcePostBatch || !b.building.haveUsefulMessage { From 8b0424b6307bbc4b1974d5492431c6813929bb17 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Wed, 3 Jul 2024 17:48:27 +0530 Subject: [PATCH 21/29] fix --- execution/nodeInterface/virtual-contracts.go | 3 ++- system_tests/estimation_test.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/execution/nodeInterface/virtual-contracts.go b/execution/nodeInterface/virtual-contracts.go index d72ad0da8e..d04be10857 100644 --- a/execution/nodeInterface/virtual-contracts.go +++ b/execution/nodeInterface/virtual-contracts.go @@ -141,7 +141,8 @@ func init() { return } posterCost, _ := state.L1PricingState().PosterDataCost(msg, l1pricing.BatchPosterAddress, brotliCompressionLevel) - posterCostInL2Gas := arbos.GetPosterGas(state, header.BaseFee, msg.TxRunMode, posterCost) + // Use estimate mode because this is used to raise the gas cap, so we don't want to underestimate. + posterCostInL2Gas := arbos.GetPosterGas(state, header.BaseFee, core.MessageGasEstimationMode, posterCost) *gascap = arbmath.SaturatingUAdd(*gascap, posterCostInL2Gas) } diff --git a/system_tests/estimation_test.go b/system_tests/estimation_test.go index 0e0b329708..284c709fad 100644 --- a/system_tests/estimation_test.go +++ b/system_tests/estimation_test.go @@ -352,5 +352,7 @@ func TestGasEstimationWithRPCGasLimit(t *testing.T) { testClientB, cleanupB := builder.Build2ndNode(t, &SecondNodeParams{execConfig: execConfigB}) defer cleanupB() _, err = testClientB.Client.EstimateGas(ctx, ethereum.CallMsg{To: &addr}) - Require(t, err) + if err == nil { + Fatal(t, "EstimateGas passed with insufficient gas") + } } From a1d1fc464203a2b7ab071e9ec281a9e2145f8b52 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Mon, 8 Jul 2024 10:01:44 -0500 Subject: [PATCH 22/29] update option usage description --- arbnode/batch_poster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 9a8a7667a6..2617a9a629 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -220,7 +220,7 @@ func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Duration(prefix+".l1-block-bound-bypass", DefaultBatchPosterConfig.L1BlockBoundBypass, "post batches even if not within the layer 1 future bounds if we're within this margin of the max delay") f.Bool(prefix+".use-access-lists", DefaultBatchPosterConfig.UseAccessLists, "post batches with access lists to reduce gas usage (disabled for L3s)") f.Uint64(prefix+".gas-estimate-base-fee-multiple-bips", uint64(DefaultBatchPosterConfig.GasEstimateBaseFeeMultipleBips), "for gas estimation, use this multiple of the basefee (measured in basis points) as the max fee per gas") - f.Duration(prefix+".reorg-resistance-margin", DefaultBatchPosterConfig.ReorgResistanceMargin, "do not post batch if its within this duration from layer 1 minimum bounds ") + f.Duration(prefix+".reorg-resistance-margin", DefaultBatchPosterConfig.ReorgResistanceMargin, "do not post batch if its within this duration from layer 1 minimum bounds. Requires l1-block-bound option not be set to \"ignore\"") redislock.AddConfigOptions(prefix+".redis-lock", f) dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f, dataposter.DefaultDataPosterConfig) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultBatchPosterConfig.ParentChainWallet.Pathname) From 2a09c3e1a65bbc6c3b8319efaaccfd543dfd0cbb Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 8 Jul 2024 10:20:30 -0500 Subject: [PATCH 23/29] 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) From 12affc858deec7d2132e4aea9e387a45b7d69a17 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Wed, 10 Jul 2024 13:38:06 +0200 Subject: [PATCH 24/29] Always make expiry index --- das/local_file_storage_service.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/das/local_file_storage_service.go b/das/local_file_storage_service.go index 6b0a5f0070..621cf3efdb 100644 --- a/das/local_file_storage_service.go +++ b/das/local_file_storage_service.go @@ -199,7 +199,7 @@ func (s *LocalFileStorageService) Put(ctx context.Context, data []byte, expiry u } } - if !s.enableLegacyLayout && s.layout.expiryEnabled { + if !s.enableLegacyLayout { if err := createHardLink(batchPath, s.layout.expiryPath(key, expiry)); err != nil { return fmt.Errorf("couldn't create by-expiry-path index entry: %w", err) } @@ -360,11 +360,9 @@ func migrate(fl *flatLayout, tl *trieLayout) error { return err } - if tl.expiryEnabled { - expiryPath := tl.expiryPath(batch.key, uint64(batch.expiry.Unix())) - if err = createHardLink(newPath, expiryPath); err != nil { - return err - } + expiryPath := tl.expiryPath(batch.key, uint64(batch.expiry.Unix())) + if err = createHardLink(newPath, expiryPath); err != nil { + return err } migrated++ } @@ -698,10 +696,8 @@ func (l *trieLayout) startMigration() error { return err } - if l.expiryEnabled { - if err := os.MkdirAll(filepath.Join(l.root, byExpiryTimestamp+migratingSuffix), 0o700); err != nil { - return err - } + if err := os.MkdirAll(filepath.Join(l.root, byExpiryTimestamp+migratingSuffix), 0o700); err != nil { + return err } return nil @@ -726,10 +722,8 @@ func (l *trieLayout) commitMigration() error { return err } - if l.expiryEnabled { - if err := removeSuffix(byExpiryTimestamp); err != nil { - return err - } + if err := removeSuffix(byExpiryTimestamp); err != nil { + return err } syscall.Sync() From 55e7d456981a65757911c6a22e22bb48198e3601 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Wed, 10 Jul 2024 17:20:44 +0200 Subject: [PATCH 25/29] Fix check for iter by expiry when expiry disabled --- das/local_file_storage_service_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/das/local_file_storage_service_test.go b/das/local_file_storage_service_test.go index 0b2ba9749d..cc27e293e3 100644 --- a/das/local_file_storage_service_test.go +++ b/das/local_file_storage_service_test.go @@ -98,10 +98,9 @@ func TestMigrationNoExpiry(t *testing.T) { countEntries(t, &s.layout, 4) getByHashAndCheck(t, s, "a", "b", "c", "d") - _, err = s.layout.iterateBatchesByTimestamp(time.Unix(int64(now+10), 0)) - if err == nil { - Fail(t, "can't iterate by timestamp when expiry is disabled") - } + // Can still iterate by timestamp even if expiry disabled + countTimestampEntries(t, &s.layout, time.Unix(int64(now+11), 0), 4) + } func TestMigrationExpiry(t *testing.T) { From fdf8beab5f5d2858c230d19a687155c710d8c75d Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 10 Jul 2024 14:25:48 -0600 Subject: [PATCH 26/29] Don't close the sigint channel --- cmd/nitro/nitro.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index 572e6d2f06..04bdeb3228 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -679,8 +679,6 @@ func mainImpl() int { exitCode = 1 } if nodeConfig.Init.ThenQuit { - close(sigint) - return exitCode } } @@ -694,9 +692,6 @@ func mainImpl() int { log.Info("shutting down because of sigint") } - // cause future ctrl+c's to panic - close(sigint) - return exitCode } From 5e3bb7b2e762afbd821b1565a07ca2559ceb8680 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 10 Jul 2024 15:27:34 -0600 Subject: [PATCH 27/29] Fix validator pending validations metric --- staker/block_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staker/block_validator.go b/staker/block_validator.go index 0fea05469f..94ee907da5 100644 --- a/staker/block_validator.go +++ b/staker/block_validator.go @@ -815,7 +815,6 @@ validationsLoop: v.possiblyFatal(errors.New("failed to set SendingValidation status")) } validatorPendingValidationsGauge.Inc(1) - defer validatorPendingValidationsGauge.Dec(1) var runs []validator.ValidationRun for _, moduleRoot := range wasmRoots { run := v.chosenValidator[moduleRoot].Launch(input, moduleRoot) @@ -826,6 +825,7 @@ validationsLoop: validationStatus.Runs = runs validationStatus.Cancel = cancel v.LaunchUntrackedThread(func() { + defer validatorPendingValidationsGauge.Dec(1) defer cancel() replaced = validationStatus.replaceStatus(SendingValidation, ValidationSent) if !replaced { From 3e12fd6faaabf76c5e6c6aec01f396742d6f4a55 Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Wed, 10 Jul 2024 18:26:42 -0300 Subject: [PATCH 28/29] init: fix loading db with custom ancient path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a previous commit, we added a check to verify whether the database directory was empty before initializing it. This check highlighted a bug where an existing database wasn’t being loaded properly when the persistent.ancient flag was set. Before the empty check, the code worked because the database was reinitialized instead of just being loaded. --- cmd/nitro/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/nitro/init.go b/cmd/nitro/init.go index 97678a7d23..ea48ec8784 100644 --- a/cmd/nitro/init.go +++ b/cmd/nitro/init.go @@ -408,7 +408,7 @@ func isLeveldbNotExistError(err error) bool { func openInitializeChainDb(ctx context.Context, stack *node.Node, config *NodeConfig, chainId *big.Int, cacheConfig *core.CacheConfig, persistentConfig *conf.PersistentConfig, l1Client arbutil.L1Interface, rollupAddrs chaininfo.RollupAddresses) (ethdb.Database, *core.BlockChain, error) { if !config.Init.Force { - if readOnlyDb, err := stack.OpenDatabaseWithFreezerWithExtraOptions("l2chaindata", 0, 0, "", "l2chaindata/", true, persistentConfig.Pebble.ExtraOptions("l2chaindata")); err == nil { + if readOnlyDb, err := stack.OpenDatabaseWithFreezerWithExtraOptions("l2chaindata", 0, 0, config.Persistent.Ancient, "l2chaindata/", true, persistentConfig.Pebble.ExtraOptions("l2chaindata")); err == nil { if chainConfig := gethexec.TryReadStoredChainConfig(readOnlyDb); chainConfig != nil { readOnlyDb.Close() if !arbmath.BigEquals(chainConfig.ChainID, chainId) { From 8f2dc2ec62cd763cf5fd5b0919c9f02c365ddfaf Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 10 Jul 2024 22:50:47 -0600 Subject: [PATCH 29/29] Switch merge checks action to pull_request_target event --- .github/workflows/merge-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/merge-checks.yml b/.github/workflows/merge-checks.yml index b729df2b26..6561c429e2 100644 --- a/.github/workflows/merge-checks.yml +++ b/.github/workflows/merge-checks.yml @@ -1,7 +1,7 @@ name: Merge Checks on: - pull_request: + pull_request_target: branches: [ master ] types: [synchronize, opened, reopened, labeled, unlabeled]