cs: reset triggered timeout precommit (#3310)

* Reset TriggeredTimeoutPrecommit as part of updateToState

* Add failing test and fix

* fix DATA RACE in TestResetTimeoutPrecommitUponNewHeight

```
WARNING: DATA RACE
Read at 0x00c001691d28 by goroutine 691:
  github.com/tendermint/tendermint/consensus.decideProposal()
      /go/src/github.com/tendermint/tendermint/consensus/common_test.go:133 +0x121
  github.com/tendermint/tendermint/consensus.TestResetTimeoutPrecommitUponNewHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state_test.go:1389 +0x958
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous write at 0x00c001691d28 by goroutine 931:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateToState()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:562 +0x5b2
  github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1340 +0x141e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1255 +0x66e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1201 +0x135
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1232 +0x94b
  github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1657 +0x132e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1503 +0x8f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:694 +0xa1e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd

Goroutine 691 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:878 +0x659
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1119 +0xa8
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162
  testing.runTests()
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1034 +0x2ee
  main.main()
      _testmain.go:216 +0x332
```

* fix another DATA RACE by locking consensus

```
WARNING: DATA RACE
Read at 0x00c009b835a8 by goroutine 871:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).createProposalBlock()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:955 +0x7c
  github.com/tendermint/tendermint/consensus.decideProposal()
      /go/src/github.com/tendermint/tendermint/consensus/common_test.go:127 +0x53
  github.com/tendermint/tendermint/consensus.TestResetTimeoutPrecommitUponNewHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state_test.go:1389 +0x958
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:827 +0x162

Previous write at 0x00c009b835a8 by goroutine 931:
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateHeight()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:446 +0xb7
  github.com/tendermint/tendermint/consensus.(*ConsensusState).updateToState()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:542 +0x22f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1340 +0x141e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1255 +0x66e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1201 +0x135
  github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1232 +0x94b
  github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1657 +0x132e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:1503 +0x8f
  github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:694 +0xa1e
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:642 +0x948
  github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine()
      /go/src/github.com/tendermint/tendermint/consensus/state.go:655 +0x7dd
```

* Fix failing test

* Delete profile.out

* fix data races

CHANGELOG_PENDING.md

@@ -24,6 +24,7 @@ Special thanks to external contributors on this release:
 
 * [consensus] \#3297 Flush WAL on stop to prevent data corruption during
   graceful shutdown
+- [consensus] \#3310 Reset TriggeredTimeoutPrecommit before starting next height
 - [rpc] \#3251 Fix /net_info#peers#remote_ip format. New format spec:
   * dotted decimal ("192.0.2.1"), if ip is an IPv4 or IP4-mapped IPv6 address
   * IPv6 ("2001:db8::1"), if ip is a valid IPv6 address

consensus/common_test.go

@@ -124,15 +124,21 @@ func startTestRound(cs *ConsensusState, height int64, round int) {
 
 // Create proposal block from cs1 but sign it with vs
 func decideProposal(cs1 *ConsensusState, vs *validatorStub, height int64, round int) (proposal *types.Proposal, block *types.Block) {
+	cs1.mtx.Lock()
 	block, blockParts := cs1.createProposalBlock()
+	cs1.mtx.Unlock()
 	if block == nil { // on error
 		panic("error creating proposal block")
 	}
 
 	// Make proposal
-	polRound, propBlockID := cs1.ValidRound, types.BlockID{block.Hash(), blockParts.Header()}
+	cs1.mtx.RLock()
+	validRound := cs1.ValidRound
+	chainID := cs1.state.ChainID
+	cs1.mtx.RUnlock()
+	polRound, propBlockID := validRound, types.BlockID{block.Hash(), blockParts.Header()}
 	proposal = types.NewProposal(height, round, polRound, propBlockID)
-	if err := vs.SignProposal(cs1.state.ChainID, proposal); err != nil {
+	if err := vs.SignProposal(chainID, proposal); err != nil {
 		panic(err)
 	}
 	return

consensus/state.go

@@ -566,6 +566,7 @@ func (cs *ConsensusState) updateToState(state sm.State) {
 	cs.CommitRound = -1
 	cs.LastCommit = lastPrecommits
 	cs.LastValidators = state.LastValidators
+	cs.TriggeredTimeoutPrecommit = false
 
 	cs.state = state
 

consensus/state_test.go

@@ -1288,8 +1288,8 @@ func (n *fakeTxNotifier) Notify() {
 }
 
 func TestStartNextHeightCorrectly(t *testing.T) {
+	config.Consensus.SkipTimeoutCommit = false
 	cs1, vss := randConsensusState(4)
-	cs1.config.SkipTimeoutCommit = false
 	cs1.txNotifier = &fakeTxNotifier{ch: make(chan struct{})}
 
 	vs2, vs3, vs4 := vss[1], vss[2], vss[3]
@@ -1326,13 +1326,14 @@ func TestStartNextHeightCorrectly(t *testing.T) {
 	// add precommits
 	signAddVotes(cs1, types.PrecommitType, nil, types.PartSetHeader{}, vs2)
 	signAddVotes(cs1, types.PrecommitType, theBlockHash, theBlockParts, vs3)
+	time.Sleep(5 * time.Millisecond)
 	signAddVotes(cs1, types.PrecommitType, theBlockHash, theBlockParts, vs4)
 
-	ensureNewBlockHeader(newBlockHeader, height, theBlockHash)
-
 	rs = cs1.GetRoundState()
 	assert.True(t, rs.TriggeredTimeoutPrecommit)
 
+	ensureNewBlockHeader(newBlockHeader, height, theBlockHash)
+
 	cs1.txNotifier.(*fakeTxNotifier).Notify()
 
 	ensureNewTimeout(timeoutProposeCh, height+1, round, cs1.config.TimeoutPropose.Nanoseconds())
@@ -1340,6 +1341,64 @@ func TestStartNextHeightCorrectly(t *testing.T) {
 	assert.False(t, rs.TriggeredTimeoutPrecommit, "triggeredTimeoutPrecommit should be false at the beginning of each round")
 }
 
+func TestResetTimeoutPrecommitUponNewHeight(t *testing.T) {
+	config.Consensus.SkipTimeoutCommit = false
+	cs1, vss := randConsensusState(4)
+
+	vs2, vs3, vs4 := vss[1], vss[2], vss[3]
+	height, round := cs1.Height, cs1.Round
+
+	partSize := types.BlockPartSizeBytes
+
+	proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal)
+
+	newRoundCh := subscribe(cs1.eventBus, types.EventQueryNewRound)
+	newBlockHeader := subscribe(cs1.eventBus, types.EventQueryNewBlockHeader)
+	addr := cs1.privValidator.GetPubKey().Address()
+	voteCh := subscribeToVoter(cs1, addr)
+
+	// start round and wait for propose and prevote
+	startTestRound(cs1, height, round)
+	ensureNewRound(newRoundCh, height, round)
+
+	ensureNewProposal(proposalCh, height, round)
+	rs := cs1.GetRoundState()
+	theBlockHash := rs.ProposalBlock.Hash()
+	theBlockParts := rs.ProposalBlockParts.Header()
+
+	ensurePrevote(voteCh, height, round)
+	validatePrevote(t, cs1, round, vss[0], theBlockHash)
+
+	signAddVotes(cs1, types.PrevoteType, theBlockHash, theBlockParts, vs2, vs3, vs4)
+
+	ensurePrecommit(voteCh, height, round)
+	validatePrecommit(t, cs1, round, round, vss[0], theBlockHash, theBlockHash)
+
+	rs = cs1.GetRoundState()
+
+	// add precommits
+	signAddVotes(cs1, types.PrecommitType, nil, types.PartSetHeader{}, vs2)
+	signAddVotes(cs1, types.PrecommitType, theBlockHash, theBlockParts, vs3)
+	time.Sleep(5 * time.Millisecond)
+	signAddVotes(cs1, types.PrecommitType, theBlockHash, theBlockParts, vs4)
+
+	rs = cs1.GetRoundState()
+	assert.True(t, rs.TriggeredTimeoutPrecommit)
+
+	ensureNewBlockHeader(newBlockHeader, height, theBlockHash)
+
+	prop, propBlock := decideProposal(cs1, vs2, height+1, 0)
+	propBlockParts := propBlock.MakePartSet(partSize)
+
+	if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil {
+		t.Fatal(err)
+	}
+	ensureNewProposal(proposalCh, height+1, 0)
+
+	rs = cs1.GetRoundState()
+	assert.False(t, rs.TriggeredTimeoutPrecommit, "triggeredTimeoutPrecommit should be false at the beginning of each height")
+}
+
 //------------------------------------------------------------------------------------------
 // SlashingSuite
 // TODO: Slashing