From 66d9a3351f275e6ff5247fdd57e3c81e2e316ced Mon Sep 17 00:00:00 2001 From: William Banfield Date: Tue, 22 Mar 2022 12:07:24 -0400 Subject: [PATCH 1/4] timeout params in toml used as overrides --- config/config.go | 95 ++++++++++++++------------------ config/config_test.go | 42 +++++++------- config/toml.go | 59 +++++++++++++------- internal/consensus/state.go | 61 +++++++++++++++++--- internal/consensus/state_test.go | 70 +++++++++++------------ 5 files changed, 189 insertions(+), 138 deletions(-) diff --git a/config/config.go b/config/config.go index e33b8dd7e..6904a7431 100644 --- a/config/config.go +++ b/config/config.go @@ -956,27 +956,6 @@ type ConsensusConfig struct { WalPath string `mapstructure:"wal-file"` walFile string // overrides WalPath if set - // TODO: remove timeout configs, these should be global not local - // How long we wait for a proposal block before prevoting nil - TimeoutPropose time.Duration `mapstructure:"timeout-propose"` - // How much timeout-propose increases with each round - TimeoutProposeDelta time.Duration `mapstructure:"timeout-propose-delta"` - // How long we wait after receiving +2/3 prevotes for “anything” (ie. not a single block or nil) - TimeoutPrevote time.Duration `mapstructure:"timeout-prevote"` - // How much the timeout-prevote increases with each round - TimeoutPrevoteDelta time.Duration `mapstructure:"timeout-prevote-delta"` - // How long we wait after receiving +2/3 precommits for “anything” (ie. not a single block or nil) - TimeoutPrecommit time.Duration `mapstructure:"timeout-precommit"` - // How much the timeout-precommit increases with each round - TimeoutPrecommitDelta time.Duration `mapstructure:"timeout-precommit-delta"` - // How long we wait after committing a block, before starting on the new - // height (this gives us a chance to receive some more precommits, even - // though we already have +2/3). - TimeoutCommit time.Duration `mapstructure:"timeout-commit"` - - // Make progress as soon as we have all the precommits (as if TimeoutCommit = 0) - SkipTimeoutCommit bool `mapstructure:"skip-timeout-commit"` - // EmptyBlocks mode and possible interval between empty blocks CreateEmptyBlocks bool `mapstructure:"create-empty-blocks"` CreateEmptyBlocksInterval time.Duration `mapstructure:"create-empty-blocks-interval"` @@ -986,20 +965,44 @@ type ConsensusConfig struct { PeerQueryMaj23SleepDuration time.Duration `mapstructure:"peer-query-maj23-sleep-duration"` DoubleSignCheckHeight int64 `mapstructure:"double-sign-check-height"` + + // 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. + + // 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"` + // 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"` + // 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"` + // 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"` + // 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"` + + // UnsafeBypassCommitTimeoutOverride provides an unsafe override of the + // BypassCommitTimeout consensus parameter. It configures if the consensus + // engine will wait for the full Commit timeout before proceeding to the next height. + // If it is set to true, the consensus engine will proceed to the next height + // as soon as the node has gathered votes from all of the validators on the network. + UnsafeBypassCommitTimeoutOverride *bool `mapstructure:"unsafe-bypass-commit-timeout-override"` } // DefaultConsensusConfig returns a default configuration for the consensus service func DefaultConsensusConfig() *ConsensusConfig { return &ConsensusConfig{ WalPath: filepath.Join(defaultDataDir, "cs.wal", "wal"), - TimeoutPropose: 3000 * time.Millisecond, - TimeoutProposeDelta: 500 * time.Millisecond, - TimeoutPrevote: 1000 * time.Millisecond, - TimeoutPrevoteDelta: 500 * time.Millisecond, - TimeoutPrecommit: 1000 * time.Millisecond, - TimeoutPrecommitDelta: 500 * time.Millisecond, - TimeoutCommit: 1000 * time.Millisecond, - SkipTimeoutCommit: false, CreateEmptyBlocks: true, CreateEmptyBlocksInterval: 0 * time.Second, PeerGossipSleepDuration: 100 * time.Millisecond, @@ -1011,14 +1014,6 @@ func DefaultConsensusConfig() *ConsensusConfig { // TestConsensusConfig returns a configuration for testing the consensus service func TestConsensusConfig() *ConsensusConfig { cfg := DefaultConsensusConfig() - cfg.TimeoutPropose = 40 * time.Millisecond - cfg.TimeoutProposeDelta = 1 * time.Millisecond - cfg.TimeoutPrevote = 10 * time.Millisecond - cfg.TimeoutPrevoteDelta = 1 * time.Millisecond - cfg.TimeoutPrecommit = 10 * time.Millisecond - cfg.TimeoutPrecommitDelta = 1 * time.Millisecond - cfg.TimeoutCommit = 10 * time.Millisecond - cfg.SkipTimeoutCommit = true cfg.PeerGossipSleepDuration = 5 * time.Millisecond cfg.PeerQueryMaj23SleepDuration = 250 * time.Millisecond cfg.DoubleSignCheckHeight = int64(0) @@ -1046,26 +1041,20 @@ 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.TimeoutPropose < 0 { - return errors.New("timeout-propose can't be negative") - } - if cfg.TimeoutProposeDelta < 0 { - return errors.New("timeout-propose-delta can't be negative") - } - if cfg.TimeoutPrevote < 0 { - return errors.New("timeout-prevote can't be negative") + if cfg.UnsafeProposeTimeoutOverride != nil && *cfg.UnsafeProposeTimeoutOverride < 0 { + return errors.New("unsafe-propose-timeout-override can't be negative") } - if cfg.TimeoutPrevoteDelta < 0 { - return errors.New("timeout-prevote-delta can't be negative") + if cfg.UnsafeProposeTimeoutDeltaOverride != nil && *cfg.UnsafeProposeTimeoutDeltaOverride < 0 { + return errors.New("unsafe-propose-timeout-delta-override can't be negative") } - if cfg.TimeoutPrecommit < 0 { - return errors.New("timeout-precommit can't be negative") + if cfg.UnsafeVoteTimeoutOverride != nil && *cfg.UnsafeVoteTimeoutOverride < 0 { + return errors.New("unsafe-vote-timeout-override can't be negative") } - if cfg.TimeoutPrecommitDelta < 0 { - return errors.New("timeout-precommit-delta can't be negative") + if cfg.UnsafeVoteTimeoutDeltaOverride != nil && *cfg.UnsafeVoteTimeoutDeltaOverride < 0 { + return errors.New("unsafe-vote-timeout-delta-override can't be negative") } - if cfg.TimeoutCommit < 0 { - return errors.New("timeout-commit can't be negative") + if cfg.UnsafeCommitTimeoutOverride != nil && *cfg.UnsafeCommitTimeoutOverride < 0 { + return errors.New("unsafe-commit-timeout-override can't be negative") } if cfg.CreateEmptyBlocksInterval < 0 { return errors.New("create-empty-blocks-interval can't be negative") diff --git a/config/config_test.go b/config/config_test.go index d768a1702..db897ee12 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -29,8 +29,8 @@ func TestConfigValidateBasic(t *testing.T) { cfg := DefaultConfig() assert.NoError(t, cfg.ValidateBasic()) - // tamper with timeout_propose - cfg.Consensus.TimeoutPropose = -10 * time.Second + // tamper with unsafe-propose-timeout-override + cfg.Consensus.UnsafeProposeTimeoutOverride = durationPtr(-10 * time.Second) assert.Error(t, cfg.ValidateBasic()) } @@ -106,25 +106,21 @@ func TestConsensusConfig_ValidateBasic(t *testing.T) { modify func(*ConsensusConfig) expectErr bool }{ - "TimeoutPropose": {func(c *ConsensusConfig) { c.TimeoutPropose = time.Second }, false}, - "TimeoutPropose negative": {func(c *ConsensusConfig) { c.TimeoutPropose = -1 }, true}, - "TimeoutProposeDelta": {func(c *ConsensusConfig) { c.TimeoutProposeDelta = time.Second }, false}, - "TimeoutProposeDelta negative": {func(c *ConsensusConfig) { c.TimeoutProposeDelta = -1 }, true}, - "TimeoutPrevote": {func(c *ConsensusConfig) { c.TimeoutPrevote = time.Second }, false}, - "TimeoutPrevote negative": {func(c *ConsensusConfig) { c.TimeoutPrevote = -1 }, true}, - "TimeoutPrevoteDelta": {func(c *ConsensusConfig) { c.TimeoutPrevoteDelta = time.Second }, false}, - "TimeoutPrevoteDelta negative": {func(c *ConsensusConfig) { c.TimeoutPrevoteDelta = -1 }, true}, - "TimeoutPrecommit": {func(c *ConsensusConfig) { c.TimeoutPrecommit = time.Second }, false}, - "TimeoutPrecommit negative": {func(c *ConsensusConfig) { c.TimeoutPrecommit = -1 }, true}, - "TimeoutPrecommitDelta": {func(c *ConsensusConfig) { c.TimeoutPrecommitDelta = time.Second }, false}, - "TimeoutPrecommitDelta negative": {func(c *ConsensusConfig) { c.TimeoutPrecommitDelta = -1 }, true}, - "TimeoutCommit": {func(c *ConsensusConfig) { c.TimeoutCommit = time.Second }, false}, - "TimeoutCommit negative": {func(c *ConsensusConfig) { c.TimeoutCommit = -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}, - "PeerQueryMaj23SleepDuration negative": {func(c *ConsensusConfig) { c.PeerQueryMaj23SleepDuration = -1 }, true}, - "DoubleSignCheckHeight negative": {func(c *ConsensusConfig) { c.DoubleSignCheckHeight = -1 }, true}, + "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}, + "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}, + "PeerQueryMaj23SleepDuration negative": {func(c *ConsensusConfig) { c.PeerQueryMaj23SleepDuration = -1 }, true}, + "DoubleSignCheckHeight negative": {func(c *ConsensusConfig) { c.DoubleSignCheckHeight = -1 }, true}, } for desc, tc := range testcases { tc := tc // appease linter @@ -168,3 +164,7 @@ 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/config/toml.go b/config/toml.go index a82e7f59d..d225a2038 100644 --- a/config/toml.go +++ b/config/toml.go @@ -450,32 +450,12 @@ fetchers = "{{ .StateSync.Fetchers }}" wal-file = "{{ js .Consensus.WalPath }}" -# How long we wait for a proposal block before prevoting nil -timeout-propose = "{{ .Consensus.TimeoutPropose }}" -# How much timeout-propose increases with each round -timeout-propose-delta = "{{ .Consensus.TimeoutProposeDelta }}" -# How long we wait after receiving +2/3 prevotes for “anything” (ie. not a single block or nil) -timeout-prevote = "{{ .Consensus.TimeoutPrevote }}" -# How much the timeout-prevote increases with each round -timeout-prevote-delta = "{{ .Consensus.TimeoutPrevoteDelta }}" -# How long we wait after receiving +2/3 precommits for “anything” (ie. not a single block or nil) -timeout-precommit = "{{ .Consensus.TimeoutPrecommit }}" -# How much the timeout-precommit increases with each round -timeout-precommit-delta = "{{ .Consensus.TimeoutPrecommitDelta }}" -# How long we wait after committing a block, before starting on the new -# height (this gives us a chance to receive some more precommits, even -# though we already have +2/3). -timeout-commit = "{{ .Consensus.TimeoutCommit }}" - # How many blocks to look back to check existence of the node's consensus votes before joining consensus # When non-zero, the node will panic upon restart # if the same consensus key was used to sign {double-sign-check-height} last blocks. # So, validators should stop the state machine, wait for some blocks, and then restart the state machine to avoid panic. double-sign-check-height = {{ .Consensus.DoubleSignCheckHeight }} -# Make progress as soon as we have all the precommits (as if TimeoutCommit = 0) -skip-timeout-commit = {{ .Consensus.SkipTimeoutCommit }} - # EmptyBlocks mode and possible interval between empty blocks create-empty-blocks = {{ .Consensus.CreateEmptyBlocks }} create-empty-blocks-interval = "{{ .Consensus.CreateEmptyBlocksInterval }}" @@ -484,6 +464,45 @@ create-empty-blocks-interval = "{{ .Consensus.CreateEmptyBlocksInterval }}" peer-gossip-sleep-duration = "{{ .Consensus.PeerGossipSleepDuration }}" peer-query-maj23-sleep-duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}" +### Unsafe Timeout Overrides ### + +# These fields provide temporary overrides for the Timeout consensus parameters. +# Use of these parameters is strongly discouraged. Using these parameters may have serious +# liveness implications for the validator and for the chain. +# +# These fields will be removed from the configuration file in the v0.37 release of Tendermint. +# For additional information, see ADR-74: +# https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-074-timeout-params.md + +# This field provides an unsafe override of the Propose timeout consensus parameter. +# This field configures how long the consensus engine will wait for a proposal block before prevoting nil. +# unsafe-propose-timeout-override = + +# This field provides an unsafe override of the ProposeDelta timeout consensus parameter. +# This field configures how much the propose timeout increases with each round. +# unsafe-propose-timeout-delta-override = + +# This field provides an unsafe override of the Vote timeout consensus parameter. +# This field configures how long the consensus engine will wait after +# receiving +2/3 votes in a around. +# unsafe-vote-timeout-override = + +# This field provides an unsafe override of the VoteDelta timeout consensus parameter. +# This field configures how much the vote timeout increases with each round. +# unsafe-vote-timeout-delta-override = + +# This field provides an unsafe override of the Commit timeout consensus parameter. +# This field configures how long the consensus engine will wait after receiving +# +2/3 precommits before beginning the next height. +# unsafe-commit-timeout-override = + +# This field provides an unsafe override of the BypassCommitTimeout consensus parameter. +# This field configures if the consensus engine will wait for the full Commit timeout +# before proceeding to the next height. +# If this field is set to true, the consensus engine will proceed to the next height +# as soon as the node has gathered votes from all of the validators on the network. +# unsafe-bypass-commit-timeout-override = + ####################################################### ### Transaction Indexer Configuration Options ### ####################################################### diff --git a/internal/consensus/state.go b/internal/consensus/state.go index e27d971ca..cdb121b96 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -787,9 +787,9 @@ func (cs *State) updateToState(ctx context.Context, state sm.State) { // to be gathered for the first block. // And alternative solution that relies on clocks: // cs.StartTime = state.LastBlockTime.Add(timeoutCommit) - cs.StartTime = cs.state.ConsensusParams.Timeout.CommitTime(tmtime.Now()) + cs.StartTime = cs.commitTime(tmtime.Now()) } else { - cs.StartTime = cs.state.ConsensusParams.Timeout.CommitTime(cs.CommitTime) + cs.StartTime = cs.commitTime(cs.CommitTime) } cs.Validators = validators @@ -1262,7 +1262,7 @@ func (cs *State) enterPropose(ctx context.Context, height int64, round int32) { }() // If we don't get the proposal and all block parts quick enough, enterPrevote - cs.scheduleTimeout(cs.state.ConsensusParams.Timeout.ProposeTimeout(round), height, round, cstypes.RoundStepPropose) + cs.scheduleTimeout(cs.proposeTimeout(round), height, round, cstypes.RoundStepPropose) // Nothing more to do if we're not a validator if cs.privValidator == nil { @@ -1620,7 +1620,7 @@ func (cs *State) enterPrevoteWait(ctx context.Context, height int64, round int32 }() // Wait for some more prevotes; enterPrecommit - cs.scheduleTimeout(cs.state.ConsensusParams.Timeout.VoteTimeout(round), height, round, cstypes.RoundStepPrevoteWait) + cs.scheduleTimeout(cs.voteTimeout(round), height, round, cstypes.RoundStepPrevoteWait) } // Enter: `timeoutPrevote` after any +2/3 prevotes. @@ -1773,7 +1773,7 @@ func (cs *State) enterPrecommitWait(ctx context.Context, height int64, round int }() // wait for some more precommits; enterNewRound - cs.scheduleTimeout(cs.state.ConsensusParams.Timeout.VoteTimeout(round), height, round, cstypes.RoundStepPrecommitWait) + cs.scheduleTimeout(cs.voteTimeout(round), height, round, cstypes.RoundStepPrecommitWait) } // Enter: +2/3 precommits for block @@ -2309,7 +2309,7 @@ func (cs *State) addVote( cs.evsw.FireEvent(ctx, types.EventVoteValue, vote) // if we can skip timeoutCommit and have all the votes now, - if cs.state.ConsensusParams.Timeout.BypassCommitTimeout && cs.LastCommit.HasAll() { + if cs.bypassCommitTimeout() && cs.LastCommit.HasAll() { // go straight to new round (skip timeout commit) // cs.scheduleTimeout(time.Duration(0), cs.Height, 0, cstypes.RoundStepNewHeight) cs.enterNewRound(ctx, cs.Height, 0) @@ -2422,7 +2422,7 @@ func (cs *State) addVote( if !blockID.IsNil() { cs.enterCommit(ctx, height, vote.Round) - if cs.state.ConsensusParams.Timeout.BypassCommitTimeout && precommits.HasAll() { + if cs.bypassCommitTimeout() && precommits.HasAll() { cs.enterNewRound(ctx, cs.Height, 0) } } else { @@ -2472,7 +2472,7 @@ func (cs *State) signVote( // If the signedMessageType is for precommit, // use our local precommit Timeout as the max wait time for getting a singed commit. The same goes for prevote. - timeout := cs.state.ConsensusParams.Timeout.VoteTimeout(cs.Round) + timeout := cs.voteTimeout(cs.Round) switch msgType { case tmproto.PrecommitType: @@ -2540,7 +2540,7 @@ func (cs *State) updatePrivValidatorPubKey(rctx context.Context) error { return nil } - timeout := cs.state.ConsensusParams.Timeout.VoteTimeout(cs.Round) + timeout := cs.voteTimeout(cs.Round) // no GetPubKey retry beyond the proposal/voting in RetrySignerClient if cs.Step >= cstypes.RoundStepPrecommit && cs.privValidatorType == types.RetrySignerClient { @@ -2666,6 +2666,49 @@ func repairWalFile(src, dst string) error { return nil } +func (cs *State) proposeTimeout(round int32) time.Duration { + p := cs.state.ConsensusParams.Timeout.Propose + if cs.config.UnsafeProposeTimeoutOverride != nil { + p = *cs.config.UnsafeProposeTimeoutOverride + } + pd := cs.state.ConsensusParams.Timeout.ProposeDelta + if cs.config.UnsafeProposeTimeoutDeltaOverride != nil { + pd = *cs.config.UnsafeProposeTimeoutDeltaOverride + } + return time.Duration( + p.Nanoseconds()+pd.Nanoseconds()*int64(round), + ) * time.Nanosecond +} + +func (cs *State) voteTimeout(round int32) time.Duration { + v := cs.state.ConsensusParams.Timeout.Vote + if cs.config.UnsafeVoteTimeoutOverride != nil { + v = *cs.config.UnsafeVoteTimeoutOverride + } + vd := cs.state.ConsensusParams.Timeout.VoteDelta + if cs.config.UnsafeVoteTimeoutDeltaOverride != nil { + vd = *cs.config.UnsafeVoteTimeoutDeltaOverride + } + return time.Duration( + v.Nanoseconds()+vd.Nanoseconds()*int64(round), + ) * time.Nanosecond +} + +func (cs *State) commitTime(t time.Time) time.Time { + c := cs.state.ConsensusParams.Timeout.Commit + if cs.config.UnsafeCommitTimeoutOverride != nil { + c = *cs.config.UnsafeProposeTimeoutOverride + } + return t.Add(c) +} + +func (cs *State) bypassCommitTimeout() bool { + if cs.config.UnsafeBypassCommitTimeoutOverride != nil { + return *cs.config.UnsafeBypassCommitTimeoutOverride + } + return cs.state.ConsensusParams.Timeout.BypassCommitTimeout +} + func (cs *State) calculateProposalTimestampDifferenceMetric() { if cs.Proposal != nil && cs.Proposal.POLRound == -1 { tp := types.SynchronyParams{ diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index fa614beb3..03bb85cf2 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -313,7 +313,7 @@ func TestStateOversizedBlock(t *testing.T) { // c1 should log an error with the block part message as it exceeds the consensus params. The // block is not added to cs.ProposalBlock so the node timeouts. - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) // and then should send nil prevote and precommit regardless of whether other validators prevote and // precommit on it @@ -481,7 +481,7 @@ func TestStateLock_NoPOL(t *testing.T) { // (note we're entering precommit for a second time this round) // but with invalid args. then we enterPrecommitWait, and the timeout to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /// @@ -494,7 +494,7 @@ func TestStateLock_NoPOL(t *testing.T) { incrementRound(vs2) // now we're on a new round and not the proposer, so wait for timeout - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) rs := cs1.GetRoundState() @@ -513,7 +513,7 @@ func TestStateLock_NoPOL(t *testing.T) { // now we're going to enter prevote again, but with invalid args // and then prevote wait, which should timeout. then wait for precommit - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) // the proposed block should still be locked block. // we should precommit nil and be locked on the proposal. ensurePrecommit(t, voteCh, height, round) @@ -525,7 +525,7 @@ func TestStateLock_NoPOL(t *testing.T) { // (note we're entering precommit for a second time this round, but with invalid args // then we enterPrecommitWait and timeout into NewRound - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) round++ // entering new round ensureNewRound(t, newRoundCh, height, round) @@ -552,7 +552,7 @@ func TestStateLock_NoPOL(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), newBlockID, vs2) ensurePrevote(t, voteCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensurePrecommit(t, voteCh, height, round) // precommit validatePrecommit(ctx, t, cs1, round, 0, vss[0], nil, initialBlockID.Hash) // precommit nil but be locked on proposal @@ -567,7 +567,7 @@ func TestStateLock_NoPOL(t *testing.T) { vs2) // NOTE: conflicting precommits at same height ensurePrecommit(t, voteCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) // cs1 is locked on a block at this point, so we must generate a new consensus // state to force a new proposal block to be generated. @@ -606,7 +606,7 @@ func TestStateLock_NoPOL(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), propBlockID, vs2) ensurePrevote(t, voteCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensurePrecommit(t, voteCh, height, round) validatePrecommit(ctx, t, cs1, round, 0, vss[0], nil, initialBlockID.Hash) // precommit nil but locked on proposal @@ -683,7 +683,7 @@ func TestStateLock_POLUpdateLock(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -789,7 +789,7 @@ func TestStateLock_POLRelock(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -884,7 +884,7 @@ func TestStateLock_PrevoteNilWhenLockedAndMissProposal(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -970,7 +970,7 @@ func TestStateLock_PrevoteNilWhenLockedAndDifferentProposal(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -1078,7 +1078,7 @@ func TestStateLock_POLDoesNotUnlock(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vs3) // timeout to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -1112,7 +1112,7 @@ func TestStateLock_POLDoesNotUnlock(t *testing.T) { validatePrecommit(ctx, t, cs1, round, 0, vss[0], nil, blockID.Hash) signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 2: @@ -1198,7 +1198,7 @@ func TestStateLock_MissingProposalWhenPOLSeenDoesNotUpdateLock(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -1285,7 +1285,7 @@ func TestStateLock_DoesNotLockOnOldProposal(t *testing.T) { incrementRound(vs2, vs3, vs4) // timeout to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -1356,7 +1356,7 @@ func TestStateLock_POLSafety1(t *testing.T) { // cs1 precommit nil ensurePrecommit(t, voteCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) incrementRound(vs2, vs3, vs4) round++ // moving to the next round @@ -1397,7 +1397,7 @@ func TestStateLock_POLSafety1(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) incrementRound(vs2, vs3, vs4) round++ // moving to the next round @@ -1409,7 +1409,7 @@ func TestStateLock_POLSafety1(t *testing.T) { */ // timeout of propose - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) // finish prevote ensurePrevoteMatch(t, voteCh, height, round, nil) @@ -1493,7 +1493,7 @@ func TestStateLock_POLSafety2(t *testing.T) { incrementRound(vs2, vs3, vs4) // timeout of precommit wait to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) round++ // moving to the next round // in round 2 we see the polkad block from round 0 @@ -1580,7 +1580,7 @@ func TestState_PrevotePOLFromPreviousRound(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Round 1: @@ -1621,7 +1621,7 @@ func TestState_PrevotePOLFromPreviousRound(t *testing.T) { ensurePrecommit(t, voteCh, height, round) // timeout to new round. - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) /* Create a new proposal for D, the same block from Round 1. @@ -1714,7 +1714,7 @@ func TestProposeValidBlock(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) incrementRound(vs2, vs3, vs4) round++ // moving to the next round @@ -1722,7 +1722,7 @@ func TestProposeValidBlock(t *testing.T) { ensureNewRound(t, newRoundCh, height, round) // timeout of propose - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) // We did not see a valid proposal within this round, so prevote nil. ensurePrevoteMatch(t, voteCh, height, round, nil) @@ -1743,7 +1743,7 @@ func TestProposeValidBlock(t *testing.T) { ensureNewRound(t, newRoundCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) round++ // moving to the next round @@ -1802,7 +1802,7 @@ func TestSetValidBlockOnDelayedPrevote(t *testing.T) { // vs3 send prevote nil signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), types.BlockID{}, vs3) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensurePrecommit(t, voteCh, height, round) // we should have precommitted @@ -1856,7 +1856,7 @@ func TestSetValidBlockOnDelayedProposal(t *testing.T) { startTestRound(ctx, cs1, cs1.Height, round) ensureNewRound(t, newRoundCh, height, round) - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) ensurePrevoteMatch(t, voteCh, height, round, nil) @@ -1872,7 +1872,7 @@ func TestSetValidBlockOnDelayedProposal(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vs2, vs3, vs4) ensureNewValidBlock(t, validBlockCh, height, round) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensurePrecommit(t, voteCh, height, round) validatePrecommit(ctx, t, cs1, round, -1, vss[0], nil, nil) @@ -2036,7 +2036,7 @@ func TestWaitingTimeoutOnNilPolka(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensureNewRound(t, newRoundCh, height, round+1) } @@ -2074,7 +2074,7 @@ func TestWaitingTimeoutProposeOnNewRound(t *testing.T) { rs := cs1.GetRoundState() assert.True(t, rs.Step == cstypes.RoundStepPropose) // P0 does not prevote before timeoutPropose expires - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) ensurePrevoteMatch(t, voteCh, height, round, nil) } @@ -2113,7 +2113,7 @@ func TestRoundSkipOnNilPolkaFromHigherRound(t *testing.T) { ensurePrecommit(t, voteCh, height, round) validatePrecommit(ctx, t, cs1, round, -1, vss[0], nil, nil) - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) round++ // moving to the next round ensureNewRound(t, newRoundCh, height, round) @@ -2145,7 +2145,7 @@ func TestWaitTimeoutProposeOnNilPolkaForTheCurrentRound(t *testing.T) { incrementRound(vss[1:]...) signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), types.BlockID{}, vs2, vs3, vs4) - ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height, round, cs1.proposeTimeout(round).Nanoseconds()) ensurePrevoteMatch(t, voteCh, height, round, nil) } @@ -2302,7 +2302,7 @@ func TestStartNextHeightCorrectlyAfterTimeout(t *testing.T) { signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vs3) // wait till timeout occurs - ensureNewTimeout(t, precommitTimeoutCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, precommitTimeoutCh, height, round, cs1.voteTimeout(round).Nanoseconds()) ensureNewRound(t, newRoundCh, height, round+1) @@ -2313,7 +2313,7 @@ func TestStartNextHeightCorrectlyAfterTimeout(t *testing.T) { cs1.txNotifier.(*fakeTxNotifier).Notify() - ensureNewTimeout(t, timeoutProposeCh, height+1, round, cs1.state.ConsensusParams.Timeout.ProposeTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutProposeCh, height+1, round, cs1.proposeTimeout(round).Nanoseconds()) rs = cs1.GetRoundState() assert.False( t, @@ -2441,7 +2441,7 @@ func TestStateHalt1(t *testing.T) { incrementRound(vs2, vs3, vs4) // timeout to new round - ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds()) + ensureNewTimeout(t, timeoutWaitCh, height, round, cs1.voteTimeout(round).Nanoseconds()) round++ // moving to the next round From fbb7f51f49adf3d12fcf1bb0eb4acf11ab0c972d Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 23 Mar 2022 19:01:01 -0400 Subject: [PATCH 2/4] 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 { From 977638b9e521903487e6f708826dd3e25e57896a Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 23 Mar 2022 19:03:05 -0400 Subject: [PATCH 3/4] update comments to add > 0 logic --- config/toml.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/toml.go b/config/toml.go index d225a2038..a0a4b2a2c 100644 --- a/config/toml.go +++ b/config/toml.go @@ -476,24 +476,29 @@ peer-query-maj23-sleep-duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}" # This field provides an unsafe override of the Propose timeout consensus parameter. # This field configures how long the consensus engine will wait for a proposal block before prevoting nil. +# If this field is set to a value greater than 0, it will take effect. # unsafe-propose-timeout-override = # This field provides an unsafe override of the ProposeDelta timeout consensus parameter. # This field configures how much the propose timeout increases with each round. +# If this field is set to a value greater than 0, it will take effect. # unsafe-propose-timeout-delta-override = # This field provides an unsafe override of the Vote timeout consensus parameter. # This field configures how long the consensus engine will wait after # receiving +2/3 votes in a around. +# If this field is set to a value greater than 0, it will take effect. # unsafe-vote-timeout-override = # This field provides an unsafe override of the VoteDelta timeout consensus parameter. # This field configures how much the vote timeout increases with each round. +# If this field is set to a value greater than 0, it will take effect. # unsafe-vote-timeout-delta-override = # This field provides an unsafe override of the Commit timeout consensus parameter. # This field configures how long the consensus engine will wait after receiving # +2/3 precommits before beginning the next height. +# If this field is set to a value greater than 0, it will take effect. # unsafe-commit-timeout-override = # This field provides an unsafe override of the BypassCommitTimeout consensus parameter. From 973003bda44cc1802f7d9552dad27f17bcbee97a Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 23 Mar 2022 19:11:51 -0400 Subject: [PATCH 4/4] add values back to config toml tmpl --- config/toml.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/toml.go b/config/toml.go index a0a4b2a2c..7ed1aaabf 100644 --- a/config/toml.go +++ b/config/toml.go @@ -477,29 +477,29 @@ peer-query-maj23-sleep-duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}" # This field provides an unsafe override of the Propose timeout consensus parameter. # This field configures how long the consensus engine will wait for a proposal block before prevoting nil. # If this field is set to a value greater than 0, it will take effect. -# unsafe-propose-timeout-override = +# unsafe-propose-timeout-override = {{ .Consensus.UnsafeProposeTimeoutOverride }} # This field provides an unsafe override of the ProposeDelta timeout consensus parameter. # This field configures how much the propose timeout increases with each round. # If this field is set to a value greater than 0, it will take effect. -# unsafe-propose-timeout-delta-override = +# unsafe-propose-timeout-delta-override = {{ .Consensus.UnsafeProposeTimeoutDeltaOverride }} # This field provides an unsafe override of the Vote timeout consensus parameter. # This field configures how long the consensus engine will wait after # receiving +2/3 votes in a around. # If this field is set to a value greater than 0, it will take effect. -# unsafe-vote-timeout-override = +# unsafe-vote-timeout-override = {{ .Consensus.UnsafeVoteTimeoutOverride }} # This field provides an unsafe override of the VoteDelta timeout consensus parameter. # This field configures how much the vote timeout increases with each round. # If this field is set to a value greater than 0, it will take effect. -# unsafe-vote-timeout-delta-override = +# unsafe-vote-timeout-delta-override = {{ .Consensus.UnsafeVoteTimeoutDeltaOverride }} # This field provides an unsafe override of the Commit timeout consensus parameter. # This field configures how long the consensus engine will wait after receiving # +2/3 precommits before beginning the next height. # If this field is set to a value greater than 0, it will take effect. -# unsafe-commit-timeout-override = +# unsafe-commit-timeout-override = {{ .Consensus.UnsafeCommitTimeoutOverride }} # This field provides an unsafe override of the BypassCommitTimeout consensus parameter. # This field configures if the consensus engine will wait for the full Commit timeout