Obsolete block time flag from server command (#1365)

* refactor(server): remove block time configuration from raw config and server params (#1363) The block time configuration was removed from the raw config and server params. The initBlockTime function was also removed from the server params. * Resolve block time from consensus engine parameters * Provide zero value for block time for dummy and dev consensuses * Move DefaultBlockTime to IBFT E2E tests * Make sure that block time is not less than 1 second --------- Co-authored-by: Flavor Town <117117716+flavor-town@users.noreply.github.com>
0xPolygon · Apr 7, 2023 · 5d4c9d9 · 5d4c9d9
1 parent 3765811
commit 5d4c9d9
Show file tree

Hide file tree

Showing 19 changed files with 167 additions and 49 deletions.
diff --git a/command/genesis/params.go b/command/genesis/params.go
@@ -299,6 +299,7 @@ func (p *genesisParams) initIBFTEngineMap(ibftType fork.IBFTType) {
 		string(server.IBFTConsensus): map[string]interface{}{
 			fork.KeyType:          ibftType,
 			fork.KeyValidatorType: p.ibftValidatorType,
+			fork.KeyBlockTime:     p.blockTime,
 			ibft.KeyEpochSize:     p.epochSize,
 		},
 	}

diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi/artifact"
+	"github.com/0xPolygon/polygon-edge/helper/common"
 
 	"github.com/0xPolygon/polygon-edge/chain"
 	"github.com/0xPolygon/polygon-edge/command"
@@ -88,7 +89,7 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er
 
 	polyBftConfig := &polybft.PolyBFTConfig{
 		InitialValidatorSet: manifest.GenesisValidators,
-		BlockTime:           p.blockTime,
+		BlockTime:           common.Duration{Duration: p.blockTime},
 		EpochSize:           p.epochSize,
 		SprintSize:          p.sprintSize,
 		EpochReward:         p.epochReward,

diff --git a/command/ibft/switch/params.go b/command/ibft/switch/params.go
@@ -403,6 +403,7 @@ func appendIBFTForks(
 		Type:          ibftType,
 		ValidatorType: validatorType,
 		From:          common.JSONNumber{Value: from},
+		BlockTime:     lastFork.BlockTime,
 	}
 
 	switch ibftType {

diff --git a/command/server/config/config.go b/command/server/config/config.go
@@ -25,7 +25,6 @@ type Config struct {
 	TxPool                   *TxPool    `json:"tx_pool" yaml:"tx_pool"`
 	LogLevel                 string     `json:"log_level" yaml:"log_level"`
 	RestoreFile              string     `json:"restore_file" yaml:"restore_file"`
-	BlockTime                uint64     `json:"block_time_s" yaml:"block_time_s"`
 	Headers                  *Headers   `json:"headers" yaml:"headers"`
 	LogFilePath              string     `json:"log_to" yaml:"log_to"`
 	JSONRPCBatchRequestLimit uint64     `json:"json_rpc_batch_request_limit" yaml:"json_rpc_batch_request_limit"`
@@ -65,9 +64,6 @@ type Headers struct {
 }
 
 const (
-	// DefaultBlockTime minimum block generation time in seconds
-	DefaultBlockTime uint64 = 2
-
 	// BlockTimeMultiplierForTimeout Multiplier to get IBFT timeout from block time
 	// timeout is calculated when IBFT timeout is not specified
 	BlockTimeMultiplierForTimeout uint64 = 5
@@ -111,7 +107,6 @@ func DefaultConfig() *Config {
 		},
 		LogLevel:    "INFO",
 		RestoreFile: "",
-		BlockTime:   DefaultBlockTime,
 		Headers: &Headers{
 			AccessControlAllowOrigins: []string{"*"},
 		},

diff --git a/command/server/init.go b/command/server/init.go
@@ -19,7 +19,6 @@ import (
 )
 
 var (
-	errInvalidBlockTime       = errors.New("invalid block time specified")
 	errDataDirectoryUndefined = errors.New("data directory not defined")
 )
 
@@ -50,10 +49,6 @@ func (p *serverParams) initRawParams() error {
 		return err
 	}
 
-	if err := p.initBlockTime(); err != nil {
-		return err
-	}
-
 	if p.isDevMode {
 		p.initDevMode()
 	}
@@ -66,14 +61,6 @@ func (p *serverParams) initRawParams() error {
 	return p.initAddresses()
 }
 
-func (p *serverParams) initBlockTime() error {
-	if p.rawConfig.BlockTime < 1 {
-		return errInvalidBlockTime
-	}
-
-	return nil
-}
-
 func (p *serverParams) initDataDirLocation() error {
 	if p.rawConfig.DataDir == "" {
 		return errDataDirectoryUndefined

diff --git a/command/server/params.go b/command/server/params.go
@@ -33,7 +33,6 @@ const (
 	blockGasTargetFlag           = "block-gas-target"
 	secretsConfigFlag            = "secrets-config"
 	restoreFlag                  = "restore"
-	blockTimeFlag                = "block-time"
 	devIntervalFlag              = "dev-interval"
 	devFlag                      = "dev"
 	corsOriginFlag               = "access-control-allow-origins"
@@ -179,7 +178,6 @@ func (p *serverParams) generateConfig() *server.Config {
 		MaxAccountEnqueued: p.rawConfig.TxPool.MaxAccountEnqueued,
 		SecretsManager:     p.secretsConfig,
 		RestoreFile:        p.getRestoreFilePath(),
-		BlockTime:          p.rawConfig.BlockTime,
 		LogLevel:           hclog.LevelFromString(p.rawConfig.LogLevel),
 		JSONLogFormat:      p.rawConfig.JSONLogFormat,
 		LogFilePath:        p.logFileLocation,

diff --git a/command/server/server.go b/command/server/server.go
@@ -185,13 +185,6 @@ func setFlags(cmd *cobra.Command) {
 		"maximum number of enqueued transactions per account",
 	)
 
-	cmd.Flags().Uint64Var(
-		&params.rawConfig.BlockTime,
-		blockTimeFlag,
-		defaultConfig.BlockTime,
-		"minimum block time in seconds (at least 1s)",
-	)
-
 	cmd.Flags().StringArrayVar(
 		&params.corsAllowedOrigins,
 		corsOriginFlag,

diff --git a/consensus/ibft/fork/fork.go b/consensus/ibft/fork/fork.go
@@ -13,10 +13,12 @@ const (
 	KeyType          = "type"
 	KeyTypes         = "types"
 	KeyValidatorType = "validator_type"
+	KeyBlockTime     = "blockTime"
 )
 
 var (
 	ErrUndefinedIBFTConfig = errors.New("IBFT config is not defined")
+	errInvalidBlockTime    = errors.New("invalid block time provided")
 )
 
 // IBFT Fork represents setting in params.engine.ibft of genesis.json
@@ -26,6 +28,7 @@ type IBFTFork struct {
 	Deployment    *common.JSONNumber       `json:"deployment,omitempty"`
 	From          common.JSONNumber        `json:"from"`
 	To            *common.JSONNumber       `json:"to,omitempty"`
+	BlockTime     *common.Duration         `json:"blockTime,omitempty"`
 
 	// PoA
 	Validators validators.Validators `json:"validators,omitempty"`
@@ -42,6 +45,7 @@ func (f *IBFTFork) UnmarshalJSON(data []byte) error {
 		Deployment        *common.JSONNumber        `json:"deployment,omitempty"`
 		From              common.JSONNumber         `json:"from"`
 		To                *common.JSONNumber        `json:"to,omitempty"`
+		BlockTime         *common.Duration          `json:"blockTime,omitempty"`
 		Validators        interface{}               `json:"validators,omitempty"`
 		MaxValidatorCount *common.JSONNumber        `json:"maxValidatorCount,omitempty"`
 		MinValidatorCount *common.JSONNumber        `json:"minValidatorCount,omitempty"`
@@ -55,6 +59,7 @@ func (f *IBFTFork) UnmarshalJSON(data []byte) error {
 	f.Deployment = raw.Deployment
 	f.From = raw.From
 	f.To = raw.To
+	f.BlockTime = raw.BlockTime
 	f.MaxValidatorCount = raw.MaxValidatorCount
 	f.MinValidatorCount = raw.MinValidatorCount
 
@@ -74,11 +79,7 @@ func (f *IBFTFork) UnmarshalJSON(data []byte) error {
 		return err
 	}
 
-	if err := json.Unmarshal(validatorsBytes, f.Validators); err != nil {
-		return err
-	}
-
-	return nil
+	return json.Unmarshal(validatorsBytes, f.Validators)
 }
 
 // GetIBFTForks returns IBFT fork configurations from chain config
@@ -98,13 +99,28 @@ func GetIBFTForks(ibftConfig map[string]interface{}) (IBFTForks, error) {
 			}
 		}
 
+		var blockTime *common.Duration = nil
+
+		blockTimeGeneric, ok := ibftConfig[KeyBlockTime]
+		if ok {
+			blockTimeRaw, err := json.Marshal(blockTimeGeneric)
+			if err != nil {
+				return nil, errInvalidBlockTime
+			}
+
+			if err := json.Unmarshal(blockTimeRaw, &blockTime); err != nil {
+				return nil, errInvalidBlockTime
+			}
+		}
+
 		return IBFTForks{
 			{
 				Type:          typ,
 				Deployment:    nil,
 				ValidatorType: validatorType,
 				From:          common.JSONNumber{Value: 0},
 				To:            nil,
+				BlockTime:     blockTime,
 			},
 		}, nil
 	}

diff --git a/consensus/ibft/fork/fork_test.go b/consensus/ibft/fork/fork_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/0xPolygon/polygon-edge/helper/common"
 	testHelper "github.com/0xPolygon/polygon-edge/helper/tests"
@@ -62,6 +63,7 @@ func TestIBFTForkUnmarshalJSON(t *testing.T) {
 			data: fmt.Sprintf(`{
 				"type": "%s",
 				"validator_type": "%s",
+				"blockTime": %d,
 				"validators": [
 					{
 						"Address": "%s"
@@ -72,8 +74,9 @@ func TestIBFTForkUnmarshalJSON(t *testing.T) {
 				],
 				"from": %d,
 				"to": %d
-			}`,
+				}`,
 				PoA, validators.ECDSAValidatorType,
+				3000000000,
 				types.StringToAddress("1"), types.StringToAddress("2"),
 				16, 20,
 			),
@@ -86,6 +89,7 @@ func TestIBFTForkUnmarshalJSON(t *testing.T) {
 					validators.NewECDSAValidator(types.StringToAddress("1")),
 					validators.NewECDSAValidator(types.StringToAddress("2")),
 				),
+				BlockTime:         &common.Duration{Duration: 3 * time.Second},
 				MaxValidatorCount: nil,
 				MinValidatorCount: nil,
 			},

diff --git a/consensus/polybft/consensus_runtime.go b/consensus/polybft/consensus_runtime.go
@@ -310,7 +310,7 @@ func (c *consensusRuntime) FSM() error {
 		parent,
 		types.Address(c.config.Key.Address()),
 		c.config.txPool,
-		c.config.PolyBFTConfig.BlockTime,
+		c.config.PolyBFTConfig.BlockTime.Duration,
 		c.logger,
 	)
 

diff --git a/consensus/polybft/consensus_runtime_test.go b/consensus/polybft/consensus_runtime_test.go
@@ -15,6 +15,7 @@ import (
 	bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/wallet"
 	"github.com/0xPolygon/polygon-edge/contracts"
+	"github.com/0xPolygon/polygon-edge/helper/common"
 	"github.com/0xPolygon/polygon-edge/types"
 	"github.com/hashicorp/go-hclog"
 	"github.com/stretchr/testify/assert"
@@ -447,7 +448,7 @@ func Test_NewConsensusRuntime(t *testing.T) {
 		},
 		EpochSize:  10,
 		SprintSize: 10,
-		BlockTime:  2 * time.Second,
+		BlockTime:  common.Duration{Duration: 2 * time.Second},
 	}
 
 	validators := newTestValidators(t, 3).getPublicIdentities()

diff --git a/consensus/polybft/polybft_config.go b/consensus/polybft/polybft_config.go
@@ -7,7 +7,6 @@ import (
 	"math/big"
 	"os"
 	"path/filepath"
-	"time"
 
 	"github.com/0xPolygon/polygon-edge/chain"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi"
@@ -34,7 +33,7 @@ type PolyBFTConfig struct {
 	SprintSize uint64 `json:"sprintSize"`
 
 	// BlockTime is target frequency of blocks production
-	BlockTime time.Duration `json:"blockTime"`
+	BlockTime common.Duration `json:"blockTime"`
 
 	// Governance is the initial governance address
 	Governance types.Address `json:"governance"`

diff --git a/consensus/polybft/sc_integration_test.go b/consensus/polybft/sc_integration_test.go
@@ -6,7 +6,6 @@ import (
 	"math/big"
 	"strconv"
 	"testing"
-	"time"
 
 	"github.com/0xPolygon/polygon-edge/chain"
 	"github.com/0xPolygon/polygon-edge/consensus/polybft/bitmap"
@@ -278,7 +277,6 @@ func TestIntegration_CommitEpoch(t *testing.T) {
 
 		polyBFTConfig := PolyBFTConfig{
 			InitialValidatorSet: initValidators,
-			BlockTime:           2 * time.Second,
 			EpochSize:           24 * 60 * 60 / 2,
 			SprintSize:          5,
 			EpochReward:         reward,

diff --git a/e2e/framework/testserver.go b/e2e/framework/testserver.go
@@ -277,6 +277,13 @@ func (t *TestServer) GenerateGenesis() error {
 		args = append(args, "--premine", acct.Addr.String()+":0x"+acct.Balance.Text(16))
 	}
 
+	// provide block time flag
+	// (e2e framework expects BlockTime parameter to be provided in seconds)
+	if t.Config.BlockTime != 0 {
+		args = append(args, "--block-time",
+			(time.Duration(t.Config.BlockTime) * time.Second).String())
+	}
+
 	// add consensus flags
 	switch t.Config.Consensus {
 	case ConsensusIBFT:
@@ -429,10 +436,6 @@ func (t *TestServer) Start(ctx context.Context) error {
 		args = append(args, "--block-gas-target", *types.EncodeUint64(t.Config.BlockGasTarget))
 	}
 
-	if t.Config.BlockTime != 0 {
-		args = append(args, "--block-time", strconv.FormatUint(t.Config.BlockTime, 10))
-	}
-
 	if t.Config.IBFTBaseTimeout != 0 {
 		args = append(args, "--ibft-base-timeout", strconv.FormatUint(t.Config.IBFTBaseTimeout, 10))
 	}

diff --git a/e2e/ibft_test.go b/e2e/ibft_test.go
@@ -6,7 +6,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/0xPolygon/polygon-edge/command/server/config"
 	ibftSigner "github.com/0xPolygon/polygon-edge/consensus/ibft/signer"
 	"github.com/0xPolygon/polygon-edge/e2e/framework"
 	"github.com/0xPolygon/polygon-edge/helper/tests"
@@ -19,6 +18,8 @@ import (
 // TestIbft_Transfer sends a transfer transaction (EOA -> EOA)
 // and verifies it was mined
 func TestIbft_Transfer(t *testing.T) {
+	const defaultBlockTime uint64 = 2
+
 	testCases := []struct {
 		name            string
 		blockTime       uint64
@@ -27,7 +28,7 @@ func TestIbft_Transfer(t *testing.T) {
 	}{
 		{
 			name:            "default block time",
-			blockTime:       config.DefaultBlockTime,
+			blockTime:       defaultBlockTime,
 			ibftBaseTimeout: 0, // use default value
 			validatorType:   validators.ECDSAValidatorType,
 		},
@@ -39,7 +40,7 @@ func TestIbft_Transfer(t *testing.T) {
 		},
 		{
 			name:            "with BLS",
-			blockTime:       config.DefaultBlockTime,
+			blockTime:       defaultBlockTime,
 			ibftBaseTimeout: 0, // use default value
 			validatorType:   validators.BLSValidatorType,
 		},