-
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 5 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,2 @@ | ||
| ### Internal | ||
|
gligneul marked this conversation as resolved.
|
||
| - Add bench sequencer RPC, config, and tests behind the benchsequencer 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,90 @@ | ||
| //go:build benchsequencer | ||
|
|
||
| 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 BenchSequencerConfigAddOptions(prefix string, f *pflag.FlagSet) { | ||
| f.Bool(prefix+".enable", BenchSequencerConfigDefault.Enable, "enables transaction indexer") | ||
|
gligneul marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| func (c *BenchSequencerConfig) Validate() error { | ||
| if c.Enable { | ||
| log.Warn("DANGER! BenchSequencer enabled") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func NewBenchSequencer(sequencer *Sequencer) (TransactionPublisher, interface{}) { | ||
| benchSequencer := &BenchSequencer{ | ||
| Sequencer: sequencer, | ||
| semaphore: make(chan struct{}, 1), | ||
| } | ||
| return benchSequencer, NewBenchSequencerAPI(benchSequencer) | ||
| } | ||
|
|
||
| type BenchSequencer struct { | ||
| *Sequencer | ||
| semaphore chan struct{} | ||
| } | ||
|
|
||
| func (s *BenchSequencer) 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 *BenchSequencer) TxQueueLength(includeRetryTxQueue bool) int { | ||
| if includeRetryTxQueue { | ||
| return len(s.Sequencer.txQueue) + s.Sequencer.txRetryQueue.Len() | ||
| } | ||
| return len(s.Sequencer.txQueue) | ||
| } | ||
|
|
||
| func (s *BenchSequencer) 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 *BenchSequencer) 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 BenchSequencerAPI struct { | ||
| benchSequencer *BenchSequencer | ||
| } | ||
|
|
||
| func (a *BenchSequencerAPI) TxQueueLength(includeRetryTxQueue bool) int { | ||
| return a.benchSequencer.TxQueueLength(includeRetryTxQueue) | ||
| } | ||
|
|
||
| func (a *BenchSequencerAPI) TxRetryQueueLength() int { | ||
| return a.benchSequencer.TxRetryQueueLength() | ||
| } | ||
|
|
||
| func (a *BenchSequencerAPI) CreateBlock(ctx context.Context) (bool, error) { | ||
| return a.benchSequencer.CreateBlock().Await(ctx) | ||
| } | ||
|
|
||
| func NewBenchSequencerAPI(benchSequencer *BenchSequencer) *BenchSequencerAPI { | ||
| return &BenchSequencerAPI{benchSequencer: benchSequencer} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // DANGER! this file is included in all builds | ||
| // DANGER! do not place any of the experimental logic and features here | ||
|
|
||
| package gethexec | ||
|
|
||
| type BenchSequencerConfig struct { | ||
| Enable bool `koanf:"enable"` | ||
| } | ||
|
|
||
| var BenchSequencerConfigDefault = BenchSequencerConfig{ | ||
| Enable: false, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| //go:build !benchsequencer | ||
|
|
||
| package gethexec | ||
|
|
||
| import ( | ||
| "github.com/spf13/pflag" | ||
|
|
||
| "github.com/ethereum/go-ethereum/log" | ||
| ) | ||
|
|
||
| func BenchSequencerConfigAddOptions(_ string, _ *pflag.FlagSet) { | ||
| // don't add any options | ||
| } | ||
|
|
||
| func (c *BenchSequencerConfig) Validate() error { | ||
| if c.Enable { | ||
| log.Warn("BenchSequencer is not supported in this build") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func NewBenchSequencer(sequencer *Sequencer) (TransactionPublisher, interface{}) { | ||
| // do nothing | ||
| return sequencer, nil | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -139,6 +139,7 @@ type Config struct { | |||||||||||
| RPCServer rpcserver.Config `koanf:"rpc-server"` | ||||||||||||
| ConsensusRPCClient rpcclient.ClientConfig `koanf:"consensus-rpc-client" reload:"hot"` | ||||||||||||
| AddressFilter addressfilter.Config `koanf:"address-filter" reload:"hot"` | ||||||||||||
| Dangerous DangerousConfig `koanf:"dangerous"` | ||||||||||||
|
|
||||||||||||
| forwardingTarget string | ||||||||||||
| } | ||||||||||||
|
|
@@ -173,6 +174,9 @@ func (c *Config) Validate() error { | |||||||||||
| if err := c.AddressFilter.Validate(); err != nil { | ||||||||||||
| return fmt.Errorf("error validating addressfilter 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 | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -197,6 +201,7 @@ func ConfigAddOptions(prefix string, f *pflag.FlagSet) { | |||||||||||
| rpcserver.ConfigAddOptions(prefix+".rpc-server", "execution", f) | ||||||||||||
| rpcclient.RPCClientAddOptions(prefix+".consensus-rpc-client", f, &ConfigDefault.ConsensusRPCClient) | ||||||||||||
| addressfilter.ConfigAddOptions(prefix+".address-filter", f) | ||||||||||||
| DangerousConfigAddOptions(prefix+".dangerous", f) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type LiveTracingConfig struct { | ||||||||||||
|
|
@@ -214,6 +219,25 @@ func LiveTracingConfigAddOptions(prefix string, f *pflag.FlagSet) { | |||||||||||
| f.String(prefix+".json-config", DefaultLiveTracingConfig.JSONConfig, "(experimental) Tracer configuration in JSON format") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type DangerousConfig struct { | ||||||||||||
| BenchSequencer BenchSequencerConfig `koanf:"bench-sequencer"` | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var DefaultDangerousConfig = DangerousConfig{ | ||||||||||||
| BenchSequencer: BenchSequencerConfigDefault, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func DangerousConfigAddOptions(prefix string, f *pflag.FlagSet) { | ||||||||||||
| BenchSequencerConfigAddOptions(prefix+".bench-sequencer", f) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func (c *DangerousConfig) Validate() error { | ||||||||||||
| if err := c.BenchSequencer.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, | ||||||||||||
|
|
@@ -244,6 +268,7 @@ var ConfigDefault = Config{ | |||||||||||
| WebsocketMessageSizeLimit: 256 * 1024 * 1024, | ||||||||||||
| }, | ||||||||||||
|
|
||||||||||||
| Dangerous: DefaultDangerousConfig, | ||||||||||||
| AddressFilter: addressfilter.DefaultConfig, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -309,13 +334,17 @@ func CreateExecutionNode( | |||||||||||
| log.Warn("sequencer enabled without l1 client") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| var benchSequencerService 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.BenchSequencer.Enable { | ||||||||||||
| txPublisher, benchSequencerService = NewBenchSequencer(sequencer) | ||||||||||||
| } | ||||||||||||
| } else { | ||||||||||||
| if config.Forwarder.RedisUrl != "" { | ||||||||||||
| txPublisher = NewRedisTxForwarder(config.forwardingTarget, &config.Forwarder) | ||||||||||||
|
|
@@ -447,6 +476,14 @@ func CreateExecutionNode( | |||||||||||
| }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if benchSequencerService != nil { | ||||||||||||
| apis = append(apis, rpc.API{ | ||||||||||||
| Namespace: "benchseq", | ||||||||||||
| Service: benchSequencerService, | ||||||||||||
| Public: false, | ||||||||||||
| }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| stack.RegisterAPIs(apis) | ||||||||||||
|
|
||||||||||||
| return execNode, nil | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.