From fbb7f51f49adf3d12fcf1bb0eb4acf11ab0c972d Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 23 Mar 2022 19:01:01 -0400 Subject: [PATCH] params are not pointers --- config/config.go | 21 +++++++++++---------- config/config_test.go | 26 +++++++++++--------------- internal/consensus/state.go | 20 ++++++++++---------- types/params.go | 20 ++++++++++---------- 4 files changed, 42 insertions(+), 45 deletions(-) diff --git a/config/config.go b/config/config.go index 6904a7431..fd4923cce 100644 --- a/config/config.go +++ b/config/config.go @@ -969,27 +969,28 @@ type ConsensusConfig struct { // TODO: The following fields are all temporary overrides that should exist only // for the duration of the v0.36 release. The below fields should be completely // removed in the v0.37 release of Tendermint. + // See: https://github.com/tendermint/tendermint/issues/8188 // UnsafeProposeTimeoutOverride provides an unsafe override of the Propose // timeout consensus parameter. It configures how long the consensus engine // will wait to receive a proposal block before prevoting nil. - UnsafeProposeTimeoutOverride *time.Duration `mapstructure:"unsafe-propose-timeout-override"` + UnsafeProposeTimeoutOverride time.Duration `mapstructure:"unsafe-propose-timeout-override"` // UnsafeProposeTimeoutDeltaOverride provides an unsafe override of the // ProposeDelta timeout consensus parameter. It configures how much the // propose timeout increases with each round. - UnsafeProposeTimeoutDeltaOverride *time.Duration `mapstructure:"unsafe-propose-timeout-delta-override"` + UnsafeProposeTimeoutDeltaOverride time.Duration `mapstructure:"unsafe-propose-timeout-delta-override"` // UnsafeVoteTimeoutOverride provides an unsafe override of the Vote timeout // consensus parameter. It configures how long the consensus engine will wait // to gather additional votes after receiving +2/3 votes in a round. - UnsafeVoteTimeoutOverride *time.Duration `mapstructure:"unsafe-vote-timeout-override"` + UnsafeVoteTimeoutOverride time.Duration `mapstructure:"unsafe-vote-timeout-override"` // UnsafeVoteTimeoutDeltaOverride provides an unsafe override of the VoteDelta // timeout consensus parameter. It configures how much the vote timeout // increases with each round. - UnsafeVoteTimeoutDeltaOverride *time.Duration `mapstructure:"unsafe-vote-timeout-delta-override"` + UnsafeVoteTimeoutDeltaOverride time.Duration `mapstructure:"unsafe-vote-timeout-delta-override"` // UnsafeCommitTimeoutOverride provides an unsafe override of the Commit timeout // consensus parameter. It configures how long the consensus engine will wait // after receiving +2/3 precommits before beginning the next height. - UnsafeCommitTimeoutOverride *time.Duration `mapstructure:"unsafe-commit-timeout-override"` + UnsafeCommitTimeoutOverride time.Duration `mapstructure:"unsafe-commit-timeout-override"` // UnsafeBypassCommitTimeoutOverride provides an unsafe override of the // BypassCommitTimeout consensus parameter. It configures if the consensus @@ -1041,19 +1042,19 @@ func (cfg *ConsensusConfig) SetWalFile(walFile string) { // ValidateBasic performs basic validation (checking param bounds, etc.) and // returns an error if any check fails. func (cfg *ConsensusConfig) ValidateBasic() error { - if cfg.UnsafeProposeTimeoutOverride != nil && *cfg.UnsafeProposeTimeoutOverride < 0 { + if cfg.UnsafeProposeTimeoutOverride < 0 { return errors.New("unsafe-propose-timeout-override can't be negative") } - if cfg.UnsafeProposeTimeoutDeltaOverride != nil && *cfg.UnsafeProposeTimeoutDeltaOverride < 0 { + if cfg.UnsafeProposeTimeoutDeltaOverride < 0 { return errors.New("unsafe-propose-timeout-delta-override can't be negative") } - if cfg.UnsafeVoteTimeoutOverride != nil && *cfg.UnsafeVoteTimeoutOverride < 0 { + if cfg.UnsafeVoteTimeoutOverride < 0 { return errors.New("unsafe-vote-timeout-override can't be negative") } - if cfg.UnsafeVoteTimeoutDeltaOverride != nil && *cfg.UnsafeVoteTimeoutDeltaOverride < 0 { + if cfg.UnsafeVoteTimeoutDeltaOverride < 0 { return errors.New("unsafe-vote-timeout-delta-override can't be negative") } - if cfg.UnsafeCommitTimeoutOverride != nil && *cfg.UnsafeCommitTimeoutOverride < 0 { + if cfg.UnsafeCommitTimeoutOverride < 0 { return errors.New("unsafe-commit-timeout-override can't be negative") } if cfg.CreateEmptyBlocksInterval < 0 { diff --git a/config/config_test.go b/config/config_test.go index db897ee12..a86ab8463 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -30,7 +30,7 @@ func TestConfigValidateBasic(t *testing.T) { assert.NoError(t, cfg.ValidateBasic()) // tamper with unsafe-propose-timeout-override - cfg.Consensus.UnsafeProposeTimeoutOverride = durationPtr(-10 * time.Second) + cfg.Consensus.UnsafeProposeTimeoutOverride = -10 * time.Second assert.Error(t, cfg.ValidateBasic()) } @@ -106,16 +106,16 @@ func TestConsensusConfig_ValidateBasic(t *testing.T) { modify func(*ConsensusConfig) expectErr bool }{ - "UnsafeProposeTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutOverride = durationPtr(time.Second) }, false}, - "UnsafeProposeTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutOverride = durationPtr(-1) }, true}, - "UnsafeProposeTimeoutDeltaOverride": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutDeltaOverride = durationPtr(time.Second) }, false}, - "UnsafeProposeTimeoutDeltaOverride negative": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutDeltaOverride = durationPtr(-1) }, true}, - "UnsafePrevoteTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutOverride = durationPtr(time.Second) }, false}, - "UnsafePrevoteTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutOverride = durationPtr(-1) }, true}, - "UnsafePrevoteTimeoutDeltaOverride": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutDeltaOverride = durationPtr(time.Second) }, false}, - "UnsafePrevoteTimeoutDeltaOverride negative": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutDeltaOverride = durationPtr(-1) }, true}, - "UnsafeCommitTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeCommitTimeoutOverride = durationPtr(time.Second) }, false}, - "UnsafeCommitTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeCommitTimeoutOverride = durationPtr(-1) }, true}, + "UnsafeProposeTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutOverride = time.Second }, false}, + "UnsafeProposeTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutOverride = -1 }, true}, + "UnsafeProposeTimeoutDeltaOverride": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutDeltaOverride = time.Second }, false}, + "UnsafeProposeTimeoutDeltaOverride negative": {func(c *ConsensusConfig) { c.UnsafeProposeTimeoutDeltaOverride = -1 }, true}, + "UnsafePrevoteTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutOverride = time.Second }, false}, + "UnsafePrevoteTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutOverride = -1 }, true}, + "UnsafePrevoteTimeoutDeltaOverride": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutDeltaOverride = time.Second }, false}, + "UnsafePrevoteTimeoutDeltaOverride negative": {func(c *ConsensusConfig) { c.UnsafeVoteTimeoutDeltaOverride = -1 }, true}, + "UnsafeCommitTimeoutOverride": {func(c *ConsensusConfig) { c.UnsafeCommitTimeoutOverride = time.Second }, false}, + "UnsafeCommitTimeoutOverride negative": {func(c *ConsensusConfig) { c.UnsafeCommitTimeoutOverride = -1 }, true}, "PeerGossipSleepDuration": {func(c *ConsensusConfig) { c.PeerGossipSleepDuration = time.Second }, false}, "PeerGossipSleepDuration negative": {func(c *ConsensusConfig) { c.PeerGossipSleepDuration = -1 }, true}, "PeerQueryMaj23SleepDuration": {func(c *ConsensusConfig) { c.PeerQueryMaj23SleepDuration = time.Second }, false}, @@ -164,7 +164,3 @@ func TestP2PConfigValidateBasic(t *testing.T) { reflect.ValueOf(cfg).Elem().FieldByName(fieldName).SetInt(0) } } - -func durationPtr(t time.Duration) *time.Duration { - return &t -} diff --git a/internal/consensus/state.go b/internal/consensus/state.go index cdb121b96..74e07ca9d 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -2668,12 +2668,12 @@ func repairWalFile(src, dst string) error { func (cs *State) proposeTimeout(round int32) time.Duration { p := cs.state.ConsensusParams.Timeout.Propose - if cs.config.UnsafeProposeTimeoutOverride != nil { - p = *cs.config.UnsafeProposeTimeoutOverride + if cs.config.UnsafeProposeTimeoutOverride != 0 { + p = cs.config.UnsafeProposeTimeoutOverride } pd := cs.state.ConsensusParams.Timeout.ProposeDelta - if cs.config.UnsafeProposeTimeoutDeltaOverride != nil { - pd = *cs.config.UnsafeProposeTimeoutDeltaOverride + if cs.config.UnsafeProposeTimeoutDeltaOverride != 0 { + pd = cs.config.UnsafeProposeTimeoutDeltaOverride } return time.Duration( p.Nanoseconds()+pd.Nanoseconds()*int64(round), @@ -2682,12 +2682,12 @@ func (cs *State) proposeTimeout(round int32) time.Duration { func (cs *State) voteTimeout(round int32) time.Duration { v := cs.state.ConsensusParams.Timeout.Vote - if cs.config.UnsafeVoteTimeoutOverride != nil { - v = *cs.config.UnsafeVoteTimeoutOverride + if cs.config.UnsafeVoteTimeoutOverride != 0 { + v = cs.config.UnsafeVoteTimeoutOverride } vd := cs.state.ConsensusParams.Timeout.VoteDelta - if cs.config.UnsafeVoteTimeoutDeltaOverride != nil { - vd = *cs.config.UnsafeVoteTimeoutDeltaOverride + if cs.config.UnsafeVoteTimeoutDeltaOverride != 0 { + vd = cs.config.UnsafeVoteTimeoutDeltaOverride } return time.Duration( v.Nanoseconds()+vd.Nanoseconds()*int64(round), @@ -2696,8 +2696,8 @@ func (cs *State) voteTimeout(round int32) time.Duration { func (cs *State) commitTime(t time.Time) time.Time { c := cs.state.ConsensusParams.Timeout.Commit - if cs.config.UnsafeCommitTimeoutOverride != nil { - c = *cs.config.UnsafeProposeTimeoutOverride + if cs.config.UnsafeCommitTimeoutOverride != 0 { + c = cs.config.UnsafeProposeTimeoutOverride } return t.Add(c) } diff --git a/types/params.go b/types/params.go index b8cf7f2d8..be9217fa3 100644 --- a/types/params.go +++ b/types/params.go @@ -247,24 +247,24 @@ func (params ConsensusParams) ValidateConsensusParams() error { params.Synchrony.Precision) } - if params.Timeout.Propose < 0 { - return fmt.Errorf("timeout.ProposeDelta must not be negative. Got: %d", params.Timeout.Propose) + if params.Timeout.Propose <= 0 { + return fmt.Errorf("timeout.ProposeDelta must be greater than 0. Got: %d", params.Timeout.Propose) } - if params.Timeout.ProposeDelta < 0 { - return fmt.Errorf("timeout.ProposeDelta must not be negative. Got: %d", params.Timeout.ProposeDelta) + if params.Timeout.ProposeDelta <= 0 { + return fmt.Errorf("timeout.ProposeDelta must be greater than 0. Got: %d", params.Timeout.ProposeDelta) } - if params.Timeout.Vote < 0 { - return fmt.Errorf("timeout.Vote must not be negative. Got: %d", params.Timeout.Vote) + if params.Timeout.Vote <= 0 { + return fmt.Errorf("timeout.Vote must be greater than 0. Got: %d", params.Timeout.Vote) } - if params.Timeout.VoteDelta < 0 { - return fmt.Errorf("timeout.VoteDelta must not be negative. Got: %d", params.Timeout.VoteDelta) + if params.Timeout.VoteDelta <= 0 { + return fmt.Errorf("timeout.VoteDelta must be greater than 0. Got: %d", params.Timeout.VoteDelta) } - if params.Timeout.Commit < 0 { - return fmt.Errorf("timeout.Commit must not be negative. Got: %d", params.Timeout.Commit) + if params.Timeout.Commit <= 0 { + return fmt.Errorf("timeout.Commit must be greater than 0. Got: %d", params.Timeout.Commit) } if len(params.Validator.PubKeyTypes) == 0 {