[ibft] Fix validator build block when syncing (#313)

# Description

Validator nodes build blocks during sync, which makes it slow to sync
and full of error logs.
This PR fixes it to only start building blocks when syncing is done.

# Changes include

- [x] Bugfix (non-breaking change that solves an issue)

## Testing

- [x] I have tested this code with the official test suite
- [x] I have tested this code manually

### Manual tests

1. start up a 4-validators network.
2. stop a validator for about 2-3 minutes.
3. restart the stopped validator and check its logs.

On current branch, validator will print out "sequence started" only when
it catch up to the latest height.
On the target branch, validator will print out "sequence started" every
time a new block arrives, and try to build an outdated block every turn.

consensus/ibft/ibft.go

@@ -544,8 +544,8 @@ func (i *Ibft) startConsensus() {
 
 		isValidator = i.isValidSnapshot()
 
-		if isValidator {
-			// i.startNewSequence()
+		// validator must not be in syncing mode to start a new block
+		if isValidator && !i.syncer.IsSyncing() {
 			sequenceCh = i.runSequence(pending)
 		}
 

protocol/interface.go

@@ -21,6 +21,8 @@ type Syncer interface {
 	GetSyncProgression() *progress.Progression
 	// HasSyncPeer returns whether syncer has the peer syncer can sync with
 	HasSyncPeer() bool
+	// IsSyncing returns whether syncer is syncing
+	IsSyncing() bool
 	// Sync starts routine to sync blocks
 	Sync(func(*types.Block) bool) error
 }

protocol/syncer.go

@@ -136,12 +136,16 @@ func NewSyncer(
 	return s
 }
 
-func (s *noForkSyncer) isSyncing() bool {
+func (s *noForkSyncer) IsSyncing() bool {
 	return s.syncing.Load()
 }
 
-func (s *noForkSyncer) setSyncing(syncing bool) (oldStatus bool) {
-	return s.syncing.Swap(syncing)
+func (s *noForkSyncer) startSyncingStatus() (success bool) {
+	return s.syncing.CompareAndSwap(false, true)
+}
+
+func (s *noForkSyncer) stopSyncingStatus() (success bool) {
+	return s.syncing.CompareAndSwap(true, false)
 }
 
 // GetSyncProgression returns the latest sync progression, if any
@@ -300,15 +304,13 @@ func (s *noForkSyncer) Sync(callback func(*types.Block) bool) error {
 			}
 
 			// The channel should not be blocked, otherwise it will hang when an error occurs
-			if s.isSyncing() {
+			if !s.startSyncingStatus() {
 				s.logger.Debug("skip new status event due to not done syncing")
 
 				continue
 			}
 		}
 
-		s.logger.Debug("got new status event")
-
 		if shouldTerminate := s.syncWithSkipList(skipList, callback); shouldTerminate {
 			s.logger.Error("terminate syncing")
 
@@ -323,12 +325,13 @@ func (s *noForkSyncer) syncWithSkipList(
 	skipList *sync.Map,
 	callback func(*types.Block) bool,
 ) (shouldTerminate bool) {
-	// switch syncing status
-	s.setSyncing(true)
-	defer s.setSyncing(false)
+	s.logger.Debug("got new status event and start syncing")
 
-	s.logger.Debug("start syncing")
-	defer s.logger.Debug("done syncing")
+	// switch syncing status
+	defer func() {
+		s.logger.Debug("done syncing")
+		s.stopSyncingStatus()
+	}()
 
 	var localLatest uint64
 

protocol/syncer_test.go

@@ -212,7 +212,7 @@ func Test_startPeerStatusUpdateProcess(t *testing.T) {
 		&mockProgression{},
 	)
 
-	syncer.setSyncing(true) // to skip channel blocking
+	syncer.startSyncingStatus() // to skip channel blocking
 
 	go syncer.Sync(nil)