Skip to content

Commit

Permalink
stale sequence check (temp fix)
Browse files Browse the repository at this point in the history
  • Loading branch information
goran-ethernal committed Apr 10, 2024
1 parent 1a8ae5d commit 5e24ea9
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 1 deletion.
5 changes: 4 additions & 1 deletion consensus/polybft/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ func (f *fsm) BuildProposal(currentRound uint64) ([]byte, error) {
}

f.logger.Debug("[FSM Build Proposal]",
"block", stateBlock.Block.Number(),
"round", currentRound,
"txs", len(stateBlock.Block.Transactions),
"proposal hash", checkpointHash.String())
}
Expand Down Expand Up @@ -392,7 +394,8 @@ func (f *fsm) Validate(proposal []byte) error {
return fmt.Errorf("failed to calculate proposal hash: %w", err)
}

f.logger.Debug("[FSM Validate]", "txs", len(block.Transactions), "proposal hash", checkpointHash)
f.logger.Debug("[FSM Validate] Finish", "Block", block.Number(), "Proposer", types.BytesToHash(block.Header.Miner),
"txs", len(block.Transactions), "proposal hash", checkpointHash)
}

f.target = stateBlock
Expand Down
21 changes: 21 additions & 0 deletions consensus/polybft/polybft.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,14 @@ func (p *Polybft) startConsensusProtocol() {
stopSequence func()
)

// check every block time * 1.5, so we don't artificially close a sequence before the actual sequence has ended properly

Check failure on line 632 in consensus/polybft/polybft.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

line is 121 characters (lll)
checkOffset := p.config.BlockTime
if checkOffset > 1 {
checkOffset = checkOffset / 2

Check failure on line 635 in consensus/polybft/polybft.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

assignOp: replace `checkOffset = checkOffset / 2` with `checkOffset /= 2` (gocritic)
}
checkFrequency := time.Duration(p.config.BlockTime + checkOffset)

Check failure on line 637 in consensus/polybft/polybft.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

assignments should only be cuddled with other assignments (wsl)
staleChecker := newStaleSequenceCheck(p.logger, p.blockchain.CurrentHeader, checkFrequency*time.Second)

for {
latestHeader := p.blockchain.CurrentHeader()

Expand All @@ -652,6 +660,8 @@ func (p *Polybft) startConsensusProtocol() {
}

sequenceCh, stopSequence = p.ibft.runSequence(latestHeader.Number + 1)
staleChecker.setSequence(latestHeader.Number + 1)
staleChecker.startChecking()
}

now := time.Now().UTC()
Expand All @@ -662,15 +672,26 @@ func (p *Polybft) startConsensusProtocol() {
stopSequence()
p.logger.Info("canceled sequence", "sequence", latestHeader.Number+1)
}
case <-staleChecker.sequenceShouldStop:
if isValidator {
stopSequence()
p.logger.Info("canceled sequence via stale checker", "sequence", latestHeader.Number+1)
}
case <-sequenceCh:
case <-p.closeCh:
p.logger.Debug("[BRE] stoping sequence", "block number", latestHeader.Number+1)
if isValidator {
stopSequence()
staleChecker.stopChecking()
}

return
}

if isValidator {
staleChecker.stopChecking()
}

p.logger.Debug("time to run the sequence", "seconds", time.Since(now))
}
}
Expand Down
79 changes: 79 additions & 0 deletions consensus/polybft/stale_sequence_check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package polybft

import (
"sync"
"time"

"github.com/0xPolygon/polygon-edge/types"
"github.com/hashicorp/go-hclog"
)

type staleSequenceCheck struct {
logger hclog.Logger
currentSequence uint64
mtx *sync.Mutex
checkFrequency time.Duration
sequenceShouldStop chan struct{}
stop chan struct{}
stopped chan struct{}
getHeader func() *types.Header
}

func newStaleSequenceCheck(logger hclog.Logger, getHeader func() *types.Header, checkDuration time.Duration) *staleSequenceCheck {

Check failure on line 22 in consensus/polybft/stale_sequence_check.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

line is 130 characters (lll)
return &staleSequenceCheck{
logger: logger,
currentSequence: 0,
mtx: &sync.Mutex{},
checkFrequency: checkDuration,
getHeader: getHeader,
}
}

func (s *staleSequenceCheck) startChecking() {
s.sequenceShouldStop = make(chan struct{}, 1)
s.stop = make(chan struct{})
s.stopped = make(chan struct{})
ticker := time.NewTicker(s.checkFrequency)
go func() {

Check failure on line 37 in consensus/polybft/stale_sequence_check.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

only one cuddle assignment allowed before go statement (wsl)
defer close(s.stopped)
for {

Check failure on line 39 in consensus/polybft/stale_sequence_check.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

for statement without condition should never be cuddled (wsl)
select {
case <-s.stop:
close(s.sequenceShouldStop)
ticker.Stop()
return

Check failure on line 44 in consensus/polybft/stale_sequence_check.go

View workflow job for this annotation

GitHub Actions / Lint / Run Lint

return with no blank line before (nlreturn)
case <-ticker.C:
s.checkForStaleness()
}
}
}()
}

func (s *staleSequenceCheck) stopChecking() {
close(s.stop)
<-s.stopped
}

func (s *staleSequenceCheck) setSequence(sequence uint64) {
s.mtx.Lock()
defer s.mtx.Unlock()
s.currentSequence = sequence
}

func (s *staleSequenceCheck) checkForStaleness() {
s.logger.Info("[staleSequenceCheck] checking for stale sequence")
header := s.getHeader()
s.chainHeightUpdated(header.Number)
}

func (s *staleSequenceCheck) chainHeightUpdated(height uint64) {
s.mtx.Lock()
defer s.mtx.Unlock()
if s.currentSequence == 0 {
return
}
if height >= s.currentSequence {
s.logger.Info("[staleSequenceCheck] stale sequence detected", "height", height, "currentSequence", s.currentSequence)
s.sequenceShouldStop <- struct{}{}
}
}

0 comments on commit 5e24ea9

Please sign in to comment.