-
Notifications
You must be signed in to change notification settings - Fork 737
Add Benchmark Sequencer System Test #4341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8eb5896
b00bc77
339422c
96951a7
8fd94ed
3649c75
405f475
9b90b6f
14bd65e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ### Configuration | ||
| - Add `--execution.dangerous.benchmarking-sequencer.enable` to enable the benchmarking sequencer RPC (only available in builds with the `experimental` build tag). | ||
|
|
||
| ### Internal | ||
| - Add benchmarking sequencer RPC (`benchseq`) and system tests behind the `experimental` build tag. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -209,7 +209,7 @@ func devFlagArgs() []string { | |
| "--init.empty=false", | ||
| "--http.port", "8547", | ||
| "--http.addr", "127.0.0.1", | ||
| "--http.api=net,web3,eth,arb,arbdebug,debug", | ||
| "--http.api=net,web3,eth,arb,arbdebug,debug,benchseq", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thinking out loud - in Rust, I'd definitely recommend: but in Go, I'm wondering if we should somehow tag-gate this api, so that user's won't see unsupported api in the list |
||
| "--node.transaction-streamer.track-block-metadata-from=1", | ||
| } | ||
| return args | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| // Copyright 2026, Offchain Labs, Inc. | ||
| // For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md | ||
|
|
||
| //go:build experimental | ||
|
|
||
| package gethexec | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/ethereum/go-ethereum/log" | ||
| "github.com/offchainlabs/nitro/util/containers" | ||
| "github.com/offchainlabs/nitro/util/stopwaiter" | ||
| "github.com/spf13/pflag" | ||
| ) | ||
|
|
||
| func BenchmarkingSequencerConfigAddOptions(prefix string, f *pflag.FlagSet) { | ||
| f.Bool(prefix+".enable", BenchmarkingSequencerConfigDefault.Enable, "enable benchmarking sequencer RPC (manual block creation; requires experimental build tag)") | ||
| } | ||
|
|
||
| func (c *BenchmarkingSequencerConfig) Validate() error { | ||
| if c.Enable { | ||
| log.Warn("DANGER! benchmarking sequencer enabled (manual block creation); do not use in production") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func NewBenchmarkingSequencer(sequencer *Sequencer) (TransactionPublisher, interface{}) { | ||
| benchmarkingSequencer := &BenchmarkingSequencer{ | ||
| Sequencer: sequencer, | ||
| semaphore: make(chan struct{}, 1), | ||
| } | ||
| return benchmarkingSequencer, NewBenchmarkingSequencerAPI(benchmarkingSequencer) | ||
| } | ||
|
|
||
| type BenchmarkingSequencer struct { | ||
| *Sequencer | ||
| semaphore chan struct{} | ||
| } | ||
|
|
||
| func (s *BenchmarkingSequencer) Start(ctx context.Context) error { | ||
| // override Sequencer.Start to not start the inner sequencer | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we don't start the inner sequencer, how can |
||
| s.StopWaiter.Start(ctx, s) | ||
| s.semaphore <- struct{}{} | ||
| return nil | ||
| } | ||
|
|
||
| func (s *BenchmarkingSequencer) TxQueueLength(includeRetryTxQueue bool) int { | ||
| if includeRetryTxQueue { | ||
| return len(s.Sequencer.txQueue) + s.Sequencer.txRetryQueue.Len() | ||
| } | ||
| return len(s.Sequencer.txQueue) | ||
| } | ||
|
|
||
| func (s *BenchmarkingSequencer) TxRetryQueueLength() int { | ||
| return s.Sequencer.txRetryQueue.Len() | ||
| } | ||
|
Comment on lines
+48
to
+57
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we make reads somehow synchronized / mutexed? I'm not sure if we can have some race condition here |
||
|
|
||
| func (s *BenchmarkingSequencer) CreateBlock() containers.PromiseInterface[bool] { | ||
| return stopwaiter.LaunchPromiseThread[bool](s, func(ctx context.Context) (bool, error) { | ||
| select { | ||
| // createBlock can't be run in parallel | ||
| case <-s.semaphore: | ||
| defer func() { | ||
| // release semaphore, also in case of panic | ||
| s.semaphore <- struct{}{} | ||
| }() | ||
| return s.createBlock(ctx), nil | ||
| case <-ctx.Done(): | ||
| return false, ctx.Err() | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| type BenchmarkingSequencerAPI struct { | ||
| benchmarkingSequencer *BenchmarkingSequencer | ||
| } | ||
|
|
||
| func (a *BenchmarkingSequencerAPI) TxQueueLength(includeRetryTxQueue bool) int { | ||
| return a.benchmarkingSequencer.TxQueueLength(includeRetryTxQueue) | ||
| } | ||
|
|
||
| func (a *BenchmarkingSequencerAPI) TxRetryQueueLength() int { | ||
| return a.benchmarkingSequencer.TxRetryQueueLength() | ||
| } | ||
|
|
||
| func (a *BenchmarkingSequencerAPI) CreateBlock(ctx context.Context) (bool, error) { | ||
| return a.benchmarkingSequencer.CreateBlock().Await(ctx) | ||
| } | ||
|
|
||
| func NewBenchmarkingSequencerAPI(benchmarkingSequencer *BenchmarkingSequencer) *BenchmarkingSequencerAPI { | ||
| return &BenchmarkingSequencerAPI{benchmarkingSequencer: benchmarkingSequencer} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // Copyright 2026, Offchain Labs, Inc. | ||
| // For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md | ||
|
|
||
| // DANGER! this file is included in all builds | ||
| // DANGER! do not place any experimental tag logic here | ||
|
Comment on lines
+4
to
+5
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you find these warnings helpful? I guess that nitro just won't compile, if one adds the experimental tag here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the comment means not to add experimental logic here because it is included in all builds. |
||
|
|
||
| package gethexec | ||
|
|
||
| type BenchmarkingSequencerConfig struct { | ||
| Enable bool `koanf:"enable"` | ||
| } | ||
|
|
||
| var BenchmarkingSequencerConfigDefault = BenchmarkingSequencerConfig{ | ||
| Enable: false, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Copyright 2026, Offchain Labs, Inc. | ||
| // For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md | ||
|
|
||
| //go:build !experimental | ||
|
|
||
| package gethexec | ||
|
|
||
| import ( | ||
| "github.com/spf13/pflag" | ||
|
|
||
| "github.com/ethereum/go-ethereum/log" | ||
| ) | ||
|
|
||
| func BenchmarkingSequencerConfigAddOptions(_ string, _ *pflag.FlagSet) { | ||
| // don't add any options | ||
| } | ||
|
|
||
| func (c *BenchmarkingSequencerConfig) Validate() error { | ||
| if c.Enable { | ||
| log.Warn("benchmarking sequencer requested but not supported in this build (missing experimental build tag)") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd consider returning an error here in case someone explicitly requests benchmarking, but doesn't add the tag |
||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func NewBenchmarkingSequencer(sequencer *Sequencer) (TransactionPublisher, interface{}) { | ||
| // do nothing | ||
| return sequencer, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -137,6 +137,7 @@ type Config struct { | |||||||||||
| ExposeMultiGas bool `koanf:"expose-multi-gas"` | ||||||||||||
| RPCServer rpcserver.Config `koanf:"rpc-server"` | ||||||||||||
| ConsensusRPCClient rpcclient.ClientConfig `koanf:"consensus-rpc-client" reload:"hot"` | ||||||||||||
| Dangerous DangerousConfig `koanf:"dangerous"` | ||||||||||||
|
|
||||||||||||
| forwardingTarget string | ||||||||||||
| } | ||||||||||||
|
|
@@ -168,6 +169,9 @@ func (c *Config) Validate() error { | |||||||||||
| if err := c.ConsensusRPCClient.Validate(); err != nil { | ||||||||||||
| return fmt.Errorf("error validating ConsensusRPCClient config: %w", err) | ||||||||||||
| } | ||||||||||||
| if err := c.Dangerous.Validate(); err != nil { | ||||||||||||
| return err | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just for consistency, you can add context for the error, as the checks above do |
||||||||||||
| } | ||||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -191,6 +195,7 @@ func ConfigAddOptions(prefix string, f *pflag.FlagSet) { | |||||||||||
| LiveTracingConfigAddOptions(prefix+".vmtrace", f) | ||||||||||||
| rpcserver.ConfigAddOptions(prefix+".rpc-server", "execution", f) | ||||||||||||
| rpcclient.RPCClientAddOptions(prefix+".consensus-rpc-client", f, &ConfigDefault.ConsensusRPCClient) | ||||||||||||
| DangerousConfigAddOptions(prefix+".dangerous", f) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type LiveTracingConfig struct { | ||||||||||||
|
|
@@ -208,6 +213,25 @@ func LiveTracingConfigAddOptions(prefix string, f *pflag.FlagSet) { | |||||||||||
| f.String(prefix+".json-config", DefaultLiveTracingConfig.JSONConfig, "(experimental) Tracer configuration in JSON format") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type DangerousConfig struct { | ||||||||||||
| BenchmarkingSequencer BenchmarkingSequencerConfig `koanf:"benchmarking-sequencer"` | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var DefaultDangerousConfig = DangerousConfig{ | ||||||||||||
| BenchmarkingSequencer: BenchmarkingSequencerConfigDefault, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func DangerousConfigAddOptions(prefix string, f *pflag.FlagSet) { | ||||||||||||
| BenchmarkingSequencerConfigAddOptions(prefix+".benchmarking-sequencer", f) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func (c *DangerousConfig) Validate() error { | ||||||||||||
| if err := c.BenchmarkingSequencer.Validate(); err != nil { | ||||||||||||
| return err | ||||||||||||
| } | ||||||||||||
| return nil | ||||||||||||
|
Comment on lines
+229
to
+232
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe for now:
Suggested change
|
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var ConfigDefault = Config{ | ||||||||||||
| RPC: arbitrum.DefaultConfig, | ||||||||||||
| TxIndexer: DefaultTxIndexerConfig, | ||||||||||||
|
|
@@ -237,6 +261,7 @@ var ConfigDefault = Config{ | |||||||||||
| ArgLogLimit: 2048, | ||||||||||||
| WebsocketMessageSizeLimit: 256 * 1024 * 1024, | ||||||||||||
| }, | ||||||||||||
| Dangerous: DefaultDangerousConfig, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type ConfigFetcher interface { | ||||||||||||
|
|
@@ -300,13 +325,17 @@ func CreateExecutionNode( | |||||||||||
| log.Warn("sequencer enabled without l1 client") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var benchmarkingSequencerService interface{} | ||||||||||||
| if config.Sequencer.Enable { | ||||||||||||
| seqConfigFetcher := func() *SequencerConfig { return &configFetcher.Get().Sequencer } | ||||||||||||
| sequencer, err = NewSequencer(execEngine, parentChainReader, seqConfigFetcher, parentChainID) | ||||||||||||
| if err != nil { | ||||||||||||
| return nil, err | ||||||||||||
| } | ||||||||||||
| txPublisher = sequencer | ||||||||||||
| if config.Dangerous.BenchmarkingSequencer.Enable { | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need this condition, do we? |
||||||||||||
| txPublisher, benchmarkingSequencerService = NewBenchmarkingSequencer(sequencer) | ||||||||||||
| } | ||||||||||||
| } else { | ||||||||||||
| if config.Forwarder.RedisUrl != "" { | ||||||||||||
| txPublisher = NewRedisTxForwarder(config.forwardingTarget, &config.Forwarder) | ||||||||||||
|
|
@@ -438,6 +467,14 @@ func CreateExecutionNode( | |||||||||||
| }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if benchmarkingSequencerService != nil { | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we run into a typed nil here? |
||||||||||||
| apis = append(apis, rpc.API{ | ||||||||||||
| Namespace: "benchseq", | ||||||||||||
| Service: benchmarkingSequencerService, | ||||||||||||
| Public: false, | ||||||||||||
| }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| stack.RegisterAPIs(apis) | ||||||||||||
|
|
||||||||||||
| return execNode, nil | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.