Repository: mantlenetworkio/mantle (language: go)

Issue:
Sequencer: compareSequencerSet test and fix
In the current implementation,`compareSequencerSet` cannot effectively identify the deleted sequencer. This bug needs to be fixed and the test cases of this method should be supplemented.


Discussion / hints:

Fix patch:
diff --git a/l2geth/consensus/clique/scheduler.go b/l2geth/consensus/clique/scheduler.go
index 0d6659227..72c12c7c7 100644
--- a/l2geth/consensus/clique/scheduler.go
+++ b/l2geth/consensus/clique/scheduler.go
@@ -99,7 +99,7 @@ func NewScheduler(db ethdb.Database, config *Config, schedulerAddress common.Add
 	for _, item := range seqSet {
 		var addrTemp common.Address
 		copy(addrTemp[:], item.MintAddress[:])
-		votingPower := big.NewInt(0).Div(item.Amount, scale)
+		votingPower := new(big.Int).Div(item.Amount, scale)
 		seqz = append(seqz, NewSequencer(addrTemp, votingPower.Int64(), item.NodeID))
 		log.Info("sequencer: ", "address", item.MintAddress.String(), "node_ID", hex.EncodeToString(item.NodeID))
 	}
@@ -384,22 +384,40 @@ func (schedulerInst *Scheduler) syncSequencerSetRoutine() {
 }
 
 // compareSequencerSet will return the update with Driver.seqz
-func compareSequencerSet(old []*Sequencer, newSeq synchronizer.SequencerSequencerInfos) []*Sequencer {
+func compareSequencerSet(preSeqs []*Sequencer, newSeq synchronizer.SequencerSequencerInfos) []*Sequencer {
+	notDel := make(map[common.Address]bool)
 	var tmp synchronizer.SequencerSequencerInfos
 	for i, v := range newSeq {
-		changed := true
-		for _, seq := range old {
-			power := big.NewInt(1).Div(v.Amount, scale)
-			if bytes.Equal(seq.Address.Bytes(), v.MintAddress.Bytes()) && power.Int64() == seq.Power {
-				changed = false
+		for _, seq := range preSeqs {
+			power := new(big.Int).Div(v.Amount, scale)
+			// figure out unchanged sequencer
+			if bytes.Equal(seq.Address.Bytes(), v.MintAddress.Bytes()) {
+				// find the sequencer in previous sequencer set
+				notDel[seq.Address] = true
+				if power.Int64() != seq.Power {
+					// voting power changed sequencer update
+					log.Debug("Compare sequencer set", "update_sequencer", newSeq[i].MintAddress.String())
+					tmp = append(tmp, newSeq[i])
+				}
 				break
 			}
 		}
-		if changed {
+		// sequencer add
+		if !notDel[common.Address(newSeq[i].MintAddress)] {
+			log.Debug("Compare sequencer set", "add_sequencer", newSeq[i].MintAddress.String())
 			tmp = append(tmp, newSeq[i])
 		}
 	}
 	changes := bindToSeq(tmp)
+	// select the deleted sequencer
+	for _, v := range preSeqs {
+		if !notDel[v.Address] {
+			tmpDel := v
+			tmpDel.Power = 0
+			changes = append(changes, tmpDel)
+			log.Debug("Compare sequencer set", "delete_sequencer", v.Address.String())
+		}
+	}
 	return changes
 }
 
@@ -409,7 +427,7 @@ func bindToSeq(binds synchronizer.SequencerSequencerInfos) []*Sequencer {
 		seq := &Sequencer{
 			Address: common.BytesToAddress(v.MintAddress.Bytes()),
 			NodeID:  v.NodeID,
-			Power:   v.Amount.Div(v.Amount, scale).Int64(),
+			Power:   new(big.Int).Div(v.Amount, scale).Int64(),
 		}
 		seqs = append(seqs, seq)
 	}
diff --git a/packages/contracts/docs/console.md b/packages/contracts/docs/console.md
deleted file mode 100644
index 8bab67a46..000000000
--- a/packages/contracts/docs/console.md
+++ /dev/null
@@ -1,12 +0,0 @@
-# console
-
-
-
-
-
-
-
-
-
-
-


Test patch:
diff --git a/l2geth/consensus/clique/scheduler_test.go b/l2geth/consensus/clique/scheduler_test.go
index 2f2204fc0..a46858b68 100644
--- a/l2geth/consensus/clique/scheduler_test.go
+++ b/l2geth/consensus/clique/scheduler_test.go
@@ -2,26 +2,131 @@ package clique
 
 import (
 	"crypto/ecdsa"
+	"math/big"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
+	ethc "github.com/ethereum/go-ethereum/common"
 	"github.com/mantlenetworkio/mantle/l2geth/common"
+	"github.com/mantlenetworkio/mantle/l2geth/consensus/clique/synchronizer"
+	binding "github.com/mantlenetworkio/mantle/l2geth/consensus/clique/synchronizer/bindings"
 	"github.com/mantlenetworkio/mantle/l2geth/core/types"
 	"github.com/mantlenetworkio/mantle/l2geth/crypto"
-	"github.com/stretchr/testify/require"
 )
 
+var (
+	amount     = int64(1e18)
+	sequencer0 = binding.SequencerSequencerInfo{
+		Owner:       ethc.BigToAddress(big.NewInt(1)),
+		MintAddress: ethc.BigToAddress(big.NewInt(1)),
+		NodeID:      []byte{1},
+		Amount:      big.NewInt(amount),
+		KeyIndex:    big.NewInt(0),
+	}
+	sequencer1 = binding.SequencerSequencerInfo{
+		Owner:       ethc.BigToAddress(big.NewInt(2)),
+		MintAddress: ethc.BigToAddress(big.NewInt(2)),
+		NodeID:      []byte{1},
+		Amount:      big.NewInt(amount),
+		KeyIndex:    big.NewInt(0),
+	}
+	sequencer2 = binding.SequencerSequencerInfo{
+		Owner:       ethc.BigToAddress(big.NewInt(3)),
+		MintAddress: ethc.BigToAddress(big.NewInt(3)),
+		NodeID:      []byte{1},
+		Amount:      big.NewInt(amount),
+		KeyIndex:    big.NewInt(0),
+	}
+)
+
+func TestSchedulerCompareSequencerSet(t *testing.T) {
+	testCases := []struct {
+		newSeqs synchronizer.SequencerSequencerInfos
+		msg     string
+		check   func(changes []*Sequencer)
+	}{
+		{
+			newSeqs: synchronizer.SequencerSequencerInfos{
+				sequencer0,
+				sequencer1,
+				sequencer2,
+			},
+			msg: "Add sequencer",
+			check: func(changes []*Sequencer) {
+				seqSet := NewSequencerSet(changes)
+				// only add sequencer2
+				require.Equal(t, seqSet.Len(), 1)
+				require.Equal(t, seqSet.Sequencers[0].Address.String(), sequencer2.MintAddress.String())
+			},
+		},
+		{
+			newSeqs: synchronizer.SequencerSequencerInfos{},
+			msg:     "Delete all sequencer",
+			check: func(changes []*Sequencer) {
+				require.Equal(t, len(changes), 2)
+				for _, v := range changes {
+					require.Equal(t, v.Power, int64(0))
+				}
+			},
+		},
+		{
+			newSeqs: synchronizer.SequencerSequencerInfos{
+				sequencer0,
+				binding.SequencerSequencerInfo{
+					Owner:       sequencer1.Owner,
+					MintAddress: sequencer1.MintAddress,
+					NodeID:      sequencer1.NodeID,
+					Amount:      big.NewInt(0),
+					KeyIndex:    sequencer1.KeyIndex,
+				},
+			},
+			msg: "Delete sequencer0",
+			check: func(changes []*Sequencer) {
+				require.Equal(t, len(changes), 1)
+				require.Equal(t, changes[0].Power, int64(0))
+			},
+		},
+		{
+			newSeqs: synchronizer.SequencerSequencerInfos{
+				sequencer0, sequencer1,
+			},
+			msg: "Do nothing",
+			check: func(changes []*Sequencer) {
+			},
+		},
+	}
+	for _, tc := range testCases {
+		// init sequencer set
+		var seqz []*Sequencer
+		seqs := synchronizer.SequencerSequencerInfos{
+			sequencer0, sequencer1,
+		}
+		for _, item := range seqs {
+			var addrTemp common.Address
+			copy(addrTemp[:], item.MintAddress[:])
+			votingPower := big.NewInt(0).Div(item.Amount, scale)
+			seqz = append(seqz, NewSequencer(addrTemp, votingPower.Int64(), item.NodeID))
+		}
+		seqSet := NewSequencerSet(seqz)
+		// get changes from test case
+		changes := compareSequencerSet(seqSet.Sequencers, tc.newSeqs)
+		tc.check(changes)
+	}
+}
+
 func TestSigAndVerify(t *testing.T) {
 	prv, err := crypto.GenerateKey()
 	require.NoError(t, err)
 	pub := prv.Public().(*ecdsa.PublicKey)
 	address := crypto.PubkeyToAddress(*pub).Hex()
 	bps := &types.BatchPeriodStartMsg{
-		ReorgIndex:  0,
-		BatchIndex:  0,
-		StartHeight: 1,
-		MaxHeight:   100,
-		ExpireTime:  1669787879,
-		Sequencer:   common.HexToAddress("0xe86c354b11bdc9f295eb2aca01640727dc332d43"),
+		RollbackStates: nil,
+		BatchIndex:     0,
+		StartHeight:    1,
+		MaxHeight:      100,
+		ExpireTime:     100,
+		Sequencer:      common.HexToAddress("0xe86c354b11bdc9f295eb2aca01640727dc332d43"),
 	}
 
 	bpsHash := bps.GetSignData()