Skip to content

Commit

Permalink
Add check for non-nil in enterCommit (cometbft#1208)
Browse files Browse the repository at this point in the history
* Minor

* Add check for non-`nil` in `enterCommit`
  • Loading branch information
sergio-mena authored and roy-dydx committed Apr 14, 2024
1 parent da876e0 commit 2986ef7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
2 changes: 1 addition & 1 deletion consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,7 @@ func (cs *State) enterCommit(height int64, commitRound int32) {
}()

blockID, ok := cs.Votes.Precommits(commitRound).TwoThirdsMajority()
if !ok {
if !ok || blockID.IsNil() {
panic("RunActionCommit() expects +2/3 precommits")
}

Expand Down
19 changes: 10 additions & 9 deletions consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ func TestStateLockPOLUpdateLock(t *testing.T) {
cs1 creates a proposal for block B.
Send a prevote for B from each of the validators to cs1.
Send a precommit for nil from all of the validators to cs1.
This ensures that cs1 will lock on B in this round but not precommit it.
*/
t.Log("### Starting Round 0")
Expand Down Expand Up @@ -708,6 +709,7 @@ func TestStateLockPOLUpdateLock(t *testing.T) {
Create a block, D and send a proposal for it to cs1
Send a prevote for D from each of the validators to cs1.
Send a precommit for nil from all of the validtors to cs1.
Check that cs1 is now locked on the new block, D and no longer on the old block.
*/
t.Log("### Starting Round 1")
Expand Down Expand Up @@ -772,6 +774,7 @@ func TestStateLockPOLRelock(t *testing.T) {
cs1 creates a proposal for block B.
Send a prevote for B from each of the validators to cs1.
Send a precommit for nil from all of the validators to cs1.
This ensures that cs1 will lock on B in this round but not precommit it.
*/
t.Log("### Starting Round 0")
Expand All @@ -788,7 +791,6 @@ func TestStateLockPOLRelock(t *testing.T) {
ensurePrevote(voteCh, height, round)

signAddVotes(cs1, cmtproto.PrevoteType, theBlockHash, theBlockParts.Header(), false, vs2, vs3, vs4)

// check that the validator generates a Lock event.
ensureLock(lockCh, height, round)

Expand All @@ -807,6 +809,7 @@ func TestStateLockPOLRelock(t *testing.T) {
Create a proposal for block B, the same block from round 1.
Send a prevote for B from each of the validators to cs1.
Send a precommit for nil from all of the validtors to cs1.
Check that cs1 updates its 'locked round' value to the current round.
*/
t.Log("### Starting Round 1")
Expand Down Expand Up @@ -854,7 +857,6 @@ func TestStateLockPOLRelock(t *testing.T) {
func TestStateLockPOLDoesNotUnlock(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

/*
All of the assertions in this test occur on the `cs1` validator.
The test sends signed votes from the other validators to cs1 and
Expand Down Expand Up @@ -930,6 +932,7 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) {
prop, propBlock := decideProposal(ctx, t, cs1, vs2, vs2.Height, vs2.Round)
propBlockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)

if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, ""); err != nil {
t.Fatal(err)
}
Expand All @@ -954,7 +957,6 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) {

signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, true, vs2, vs3, vs4)
ensureNewTimeout(timeoutWaitCh, height, round, cs1.config.Precommit(round).Nanoseconds())

/*
Round 2:
The validator cs1 saw >2/3 precommits for nil in the previous round.
Expand All @@ -967,6 +969,7 @@ func TestStateLockPOLDoesNotUnlock(t *testing.T) {
prop, propBlock = decideProposal(ctx, t, cs1, vs3, vs3.Height, vs3.Round)
propBlockParts, err = propBlock.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)

if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, ""); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1406,7 +1409,8 @@ func TestProposeValidBlock(t *testing.T) {
signAddVotes(cs1, cmtproto.PrevoteType, propBlockHash, bps.Header(), false, vs2, vs3, vs4)

ensurePrecommit(voteCh, height, round)
// we should have precommitted
// we should have precommitted the proposed block in this round.

validatePrecommit(t, cs1, round, round, vss[0], propBlockHash, propBlockHash)

signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, true, vs2, vs3, vs4)
Expand All @@ -1417,8 +1421,7 @@ func TestProposeValidBlock(t *testing.T) {
round++ // moving to the next round

ensureNewRound(newRoundCh, height, round)

t.Log("### ONTO ROUND 2")
t.Log("### ONTO ROUND 1")

// timeout of propose
ensureNewTimeout(timeoutProposeCh, height, round, cs1.config.Propose(round).Nanoseconds())
Expand All @@ -1440,7 +1443,7 @@ func TestProposeValidBlock(t *testing.T) {

signAddVotes(cs1, cmtproto.PrecommitType, nil, types.PartSetHeader{}, true, vs2, vs3, vs4)

round += 2 // moving to the next round
round += 2 // increment by multiple rounds

ensureNewRound(newRoundCh, height, round)
t.Log("### ONTO ROUND 3")
Expand All @@ -1451,8 +1454,6 @@ func TestProposeValidBlock(t *testing.T) {

ensureNewRound(newRoundCh, height, round)

t.Log("### ONTO ROUND 4")

ensureNewProposal(proposalCh, height, round)

rs = cs1.GetRoundState()
Expand Down

0 comments on commit 2986ef7

Please sign in to comment.