From 458649580cb3adc3f5553099cfe9afc3a6f1a70b Mon Sep 17 00:00:00 2001 From: Yashk767 <76935991+Yashk767@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:08:56 +0530 Subject: [PATCH] feat: added warnings and validations for incorrect config parameters (#1188) * refactor: added validations for config parameters * refactor: modified default values for config parameters * fix: removed equal operator from comparison of config values * refactor: fixed config getter tests --- cmd/config-utils.go | 129 ++++++++++++++++++++++++++++-- cmd/config-utils_test.go | 169 ++++++++++++++++++++++++++++++++++++--- core/constants.go | 4 +- 3 files changed, 283 insertions(+), 19 deletions(-) diff --git a/cmd/config-utils.go b/cmd/config-utils.go index 7e2dbb647..d4e856cd7 100644 --- a/cmd/config-utils.go +++ b/cmd/config-utils.go @@ -185,29 +185,77 @@ func (*UtilsStruct) GetAlternateProvider() (string, error) { //This function returns the multiplier func (*UtilsStruct) GetMultiplier() (float32, error) { + const ( + MinMultiplier = 1.0 // Minimum multiplier value + MaxMultiplier = 3.0 // Maximum multiplier value + ) + gasMultiplier, err := getConfigValue("gasmultiplier", "float32", core.DefaultGasMultiplier, "gasmultiplier") if err != nil { return core.DefaultGasMultiplier, err } - return gasMultiplier.(float32), nil + + multiplierFloat32 := gasMultiplier.(float32) + + // Validate multiplier range + if multiplierFloat32 < MinMultiplier || multiplierFloat32 > MaxMultiplier { + log.Infof("GasMultiplier %.2f is out of the valid range (%.1f-%.1f), using default value %.2f", multiplierFloat32, MinMultiplier, MaxMultiplier, core.DefaultGasMultiplier) + return core.DefaultGasMultiplier, nil + } + + return multiplierFloat32, nil } //This function returns the buffer percent func (*UtilsStruct) GetBufferPercent() (int32, error) { + const ( + MinBufferPercent = 10 + MaxBufferPercent = 30 + ) + bufferPercent, err := getConfigValue("buffer", "int32", core.DefaultBufferPercent, "buffer") if err != nil { return core.DefaultBufferPercent, err } - return bufferPercent.(int32), nil + + bufferPercentInt32 := bufferPercent.(int32) + + // Check if bufferPercent is explicitly set and not within the valid range. + if bufferPercentInt32 < MinBufferPercent || bufferPercentInt32 > MaxBufferPercent { + log.Infof("BufferPercent %d is out of the valid range (%d-%d), using default value %d", bufferPercentInt32, MinBufferPercent, MaxBufferPercent, core.DefaultBufferPercent) + return core.DefaultBufferPercent, nil + } + + // If bufferPercent is 0, use the default value. + if bufferPercentInt32 == 0 { + log.Debugf("BufferPercent is unset, using default value %d", core.DefaultBufferPercent) + return core.DefaultBufferPercent, nil + } + + return bufferPercentInt32, nil } //This function returns the wait time func (*UtilsStruct) GetWaitTime() (int32, error) { + const ( + MinWaitTime = 1 // Minimum wait time in seconds + MaxWaitTime = 30 // Maximum wait time in seconds + ) + waitTime, err := getConfigValue("wait", "int32", core.DefaultWaitTime, "wait") if err != nil { return core.DefaultWaitTime, err } - return waitTime.(int32), nil + + waitTimeInt32 := waitTime.(int32) + + // Validate waitTime range + if waitTimeInt32 < MinWaitTime || waitTimeInt32 > MaxWaitTime { + log.Infof("WaitTime %d is out of the valid range (%d-%d), using default value %d", waitTimeInt32, MinWaitTime, MaxWaitTime, core.DefaultWaitTime) + return core.DefaultWaitTime, nil + } + + return waitTimeInt32, nil } //This function returns the gas price @@ -216,7 +264,16 @@ func (*UtilsStruct) GetGasPrice() (int32, error) { if err != nil { return core.DefaultGasPrice, err } - return gasPrice.(int32), nil + + gasPriceInt32 := gasPrice.(int32) + + // Validate gasPrice value + if gasPriceInt32 != 0 && gasPriceInt32 != 1 { + log.Infof("GasPrice %d is invalid, using default value %d", gasPriceInt32, core.DefaultGasPrice) + return core.DefaultGasPrice, nil + } + + return gasPriceInt32, nil } //This function returns the log level @@ -230,37 +287,93 @@ func (*UtilsStruct) GetLogLevel() (string, error) { //This function returns the gas limit func (*UtilsStruct) GetGasLimit() (float32, error) { + //gasLimit in the config acts as a gasLimit multiplier + const ( + MinGasLimit = 1.0 // Minimum gas limit + MaxGasLimit = 3.0 // Maximum gas limit + ) + gasLimit, err := getConfigValue("gasLimit", "float32", core.DefaultGasLimit, "gasLimit") if err != nil { return core.DefaultGasLimit, err } - return gasLimit.(float32), nil + + gasLimitFloat32 := gasLimit.(float32) + + // Validate gasLimit range + if gasLimitFloat32 < MinGasLimit || gasLimitFloat32 > MaxGasLimit { + log.Warnf("GasLimit %.2f is out of the suggested range (%.1f-%.1f), using default value %.2f", gasLimitFloat32, MinGasLimit, MaxGasLimit, core.DefaultGasLimit) + } + + return gasLimitFloat32, nil } //This function returns the gas limit to override func (*UtilsStruct) GetGasLimitOverride() (uint64, error) { + const ( + MinGasLimitOverride = 10000000 // Minimum gas limit override + MaxGasLimitOverride = 50000000 // Maximum gas limit override + ) + gasLimitOverride, err := getConfigValue("gasLimitOverride", "uint64", core.DefaultGasLimitOverride, "gasLimitOverride") if err != nil { return core.DefaultGasLimitOverride, err } - return gasLimitOverride.(uint64), nil + + gasLimitOverrideUint64 := gasLimitOverride.(uint64) + + // Validate gasLimitOverride range + if gasLimitOverrideUint64 < MinGasLimitOverride || gasLimitOverrideUint64 > MaxGasLimitOverride { + log.Infof("GasLimitOverride %d is out of the valid range (%d-%d), using default value %d", gasLimitOverrideUint64, MinGasLimitOverride, MaxGasLimitOverride, core.DefaultGasLimitOverride) + return core.DefaultGasLimitOverride, nil + } + + return gasLimitOverrideUint64, nil } //This function returns the RPC timeout func (*UtilsStruct) GetRPCTimeout() (int64, error) { + const ( + MinRPCTimeout = 10 // Minimum RPC timeout in seconds + MaxRPCTimeout = 60 // Maximum RPC timeout in seconds + ) + rpcTimeout, err := getConfigValue("rpcTimeout", "int64", core.DefaultRPCTimeout, "rpcTimeout") if err != nil { return core.DefaultRPCTimeout, err } - return rpcTimeout.(int64), nil + + rpcTimeoutInt64 := rpcTimeout.(int64) + + // Validate rpcTimeout range + if rpcTimeoutInt64 < MinRPCTimeout || rpcTimeoutInt64 > MaxRPCTimeout { + log.Infof("RPCTimeout %d is out of the valid range (%d-%d), using default value %d", rpcTimeoutInt64, MinRPCTimeout, MaxRPCTimeout, core.DefaultRPCTimeout) + return core.DefaultRPCTimeout, nil + } + + return rpcTimeoutInt64, nil } func (*UtilsStruct) GetHTTPTimeout() (int64, error) { + const ( + MinHTTPTimeout = 10 // Minimum HTTP timeout in seconds + MaxHTTPTimeout = 60 // Maximum HTTP timeout in seconds + ) + httpTimeout, err := getConfigValue("httpTimeout", "int64", core.DefaultHTTPTimeout, "httpTimeout") if err != nil { return core.DefaultHTTPTimeout, err } - return httpTimeout.(int64), nil + + httpTimeoutInt64 := httpTimeout.(int64) + + // Validate httpTimeout range + if httpTimeoutInt64 < MinHTTPTimeout || httpTimeoutInt64 > MaxHTTPTimeout { + log.Infof("HTTPTimeout %d is out of the valid range (%d-%d), using default value %d", httpTimeoutInt64, MinHTTPTimeout, MaxHTTPTimeout, core.DefaultHTTPTimeout) + return core.DefaultHTTPTimeout, nil + } + + return httpTimeoutInt64, nil } func (*UtilsStruct) GetLogFileMaxSize() (int, error) { diff --git a/cmd/config-utils_test.go b/cmd/config-utils_test.go index 049b47623..736c3b9ac 100644 --- a/cmd/config-utils_test.go +++ b/cmd/config-utils_test.go @@ -251,7 +251,7 @@ func TestGetBufferPercent(t *testing.T) { } tests := []struct { name string - useDummyConfigFile bool // Add this flag to decide whether to use dummy file or viper.Set + useDummyConfigFile bool args args want int32 wantErr error @@ -288,6 +288,15 @@ func TestGetBufferPercent(t *testing.T) { want: core.DefaultBufferPercent, wantErr: nil, }, + { + name: "Test 5: When buffer value is out of a valid range", + useDummyConfigFile: true, + args: args{ + bufferInTestConfig: 40, + }, + want: core.DefaultBufferPercent, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -362,9 +371,9 @@ func TestGetGasLimit(t *testing.T) { name: "Test 3: When gas value is fetched from config", useDummyConfigFile: true, args: args{ - gasLimitInTestConfig: 3.5, + gasLimitInTestConfig: 2.5, }, - want: 3.5, + want: 2.5, wantErr: nil, }, { @@ -372,6 +381,15 @@ func TestGetGasLimit(t *testing.T) { want: core.DefaultGasLimit, wantErr: nil, }, + { + name: "Test 5: When gas limit value is out of valid range", + useDummyConfigFile: true, + args: args{ + gasLimitInTestConfig: 3.5, + }, + want: 3.5, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -423,9 +441,9 @@ func TestGetGasLimitOverride(t *testing.T) { name: "Test 1: When gasLimitOverride is fetched from root flag", args: args{ isFlagSet: true, - gasLimitOverride: 4000000, + gasLimitOverride: 40000000, }, - want: 4000000, + want: 40000000, wantErr: nil, }, { @@ -441,9 +459,9 @@ func TestGetGasLimitOverride(t *testing.T) { name: "Test 3: When gasLimitOverride is fetched from config", useDummyConfigFile: true, args: args{ - gasLimitOverrideInConfig: 3000000, + gasLimitOverrideInConfig: 30000000, }, - want: 3000000, + want: 30000000, wantErr: nil, }, { @@ -451,6 +469,15 @@ func TestGetGasLimitOverride(t *testing.T) { want: core.DefaultGasLimitOverride, wantErr: nil, }, + { + name: "Test 3: When gasLimitOverride is fetched from config", + useDummyConfigFile: true, + args: args{ + gasLimitOverrideInConfig: 60000000, + }, + want: core.DefaultGasLimitOverride, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -521,9 +548,9 @@ func TestGetGasPrice(t *testing.T) { name: "Test 3: When gasPrice value is fetched from config", useDummyConfigFile: true, args: args{ - gasPriceInTestConfig: 2, + gasPriceInTestConfig: 0, }, - want: 2, + want: 0, wantErr: nil, }, { @@ -531,6 +558,15 @@ func TestGetGasPrice(t *testing.T) { want: core.DefaultGasPrice, wantErr: nil, }, + { + name: "Test 5: When gasPrice is out of valid range", + useDummyConfigFile: true, + args: args{ + gasPriceInTestConfig: 3, + }, + want: core.DefaultGasPrice, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -690,6 +726,15 @@ func TestGetMultiplier(t *testing.T) { want: core.DefaultGasMultiplier, wantErr: nil, }, + { + name: "Test 5: When gasMultiplier is out of a valid range", + useDummyConfigFile: true, + args: args{ + gasMultiplierInTestConfig: 4, + }, + want: core.DefaultGasMultiplier, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -937,6 +982,15 @@ func TestGetRPCTimeout(t *testing.T) { want: core.DefaultRPCTimeout, wantErr: nil, }, + { + name: "Test 5: When rpcTimeout value is out of a valid range", + useDummyConfigFile: true, + args: args{ + rpcTimeoutInTestConfig: 70, + }, + want: core.DefaultRPCTimeout, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1016,6 +1070,15 @@ func TestGetHTTPTimeout(t *testing.T) { want: core.DefaultHTTPTimeout, wantErr: nil, }, + { + name: "Test 5: When httpTimeout is out of valid range", + useDummyConfigFile: true, + args: args{ + httpTimeoutInTestConfig: 70, + }, + want: core.DefaultHTTPTimeout, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1048,3 +1111,91 @@ func TestGetHTTPTimeout(t *testing.T) { }) } } + +func TestGetWaitTime(t *testing.T) { + type args struct { + isFlagSet bool + waitTime int32 + waitTimeErr error + waitInTestConfig int32 + } + tests := []struct { + name string + useDummyConfigFile bool + args args + want int32 + wantErr error + }{ + { + name: "Test 1: When wait time is fetched from root flag", + args: args{ + isFlagSet: true, + waitTime: 10, + }, + want: 10, + wantErr: nil, + }, + { + name: "Test 2: When there is an error in fetching wait time from root flag", + args: args{ + isFlagSet: true, + waitTimeErr: errors.New("wait time error"), + }, + want: core.DefaultWaitTime, + wantErr: errors.New("wait time error"), + }, + { + name: "Test 3: When wait time value is fetched from config", + useDummyConfigFile: true, + args: args{ + waitInTestConfig: 20, + }, + want: 20, + wantErr: nil, + }, + { + name: "Test 4: When wait time is not passed in root nor set in config", + want: core.DefaultWaitTime, + wantErr: nil, + }, + { + name: "Test 5: When wait time value is out of valid range", + useDummyConfigFile: true, + args: args{ + waitInTestConfig: 40, + }, + want: core.DefaultWaitTime, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + viper.Reset() // Reset viper state + + if tt.useDummyConfigFile { + createTestConfig(t, "wait", tt.args.waitInTestConfig) + defer removeTestConfig(tempConfigPath) + } + + SetUpMockInterfaces() + + flagSetMock.On("FetchRootFlagInput", mock.Anything, mock.Anything).Return(tt.args.waitTime, tt.args.waitTimeErr) + flagSetMock.On("Changed", mock.Anything, mock.Anything).Return(tt.args.isFlagSet) + + utils := &UtilsStruct{} + got, err := utils.GetWaitTime() + if got != tt.want { + t.Errorf("GetWaitTime() got = %v, want %v", got, tt.want) + } + if err == nil || tt.wantErr == nil { + if err != tt.wantErr { + t.Errorf("Error for GetWaitTime function, got = %v, want = %v", err, tt.wantErr) + } + } else { + if err.Error() != tt.wantErr.Error() { + t.Errorf("Error for GetWaitTime function, got = %v, want = %v", err, tt.wantErr) + } + } + }) + } +} diff --git a/core/constants.go b/core/constants.go index 107fafacc..d7b052886 100644 --- a/core/constants.go +++ b/core/constants.go @@ -22,10 +22,10 @@ var BlockCompletionTimeout = 30 var DefaultGasMultiplier float32 = 1.0 var DefaultBufferPercent int32 = 20 -var DefaultGasPrice int32 = 1 +var DefaultGasPrice int32 = 0 var DefaultWaitTime int32 = 1 var DefaultGasLimit float32 = 2 -var DefaultGasLimitOverride uint64 = 50000000 +var DefaultGasLimitOverride uint64 = 30000000 var DefaultRPCTimeout int64 = 10 var DefaultHTTPTimeout int64 = 10 var DefaultLogLevel = ""