Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next validator in view change. #4492

Merged
merged 4 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion consensus/consensus_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
31 changes: 31 additions & 0 deletions consensus/quorum/quorom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
46 changes: 43 additions & 3 deletions consensus/quorum/quorum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Frozen marked this conversation as resolved.
Show resolved Hide resolved
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
Frozen marked this conversation as resolved.
Show resolved Hide resolved
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

Expand Down Expand Up @@ -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{},
Expand All @@ -420,6 +456,10 @@ func newBallotsBackedSignatureReader() *cIdentities {
}
}

func newBallotsBackedSignatureReader() *cIdentities {
return newCIdentities()
}

// NewDecider ..
func NewDecider(p Policy, shardID uint32) Decider {
switch p {
Expand Down
18 changes: 15 additions & 3 deletions consensus/view_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions consensus/view_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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])
}