Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: added validation on buffer percent limits #1228

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/cmd-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions cmd/cmd-utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,6 +39,7 @@ func TestGetEpochAndState(t *testing.T) {
epoch: 4,
latestHeader: &Types.Header{},
bufferPercent: 20,
stateBuffer: 5,
state: 0,
stateName: "commit",
},
Expand All @@ -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",
},
Expand All @@ -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,
Expand All @@ -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)

Expand Down
47 changes: 32 additions & 15 deletions cmd/config-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd

import (
"errors"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/sirupsen/logrus"
"razor/client"
"razor/core"
Expand Down Expand Up @@ -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")
ashish10677 marked this conversation as resolved.
Show resolved Hide resolved
}

return bufferPercentInt32, nil
Expand Down Expand Up @@ -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)
}
72 changes: 66 additions & 6 deletions cmd/config-utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"errors"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/spf13/viper"
"github.com/stretchr/testify/mock"
"os"
Expand Down Expand Up @@ -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,
},
{
Expand All @@ -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,
},
{
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
}

})
}
}
9 changes: 2 additions & 7 deletions cmd/propose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand Down
21 changes: 3 additions & 18 deletions cmd/propose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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{},
Expand Down Expand Up @@ -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{}
Expand Down
3 changes: 3 additions & 0 deletions cmd/vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions cmd/vote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading