Revert "feat!: account for time already elapsed when waiting after the commit (#965)" (#1033)

This reverts commit cc1bc3f.

Author: evan-forbes

DOCKER/docker-entrypoint.sh

@@ -9,7 +9,7 @@ if [ ! -d "$CMTHOME/config" ]; then
 		-e "s/^proxy_app\s*=.*/proxy_app = \"$PROXY_APP\"/" \
 		-e "s/^moniker\s*=.*/moniker = \"$MONIKER\"/" \
 		-e 's/^addr_book_strict\s*=.*/addr_book_strict = false/' \
-		-e 's/^target_height_duration\s*=.*/target_height_duration = "1000ms"/' \
+		-e 's/^timeout_commit\s*=.*/timeout_commit = "500ms"/' \
 		-e 's/^index_all_tags\s*=.*/index_all_tags = true/' \
 		-e 's,^laddr = "tcp://127.0.0.1:26657",laddr = "tcp://0.0.0.0:26657",' \
 		-e 's/^prometheus\s*=.*/prometheus = true/' \

config/config.go

@@ -8,8 +8,6 @@ import (
 	"os"
 	"path/filepath"
 	"time"
-
-	tmtime "github.com/tendermint/tendermint/types/time"
 )
 
 const (
@@ -946,10 +944,11 @@ type ConsensusConfig struct {
 	TimeoutPrecommit time.Duration `mapstructure:"timeout_precommit"`
 	// How much the timeout_precommit increases with each round
 	TimeoutPrecommitDelta time.Duration `mapstructure:"timeout_precommit_delta"`
-	// TargetHeigtDuration is used to determine how long we wait after a
-	// block is committed. If this time is shorter than the actual time to reach
-	// consensus for that height, then we do not wait at all.
-	TargetHeightDuration time.Duration `mapstructure:"target_height_duration"`
+	// 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).
+	// NOTE: when modifying, make sure to update time_iota_ms genesis parameter
+	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"`
@@ -969,13 +968,13 @@ type ConsensusConfig struct {
 func DefaultConsensusConfig() *ConsensusConfig {
 	return &ConsensusConfig{
 		WalPath:                     filepath.Join(defaultDataDir, "cs.wal", "wal"),
-		TimeoutPropose:              2000 * time.Millisecond,
+		TimeoutPropose:              3000 * time.Millisecond,
 		TimeoutProposeDelta:         500 * time.Millisecond,
 		TimeoutPrevote:              1000 * time.Millisecond,
 		TimeoutPrevoteDelta:         500 * time.Millisecond,
 		TimeoutPrecommit:            1000 * time.Millisecond,
 		TimeoutPrecommitDelta:       500 * time.Millisecond,
-		TargetHeightDuration:        3000 * time.Millisecond,
+		TimeoutCommit:               1000 * time.Millisecond,
 		SkipTimeoutCommit:           false,
 		CreateEmptyBlocks:           true,
 		CreateEmptyBlocksInterval:   0 * time.Second,
@@ -995,7 +994,7 @@ func TestConsensusConfig() *ConsensusConfig {
 	cfg.TimeoutPrecommit = 10 * time.Millisecond
 	cfg.TimeoutPrecommitDelta = 1 * time.Millisecond
 	// NOTE: when modifying, make sure to update time_iota_ms (testGenesisFmt) in toml.go
-	cfg.TargetHeightDuration = 70 * time.Millisecond
+	cfg.TimeoutCommit = 10 * time.Millisecond
 	cfg.SkipTimeoutCommit = true
 	cfg.PeerGossipSleepDuration = 5 * time.Millisecond
 	cfg.PeerQueryMaj23SleepDuration = 250 * time.Millisecond
@@ -1029,14 +1028,10 @@ func (cfg *ConsensusConfig) Precommit(round int32) time.Duration {
 	) * time.Nanosecond
 }
 
-// NextStartTime adds the TargetHeightDuration to the provided starting time.
-func (cfg *ConsensusConfig) NextStartTime(t time.Time) time.Time {
-	newStartTime := t.Add(cfg.TargetHeightDuration)
-	now := tmtime.Now()
-	if newStartTime.Before(now) {
-		return now
-	}
-	return newStartTime
+// Commit returns the amount of time to wait for straggler votes after receiving +2/3 precommits
+// for a single block (ie. a commit).
+func (cfg *ConsensusConfig) Commit(t time.Time) time.Time {
+	return t.Add(cfg.TimeoutCommit)
 }
 
 // WalFile returns the full path to the write-ahead log file
@@ -1073,8 +1068,8 @@ func (cfg *ConsensusConfig) ValidateBasic() error {
 	if cfg.TimeoutPrecommitDelta < 0 {
 		return errors.New("timeout_precommit_delta can't be negative")
 	}
-	if cfg.TargetHeightDuration < 0 {
-		return errors.New("target_height_duration can't be negative")
+	if cfg.TimeoutCommit < 0 {
+		return errors.New("timeout_commit can't be negative")
 	}
 	if cfg.CreateEmptyBlocksInterval < 0 {
 		return errors.New("create_empty_blocks_interval can't be negative")

config/config_test.go

@@ -157,8 +157,8 @@ func TestConsensusConfig_ValidateBasic(t *testing.T) {
 		"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},
-		"TargetHeightDuration":                 {func(c *ConsensusConfig) { c.TargetHeightDuration = time.Second }, false},
-		"TargetHeightDuration negative":        {func(c *ConsensusConfig) { c.TargetHeightDuration = -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},

config/toml.go

@@ -464,10 +464,10 @@ timeout_prevote_delta = "{{ .Consensus.TimeoutPrevoteDelta }}"
 timeout_precommit = "{{ .Consensus.TimeoutPrecommit }}"
 # How much the timeout_precommit increases with each round
 timeout_precommit_delta = "{{ .Consensus.TimeoutPrecommitDelta }}"
-# TargetHeigtDuration is used to determine how long we wait after a
-# block is committed. If this time is shorter than the actual time to reach
-# consensus for that height, then we do not wait at all.
-target_height_duration = "{{ .Consensus.TargetHeightDuration }}"
+# 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

consensus/state.go

@@ -664,11 +664,14 @@ func (cs *State) updateToState(state sm.State) {
 	cs.updateRoundStep(0, cstypes.RoundStepNewHeight)
 
 	if cs.CommitTime.IsZero() {
-		// If it is the first block, start time is equal to
-		// states last block time which is the genesis time.
-		cs.StartTime = state.LastBlockTime
+		// "Now" makes it easier to sync up dev nodes.
+		// We add timeoutCommit to allow transactions
+		// to be gathered for the first block.
+		// And alternative solution that relies on clocks:
+		// cs.StartTime = state.LastBlockTime.Add(timeoutCommit)
+		cs.StartTime = cs.config.Commit(cmttime.Now())
 	} else {
-		cs.StartTime = cs.config.NextStartTime(cs.StartTime)
+		cs.StartTime = cs.config.Commit(cs.CommitTime)
 	}
 
 	cs.Validators = validators

docs/core/configuration.md

@@ -414,10 +414,10 @@ timeout_prevote_delta = "500ms"
 timeout_precommit = "1s"
 # How much the timeout_precommit increases with each round
 timeout_precommit_delta = "500ms"
-# TargetHeigtDuration is used to determine how long we wait after a
-# block is committed. If this time is shorter than the actual time to reach
-# consensus for that height, then we do not wait at all.
-target_height_duration = "15s"
+# 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 = "1s"
 
 # 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
@@ -537,9 +537,12 @@ timeout_prevote = "1s"
 timeout_prevote_delta = "500ms"
 timeout_precommit = "1s"
 timeout_precommit_delta = "500ms"
-target_height_duration = "1s"
+timeout_commit = "1s"
 ```
 
+Note that in a successful round, the only timeout that we absolutely wait no
+matter what is `timeout_commit`.
+
 Here's a brief summary of the timeouts:
 
 - `timeout_propose` = how long we wait for a proposal block before prevoting nil
@@ -549,8 +552,7 @@ Here's a brief summary of the timeouts:
 - `timeout_prevote_delta` = how much the `timeout_prevote` increases with each round
 - `timeout_precommit` = how long we wait after receiving +2/3 precommits for
   anything (ie. not a single block or nil)
-- `timeout_precommit_delta` = how much the timeout_precommit increases with
-  each round
-- `target_height_duration` = used to determine how long we wait after a
-	block is committed. If this time is shorter than the actual time to reach
-	consensus for that height, then we do not wait at all.
+- `timeout_precommit_delta` = how much the `timeout_precommit` increases with each round
+- `timeout_commit` = 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)

state/execution.go

@@ -407,7 +407,7 @@ func execBlockOnProxyApp(
 		return nil, err
 	}
 
-	logger.Info("executed block", "height", block.Height, "num_valid_txs", validTxs, "num_invalid_txs", invalidTxs, "time", block.Time)
+	logger.Info("executed block", "height", block.Height, "num_valid_txs", validTxs, "num_invalid_txs", invalidTxs)
 	return abciResponses, nil
 }
 

test/maverick/consensus/state.go

@@ -900,9 +900,9 @@ func (cs *State) updateToState(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 = state.LastBlockTime
+		cs.StartTime = cs.config.Commit(cmttime.Now())
 	} else {
-		cs.StartTime = cs.config.NextStartTime(cs.StartTime)
+		cs.StartTime = cs.config.Commit(cs.CommitTime)
 	}
 
 	cs.Validators = validators