Next validator in view change. (#4492)

* NthNextValidatorHmy

* Test.

* Fixed func usage.

* Additional checks.

consensus/consensus_v2.go

@@ -749,7 +749,7 @@ func (consensus *Consensus) rotateLeader(epoch *big.Int) {
 		next     *bls.PublicKeyWrapper
 	)
 	if bc.Config().IsLeaderRotationExternalValidatorsAllowed(epoch, consensus.ShardID) {
-		wasFound, next = consensus.Decider.NthNext(leader, 1)
+		wasFound, next = consensus.Decider.NthNextValidator(committee.Slots, leader, 1)
 	} else {
 		wasFound, next = consensus.Decider.NthNextHmy(shard.Schedule.InstanceForEpoch(epoch), leader, 1)
 	}

consensus/quorum/quorom_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/crypto/bls"
@@ -593,3 +594,33 @@ func TestNthNextHmyExt(test *testing.T) {
 		}
 	}
 }
+
+func TestCIdentities_NthNextValidatorHmy(t *testing.T) {
+	address := []common.Address{
+		common.HexToAddress("0x1"),
+		common.HexToAddress("0x2"),
+		common.HexToAddress("0x3"),
+	}
+	slots := shard.SlotList{}
+	list := []harmony_bls.PublicKeyWrapper{}
+	for i := 0; i < 3; i++ {
+		for j := 0; j < 3; j++ {
+			blsKey := harmony_bls.RandPrivateKey()
+			wrapper := harmony_bls.PublicKeyWrapper{Object: blsKey.GetPublicKey()}
+			wrapper.Bytes.FromLibBLSPublicKey(wrapper.Object)
+			slots = append(slots, shard.Slot{
+				EcdsaAddress:   address[i%3],
+				BLSPublicKey:   wrapper.Bytes,
+				EffectiveStake: nil,
+			})
+			list = append(list, wrapper)
+		}
+	}
+
+	c := newCIdentities()
+	c.UpdateParticipants(list, []bls.PublicKeyWrapper{})
+	found, key := c.NthNextValidator(slots, &list[0], 1)
+	require.Equal(t, true, found)
+	// because we skip 3 keys of current validator
+	require.Equal(t, 3, c.IndexOf(key.Bytes))
+}

consensus/quorum/quorum.go

@@ -75,7 +75,8 @@ type ParticipantTracker interface {
 	Participants() multibls.PublicKeys
 	IndexOf(bls.SerializedPublicKey) int
 	ParticipantsCount() int64
-	NthNext(*bls.PublicKeyWrapper, int) (bool, *bls.PublicKeyWrapper)
+	// NthNextValidator returns key for next validator. It assumes external validators and leader rotation.
+	NthNextValidator(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper)
 	NthNextHmy(shardingconfig.Instance, *bls.PublicKeyWrapper, int) (bool, *bls.PublicKeyWrapper)
 	NthNextHmyExt(shardingconfig.Instance, *bls.PublicKeyWrapper, int) (bool, *bls.PublicKeyWrapper)
 	FirstParticipant(shardingconfig.Instance) *bls.PublicKeyWrapper
@@ -217,7 +218,42 @@ func (s *cIdentities) NthNext(pubKey *bls.PublicKeyWrapper, next int) (bool, *bl
 	return found, &s.publicKeys[idx]
 }
 
-// NthNextHmy return the Nth next pubkey of Harmony nodes, next can be negative number
+// NthNextValidator return the Nth next pubkey nodes, but from another validator.
+func (s *cIdentities) NthNextValidator(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) {
+	found := false
+
+	if len(s.publicKeys) == 0 {
+		return false, pubKey
+	}
+	if next < 0 {
+		return false, pubKey
+	}
+
+	publicToAddress := make(map[bls.SerializedPublicKey]common.Address)
+	for _, slot := range slotList {
+		publicToAddress[slot.BLSPublicKey] = slot.EcdsaAddress
+	}
+
+	idx := s.IndexOf(pubKey.Bytes)
+	if idx != -1 {
+		found = true
+	} else {
+		utils.Logger().Error().
+			Str("key", pubKey.Bytes.Hex()).
+			Msg("[NthNextHmy] pubKey not found")
+	}
+	for {
+		numNodes := len(s.publicKeys)
+		idx = (idx + next) % numNodes
+		if publicToAddress[s.publicKeys[idx].Bytes] == publicToAddress[pubKey.Bytes] {
+			// same validator, go next
+			idx++
+			continue
+		}
+		return found, &s.publicKeys[idx]
+	}
+}
+
 func (s *cIdentities) NthNextHmy(instance shardingconfig.Instance, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) {
 	found := false
 
@@ -410,7 +446,7 @@ func (s *cIdentities) ReadAllBallots(p Phase) []*votepower.Ballot {
 	return ballots
 }
 
-func newBallotsBackedSignatureReader() *cIdentities {
+func newCIdentities() *cIdentities {
 	return &cIdentities{
 		publicKeys:  []bls.PublicKeyWrapper{},
 		keyIndexMap: map[bls.SerializedPublicKey]int{},
@@ -420,6 +456,10 @@ func newBallotsBackedSignatureReader() *cIdentities {
 	}
 }
 
+func newBallotsBackedSignatureReader() *cIdentities {
+	return newCIdentities()
+}
+
 // NewDecider ..
 func NewDecider(p Policy, shardID uint32) Decider {
 	switch p {

consensus/view_change.go

@@ -149,7 +149,7 @@ func (consensus *Consensus) getNextViewID() (uint64, time.Duration) {
 // It reads the current leader's pubkey based on the blockchain data and returns
 // the next leader based on the gap of the viewID of the view change and the last
 // know view id of the block.
-func (consensus *Consensus) getNextLeaderKey(viewID uint64) *bls.PublicKeyWrapper {
+func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Committee) *bls.PublicKeyWrapper {
 	gap := 1
 
 	cur := consensus.getCurBlockViewID()
@@ -204,7 +204,8 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64) *bls.PublicKeyWrappe
 	var next *bls.PublicKeyWrapper
 	if blockchain != nil && blockchain.Config().IsLeaderRotation(epoch) {
 		if blockchain.Config().IsLeaderRotationExternalValidatorsAllowed(epoch, consensus.ShardID) {
-			wasFound, next = consensus.Decider.NthNext(
+			wasFound, next = consensus.Decider.NthNextValidator(
+				committee.Slots,
 				lastLeaderPubKey,
 				gap)
 		} else {
@@ -249,13 +250,24 @@ func (consensus *Consensus) startViewChange() {
 	consensus.current.SetMode(ViewChanging)
 	nextViewID, duration := consensus.getNextViewID()
 	consensus.setViewChangingID(nextViewID)
+	epoch := consensus.Blockchain().CurrentHeader().Epoch()
+	ss, err := consensus.Blockchain().ReadShardState(epoch)
+	if err != nil {
+		utils.Logger().Error().Err(err).Msg("Failed to read shard state")
+		return
+	}
+	committee, err := ss.FindCommitteeByID(consensus.ShardID)
+	if err != nil {
+		utils.Logger().Error().Err(err).Msg("Failed to find committee")
+		return
+	}
 	// TODO: set the Leader PubKey to the next leader for view change
 	// this is dangerous as the leader change is not succeeded yet
 	// we use it this way as in many code we validate the messages
 	// aganist the consensus.LeaderPubKey variable.
 	// Ideally, we shall use another variable to keep track of the
 	// leader pubkey in viewchange mode
-	consensus.LeaderPubKey = consensus.getNextLeaderKey(nextViewID)
+	consensus.LeaderPubKey = consensus.getNextLeaderKey(nextViewID, committee)
 
 	consensus.getLogger().Warn().
 		Uint64("nextViewID", nextViewID).

consensus/view_change_test.go

@@ -87,7 +87,7 @@ func TestGetNextLeaderKeyShouldFailForStandardGeneratedConsensus(t *testing.T) {
 
 	// The below results in: "panic: runtime error: integer divide by zero"
 	// This happens because there's no check for if there are any participants or not in https://github.com/harmony-one/harmony/blob/main/consensus/quorum/quorum.go#L188-L197
-	assert.Panics(t, func() { consensus.getNextLeaderKey(uint64(1)) })
+	assert.Panics(t, func() { consensus.getNextLeaderKey(uint64(1), nil) })
 }
 
 func TestGetNextLeaderKeyShouldSucceed(t *testing.T) {
@@ -115,7 +115,7 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) {
 	assert.Equal(t, keyCount, consensus.Decider.ParticipantsCount())
 
 	consensus.LeaderPubKey = &wrappedBLSKeys[0]
-	nextKey := consensus.getNextLeaderKey(uint64(1))
+	nextKey := consensus.getNextLeaderKey(uint64(1), nil)
 
 	assert.Equal(t, nextKey, &wrappedBLSKeys[1])
 }