refactor: added validation on buffer percent limits (#1228)

* refactor: added validation on buffer percent limits

* refactor: fixed tests

* refactor: updated default value when builded from source in config script

* refactor: fixed lint error

* refactor: fixed vote tests

* refactor: fixed getBufferPercent() tests

* refactor: recorrected the comment for max buffer formula used

cmd/cmd-utils.go

@@ -23,6 +23,10 @@ func (*UtilsStruct) GetEpochAndState(client *ethclient.Client) (uint32, int64, e
 	if err != nil {
 		return 0, 0, err
 	}
+	err = ValidateBufferPercentLimit(client, bufferPercent)
+	if err != nil {
+		return 0, 0, err
+	}
 	latestHeader, err := clientUtils.GetLatestBlockWithRetry(client)
 	if err != nil {
 		log.Error("Error in fetching block: ", err)

cmd/cmd-utils_test.go

@@ -20,6 +20,8 @@ func TestGetEpochAndState(t *testing.T) {
 		latestHeaderErr  error
 		bufferPercent    int32
 		bufferPercentErr error
+		stateBuffer      uint64
+		stateBufferErr   error
 		state            int64
 		stateErr         error
 		stateName        string
@@ -37,6 +39,7 @@ func TestGetEpochAndState(t *testing.T) {
 				epoch:         4,
 				latestHeader:  &Types.Header{},
 				bufferPercent: 20,
+				stateBuffer:   5,
 				state:         0,
 				stateName:     "commit",
 			},
@@ -50,6 +53,7 @@ func TestGetEpochAndState(t *testing.T) {
 				epochErr:      errors.New("epoch error"),
 				latestHeader:  &Types.Header{},
 				bufferPercent: 20,
+				stateBuffer:   5,
 				state:         0,
 				stateName:     "commit",
 			},
@@ -76,6 +80,7 @@ func TestGetEpochAndState(t *testing.T) {
 				epoch:         4,
 				latestHeader:  &Types.Header{},
 				bufferPercent: 20,
+				stateBuffer:   5,
 				stateErr:      errors.New("state error"),
 			},
 			wantEpoch: 0,
@@ -88,20 +93,46 @@ func TestGetEpochAndState(t *testing.T) {
 				epoch:           4,
 				latestHeaderErr: errors.New("header error"),
 				bufferPercent:   20,
+				stateBuffer:     5,
 				state:           0,
 				stateName:       "commit",
 			},
 			wantEpoch: 0,
 			wantState: 0,
 			wantErr:   errors.New("header error"),
 		},
+		{
+			name: "Test 6: When validating buffer percent limit fails",
+			args: args{
+				epoch:         4,
+				latestHeader:  &Types.Header{},
+				bufferPercent: 50,
+				stateBuffer:   10,
+			},
+			wantEpoch: 0,
+			wantState: 0,
+			wantErr:   errors.New("buffer percent exceeds limit"),
+		},
+		{
+			name: "Test 7: When there is an error in validating buffer percent limit",
+			args: args{
+				epoch:          4,
+				latestHeader:   &Types.Header{},
+				bufferPercent:  50,
+				stateBufferErr: errors.New("state buffer error"),
+			},
+			wantEpoch: 0,
+			wantState: 0,
+			wantErr:   errors.New("state buffer error"),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			SetUpMockInterfaces()
 
 			utilsMock.On("GetEpoch", mock.AnythingOfType("*ethclient.Client")).Return(tt.args.epoch, tt.args.epochErr)
 			cmdUtilsMock.On("GetBufferPercent").Return(tt.args.bufferPercent, tt.args.bufferPercentErr)
+			utilsMock.On("GetStateBuffer", mock.Anything).Return(tt.args.stateBuffer, tt.args.stateBufferErr)
 			clientUtilsMock.On("GetLatestBlockWithRetry", mock.Anything).Return(tt.args.latestHeader, tt.args.latestHeaderErr)
 			utilsMock.On("GetBufferedState", mock.Anything, mock.Anything, mock.Anything).Return(tt.args.state, tt.args.stateErr)
 

cmd/config-utils.go

@@ -3,6 +3,7 @@ package cmd
 
 import (
 	"errors"
+	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/sirupsen/logrus"
 	"razor/client"
 	"razor/core"
@@ -207,28 +208,17 @@ func (*UtilsStruct) GetMultiplier() (float32, error) {
 
 //This function returns the buffer percent
 func (*UtilsStruct) GetBufferPercent() (int32, error) {
-	const (
-		MinBufferPercent = 0
-		MaxBufferPercent = 30
-	)
-
 	bufferPercent, err := getConfigValue("buffer", "int32", core.DefaultBufferPercent, "buffer")
 	if err != nil {
 		return core.DefaultBufferPercent, err
 	}
 
 	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 or set to 0, using its default %d value", core.DefaultBufferPercent)
-		return core.DefaultBufferPercent, nil
+	// bufferPercent cannot be less than core.DefaultBufferPercent else through an error
+	if bufferPercentInt32 < core.DefaultBufferPercent {
+		log.Infof("BufferPercent should be greater than or equal to %v", core.DefaultBufferPercent)
+		return core.DefaultBufferPercent, errors.New("invalid buffer percent")
 	}
 
 	return bufferPercentInt32, nil
@@ -413,3 +403,30 @@ func setLogLevel(config types.Configurations) {
 		log.Debugf("Log File Max Age (max number of days to retain old log files): %d", config.LogFileMaxAge)
 	}
 }
+
+func ValidateBufferPercentLimit(client *ethclient.Client, bufferPercent int32) error {
+	stateBuffer, err := razorUtils.GetStateBuffer(client)
+	if err != nil {
+		return err
+	}
+	maxBufferPercent := calculateMaxBufferPercent(stateBuffer, core.StateLength)
+	if bufferPercent >= maxBufferPercent {
+		log.Errorf("Buffer percent %v is greater than or equal to maximum possible buffer percent", maxBufferPercent)
+		return errors.New("buffer percent exceeds limit")
+	}
+	return nil
+}
+
+// calculateMaxBuffer calculates the maximum buffer percent value.
+func calculateMaxBufferPercent(stateBuffer, stateLength uint64) int32 {
+	if stateLength == 0 {
+		return 0
+	}
+
+	// The formula is derived from the condition:
+	// 2(maxBuffer % stateLength) < (stateLength - 2*StateBuffer)
+
+	// Perform the calculation with float64 for precision
+	maxBufferPercent := 50 * (1 - (float64(2*stateBuffer) / float64(stateLength)))
+	return int32(maxBufferPercent)
+}

cmd/config-utils_test.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"errors"
+	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/mock"
 	"os"
@@ -260,9 +261,9 @@ func TestGetBufferPercent(t *testing.T) {
 			name: "Test 1: When buffer percent is fetched from root flag",
 			args: args{
 				isFlagSet:     true,
-				bufferPercent: 5,
+				bufferPercent: 10,
 			},
-			want:    5,
+			want:    10,
 			wantErr: nil,
 		},
 		{
@@ -278,9 +279,9 @@ func TestGetBufferPercent(t *testing.T) {
 			name:               "Test 3: When buffer value is fetched from config",
 			useDummyConfigFile: true,
 			args: args{
-				bufferInTestConfig: 1,
+				bufferInTestConfig: 6,
 			},
-			want:    1,
+			want:    6,
 			wantErr: nil,
 		},
 		{
@@ -292,10 +293,10 @@ func TestGetBufferPercent(t *testing.T) {
 			name:               "Test 5: When buffer value is out of a valid range",
 			useDummyConfigFile: true,
 			args: args{
-				bufferInTestConfig: 40,
+				bufferInTestConfig: 0,
 			},
 			want:    core.DefaultBufferPercent,
-			wantErr: nil,
+			wantErr: errors.New("invalid buffer percent"),
 		},
 	}
 	for _, tt := range tests {
@@ -1199,3 +1200,62 @@ func TestGetWaitTime(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateBufferPercentLimit(t *testing.T) {
+	var client *ethclient.Client
+
+	type args struct {
+		bufferPercent  int32
+		stateBuffer    uint64
+		stateBufferErr error
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr error
+	}{
+		{
+			name: "Buffer percent less than max buffer percent",
+			args: args{
+				stateBuffer:   10,
+				bufferPercent: 20,
+			},
+			wantErr: nil,
+		},
+		{
+			name: "Buffer percent greater than max buffer percent",
+			args: args{
+				stateBuffer:   10,
+				bufferPercent: 60,
+			},
+			wantErr: errors.New("buffer percent exceeds limit"),
+		},
+		{
+			name: "GetStateBuffer returns an error",
+			args: args{
+				stateBufferErr: errors.New("state buffer error"),
+				bufferPercent:  10,
+			},
+			wantErr: errors.New("state buffer error"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			SetUpMockInterfaces()
+
+			utilsMock.On("GetStateBuffer", mock.Anything).Return(tt.args.stateBuffer, tt.args.stateBufferErr)
+
+			err := ValidateBufferPercentLimit(client, tt.args.bufferPercent)
+			if err == nil || tt.wantErr == nil {
+				if err != tt.wantErr {
+					t.Errorf("Error for GetEpochAndState function, got = %v, want = %v", err, tt.wantErr)
+				}
+			} else {
+				if err.Error() != tt.wantErr.Error() {
+					t.Errorf("Error for GetEpochAndState function, got = %v, want = %v", err, tt.wantErr)
+				}
+			}
+
+		})
+	}
+}

cmd/propose.go

@@ -76,11 +76,6 @@ func (*UtilsStruct) Propose(client *ethclient.Client, config types.Configuration
 	}
 
 	log.Debugf("Biggest staker Id: %d Biggest stake: %s, Stake: %s, Staker Id: %d, Number of Stakers: %d, Salt: %s", biggestStakerId, biggestStake, staker.Stake, staker.Id, numStakers, hex.EncodeToString(salt[:]))
-
-	bufferPercent, err := cmdUtils.GetBufferPercent()
-	if err != nil {
-		return err
-	}
 	proposer := types.ElectedProposer{
 		Stake:           staker.Stake,
 		StakerId:        staker.Id,
@@ -89,8 +84,8 @@ func (*UtilsStruct) Propose(client *ethclient.Client, config types.Configuration
 		Salt:            salt,
 		Epoch:           epoch,
 	}
-	log.Debugf("Propose: Calling GetIteration with arguments proposer = %+v, buffer percent = %d", proposer, bufferPercent)
-	iteration := cmdUtils.GetIteration(client, proposer, bufferPercent)
+	log.Debugf("Propose: Calling GetIteration with arguments proposer = %+v, buffer percent = %d", proposer, config.BufferPercent)
+	iteration := cmdUtils.GetIteration(client, proposer, config.BufferPercent)
 
 	log.Debug("Iteration: ", iteration)
 

cmd/propose_test.go

@@ -52,8 +52,6 @@ func TestPropose(t *testing.T) {
 		smallestStakerIdErr        error
 		randaoHash                 [32]byte
 		randaoHashErr              error
-		bufferPercent              int32
-		bufferPercentErr           error
 		salt                       [32]byte
 		saltErr                    error
 		iteration                  int
@@ -429,19 +427,7 @@ func TestPropose(t *testing.T) {
 			wantErr: errors.New("error in saving data"),
 		},
 		{
-			name: "Test 17: When there is an error in getting buffer percent",
-			args: args{
-				state:            2,
-				staker:           bindings.StructsStaker{},
-				numStakers:       5,
-				biggestStake:     big.NewInt(1).Mul(big.NewInt(5356), big.NewInt(1e18)),
-				biggestStakerId:  2,
-				bufferPercentErr: errors.New("buffer error"),
-			},
-			wantErr: errors.New("buffer error"),
-		},
-		{
-			name: "Test 18: When rogue mode is on for biggestStakerId and propose exceutes successfully",
+			name: "Test 17: When rogue mode is on for biggestStakerId and propose exceutes successfully",
 			args: args{
 				rogueData: types.Rogue{
 					IsRogue:   true,
@@ -468,7 +454,7 @@ func TestPropose(t *testing.T) {
 			wantErr: nil,
 		},
 		{
-			name: "Test 19: When rogue mode is on for biggestStakerId and there is an error in getting smallestStakerId",
+			name: "Test 18: When rogue mode is on for biggestStakerId and there is an error in getting smallestStakerId",
 			args: args{
 				rogueData: types.Rogue{
 					IsRogue:   true,
@@ -494,7 +480,7 @@ func TestPropose(t *testing.T) {
 			wantErr: errors.New("smallestStakerId error"),
 		},
 		{
-			name: "Test 20: When there is an error in waitForCompletion",
+			name: "Test 19: When there is an error in waitForCompletion",
 			args: args{
 				state:                     2,
 				staker:                    bindings.StructsStaker{},
@@ -539,7 +525,6 @@ func TestPropose(t *testing.T) {
 		utilsMock.On("GetTxnOpts", mock.AnythingOfType("types.TransactionOptions")).Return(TxnOpts)
 		blockManagerMock.On("Propose", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tt.args.proposeTxn, tt.args.proposeErr)
 		transactionMock.On("Hash", mock.Anything).Return(tt.args.hash)
-		cmdUtilsMock.On("GetBufferPercent").Return(tt.args.bufferPercent, tt.args.bufferPercentErr)
 		utilsMock.On("WaitForBlockCompletion", mock.AnythingOfType("*ethclient.Client"), mock.AnythingOfType("string")).Return(tt.args.waitForBlockCompletionErr)
 
 		utils := &UtilsStruct{}

cmd/vote.go

@@ -53,6 +53,9 @@ func (*UtilsStruct) ExecuteVote(flagSet *pflag.FlagSet) {
 
 	client := razorUtils.ConnectToClient(config.Provider)
 
+	err = ValidateBufferPercentLimit(client, config.BufferPercent)
+	utils.CheckError("Error in validating buffer percent: ", err)
+
 	address, err := flagSetUtils.GetStringAddress(flagSet)
 	utils.CheckError("Error in getting address: ", err)
 	log.Debug("ExecuteVote: Address: ", address)

cmd/vote_test.go

@@ -161,6 +161,7 @@ func TestExecuteVote(t *testing.T) {
 
 			fileUtilsMock.On("AssignLogFile", mock.AnythingOfType("*pflag.FlagSet"), mock.Anything)
 			cmdUtilsMock.On("GetConfigData").Return(tt.args.config, tt.args.configErr)
+			utilsMock.On("GetStateBuffer", mock.Anything).Return(uint64(5), nil)
 			utilsMock.On("AssignPassword", flagSet).Return(tt.args.password)
 			utilsMock.On("CheckPassword", mock.Anything).Return(nil)
 			utilsMock.On("AccountManagerForKeystore").Return(&accounts.AccountManager{}, nil)

config.sh

@@ -19,10 +19,10 @@ then
   GAS_MULTIPLIER=1.0
 fi
 
-read -rp "Buffer Percent: (20) " BUFFER
+read -rp "Buffer Percent: (5) " BUFFER
 if [ -z "$BUFFER" ];
 then
-  BUFFER=20
+  BUFFER=5
 fi
 
 read -rp "Wait Time: (5) " WAIT_TIME

core/constants.go

@@ -26,7 +26,7 @@ const BlockCompletionTimeout = 30
 //Following are the default config values for all the config parameters
 const (
 	DefaultGasMultiplier    float32 = 1.0
-	DefaultBufferPercent    int32   = 20
+	DefaultBufferPercent    int32   = 5
 	DefaultGasPrice         int32   = 1
 	DefaultWaitTime         int32   = 5
 	DefaultGasLimit         float32 = 2