feat: better enforcement on dymint flags

block/manager.go

@@ -114,21 +114,6 @@ func NewManager(
 		return nil, err
 	}
 
-	// TODO((#119): Probably should be validated and manage default on config init.
-	if conf.BlockBatchMaxSizeBytes == 0 {
-		logger.Info("WARNING: using default DA batch size bytes limit", "BlockBatchMaxSizeBytes", config.DefaultNodeConfig.BlockBatchMaxSizeBytes)
-		conf.BlockBatchMaxSizeBytes = config.DefaultNodeConfig.BlockBatchMaxSizeBytes
-	}
-	if conf.BatchSubmitMaxTime == 0 {
-		logger.Info("WARNING: using default DA batch submit max time", "BatchSubmitMaxTime", config.DefaultNodeConfig.BatchSubmitMaxTime)
-		conf.BatchSubmitMaxTime = config.DefaultNodeConfig.BatchSubmitMaxTime
-
-		//TODO: validate it's larger than empty blocks time
-	}
-	if conf.BlockTime == 0 {
-		panic("Block production time must be a positive number")
-	}
-
 	exec, err := state.NewBlockExecutor(proposerAddress, conf.NamespaceID, genesis.ChainID, mempool, proxyApp, eventBus, logger)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create block executor: %w", err)

block/manager_test.go

@@ -581,9 +581,10 @@ func initSettlementLayerMock(settlementlc settlement.LayerI, proposer string, pu
 
 func getManagerConfig() config.BlockManagerConfig {
 	return config.BlockManagerConfig{
-		BlockTime:          100 * time.Millisecond,
-		BlockBatchSize:     defaultBatchSize,
-		BatchSubmitMaxTime: 30 * time.Minute,
-		NamespaceID:        "0102030405060708",
+		BlockTime:              100 * time.Millisecond,
+		BlockBatchSize:         defaultBatchSize,
+		BlockBatchMaxSizeBytes: 1000,
+		BatchSubmitMaxTime:     30 * time.Minute,
+		NamespaceID:            "0102030405060708",
 	}
 }

block/production_test.go

@@ -177,10 +177,11 @@ func TestBatchSubmissionAfterTimeout(t *testing.T) {
 
 	// Init manager with empty blocks feature enabled
 	managerConfig := config.BlockManagerConfig{
-		BlockTime:          blockTime,
-		EmptyBlocksMaxTime: 0,
-		BatchSubmitMaxTime: submitTimeout,
-		BlockBatchSize:     batchSize,
+		BlockTime:              blockTime,
+		EmptyBlocksMaxTime:     0,
+		BatchSubmitMaxTime:     submitTimeout,
+		BlockBatchSize:         batchSize,
+		BlockBatchMaxSizeBytes: 1000,
 	}
 
 	manager, err := getManager(managerConfig, nil, nil, 1, 1, 0, proxyApp, nil)

config/config.go

@@ -1,11 +1,10 @@
 package config
 
 import (
+	"fmt"
 	"path/filepath"
 	"time"
 
-	tmcmd "github.com/tendermint/tendermint/cmd/cometbft/commands"
-
 	"github.com/dymensionxyz/dymint/settlement"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -18,31 +17,6 @@ const (
 	DefaultConfigFileName = "dymint.toml"
 )
 
-const (
-	flagAggregator         = "dymint.aggregator"
-	flagDALayer            = "dymint.da_layer"
-	flagDAConfig           = "dymint.da_config"
-	flagBlockTime          = "dymint.block_time"
-	flagEmptyBlocksMaxTime = "dymint.empty_blocks_max_time"
-	flagBatchSubmitMaxTime = "dymint.batch_submit_max_time"
-	// TODO(omritoptix): Namespace ID should be part of the DA config
-	flagNamespaceID            = "dymint.namespace_id"
-	flagBlockBatchSize         = "dymint.block_batch_size"
-	flagBlockBatchMaxSizeBytes = "dymint.block_batch_max_size_bytes"
-)
-
-const (
-	flagSettlementLayer  = "dymint.settlement_layer"
-	flagSLNodeAddress    = "dymint.settlement_config.node_address"
-	flagSLKeyringBackend = "dymint.settlement_config.keyring_backend"
-	flagSLKeyringHomeDir = "dymint.settlement_config.keyring_home_dir"
-	flagSLDymAccountName = "dymint.settlement_config.dym_account_name"
-	flagSLGasLimit       = "dymint.settlement_config.gas_limit"
-	flagSLGasPrices      = "dymint.settlement_config.gas_prices"
-	flagSLGasFees        = "dymint.settlement_config.gas_fees"
-	flagRollappID        = "dymint.settlement_config.rollapp_id"
-)
-
 // NodeConfig stores Dymint node configuration.
 type NodeConfig struct {
 	// parameters below are translated from existing config
@@ -70,13 +44,11 @@ type BlockManagerConfig struct {
 	NamespaceID        string        `mapstructure:"namespace_id"`
 	// The size of the batch in blocks. Every batch we'll write to the DA and the settlement layer.
 	BlockBatchSize uint64 `mapstructure:"block_batch_size"`
-	// The max size of the batch in Bytes. Every batch we'll write to the DA and the settlement layer.
+	// The size of the batch in Bytes. Every batch we'll write to the DA and the settlement layer.
 	BlockBatchMaxSizeBytes uint64 `mapstructure:"block_batch_max_size_bytes"`
 }
 
 // GetViperConfig reads configuration parameters from Viper instance.
-//
-// This method is called in cosmos-sdk.
 func (nc *NodeConfig) GetViperConfig(cmd *cobra.Command, homeDir string) error {
 	v := viper.GetViper()
 
@@ -103,94 +75,59 @@ func (nc *NodeConfig) GetViperConfig(cmd *cobra.Command, homeDir string) error {
 		return err
 	}
 
-	return nil
-}
+	err = nc.Validate()
+	if err != nil {
+		return err
+	}
 
-// AddFlags adds Dymint specific configuration options to cobra Command.
-//
-// This function is called in cosmos-sdk.
-func AddNodeFlags(cmd *cobra.Command) {
-	//Add tendermint default flags
-	tmcmd.AddNodeFlags(cmd)
-
-	def := DefaultNodeConfig
-
-	cmd.Flags().Bool(flagAggregator, false, "run node in aggregator mode")
-	cmd.Flags().String(flagDALayer, def.DALayer, "Data Availability Layer Client name (mock or grpc")
-	cmd.Flags().String(flagDAConfig, def.DAConfig, "Data Availability Layer Client config")
-	cmd.Flags().Duration(flagBlockTime, def.BlockTime, "block time (for aggregator mode)")
-	cmd.Flags().Duration(flagEmptyBlocksMaxTime, def.EmptyBlocksMaxTime, "max time for empty blocks (for aggregator mode)")
-	cmd.Flags().Duration(flagBatchSubmitMaxTime, def.BatchSubmitMaxTime, "max time for batch submit (for aggregator mode)")
-	cmd.Flags().String(flagNamespaceID, def.NamespaceID, "namespace identifies (8 bytes in hex)")
-	cmd.Flags().Uint64(flagBlockBatchSize, def.BlockBatchSize, "block batch size")
-	cmd.Flags().Uint64(flagBlockBatchMaxSizeBytes, def.BlockBatchMaxSizeBytes, "block batch max size in bytes")
-
-	cmd.Flags().String(flagSettlementLayer, def.SettlementLayer, "Settlement Layer Client name")
-	cmd.Flags().String(flagSLNodeAddress, def.SettlementConfig.NodeAddress, "Settlement Layer RPC node address")
-	cmd.Flags().String(flagSLKeyringBackend, def.SettlementConfig.KeyringBackend, "Sequencer keyring backend")
-	cmd.Flags().String(flagSLKeyringHomeDir, def.SettlementConfig.KeyringHomeDir, "Sequencer keyring path")
-	cmd.Flags().String(flagSLDymAccountName, def.SettlementConfig.DymAccountName, "Sequencer account name in keyring")
-	cmd.Flags().String(flagSLGasFees, def.SettlementConfig.GasFees, "Settlement Layer gas fees")
-	cmd.Flags().String(flagSLGasPrices, def.SettlementConfig.GasPrices, "Settlement Layer gas prices")
-	cmd.Flags().Uint64(flagSLGasLimit, def.SettlementConfig.GasLimit, "Settlement Layer batch submit gas limit")
-	cmd.Flags().String(flagRollappID, def.SettlementConfig.RollappID, "The chainID of the rollapp")
+	return nil
 }
 
-func BindDymintFlags(cmd *cobra.Command, v *viper.Viper) error {
-	if err := v.BindPFlag("aggregator", cmd.Flags().Lookup(flagAggregator)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("da_layer", cmd.Flags().Lookup(flagDALayer)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("da_config", cmd.Flags().Lookup(flagDAConfig)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("block_time", cmd.Flags().Lookup(flagBlockTime)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("empty_blocks_max_time", cmd.Flags().Lookup(flagEmptyBlocksMaxTime)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("batch_submit_max_time", cmd.Flags().Lookup(flagBatchSubmitMaxTime)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("namespace_id", cmd.Flags().Lookup(flagNamespaceID)); err != nil {
-		return err
+// validate BlockManagerConfig
+func (c BlockManagerConfig) Validate() error {
+	if c.BlockTime <= 0 {
+		return fmt.Errorf("block_time must be positive")
 	}
-	if err := v.BindPFlag("block_batch_size", cmd.Flags().Lookup(flagBlockBatchSize)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("block_batch_max_size_bytes", cmd.Flags().Lookup(flagBlockBatchMaxSizeBytes)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("settlement_layer", cmd.Flags().Lookup(flagSettlementLayer)); err != nil {
-		return err
-	}
-	if err := v.BindPFlag("node_address", cmd.Flags().Lookup(flagSLNodeAddress)); err != nil {
-		return err
+
+	if c.EmptyBlocksMaxTime < 0 {
+		return fmt.Errorf("empty_blocks_max_time must be positive or zero to disable")
 	}
-	if err := v.BindPFlag("keyring_backend", cmd.Flags().Lookup(flagSLKeyringBackend)); err != nil {
-		return err
+
+	if c.EmptyBlocksMaxTime <= c.BlockTime {
+		return fmt.Errorf("empty_blocks_max_time must be greater than block_time")
 	}
-	if err := v.BindPFlag("keyring_home_dir", cmd.Flags().Lookup(flagSLKeyringHomeDir)); err != nil {
-		return err
+
+	if c.BatchSubmitMaxTime <= 0 {
+		return fmt.Errorf("batch_submit_max_time must be positive")
 	}
-	if err := v.BindPFlag("dym_account_name", cmd.Flags().Lookup(flagSLDymAccountName)); err != nil {
-		return err
+
+	if c.BatchSubmitMaxTime <= c.EmptyBlocksMaxTime {
+		return fmt.Errorf("batch_submit_max_time must be greater than empty_blocks_max_time")
 	}
-	if err := v.BindPFlag("gas_fees", cmd.Flags().Lookup(flagSLGasFees)); err != nil {
-		return err
+
+	if c.BlockBatchSize <= 0 {
+		return fmt.Errorf("block_batch_size must be positive")
 	}
-	if err := v.BindPFlag("gas_prices", cmd.Flags().Lookup(flagSLGasPrices)); err != nil {
-		return err
+
+	if c.BlockBatchMaxSizeBytes <= 0 {
+		return fmt.Errorf("block_batch_size_bytes must be positive")
 	}
-	if err := v.BindPFlag("gas_limit", cmd.Flags().Lookup(flagSLGasLimit)); err != nil {
+
+	return nil
+}
+
+func (c NodeConfig) Validate() error {
+	if err := c.BlockManagerConfig.Validate(); err != nil {
 		return err
 	}
-	if err := v.BindPFlag("rollapp_id", cmd.Flags().Lookup(flagRollappID)); err != nil {
-		return err
+
+	if c.SettlementLayer != "mock" {
+		if err := c.SettlementConfig.Validate(); err != nil {
+			return err
+		}
 	}
 
+	//TODO: validate DA config
+
 	return nil
 }

config/config_test.go

@@ -23,6 +23,8 @@ func TestViperAndCobra(t *testing.T) {
 	assert.NoError(cmd.Flags().Set(flagDALayer, "foobar"))
 	assert.NoError(cmd.Flags().Set(flagDAConfig, `{"json":true}`))
 	assert.NoError(cmd.Flags().Set(flagBlockTime, "1234s"))
+	assert.NoError(cmd.Flags().Set(flagEmptyBlocksMaxTime, "2000s"))
+	assert.NoError(cmd.Flags().Set(flagBatchSubmitMaxTime, "3000s"))
 	assert.NoError(cmd.Flags().Set(flagNamespaceID, "0102030405060708"))
 	assert.NoError(cmd.Flags().Set(flagBlockBatchSize, "10"))
 
@@ -35,3 +37,5 @@ func TestViperAndCobra(t *testing.T) {
 	assert.Equal("0102030405060708", nc.NamespaceID)
 	assert.Equal(uint64(10), nc.BlockBatchSize)
 }
+
+//TODO: check invalid config

config/flags.go

@@ -0,0 +1,120 @@
+package config
+
+import (
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+	tmcmd "github.com/tendermint/tendermint/cmd/cometbft/commands"
+)
+
+const (
+	flagAggregator             = "dymint.aggregator"
+	flagDALayer                = "dymint.da_layer"
+	flagDAConfig               = "dymint.da_config"
+	flagBlockTime              = "dymint.block_time"
+	flagEmptyBlocksMaxTime     = "dymint.empty_blocks_max_time"
+	flagBatchSubmitMaxTime     = "dymint.batch_submit_max_time"
+	flagNamespaceID            = "dymint.namespace_id"
+	flagBlockBatchSize         = "dymint.block_batch_size"
+	flagBlockBatchMaxSizeBytes = "dymint.block_batch_max_size_bytes"
+)
+
+const (
+	flagSettlementLayer  = "dymint.settlement_layer"
+	flagSLNodeAddress    = "dymint.settlement_config.node_address"
+	flagSLKeyringBackend = "dymint.settlement_config.keyring_backend"
+	flagSLKeyringHomeDir = "dymint.settlement_config.keyring_home_dir"
+	flagSLDymAccountName = "dymint.settlement_config.dym_account_name"
+	flagSLGasLimit       = "dymint.settlement_config.gas_limit"
+	flagSLGasPrices      = "dymint.settlement_config.gas_prices"
+	flagSLGasFees        = "dymint.settlement_config.gas_fees"
+	flagRollappID        = "dymint.settlement_config.rollapp_id"
+)
+
+// AddFlags adds Dymint specific configuration options to cobra Command.
+//
+// This function is called in cosmos-sdk.
+func AddNodeFlags(cmd *cobra.Command) {
+	//Add tendermint default flags
+	tmcmd.AddNodeFlags(cmd)
+
+	def := DefaultNodeConfig
+
+	cmd.Flags().Bool(flagAggregator, false, "run node in aggregator mode")
+	cmd.Flags().String(flagDALayer, def.DALayer, "Data Availability Layer Client name (mock or grpc")
+	cmd.Flags().String(flagDAConfig, def.DAConfig, "Data Availability Layer Client config")
+	cmd.Flags().Duration(flagBlockTime, def.BlockTime, "block time (for aggregator mode)")
+	cmd.Flags().Duration(flagEmptyBlocksMaxTime, def.EmptyBlocksMaxTime, "max time for empty blocks (for aggregator mode)")
+	cmd.Flags().Duration(flagBatchSubmitMaxTime, def.BatchSubmitMaxTime, "max time for batch submit (for aggregator mode)")
+	cmd.Flags().String(flagNamespaceID, def.NamespaceID, "namespace identifies (8 bytes in hex)")
+	cmd.Flags().Uint64(flagBlockBatchSize, def.BlockBatchSize, "block batch size")
+	cmd.Flags().Uint64(flagBlockBatchMaxSizeBytes, def.BlockBatchMaxSizeBytes, "block batch size in bytes")
+
+	cmd.Flags().String(flagSettlementLayer, def.SettlementLayer, "Settlement Layer Client name")
+	cmd.Flags().String(flagSLNodeAddress, def.SettlementConfig.NodeAddress, "Settlement Layer RPC node address")
+	cmd.Flags().String(flagSLKeyringBackend, def.SettlementConfig.KeyringBackend, "Sequencer keyring backend")
+	cmd.Flags().String(flagSLKeyringHomeDir, def.SettlementConfig.KeyringHomeDir, "Sequencer keyring path")
+	cmd.Flags().String(flagSLDymAccountName, def.SettlementConfig.DymAccountName, "Sequencer account name in keyring")
+	cmd.Flags().String(flagSLGasFees, def.SettlementConfig.GasFees, "Settlement Layer gas fees")
+	cmd.Flags().String(flagSLGasPrices, def.SettlementConfig.GasPrices, "Settlement Layer gas prices")
+	cmd.Flags().Uint64(flagSLGasLimit, def.SettlementConfig.GasLimit, "Settlement Layer batch submit gas limit")
+	cmd.Flags().String(flagRollappID, def.SettlementConfig.RollappID, "The chainID of the rollapp")
+}
+
+func BindDymintFlags(cmd *cobra.Command, v *viper.Viper) error {
+	if err := v.BindPFlag("aggregator", cmd.Flags().Lookup(flagAggregator)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("da_layer", cmd.Flags().Lookup(flagDALayer)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("da_config", cmd.Flags().Lookup(flagDAConfig)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("block_time", cmd.Flags().Lookup(flagBlockTime)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("empty_blocks_max_time", cmd.Flags().Lookup(flagEmptyBlocksMaxTime)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("batch_submit_max_time", cmd.Flags().Lookup(flagBatchSubmitMaxTime)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("namespace_id", cmd.Flags().Lookup(flagNamespaceID)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("block_batch_size", cmd.Flags().Lookup(flagBlockBatchSize)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("block_batch_max_size_bytes", cmd.Flags().Lookup(flagBlockBatchMaxSizeBytes)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("settlement_layer", cmd.Flags().Lookup(flagSettlementLayer)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("node_address", cmd.Flags().Lookup(flagSLNodeAddress)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("keyring_backend", cmd.Flags().Lookup(flagSLKeyringBackend)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("keyring_home_dir", cmd.Flags().Lookup(flagSLKeyringHomeDir)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("dym_account_name", cmd.Flags().Lookup(flagSLDymAccountName)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("gas_fees", cmd.Flags().Lookup(flagSLGasFees)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("gas_prices", cmd.Flags().Lookup(flagSLGasPrices)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("gas_limit", cmd.Flags().Lookup(flagSLGasLimit)); err != nil {
+		return err
+	}
+	if err := v.BindPFlag("rollapp_id", cmd.Flags().Lookup(flagRollappID)); err != nil {
+		return err
+	}
+
+	return nil
+}

node/integration_test.go

@@ -46,9 +46,11 @@ func TestAggregatorMode(t *testing.T) {
 	anotherKey, _, _ := crypto.GenerateEd25519Key(rand.Reader)
 
 	blockManagerConfig := config.BlockManagerConfig{
-		BlockBatchSize: 1,
-		BlockTime:      1 * time.Second,
-		NamespaceID:    "0102030405060708",
+		BlockBatchSize:         1,
+		BlockTime:              1 * time.Second,
+		BatchSubmitMaxTime:     60 * time.Second,
+		BlockBatchMaxSizeBytes: 1000,
+		NamespaceID:            "0102030405060708",
 	}
 
 	nodeConfig := config.NodeConfig{