Address comments

Author: ranchalp

consensus/system_contract/consensus.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"math/big"
-	"sort"
 	"time"
 
 	"github.com/scroll-tech/go-ethereum/accounts"
@@ -176,8 +175,10 @@ func (s *SystemContract) verifyCascadingFields(chain consensus.ChainHeaderReader
 	}
 
 	if err := s.correctSigner(header); err != nil {
-		return err
+		return fmt.Errorf("block %d: signer check failed: %w", header.Number.Uint64(), err)
+
 	}
+
 	return nil
 }
 
@@ -201,20 +202,16 @@ func (s *SystemContract) correctSigner(header *types.Header) error {
 }
 
 func (s *SystemContract) findSignerSlice(blockNum uint64) (common.Address, error) {
-	// Acquire read-lock before accessing s.signerAddressesL1
 	s.lock.RLock()
 	defer s.lock.RUnlock()
-	transitions := s.signerAddressesL1
-	idx := sort.Search(len(transitions), func(i int) bool {
-		// We want the first transition with StartBlock > blockNum
-		return transitions[i].StartBlock > blockNum
-	})
-	// idx is now the insertion point for 'blockNum'
-	// so the real match is at idx-1 if idx > 0
-	if idx == 0 {
-		return common.Address{}, errors.New("no signer for block")
-	}
-	return transitions[idx-1].Signer, nil
+
+	// Assuming s.signerAddressesL1 is sorted in descending order (largest StartBlock first)
+	for _, t := range s.signerAddressesL1 {
+		if blockNum >= t.StartBlock {
+			return t.Signer, nil
+		}
+	}
+	return common.Address{}, errors.New("no signer for block")
 }
 
 // VerifyUncles implements consensus.Engine, always returning an error for any

consensus/system_contract/system_contract.go

@@ -17,7 +17,7 @@ import (
 )
 
 const (
-	defaultSyncInterval = 10
+	defaultSyncInterval = 10 * time.Second
 )
 
 type SignerAddressL1 struct {
@@ -155,18 +155,46 @@ func (s *SystemContract) Start() {
 			case <-s.ctx.Done():
 				return
 			case <-syncTicker.C:
-				s.lock.Lock()
-				signers, err := s.fetchAllSigners(nil) // nil -> latest state
+				// Fetch the latest signers from L1 outside of the lock
+				newSigners, err := s.fetchAllSigners(nil)
 				if err != nil {
-					log.Error("failed to get signer address from L1 System Contract", "err", err)
+					log.Error("failed to fetch signer addresses from L1 System Contract", "err", err)
+					continue
+				}
+
+				// Optionally, compare with the currently stored signers first.
+				s.lock.RLock()
+				currentSigners := s.signerAddressesL1
+				s.lock.RUnlock()
+				if !signersChanged(currentSigners, newSigners) {
+					// No change—no need to reacquire the write lock.
+					continue
 				}
-				s.signerAddressesL1 = signers
+
+				s.lock.Lock()
+				s.signerAddressesL1 = newSigners
 				s.lock.Unlock()
+				log.Info("Refreshed signer addresses", "count", len(newSigners))
 			}
 		}
 	}()
 }
 
+// signersChanged compares two slices of SignerAddressL1 and returns true if they differ.
+// For simplicity, we compare lengths and each element in order.
+func signersChanged(current, new []SignerAddressL1) bool {
+	if len(current) != len(new) {
+		return true
+	}
+	for i, cs := range current {
+		ns := new[i]
+		if cs.StartBlock != ns.StartBlock || cs.Signer != ns.Signer {
+			return true
+		}
+	}
+	return false
+}
+
 // Close implements consensus.Engine.
 func (s *SystemContract) Close() error {
 	s.cancel()