From a0268fe9e196b148705d7abf0484868154046c92 Mon Sep 17 00:00:00 2001
From: Nodar Ambroladze <nodar.ambroladze@gmail.com>
Date: Mon, 22 Apr 2024 16:06:28 +0200
Subject: [PATCH] Add config validation

---
 staker/block_validator.go           |  6 ++++++
 staker/stateless_block_validator.go | 13 ++++++-------
 validator/client/redisproducer.go   | 29 ++++++++++++++++++++++++++---
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/staker/block_validator.go b/staker/block_validator.go
index cd89ccf650..806e5d44a1 100644
--- a/staker/block_validator.go
+++ b/staker/block_validator.go
@@ -134,6 +134,9 @@ func (c *BlockValidatorConfig) Validate() error {
 	if err := c.ExecutionServerConfig.Validate(); err != nil {
 		return fmt.Errorf("validating execution server config: %w", err)
 	}
+	if err := c.RedisValidationClientConfig.Validate(); err != nil {
+		return fmt.Errorf("validating redis validation client configuration: %w", err)
+	}
 	return nil
 }
 
@@ -146,6 +149,8 @@ type BlockValidatorConfigFetcher func() *BlockValidatorConfig
 func BlockValidatorConfigAddOptions(prefix string, f *pflag.FlagSet) {
 	f.Bool(prefix+".enable", DefaultBlockValidatorConfig.Enable, "enable block-by-block validation")
 	rpcclient.RPCClientAddOptions(prefix+".validation-server", f, &DefaultBlockValidatorConfig.ValidationServer)
+	rpcclient.RPCClientAddOptions(prefix+".execution-server-config", f, &DefaultBlockValidatorConfig.ExecutionServerConfig)
+	validatorclient.RedisValidationClientConfigAddOptions(prefix+"redis-validation-client-config", f)
 	f.String(prefix+".validation-server-configs-list", DefaultBlockValidatorConfig.ValidationServerConfigsList, "array of validation rpc configs given as a json string. time duration should be supplied in number indicating nanoseconds")
 	f.Duration(prefix+".validation-poll", DefaultBlockValidatorConfig.ValidationPoll, "poll time to check validations")
 	f.Uint64(prefix+".forward-blocks", DefaultBlockValidatorConfig.ForwardBlocks, "prepare entries for up to that many blocks ahead of validation (small footprint)")
@@ -165,6 +170,7 @@ var DefaultBlockValidatorConfig = BlockValidatorConfig{
 	Enable:                      false,
 	ValidationServerConfigsList: "default",
 	ValidationServer:            rpcclient.DefaultClientConfig,
+	ExecutionServerConfig:       rpcclient.DefaultClientConfig,
 	ValidationPoll:              time.Second,
 	ForwardBlocks:               1024,
 	PrerecordedBlocks:           uint64(2 * runtime.NumCPU()),
diff --git a/staker/stateless_block_validator.go b/staker/stateless_block_validator.go
index 8386d0b80c..74b87f0291 100644
--- a/staker/stateless_block_validator.go
+++ b/staker/stateless_block_validator.go
@@ -208,7 +208,10 @@ func NewStatelessBlockValidator(
 		validationSpawners = append(validationSpawners, validatorclient.NewValidationClient(valConfFetcher, stack))
 	}
 
-	validator := &StatelessBlockValidator{
+	valConfFetcher := func() *rpcclient.ClientConfig {
+		return &config().ExecutionServerConfig
+	}
+	return &StatelessBlockValidator{
 		config:             config(),
 		recorder:           recorder,
 		validationSpawners: validationSpawners,
@@ -218,12 +221,8 @@ func NewStatelessBlockValidator(
 		db:                 arbdb,
 		daService:          das,
 		blobReader:         blobReader,
-	}
-	valConfFetcher := func() *rpcclient.ClientConfig {
-		return &config().ExecutionServerConfig
-	}
-	validator.execSpawner = validatorclient.NewExecutionClient(valConfFetcher, stack)
-	return validator, nil
+		execSpawner:        validatorclient.NewExecutionClient(valConfFetcher, stack),
+	}, nil
 }
 
 func (v *StatelessBlockValidator) GetModuleRootsToValidate() []common.Hash {
diff --git a/validator/client/redisproducer.go b/validator/client/redisproducer.go
index a2a9d28eb5..50e58c4e66 100644
--- a/validator/client/redisproducer.go
+++ b/validator/client/redisproducer.go
@@ -6,6 +6,7 @@ import (
 	"sync/atomic"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/log"
 	"github.com/offchainlabs/nitro/pubsub"
 	"github.com/offchainlabs/nitro/util/containers"
 	"github.com/offchainlabs/nitro/util/redisutil"
@@ -23,12 +24,35 @@ type RedisValidationClientConfig struct {
 	ProducerConfig pubsub.ProducerConfig `koanf:"producer-config"`
 	// Supported wasm module roots, when the list is empty this is disabled.
 	ModuleRoots []string `koanf:"module-roots"`
+	moduleRoots []common.Hash
 }
 
 func (c RedisValidationClientConfig) Enabled() bool {
 	return c.RedisURL != ""
 }
 
+func (c *RedisValidationClientConfig) Validate() error {
+	m := make(map[string]bool)
+	// Add all moduleRoot hashes in case Validate is called twice so that we
+	// don't add duplicate moduleRoots again.
+	for _, mr := range c.moduleRoots {
+		m[mr.Hex()] = true
+	}
+	for _, mr := range c.ModuleRoots {
+		if _, exists := m[mr]; exists {
+			log.Warn("Duplicate module root", "hash", mr)
+			continue
+		}
+		h := common.HexToHash(mr)
+		if h == (common.Hash{}) {
+			return fmt.Errorf("invalid module root hash: %q", mr)
+		}
+		m[mr] = true
+		c.moduleRoots = append(c.moduleRoots, h)
+	}
+	return nil
+}
+
 var DefaultRedisValidationClientConfig = RedisValidationClientConfig{
 	Name:           "redis validation client",
 	Room:           2,
@@ -72,11 +96,10 @@ func NewRedisValidationClient(cfg *RedisValidationClientConfig) (*RedisValidatio
 	if err != nil {
 		return nil, err
 	}
-	if len(cfg.ModuleRoots) == 0 {
+	if len(cfg.moduleRoots) == 0 {
 		return nil, fmt.Errorf("moduleRoots must be specified to enable redis streams")
 	}
-	for _, hash := range cfg.ModuleRoots {
-		mr := common.HexToHash(hash)
+	for _, mr := range cfg.moduleRoots {
 		p, err := pubsub.NewProducer[*validator.ValidationInput, validator.GoGlobalState](
 			redisClient, server_api.RedisStreamForRoot(mr), &cfg.ProducerConfig)
 		if err != nil {