From d6667e1579bfeb01056a7829cb1d70cbab97b3b5 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 17 Sep 2024 15:17:10 +0200 Subject: [PATCH 1/4] Add test sizes to small test targets. (#690) --- api/db/BUILD.bazel | 1 + assertions/BUILD.bazel | 1 + chain-abstraction/sol-implementation/BUILD.bazel | 1 + challenge-manager/BUILD.bazel | 1 + challenge-manager/chain-watcher/BUILD.bazel | 1 + challenge-manager/challenge-tree/BUILD.bazel | 1 + containers/BUILD.bazel | 1 + containers/events/BUILD.bazel | 1 + containers/fsm/BUILD.bazel | 1 + containers/threadsafe/BUILD.bazel | 1 + layer2-state-provider/BUILD.bazel | 1 + math/BUILD.bazel | 1 + runtime/BUILD.bazel | 1 + state-commitments/history/BUILD.bazel | 1 + state-commitments/inclusion-proofs/BUILD.bazel | 1 + state-commitments/prefix-proofs/BUILD.bazel | 1 + testing/mocks/state-provider/BUILD.bazel | 1 + time/BUILD.bazel | 1 + 18 files changed, 18 insertions(+) diff --git a/api/db/BUILD.bazel b/api/db/BUILD.bazel index ed5c91de6..92eead285 100644 --- a/api/db/BUILD.bazel +++ b/api/db/BUILD.bazel @@ -32,4 +32,5 @@ go_test( "@com_github_mattn_go_sqlite3//:go-sqlite3", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/assertions/BUILD.bazel b/assertions/BUILD.bazel index d4bae9068..dd36786c3 100644 --- a/assertions/BUILD.bazel +++ b/assertions/BUILD.bazel @@ -57,4 +57,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/chain-abstraction/sol-implementation/BUILD.bazel b/chain-abstraction/sol-implementation/BUILD.bazel index e4eecee10..e74b43a6f 100644 --- a/chain-abstraction/sol-implementation/BUILD.bazel +++ b/chain-abstraction/sol-implementation/BUILD.bazel @@ -69,4 +69,5 @@ go_test( "@com_github_ethereum_go_ethereum//core/types", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/challenge-manager/BUILD.bazel b/challenge-manager/BUILD.bazel index 85bc1c47a..e735c95d8 100644 --- a/challenge-manager/BUILD.bazel +++ b/challenge-manager/BUILD.bazel @@ -56,4 +56,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/challenge-manager/chain-watcher/BUILD.bazel b/challenge-manager/chain-watcher/BUILD.bazel index 14a53ee97..90378b3e5 100644 --- a/challenge-manager/chain-watcher/BUILD.bazel +++ b/challenge-manager/chain-watcher/BUILD.bazel @@ -39,4 +39,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/challenge-manager/challenge-tree/BUILD.bazel b/challenge-manager/challenge-tree/BUILD.bazel index e2c23aa68..f174fffbe 100644 --- a/challenge-manager/challenge-tree/BUILD.bazel +++ b/challenge-manager/challenge-tree/BUILD.bazel @@ -45,4 +45,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/containers/BUILD.bazel b/containers/BUILD.bazel index adcd349e1..65df879e9 100644 --- a/containers/BUILD.bazel +++ b/containers/BUILD.bazel @@ -12,4 +12,5 @@ go_test( srcs = ["slice_test.go"], embed = [":containers"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/containers/events/BUILD.bazel b/containers/events/BUILD.bazel index ef6e78331..208f13c1d 100644 --- a/containers/events/BUILD.bazel +++ b/containers/events/BUILD.bazel @@ -12,4 +12,5 @@ go_test( srcs = ["producer_test.go"], embed = [":events"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/containers/fsm/BUILD.bazel b/containers/fsm/BUILD.bazel index 2694850a8..5d911e6d5 100644 --- a/containers/fsm/BUILD.bazel +++ b/containers/fsm/BUILD.bazel @@ -13,4 +13,5 @@ go_test( srcs = ["fsm_test.go"], embed = [":fsm"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/containers/threadsafe/BUILD.bazel b/containers/threadsafe/BUILD.bazel index 4179bead5..f6b14df47 100644 --- a/containers/threadsafe/BUILD.bazel +++ b/containers/threadsafe/BUILD.bazel @@ -28,4 +28,5 @@ go_test( ], embed = [":threadsafe"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/layer2-state-provider/BUILD.bazel b/layer2-state-provider/BUILD.bazel index 5a0b3fc09..c0bfa4be3 100644 --- a/layer2-state-provider/BUILD.bazel +++ b/layer2-state-provider/BUILD.bazel @@ -30,4 +30,5 @@ go_test( "//containers/option", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/math/BUILD.bazel b/math/BUILD.bazel index 94ff3408a..04620a62b 100644 --- a/math/BUILD.bazel +++ b/math/BUILD.bazel @@ -12,4 +12,5 @@ go_test( srcs = ["math_test.go"], embed = [":math"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/runtime/BUILD.bazel b/runtime/BUILD.bazel index 03a1ec3c2..bf93f02e9 100644 --- a/runtime/BUILD.bazel +++ b/runtime/BUILD.bazel @@ -16,4 +16,5 @@ go_test( srcs = ["retry_test.go"], embed = [":runtime"], deps = ["@com_github_stretchr_testify//require"], + size = "small", ) diff --git a/state-commitments/history/BUILD.bazel b/state-commitments/history/BUILD.bazel index 2ee7d26dc..29a862486 100644 --- a/state-commitments/history/BUILD.bazel +++ b/state-commitments/history/BUILD.bazel @@ -21,4 +21,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/state-commitments/inclusion-proofs/BUILD.bazel b/state-commitments/inclusion-proofs/BUILD.bazel index 97702dd7a..ac94c7fe3 100644 --- a/state-commitments/inclusion-proofs/BUILD.bazel +++ b/state-commitments/inclusion-proofs/BUILD.bazel @@ -22,4 +22,5 @@ go_test( "@com_github_ethereum_go_ethereum//common", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/state-commitments/prefix-proofs/BUILD.bazel b/state-commitments/prefix-proofs/BUILD.bazel index 6021c7f2b..1a3274264 100644 --- a/state-commitments/prefix-proofs/BUILD.bazel +++ b/state-commitments/prefix-proofs/BUILD.bazel @@ -36,4 +36,5 @@ go_test( "@com_github_ethereum_go_ethereum//ethclient/simulated", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/testing/mocks/state-provider/BUILD.bazel b/testing/mocks/state-provider/BUILD.bazel index e2bb699ba..ab0694cab 100644 --- a/testing/mocks/state-provider/BUILD.bazel +++ b/testing/mocks/state-provider/BUILD.bazel @@ -39,4 +39,5 @@ go_test( "@com_github_ethereum_go_ethereum//crypto", "@com_github_stretchr_testify//require", ], + size = "small", ) diff --git a/time/BUILD.bazel b/time/BUILD.bazel index a084465a4..341eb765c 100644 --- a/time/BUILD.bazel +++ b/time/BUILD.bazel @@ -11,4 +11,5 @@ go_test( name = "time_test", srcs = ["time_reference_test.go"], embed = [":time"], + size = "small", ) From 8720acb03ab980ae23d78b67451819c5323428ac Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 17 Sep 2024 15:58:39 +0200 Subject: [PATCH 2/4] Remove the in-progress cache (#689) Co-authored-by: Raul Jordan --- containers/in-progress-cache/BUILD.bazel | 15 -- containers/in-progress-cache/cache.go | 77 ---------- containers/in-progress-cache/cache_test.go | 105 -------------- layer2-state-provider/BUILD.bazel | 1 - .../history_commitment_provider.go | 131 +++++++++--------- 5 files changed, 64 insertions(+), 265 deletions(-) delete mode 100644 containers/in-progress-cache/BUILD.bazel delete mode 100644 containers/in-progress-cache/cache.go delete mode 100644 containers/in-progress-cache/cache_test.go diff --git a/containers/in-progress-cache/BUILD.bazel b/containers/in-progress-cache/BUILD.bazel deleted file mode 100644 index a265bc2d9..000000000 --- a/containers/in-progress-cache/BUILD.bazel +++ /dev/null @@ -1,15 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") - -go_library( - name = "in-progress-cache", - srcs = ["cache.go"], - importpath = "github.com/OffchainLabs/bold/containers/in-progress-cache", - visibility = ["//visibility:public"], - deps = ["@com_github_ethereum_go_ethereum//metrics"], -) - -go_test( - name = "in-progress-cache_test", - srcs = ["cache_test.go"], - embed = [":in-progress-cache"], -) diff --git a/containers/in-progress-cache/cache.go b/containers/in-progress-cache/cache.go deleted file mode 100644 index a29f1cc46..000000000 --- a/containers/in-progress-cache/cache.go +++ /dev/null @@ -1,77 +0,0 @@ -package inprogresscache - -import ( - "sync" - - "github.com/ethereum/go-ethereum/metrics" -) - -var ( - inFlightRequestsCounter = metrics.NewRegisteredCounter("arb/validator/inprogresscache/inflight", nil) - pendingRequestsCounter = metrics.NewRegisteredCounter("arb/validator/inprogresscache/pending", nil) -) - -// Cache for expensive computations that ensures only -// one request is in-flight at a time. If a future request comes in with the same request id -// as the ongoing computation, a goroutine is spawned that awaits the computation's completion -// instead of kicking off two expensive computations. -type Cache[K comparable, V any] struct { - inProgress map[K]bool - awaitingCompletion map[K][]chan Response[V] - lock sync.RWMutex -} - -type Response[V any] struct { - value V - err error -} - -func New[K comparable, V any]() *Cache[K, V] { - return &Cache[K, V]{ - inProgress: make(map[K]bool), - awaitingCompletion: make(map[K][]chan Response[V]), - } -} - -// Compute an expensive closure. The request must be representable as a string. -func (c *Cache[K, V]) Compute(requestId K, f func() (V, error)) (V, error) { - c.lock.RLock() - if ok := c.inProgress[requestId]; ok { - pendingRequestsCounter.Inc(1) - - c.lock.RUnlock() - responseChan := make(chan Response[V]) - defer close(responseChan) - - c.lock.Lock() - c.awaitingCompletion[requestId] = append(c.awaitingCompletion[requestId], responseChan) - c.lock.Unlock() - response := <-responseChan - return response.value, response.err - } - c.lock.RUnlock() - - c.lock.Lock() - c.inProgress[requestId] = true - inFlightRequestsCounter.Inc(1) - c.lock.Unlock() - - // Do expensive operation and notify all waiting goroutines of the result as well as the error - result, err := f() - - c.lock.RLock() - receiversWaiting, ok := c.awaitingCompletion[requestId] - c.lock.RUnlock() - - if ok { - for _, ch := range receiversWaiting { - ch <- Response[V]{result, err} - } - } - - c.lock.Lock() - c.inProgress[requestId] = false - c.awaitingCompletion[requestId] = make([]chan Response[V], 0) - c.lock.Unlock() - return result, err -} diff --git a/containers/in-progress-cache/cache_test.go b/containers/in-progress-cache/cache_test.go deleted file mode 100644 index bdb347b2b..000000000 --- a/containers/in-progress-cache/cache_test.go +++ /dev/null @@ -1,105 +0,0 @@ -package inprogresscache - -import ( - "errors" - "strconv" - "strings" - "sync" - "testing" - "time" -) - -func TestCompute(t *testing.T) { - cache := New[string, int]() - requestId := "testRequest" - - // Define a computation function - computeFunc := func() (int, error) { - time.Sleep(100 * time.Millisecond) - return 42, nil - } - - // Call Compute and check the result - result, err := cache.Compute(requestId, computeFunc) - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - if result != 42 { - t.Errorf("Expected result to be 42, got %d", result) - } - - // Call Compute again with the same requestId and ensure the cached value is returned - cachedResult, cachedErr := cache.Compute(requestId, computeFunc) - if cachedErr != nil { - t.Errorf("Expected no error from cached result, got %v", cachedErr) - } - if cachedResult != result { - t.Errorf("Expected cached result to be %d, got %d", result, cachedResult) - } -} - -// TestConcurrentComputations tests that concurrent calls to Compute with the same request ID -// only result in a single computation. -func TestConcurrentComputations(t *testing.T) { - cache := New[string, int]() - requestId := "concurrentTest" - counter := 0 - - computeFunc := func() (int, error) { - time.Sleep(100 * time.Millisecond) - counter++ - return counter, nil - } - - var wg sync.WaitGroup - for i := 0; i < 20; i++ { - wg.Add(1) - go func() { - defer wg.Done() - if _, err := cache.Compute(requestId, computeFunc); err != nil { - t.Error(err) - } - }() - } - wg.Wait() - - // Verify that the computation was only performed once - if counter != 1 { - t.Errorf("Expected a single computation, got %d", counter) - } -} - -// TestConcurrentComputationsWithError tests that concurrent calls to Compute with the same request ID -// only result in a single computation even if the computation returns an error. -// The error should be returned to all goroutines awaiting the computation's completion. -func TestConcurrentComputationsWithError(t *testing.T) { - cache := New[string, int]() - requestId := "concurrentTest" - counter := 0 - - computeFunc := func() (int, error) { - time.Sleep(100 * time.Millisecond) - counter++ - return 0, errors.New(strconv.Itoa(counter)) - } - - expectedError := errors.New("1") - var wg sync.WaitGroup - for i := 0; i < 20; i++ { - wg.Add(1) - go func() { - defer wg.Done() - if _, err := cache.Compute(requestId, computeFunc); err != nil { - if !strings.Contains(err.Error(), expectedError.Error()) { - t.Errorf("Expected a single computation, got %s", err.Error()) - } - } - }() - } - wg.Wait() - - // Verify that the computation was only performed once - if counter != 1 { - t.Errorf("Expected a single computation, got %d", counter) - } -} diff --git a/layer2-state-provider/BUILD.bazel b/layer2-state-provider/BUILD.bazel index c0bfa4be3..ae9f80adb 100644 --- a/layer2-state-provider/BUILD.bazel +++ b/layer2-state-provider/BUILD.bazel @@ -12,7 +12,6 @@ go_library( "//api", "//api/db", "//chain-abstraction:protocol", - "//containers/in-progress-cache", "//containers/option", "//state-commitments/history", "//state-commitments/prefix-proofs", diff --git a/layer2-state-provider/history_commitment_provider.go b/layer2-state-provider/history_commitment_provider.go index af295da1d..27b53ab5c 100644 --- a/layer2-state-provider/history_commitment_provider.go +++ b/layer2-state-provider/history_commitment_provider.go @@ -8,7 +8,6 @@ import ( "time" protocol "github.com/OffchainLabs/bold/chain-abstraction" - inprogresscache "github.com/OffchainLabs/bold/containers/in-progress-cache" prefixproofs "github.com/OffchainLabs/bold/state-commitments/prefix-proofs" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/metrics" @@ -52,8 +51,8 @@ type HashCollectorConfig struct { // The block challenge height for hash collection. BlockChallengeHeight Height // Defines the heights at which we want to collect machine hashes for each challenge level. - // An index in this slice represents a challenge level, and a value represents a height within that - // challenge level. + // An index in this slice represents a challenge level, and a value represents a height within + // that challenge level. StepHeights []Height // The number of desired hashes to be collected. NumDesiredHashes uint64 @@ -85,8 +84,8 @@ func (h *HashCollectorConfig) String() string { return str } -// L2MessageStateCollector defines an interface which can obtain the machine hashes at each L2 message -// in a specified message range for a given batch index on Arbitrum. +// L2MessageStateCollector defines an interface which can obtain the machine hashes at each L2 +// message in a specified message range for a given batch index on Arbitrum. type L2MessageStateCollector interface { L2MessageStatesUpTo( ctx context.Context, @@ -105,13 +104,12 @@ type HistoryCommitmentProvider struct { machineHashCollector MachineHashCollector proofCollector ProofCollector challengeLeafHeights []Height - inFlightRequestCache *inprogresscache.Cache[string, []common.Hash] apiDB db.Database ExecutionProvider } -// NewHistoryCommitmentProvider creates an instance of a struct which can compute history commitments -// over any number of challenge levels for BOLD. +// NewHistoryCommitmentProvider creates an instance of a struct which can compute history +// commitments over any number of challenge levels for BOLD. func NewHistoryCommitmentProvider( l2MessageStateCollector L2MessageStateCollector, machineHashCollector MachineHashCollector, @@ -126,7 +124,6 @@ func NewHistoryCommitmentProvider( proofCollector: proofCollector, challengeLeafHeights: challengeLeafHeights, ExecutionProvider: executionProvider, - inFlightRequestCache: inprogresscache.New[string, []common.Hash](), apiDB: apiDB, } } @@ -208,64 +205,61 @@ func (p *HistoryCommitmentProvider) historyCommitmentImpl( WasmModuleRoot: req.WasmModuleRoot, FromBatch: req.FromBatch, BlockChallengeHeight: fromBlockChallengeHeight, - // We drop the first index of the validated heights, because the first index is for the block challenge level, - // which is over blocks and not over individual machine WASM opcodes. Starting from the second index, we are now - // dealing with challenges over ranges of opcodes which are what we care about for our implementation of machine hash collection. + // We drop the first index of the validated heights, because the first index is for the + // block challenge level, which is over blocks and not over individual machine WASM opcodes. + // Starting from the second index, we are now dealing with challenges over ranges of opcodes + // which are what we care about for our implementation of machine hash collection. StepHeights: validatedHeights[1:], NumDesiredHashes: numHashes, MachineStartIndex: machineStartIndex, StepSize: stepSize, ClaimId: req.ClaimId, } - // Requests collecting machine hashes for the specified config, and uses an in-flight - // request cache to make sure the same request is not spawned twice, but rather - // the second request would wait for the in-flight request to complete and use its result. - return p.inFlightRequestCache.Compute(cfg.String(), func() ([]common.Hash, error) { - if !api.IsNil(p.apiDB) { - var rawStepHeights string - for i, stepHeight := range cfg.StepHeights { - rawStepHeights += strconv.Itoa(int(stepHeight)) - if i != len(rawStepHeights)-1 { - rawStepHeights += "," - } + // Requests collecting machine hashes for the specified config. + if !api.IsNil(p.apiDB) { + var rawStepHeights string + for i, stepHeight := range cfg.StepHeights { + rawStepHeights += strconv.Itoa(int(stepHeight)) + if i != len(rawStepHeights)-1 { + rawStepHeights += "," } - collectMachineHashes := api.JsonCollectMachineHashes{ - WasmModuleRoot: cfg.WasmModuleRoot, - FromBatch: uint64(cfg.FromBatch), - BlockChallengeHeight: uint64(cfg.BlockChallengeHeight), - RawStepHeights: rawStepHeights, - NumDesiredHashes: cfg.NumDesiredHashes, - MachineStartIndex: uint64(cfg.MachineStartIndex), - StepSize: uint64(cfg.StepSize), - StartTime: time.Now().UTC(), - } - err := p.apiDB.InsertCollectMachineHash(&collectMachineHashes) - if err != nil { - return nil, err - } - defer func() { - finishTime := time.Now().UTC() - collectMachineHashes.FinishTime = &finishTime - err := p.apiDB.UpdateCollectMachineHash(&collectMachineHashes) - if err != nil { - return - } - }() } - startTime := time.Now() + collectMachineHashes := api.JsonCollectMachineHashes{ + WasmModuleRoot: cfg.WasmModuleRoot, + FromBatch: uint64(cfg.FromBatch), + BlockChallengeHeight: uint64(cfg.BlockChallengeHeight), + RawStepHeights: rawStepHeights, + NumDesiredHashes: cfg.NumDesiredHashes, + MachineStartIndex: uint64(cfg.MachineStartIndex), + StepSize: uint64(cfg.StepSize), + StartTime: time.Now().UTC(), + } + err := p.apiDB.InsertCollectMachineHash(&collectMachineHashes) + if err != nil { + return nil, err + } defer func() { - // TODO: Replace NewUniformSample(100) with NewBoundedHistogramSample(), once offchainlabs geth is merged in bold. - // Eg https://github.com/OffchainLabs/nitro/blob/ab6790a9e33884c3b4e81de2a97dae5bf904266e/das/restful_server.go#L30 - metrics.GetOrRegisterHistogram("arb/state_provider/collect_machine_hashes/step_size_"+strconv.Itoa(int(stepSize))+"/duration", nil, metrics.NewUniformSample(100)).Update(time.Since(startTime).Nanoseconds()) + finishTime := time.Now().UTC() + collectMachineHashes.FinishTime = &finishTime + err := p.apiDB.UpdateCollectMachineHash(&collectMachineHashes) + if err != nil { + return + } }() - return p.machineHashCollector.CollectMachineHashes(ctx, cfg) - }) + } + startTime := time.Now() + defer func() { + // TODO: Replace NewUniformSample(100) with NewBoundedHistogramSample(), once offchainlabs geth is merged in bold. + // Eg https://github.com/OffchainLabs/nitro/blob/ab6790a9e33884c3b4e81de2a97dae5bf904266e/das/restful_server.go#L30 + metrics.GetOrRegisterHistogram("arb/state_provider/collect_machine_hashes/step_size_"+strconv.Itoa(int(stepSize))+"/duration", nil, metrics.NewUniformSample(100)).Update(time.Since(startTime).Nanoseconds()) + }() + return p.machineHashCollector.CollectMachineHashes(ctx, cfg) } // AgreesWithHistoryCommitment checks if the l2 state provider agrees with a specified start and end -// history commitment for a type of edge under a specified assertion challenge. It returns an agreement struct -// which informs the caller whether (a) we agree with the start commitment, and whether (b) the edge is honest, meaning -// that we also agree with the end commitment. +// history commitment for a type of edge under a specified assertion challenge. It returns an +// agreement struct which informs the caller whether (a) we agree with the start commitment, and +// whether (b) the edge is honest, meaning that we also agree with the end commitment. func (p *HistoryCommitmentProvider) AgreesWithHistoryCommitment( ctx context.Context, challengeLevel protocol.ChallengeLevel, @@ -544,7 +538,8 @@ func (p *HistoryCommitmentProvider) computeRequiredNumberOfHashes( // = 4,199,434 // // This generalizes for any number of subchallenge levels into the algorithm below. -// It works by taking the sum of (each input * product of all challenge level height constants beneath its level). +// It works by taking the sum of (each input * product of all challenge level height constants +// beneath its level). // This means we need to start executing our machine exactly at opcode index 4,199,434. func (p *HistoryCommitmentProvider) computeMachineStartIndex( upperChallengeOriginHeights validatedStartHeights, @@ -554,9 +549,9 @@ func (p *HistoryCommitmentProvider) computeMachineStartIndex( if len(upperChallengeOriginHeights) == 0 { return 0, nil } - // The first position in the start heights slice is the block challenge level, which is over ranges of L2 messages - // and not over individual opcodes. We ignore this level and start at the next level when it comes to dealing with - // machines. + // The first position in the start heights slice is the block challenge level, which is over + // ranges of L2 messages and not over individual opcodes. We ignore this level and start at the + // next level when it comes to dealing with machines. heights := upperChallengeOriginHeights[1:] heights = append(heights, fromHeight) leafHeights := p.challengeLeafHeights[1:] @@ -581,13 +576,14 @@ func (p *HistoryCommitmentProvider) computeMachineStartIndex( } // Computes the number of individual opcodes we need to step through a machine at a time. -// Each challenge level has a different amount of ranges of opcodes, so the overall step size can be computed -// as a multiplication of all the next challenge levels needed. +// Each challenge level has a different amount of ranges of opcodes, so the overall step size can be +// computed as a multiplication of all the next challenge levels needed. // -// As an example, this function helps answer questions such as: "How many individual opcodes are there in a single step of a -// Megastep challenge?" +// As an example, this function helps answer questions such as: "How many individual opcodes are +// there in a single step of a Megastep challenge?" func (p *HistoryCommitmentProvider) computeStepSize(challengeLevel uint64) (StepSize, error) { - // The last challenge level is over individual opcodes, so the step size is always 1 opcode at a time. + // The last challenge level is over individual opcodes, so the step size is always 1 opcode at a + // time. if challengeLevel+1 == p.numberOfChallengeLevels() { return 1, nil } @@ -615,10 +611,11 @@ func (p *HistoryCommitmentProvider) validateOriginHeights( return upperChallengeOriginHeights, nil } -// A caller specifies a request for a history commitment at challenge level N. It specifies a list of -// heights at which to compute the history commitment at each challenge level on the way to level N -// as a list of heights, where each position represents a challenge level. -// The length of this list cannot be greater than the total number of challenge levels in the protocol. +// A caller specifies a request for a history commitment at challenge level N. It specifies a list +// of heights at which to compute the history commitment at each challenge level on the way to level +// N as a list of heights, where each position represents a challenge level. +// The length of this list cannot be greater than the total number of challenge levels in the +// protocol. // Takes in an input type that has already been validated for correctness. func deepestRequestedChallengeLevel(requestedHeights validatedStartHeights) uint64 { return uint64(len(requestedHeights)) From a12424d2b0fb315f361fdaae3eaa2ba96ac376da Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 17 Sep 2024 16:01:21 +0200 Subject: [PATCH 3/4] Remove some unused varialbes (#687) --- assertions/manager_test.go | 7 ++----- challenge-manager/challenge-tree/add_edge.go | 2 +- testing/endtoend/e2e_test.go | 4 ++-- testing/endtoend/helpers_test.go | 1 - 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/assertions/manager_test.go b/assertions/manager_test.go index 0f728098c..78bb76d61 100644 --- a/assertions/manager_test.go +++ b/assertions/manager_test.go @@ -418,8 +418,7 @@ func TestFastConfirmation(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - hash := protocol.AssertionHash{Hash: posted.Unwrap().AssertionHash} - expectAssertionConfirmed(t, ctx, setup.Backend, aliceChain.RollupAddress(), hash) + expectAssertionConfirmed(t, ctx, setup.Backend, aliceChain.RollupAddress()) } func TestFastConfirmationWithSafe(t *testing.T) { @@ -543,8 +542,7 @@ func TestFastConfirmationWithSafe(t *testing.T) { // Only after both Alice and Bob confirm the assertion, it should be confirmed. ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - hash := protocol.AssertionHash{Hash: posted.Unwrap().AssertionHash} - expectAssertionConfirmed(t, ctx, setup.Backend, aliceChain.RollupAddress(), hash) + expectAssertionConfirmed(t, ctx, setup.Backend, aliceChain.RollupAddress()) } type seqMessage struct { @@ -590,7 +588,6 @@ func expectAssertionConfirmed( ctx context.Context, backend protocol.ChainBackend, rollupAddr common.Address, - hash protocol.AssertionHash, ) { rc, err := rollupgen.NewRollupCore(rollupAddr, backend) require.NoError(t, err) diff --git a/challenge-manager/challenge-tree/add_edge.go b/challenge-manager/challenge-tree/add_edge.go index 87a827045..2bae74c82 100644 --- a/challenge-manager/challenge-tree/add_edge.go +++ b/challenge-manager/challenge-tree/add_edge.go @@ -94,7 +94,7 @@ func (ht *RoyalChallengeTree) checkAssertionHash(ctx context.Context, id protoco return nil } -func (ht *RoyalChallengeTree) claimedAssertionHash(ctx context.Context, eg protocol.SpecEdge) (protocol.AssertionHash, error) { +func (ht *RoyalChallengeTree) claimedAssertionHash(_ context.Context, eg protocol.SpecEdge) (protocol.AssertionHash, error) { challengeLevel := eg.GetChallengeLevel() // If this is a root challege level zero edge. if challengeLevel == protocol.NewBlockChallengeLevel() && !eg.ClaimId().IsNone() { diff --git a/testing/endtoend/e2e_test.go b/testing/endtoend/e2e_test.go index 752c91cea..b3bd49b1a 100644 --- a/testing/endtoend/e2e_test.go +++ b/testing/endtoend/e2e_test.go @@ -282,7 +282,7 @@ func runEndToEndTest(t *testing.T, cfg *e2eConfig) { challengemanager.WithName(name), ) honestManager := setupChallengeManager( - t, ctx, bk.Client(), rollupAddr.Rollup, honestStateManager, txOpts, name, honestOpts..., + t, ctx, bk.Client(), rollupAddr.Rollup, honestStateManager, txOpts, honestOpts..., ) if !api.IsNil(honestManager.Database()) { honestStateManager.UpdateAPIDatabase(honestManager.Database()) @@ -318,7 +318,7 @@ func runEndToEndTest(t *testing.T, cfg *e2eConfig) { challengemanager.WithName(name), ) evilManager := setupChallengeManager( - t, ctx, bk.Client(), rollupAddr.Rollup, evilStateManager, txOpts, name, evilOpts..., + t, ctx, bk.Client(), rollupAddr.Rollup, evilStateManager, txOpts, evilOpts..., ) evilChallengeManagers[i] = evilManager } diff --git a/testing/endtoend/helpers_test.go b/testing/endtoend/helpers_test.go index c3e4fd1a1..e58b61948 100644 --- a/testing/endtoend/helpers_test.go +++ b/testing/endtoend/helpers_test.go @@ -27,7 +27,6 @@ func setupChallengeManager( rollup common.Address, sm l2stateprovider.Provider, txOpts *bind.TransactOpts, - name string, opts ...challengemanager.Opt, ) *challengemanager.Manager { assertionChainBinding, err := rollupgen.NewRollupUserLogic( From 82f06a558ad8621ff8dadfb11bd45ab637051ab0 Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Tue, 17 Sep 2024 16:03:16 +0200 Subject: [PATCH 4/4] Fix minor style issues from buildifier. (#688) --- BUILD.bazel | 3 +-- WORKSPACE | 2 +- deps.bzl | 5 +++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 0a0711931..d7ffd2141 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,5 +1,6 @@ # gazelle:ignore load("@bazel_gazelle//:def.bzl", "gazelle") +load("@io_bazel_rules_go//go:def.bzl", "TOOLS_NOGO", "nogo") # gazelle:prefix github.com/OffchainLabs/bold gazelle(name = "gazelle") @@ -14,8 +15,6 @@ gazelle( command = "update-repos", ) -load("@io_bazel_rules_go//go:def.bzl", "TOOLS_NOGO", "go_binary", "go_library", "nogo") - nogo( name = "nogo", config = ":nogo.json", diff --git a/WORKSPACE b/WORKSPACE index 4e1570f14..ada6a36a9 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -27,8 +27,8 @@ http_archive( ], ) -load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies") +load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") load("//:deps.bzl", "go_dependencies") # gazelle:repository_macro deps.bzl%go_dependencies diff --git a/deps.bzl b/deps.bzl index 705ba2228..aa4224beb 100644 --- a/deps.bzl +++ b/deps.bzl @@ -1,6 +1,11 @@ +""" +Auto-gneerated deps file managed by gazelle. +""" + load("@bazel_gazelle//:deps.bzl", "go_repository") def go_dependencies(): + "Auto-generated dependencies function." go_repository( name = "co_honnef_go_tools", importpath = "honnef.co/go/tools",