Integrate Redis streams in to Nitro's validation #2241
Conversation
cla-bot
bot
added
the
s
anodar
changed the title
system_tests/common_test.go
Outdated
| conf.Arbitrator.RedisValidationServerConfig.RedisURL = redisURL | ||
| t.Cleanup(func() { destroyGroup(ctx, t, redisStream, redisClient) }) | ||
| conf.Arbitrator.RedisValidationServerConfig.ModuleRoots = []string{currentRootModule(t).Hex()} | ||
| } | ||
| _, valStack := createTestValidationNode(t, ctx, &conf) | ||
| configByValidationNode(t, nodeConfig, valStack) |
There was a problem hiding this comment.
Problem here that it'll create both an rpc-validation and a redis-validation.
Need to figure how to configure to use rpc for execution only (like other comments mention),
It's fine to assume here if redisURL != "" that you do want rpc for execution and not for validation.
There was a problem hiding this comment.
Changed ValidationServerConfigs to set to nil so that there's no rpc-validation, now that we have ExecutionServerConfig separately, this can be nil.
| @@ -54,49 +55,48 @@ func (e *disableReproduce) apply(_ *ConsumerConfig, prodCfg *ProducerConfig) { | |||
|
|
|||
| func producerCfg() *ProducerConfig { | |||
There was a problem hiding this comment.
cant you just return &TestProducerConfig?
There was a problem hiding this comment.
We run tests in parallel and some of them change property on it, e.g. set EnableReproduce to false. If you use same instance for concurrently running tests, tests will become flaky.
| EnableReproduce: TestProducerConfig.EnableReproduce, | ||
| CheckPendingInterval: TestProducerConfig.CheckPendingInterval, | ||
| KeepAliveTimeout: TestProducerConfig.KeepAliveTimeout, | ||
| CheckResultInterval: TestProducerConfig.CheckResultInterval, | ||
| } | ||
| } | ||
|
|
||
| func consumerCfg() *ConsumerConfig { |
There was a problem hiding this comment.
Same as above (technically we don't change consumer config, but just to follow the pattern).
staker/block_validator.go
Outdated
| @@ -173,6 +179,7 @@ var TestBlockValidatorConfig = BlockValidatorConfig{ | |||
| Enable: false, | |||
| ValidationServer: rpcclient.TestClientConfig, | |||
| ValidationServerConfigs: []rpcclient.ClientConfig{rpcclient.TestClientConfig}, | |||
| ExecutionServerConfig: rpcclient.TestClientConfig, | |||
There was a problem hiding this comment.
need to also add ExecutionServerConfig to DefaultBlockValidatorConfig with same value as ValidationServer
validator/server_api/json.go
Outdated
| DataB64 string | ||
| } | ||
|
|
||
| type RedisValidationServerConfig struct { |
There was a problem hiding this comment.
the config , defaults and addoptions belongs in valnode/redisconsumer.go
There was a problem hiding this comment.
We can't just move them there though, it will introduce cyclic dependency valnode->server_arb ->valnode.
validator_spawner.go (server_arb) initializes redis validation server config (calls ValidationServerConfigAddOptions, that would be in valnode if I moved).
valnode.go itself spawns validator spawners so has dependency on server_arb.
I have moved out redis consumer (and also producer for consistency) into separate packages to accomodate your comment.
staker/block_validator.go
Outdated
| @@ -1060,6 +1065,9 @@ func (v *BlockValidator) Initialize(ctx context.Context) error { | |||
| } | |||
| } | |||
| log.Info("BlockValidator initialized", "current", v.currentWasmModuleRoot, "pending", v.pendingWasmModuleRoot) | |||
| if err := v.StatelessBlockValidator.Initialize([]common.Hash{v.currentWasmModuleRoot, v.pendingWasmModuleRoot}); err != nil { | |||
There was a problem hiding this comment.
check if they are identical, and if so only send one - this is common and we don't want a warning on boot
| c.StopWaiter.StopAndWait() | ||
| } | ||
|
|
||
| func (c *ValidationClient) Name() string { |
There was a problem hiding this comment.
here we can just return name as it's not from cfg
| if err != nil { | ||
| log.Warn("closing execution client run got error", "err", err, "client", r.client.Name(), "id", r.id) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func ValidationInputToJson(entry *validator.ValidationInput) *server_api.InputJSON { |
There was a problem hiding this comment.
can that move to be with InputJSON?
| @@ -182,3 +182,34 @@ func (a *ExecServerAPI) CloseExec(execid uint64) { | |||
| run.run.Close() | |||
| delete(a.runs, execid) | |||
| } | |||
|
|
|||
| func ValidationInputFromJson(entry *server_api.InputJSON) (*validator.ValidationInput, error) { | |||
There was a problem hiding this comment.
can this move to be near InputJSON?
No description provided.