Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@
// GetRoundState returns a shallow copy of the internal consensus state.
func (cs *State) GetRoundState() *cstypes.RoundState {
cs.rsMtx.RLock()
rs := cs.rs // copy

Check failure on line 296 in consensus/state.go

View workflow job for this annotation

GitHub Actions / golangci-lint

copylocks: assignment copies lock value to rs: github.com/cometbft/cometbft/consensus/types.RoundState contains sync/atomic.Bool contains sync/atomic.noCopy (govet)
cs.rsMtx.RUnlock()
return &rs
}
Expand All @@ -302,7 +302,7 @@
func (cs *State) GetRoundStateJSON() ([]byte, error) {
cs.rsMtx.RLock()
defer cs.rsMtx.RUnlock()
return cmtjson.Marshal(cs.rs)

Check failure on line 305 in consensus/state.go

View workflow job for this annotation

GitHub Actions / golangci-lint

copylocks: call of cmtjson.Marshal copies lock value: github.com/cometbft/cometbft/consensus/types.RoundState contains sync/atomic.Bool contains sync/atomic.noCopy (govet)
}

// GetRoundStateSimpleJSON returns a json of RoundStateSimple
Expand Down Expand Up @@ -939,7 +939,7 @@
}
}

rs := cs.rs

Check failure on line 942 in consensus/state.go

View workflow job for this annotation

GitHub Actions / golangci-lint

copylocks: assignment copies lock value to rs: github.com/cometbft/cometbft/consensus/types.RoundState contains sync/atomic.Bool contains sync/atomic.noCopy (govet)
var mi msgInfo

select {
Expand Down Expand Up @@ -983,7 +983,7 @@

// if the timeout is relevant to the rs
// go to the next step
cs.handleTimeout(ti, rs)

Check failure on line 986 in consensus/state.go

View workflow job for this annotation

GitHub Actions / golangci-lint

copylocks: call of cs.handleTimeout copies lock value: github.com/cometbft/cometbft/consensus/types.RoundState contains sync/atomic.Bool contains sync/atomic.noCopy (govet)

case <-cs.Quit():
onExit(cs)
Expand Down Expand Up @@ -1084,7 +1084,7 @@
}
}

func (cs *State) handleTimeout(ti timeoutInfo, rs cstypes.RoundState) {

Check failure on line 1087 in consensus/state.go

View workflow job for this annotation

GitHub Actions / golangci-lint

copylocks: handleTimeout passes lock by value: github.com/cometbft/cometbft/consensus/types.RoundState contains sync/atomic.Bool contains sync/atomic.noCopy (govet)
cs.Logger.Trace("received tock", "timeout", ti.Duration, "height", ti.Height, "round", ti.Round, "step", ti.Step)

// timeouts must be for current height, round, step
Expand Down Expand Up @@ -1622,9 +1622,23 @@
return
}

if ready, waitTime := cs.isReadyToPrecommit(); !ready {
logger.Debug("rescheduling precommit", "delay(ms)", waitTime.Milliseconds())
cs.scheduleTimeout(waitTime, height, round, cstypes.RoundStepPrevoteWait)
if !cs.rs.StartedPrecommitSleep.Load() {
cs.rs.StartedPrecommitSleep.Store(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use CompareAndSwap.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waitTime := cs.precommitDelay()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid the timer and unlocking relocking, if the waitTime is 0

We could even add the check prior like:

waitTime := ps.precommitDelay()
if !cs.rs.StartedPrecommitSleep.Load() && waitTime > 0 {
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding the check above would complicate the if statement given the else branch that will return

logger.Debug("delaying precommit", "delay", waitTime)
cs.unlockAll()
t := time.NewTimer(waitTime)
select {
case <-cs.Quit():
cs.lockAll()
return
case <-t.C:
}
cs.lockAll()
cs.rs.StartedPrecommitSleep.Store(false)
} else {
logger.Debug("already entered precommit sleep")
// if any other routine tries to enter precommit, we just return
return
}

Expand Down Expand Up @@ -2105,20 +2119,20 @@
// KMSSigningDelay is a constant representing a delay used primarily to adjust for KMS signing latencies.
const KMSSigningDelay = 200 * time.Millisecond

// isReadyToPrecommit calculates if the process has waited at least a certain number of seconds
// precommitDelay calculates if the process has waited at least a certain number of seconds
// from their start time before they can vote
// If the application's DelayedPrecommitTimeout is set to 0, no precommit wait is done.
func (cs *State) isReadyToPrecommit() (bool, time.Duration) {
func (cs *State) precommitDelay() time.Duration {
if cs.state.Timeouts.DelayedPrecommitTimeout == 0 {
// setting 0 as a special case not to reschedule the pre-commit
return true, 0
return 0
}
precommitVoteTime := cs.rs.StartTime.Add(cs.state.Timeouts.DelayedPrecommitTimeout)
waitTime := time.Until(precommitVoteTime)
if _, ok := cs.privValidator.(*privval.SignerClient); ok {
waitTime = waitTime - KMSSigningDelay
}
return waitTime <= 0, waitTime
return waitTime
}

//-----------------------------------------------------------------------------
Expand Down
10 changes: 6 additions & 4 deletions consensus/types/round_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package types
import (
"encoding/json"
"fmt"
"sync/atomic"
"time"

"github.com/cometbft/cometbft/libs/bytes"
Expand Down Expand Up @@ -65,10 +66,11 @@ func (rs RoundStepType) String() string {
// NOTE: Not thread safe. Should only be manipulated by functions downstream
// of the cs.receiveRoutine
type RoundState struct {
Height int64 `json:"height"` // Height we are working on
Round int32 `json:"round"`
Step RoundStepType `json:"step"`
StartTime time.Time `json:"start_time"`
Height int64 `json:"height"` // Height we are working on
Round int32 `json:"round"`
Step RoundStepType `json:"step"`
StartTime time.Time `json:"start_time"`
StartedPrecommitSleep atomic.Bool

// Subjective time when +2/3 precommits for Block at Round were found
CommitTime time.Time `json:"commit_time"`
Expand Down
Loading