From d3a85794eea11e2e77feed28aa1a6e4748c79b6d Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:57:58 -0400 Subject: [PATCH 01/14] Support for checking if validators belongs to the same key. --- consensus/consensus_service.go | 16 +++++++++- consensus/view_change.go | 55 ++++++++++++++++++++++++++++------ consensus/view_change_test.go | 6 ++-- internal/chain/engine.go | 52 ++++++++++++++++---------------- 4 files changed, 92 insertions(+), 37 deletions(-) diff --git a/consensus/consensus_service.go b/consensus/consensus_service.go index 42e295573e..29b4c63090 100644 --- a/consensus/consensus_service.go +++ b/consensus/consensus_service.go @@ -465,7 +465,21 @@ func (consensus *Consensus) updateConsensusInformation(reason string) Mode { // a solution to take care of this case because the coinbase of the latest block doesn't really represent the // the real current leader in case of M1 view change. if !curHeader.IsLastBlockInEpoch() && curHeader.Number().Uint64() != 0 { - leaderPubKey, err := chain.GetLeaderPubKeyFromCoinbase(consensus.Blockchain(), curHeader) + ss, err := consensus.Blockchain().ReadShardState(curHeader.Epoch()) + if err != nil { + utils.Logger().Err(err).Msg("[UpdateConsensusInformation] failed to read shard state") + return Syncing + } + committee, err := ss.FindCommitteeByID(curHeader.ShardID()) + if err != nil { + utils.Logger().Err(err).Msg("[UpdateConsensusInformation] failed to find committee by ID") + return Syncing + } + leaderPubKey, err := chain.GetLeaderPubKeyFromCoinbase( + committee.Slots, + curHeader.Coinbase(), + consensus.Blockchain().Config().IsStaking(curHeader.Epoch()), + ) if err != nil || leaderPubKey == nil { consensus.getLogger().Error().Err(err). Msg("[UpdateConsensusInformation] Unable to get leaderPubKey from coinbase") diff --git a/consensus/view_change.go b/consensus/view_change.go index 6fb6def705..af2624a758 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -11,6 +11,7 @@ import ( "github.com/harmony-one/harmony/crypto/bls" "github.com/harmony-one/harmony/internal/chain" nodeconfig "github.com/harmony-one/harmony/internal/configs/node" + "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/internal/utils" "github.com/harmony-one/harmony/p2p" "github.com/harmony-one/harmony/shard" @@ -144,7 +145,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, committee *shard.Committee) *bls.PublicKeyWrapper { +func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Committee, config *params.ChainConfig, epoch *big.Int) *bls.PublicKeyWrapper { gap := 1 cur := consensus.getCurBlockViewID() @@ -154,7 +155,6 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com var lastLeaderPubKey *bls.PublicKeyWrapper var err error blockchain := consensus.Blockchain() - epoch := big.NewInt(0) if blockchain == nil { consensus.getLogger().Error().Msg("[getNextLeaderKey] Blockchain is nil. Use consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() @@ -167,13 +167,16 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com stuckBlockViewID := curHeader.ViewID().Uint64() + 1 gap = int(viewID - stuckBlockViewID) // this is the truth of the leader based on blockchain blocks - lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase(blockchain, curHeader) + lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase( + committee.Slots, + curHeader.Coinbase(), + config.IsStaking(epoch), + ) if err != nil || lastLeaderPubKey == nil { consensus.getLogger().Error().Err(err). Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() } - epoch = curHeader.Epoch() // viewchange happened at the first block of new epoch // use the LeaderPubKey as the base of the next leader // as we shouldn't use lastLeader from coinbase as the base. @@ -204,10 +207,39 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com lastLeaderPubKey, gap) } else if blockchain.Config().IsLeaderRotationExternalValidatorsAllowed(epoch) { - wasFound, next = consensus.decider.NthNextValidator( - committee.Slots, - lastLeaderPubKey, - gap) + if gap > 1 { + wasFoundCurrent, current := consensus.decider.NthNextValidator( + committee.Slots, + lastLeaderPubKey, + gap-1) + if !wasFoundCurrent { + return current + } + + publicToAddress := make(map[bls.SerializedPublicKey]common.Address) + for _, slot := range committee.Slots { + publicToAddress[slot.BLSPublicKey] = slot.EcdsaAddress + } + + for i := 0; ; i++ { + gap = gap + i + wasFound, next = consensus.decider.NthNextValidator( + committee.Slots, + lastLeaderPubKey, + gap) + if !wasFound { + return next + } + if publicToAddress[current.Bytes] != publicToAddress[next.Bytes] { + break + } + } + } else { + wasFound, next = consensus.decider.NthNextValidator( + committee.Slots, + lastLeaderPubKey, + gap) + } } else { wasFound, next = consensus.decider.NthNextHmy( shard.Schedule.InstanceForEpoch(epoch), @@ -267,7 +299,12 @@ func (consensus *Consensus) startViewChange() { // aganist the consensus.LeaderPubKey variable. // Ideally, we shall use another variable to keep track of the // leader pubkey in viewchange mode - consensus.setLeaderPubKey(consensus.getNextLeaderKey(nextViewID, committee)) + consensus.setLeaderPubKey( + consensus.getNextLeaderKey( + nextViewID, + committee, + consensus.Blockchain().Config(), + epoch)) consensus.getLogger().Warn(). Uint64("nextViewID", nextViewID). diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 42b44f7c78..4b628dc371 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -1,6 +1,7 @@ package consensus import ( + "github.com/harmony-one/harmony/internal/params" "testing" "github.com/harmony-one/harmony/crypto/bls" @@ -86,10 +87,11 @@ 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), nil) }) + assert.Panics(t, func() { consensus.getNextLeaderKey(uint64(1), nil, nil, nil) }) } func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { + t.Skip("skip because it uses blockchain instance") _, _, consensus, _, err := GenerateConsensusForTesting() assert.NoError(t, err) @@ -114,7 +116,7 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { assert.Equal(t, keyCount, consensus.Decider().ParticipantsCount()) consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - nextKey := consensus.getNextLeaderKey(uint64(1), nil) + nextKey := consensus.getNextLeaderKey(uint64(1), nil, ¶ms.ChainConfig{}, nil) assert.Equal(t, nextKey, &wrappedBLSKeys[1]) } diff --git a/internal/chain/engine.go b/internal/chain/engine.go index d5866e3887..a9c3684f43 100644 --- a/internal/chain/engine.go +++ b/internal/chain/engine.go @@ -10,7 +10,6 @@ import ( "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/numeric" - bls2 "github.com/harmony-one/bls/ffi/go/bls" blsvrf "github.com/harmony-one/harmony/crypto/vrf/bls" "github.com/ethereum/go-ethereum/common" @@ -157,7 +156,24 @@ func (e *engineImpl) VerifyVRF( return nil } - leaderPubKey, err := GetLeaderPubKeyFromCoinbase(bc, header) + ss, err := bc.ReadShardState(header.Epoch()) + if err != nil { + return errors.WithMessagef( + err, "[VerifyVRF] failed to read shard state %v", header.Epoch(), + ) + } + committee, err := ss.FindCommitteeByID(header.ShardID()) + if err != nil { + return errors.WithMessagef( + err, "[VerifyVRF] failed to find committee %d", header.ShardID(), + ) + } + + leaderPubKey, err := GetLeaderPubKeyFromCoinbase( + committee.Slots, + header.Coinbase(), + bc.Config().IsStaking(header.Epoch()), + ) if leaderPubKey == nil || err != nil { return err @@ -190,35 +206,22 @@ func (e *engineImpl) VerifyVRF( // GetLeaderPubKeyFromCoinbase retrieve corresponding blsPublicKey from Coinbase Address func GetLeaderPubKeyFromCoinbase( - blockchain engine.ChainReader, h *block.Header, + slots shard.SlotList, coinbase common.Address, isStaking bool, ) (*bls.PublicKeyWrapper, error) { - shardState, err := blockchain.ReadShardState(h.Epoch()) - if err != nil { - return nil, errors.Wrapf(err, "cannot read shard state %v %s", - h.Epoch(), - h.Coinbase().Hash().Hex(), - ) - } - - committee, err := shardState.FindCommitteeByID(h.ShardID()) - if err != nil { - return nil, err - } - - committerKey := new(bls2.PublicKey) - isStaking := blockchain.Config().IsStaking(h.Epoch()) - for _, member := range committee.Slots { + for _, member := range slots { if isStaking { // After staking the coinbase address will be the address of bls public key - if utils.GetAddressFromBLSPubKeyBytes(member.BLSPublicKey[:]) == h.Coinbase() { - if committerKey, err = bls.BytesToBLSPublicKey(member.BLSPublicKey[:]); err != nil { + if utils.GetAddressFromBLSPubKeyBytes(member.BLSPublicKey[:]) == coinbase { + committerKey, err := bls.BytesToBLSPublicKey(member.BLSPublicKey[:]) + if err != nil { return nil, err } return &bls.PublicKeyWrapper{Object: committerKey, Bytes: member.BLSPublicKey}, nil } } else { - if member.EcdsaAddress == h.Coinbase() { - if committerKey, err = bls.BytesToBLSPublicKey(member.BLSPublicKey[:]); err != nil { + if member.EcdsaAddress == coinbase { + committerKey, err := bls.BytesToBLSPublicKey(member.BLSPublicKey[:]) + if err != nil { return nil, err } return &bls.PublicKeyWrapper{Object: committerKey, Bytes: member.BLSPublicKey}, nil @@ -226,8 +229,7 @@ func GetLeaderPubKeyFromCoinbase( } } return nil, errors.Errorf( - "cannot find corresponding BLS Public Key coinbase %s", - h.Coinbase().Hex(), + "cannot find corresponding BLS Public Key coinbase %s", coinbase.Hex(), ) } From 45f0cab823bdb45284e2fedb2b8f26328da7eb44 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:12:30 -0400 Subject: [PATCH 02/14] Pass blockchain to `getNextLeaderKey`. --- consensus/view_change.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/consensus/view_change.go b/consensus/view_change.go index af2624a758..4bc773bb44 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -8,6 +8,7 @@ import ( "github.com/ethereum/go-ethereum/common" msg_pb "github.com/harmony-one/harmony/api/proto/message" "github.com/harmony-one/harmony/consensus/quorum" + "github.com/harmony-one/harmony/core" "github.com/harmony-one/harmony/crypto/bls" "github.com/harmony-one/harmony/internal/chain" nodeconfig "github.com/harmony-one/harmony/internal/configs/node" @@ -145,7 +146,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, committee *shard.Committee, config *params.ChainConfig, epoch *big.Int) *bls.PublicKeyWrapper { +func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Committee, blockchain core.BlockChain) *bls.PublicKeyWrapper { gap := 1 cur := consensus.getCurBlockViewID() @@ -154,7 +155,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com } var lastLeaderPubKey *bls.PublicKeyWrapper var err error - blockchain := consensus.Blockchain() + epoch := big.NewInt(0) if blockchain == nil { consensus.getLogger().Error().Msg("[getNextLeaderKey] Blockchain is nil. Use consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() @@ -170,7 +171,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase( committee.Slots, curHeader.Coinbase(), - config.IsStaking(epoch), + blockchain.Config().IsStaking(curHeader.Epoch()), ) if err != nil || lastLeaderPubKey == nil { consensus.getLogger().Error().Err(err). @@ -185,7 +186,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com // it can still sync with other validators. if curHeader.IsLastBlockInEpoch() { consensus.getLogger().Info().Msg("[getNextLeaderKey] view change in the first block of new epoch") - lastLeaderPubKey = consensus.decider.FirstParticipant(shard.Schedule.InstanceForEpoch(epoch)) + lastLeaderPubKey = consensus.decider.FirstParticipant(shard.Schedule.InstanceForEpoch(curHeader.Epoch())) } } } @@ -303,8 +304,7 @@ func (consensus *Consensus) startViewChange() { consensus.getNextLeaderKey( nextViewID, committee, - consensus.Blockchain().Config(), - epoch)) + consensus.Blockchain())) consensus.getLogger().Warn(). Uint64("nextViewID", nextViewID). From 4a4e4050cfb613d91fca7748771392e9af31b77a Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:45:23 -0400 Subject: [PATCH 03/14] Fixed tests. --- consensus/view_change_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 4b628dc371..21da1c8827 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -1,7 +1,6 @@ package consensus import ( - "github.com/harmony-one/harmony/internal/params" "testing" "github.com/harmony-one/harmony/crypto/bls" @@ -87,11 +86,10 @@ 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), nil, nil, nil) }) + assert.Panics(t, func() { consensus.getNextLeaderKey(uint64(1), nil, nil) }) } func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { - t.Skip("skip because it uses blockchain instance") _, _, consensus, _, err := GenerateConsensusForTesting() assert.NoError(t, err) @@ -116,7 +114,7 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { assert.Equal(t, keyCount, consensus.Decider().ParticipantsCount()) consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - nextKey := consensus.getNextLeaderKey(uint64(1), nil, ¶ms.ChainConfig{}, nil) + nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) assert.Equal(t, nextKey, &wrappedBLSKeys[1]) } From b5bc27e3abf07d12e232049178cfac82e37a8785 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Thu, 14 Nov 2024 21:37:14 -0400 Subject: [PATCH 04/14] Added tests for leader change in view change. --- consensus/quorum/quorum.go | 4 +- consensus/quorum/thread_safe_decider.go | 4 +- consensus/view_change.go | 81 ++++++++++++++----------- consensus/view_change_test.go | 66 +++++++++++++++++++- 4 files changed, 113 insertions(+), 42 deletions(-) diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 888b2c3511..6b906e73f8 100644 --- a/consensus/quorum/quorum.go +++ b/consensus/quorum/quorum.go @@ -79,7 +79,7 @@ type ParticipantTracker interface { NthNextValidator(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) NthNextValidatorV2(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) NthNextHmy(instance shardingconfig.Instance, pubkey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) - FirstParticipant(shardingconfig.Instance) *bls.PublicKeyWrapper + FirstParticipant() *bls.PublicKeyWrapper UpdateParticipants(pubKeys, allowlist []bls.PublicKeyWrapper) } @@ -314,7 +314,7 @@ func (s *cIdentities) NthNextHmy(instance shardingconfig.Instance, pubKey *bls.P } // FirstParticipant returns the first participant of the shard -func (s *cIdentities) FirstParticipant(instance shardingconfig.Instance) *bls.PublicKeyWrapper { +func (s *cIdentities) FirstParticipant() *bls.PublicKeyWrapper { return &s.publicKeys[0] } diff --git a/consensus/quorum/thread_safe_decider.go b/consensus/quorum/thread_safe_decider.go index 2cc877448a..1b6771ce23 100644 --- a/consensus/quorum/thread_safe_decider.go +++ b/consensus/quorum/thread_safe_decider.go @@ -68,10 +68,10 @@ func (a threadSafeDeciderImpl) NthNextHmy(instance shardingconfig.Instance, pubk return a.decider.NthNextHmy(instance, pubkey, next) } -func (a threadSafeDeciderImpl) FirstParticipant(instance shardingconfig.Instance) *bls.PublicKeyWrapper { +func (a threadSafeDeciderImpl) FirstParticipant() *bls.PublicKeyWrapper { a.mu.Lock() defer a.mu.Unlock() - return a.decider.FirstParticipant(instance) + return a.decider.FirstParticipant() } func (a threadSafeDeciderImpl) UpdateParticipants(pubKeys, allowlist []bls.PublicKeyWrapper) { diff --git a/consensus/view_change.go b/consensus/view_change.go index 4bc773bb44..3513893e45 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -7,8 +7,8 @@ import ( "github.com/ethereum/go-ethereum/common" msg_pb "github.com/harmony-one/harmony/api/proto/message" + "github.com/harmony-one/harmony/block" "github.com/harmony-one/harmony/consensus/quorum" - "github.com/harmony-one/harmony/core" "github.com/harmony-one/harmony/crypto/bls" "github.com/harmony-one/harmony/internal/chain" nodeconfig "github.com/harmony-one/harmony/internal/configs/node" @@ -142,11 +142,20 @@ func (consensus *Consensus) getNextViewID() (uint64, time.Duration) { return nextViewID, viewChangeDuration } +type nextLeaderParams struct { + config *params.ChainConfig + curHeader *block.Header +} + +func (a nextLeaderParams) Config() *params.ChainConfig { + return a.config +} + // getNextLeaderKey uniquely determine who is the leader for given viewID // 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, committee *shard.Committee, blockchain core.BlockChain) *bls.PublicKeyWrapper { +func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList, blockchain *nextLeaderParams) *bls.PublicKeyWrapper { gap := 1 cur := consensus.getCurBlockViewID() @@ -160,34 +169,29 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com consensus.getLogger().Error().Msg("[getNextLeaderKey] Blockchain is nil. Use consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() } else { - curHeader := blockchain.CurrentHeader() - if curHeader == nil { - consensus.getLogger().Error().Msg("[getNextLeaderKey] Failed to get current header from blockchain") + stuckBlockViewID := blockchain.curHeader.ViewID().Uint64() + 1 + gap = int(viewID - stuckBlockViewID) + // this is the truth of the leader based on blockchain blocks + lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase( + slots, + blockchain.curHeader.Coinbase(), + blockchain.Config().IsStaking(blockchain.curHeader.Epoch()), + ) + if err != nil || lastLeaderPubKey == nil { + consensus.getLogger().Error().Err(err). + Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() - } else { - stuckBlockViewID := curHeader.ViewID().Uint64() + 1 - gap = int(viewID - stuckBlockViewID) - // this is the truth of the leader based on blockchain blocks - lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase( - committee.Slots, - curHeader.Coinbase(), - blockchain.Config().IsStaking(curHeader.Epoch()), - ) - if err != nil || lastLeaderPubKey == nil { - consensus.getLogger().Error().Err(err). - Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") - lastLeaderPubKey = consensus.getLeaderPubKey() - } - // viewchange happened at the first block of new epoch - // use the LeaderPubKey as the base of the next leader - // as we shouldn't use lastLeader from coinbase as the base. - // The LeaderPubKey should be updated to the node of index 0 of the committee - // so, when validator joined the view change process later in the epoch block - // it can still sync with other validators. - if curHeader.IsLastBlockInEpoch() { - consensus.getLogger().Info().Msg("[getNextLeaderKey] view change in the first block of new epoch") - lastLeaderPubKey = consensus.decider.FirstParticipant(shard.Schedule.InstanceForEpoch(curHeader.Epoch())) - } + } + + // viewchange happened at the first block of new epoch + // use the LeaderPubKey as the base of the next leader + // as we shouldn't use lastLeader from coinbase as the base. + // The LeaderPubKey should be updated to the node of index 0 of the committee + // so, when validator joined the view change process later in the epoch block + // it can still sync with other validators. + if blockchain.curHeader.IsLastBlockInEpoch() { + consensus.getLogger().Info().Msg("[getNextLeaderKey] view change in the first block of new epoch") + lastLeaderPubKey = consensus.decider.FirstParticipant() } } consensus.getLogger().Info(). @@ -210,7 +214,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com } else if blockchain.Config().IsLeaderRotationExternalValidatorsAllowed(epoch) { if gap > 1 { wasFoundCurrent, current := consensus.decider.NthNextValidator( - committee.Slots, + slots, lastLeaderPubKey, gap-1) if !wasFoundCurrent { @@ -218,14 +222,14 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com } publicToAddress := make(map[bls.SerializedPublicKey]common.Address) - for _, slot := range committee.Slots { + for _, slot := range slots { publicToAddress[slot.BLSPublicKey] = slot.EcdsaAddress } for i := 0; ; i++ { gap = gap + i wasFound, next = consensus.decider.NthNextValidator( - committee.Slots, + slots, lastLeaderPubKey, gap) if !wasFound { @@ -237,7 +241,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com } } else { wasFound, next = consensus.decider.NthNextValidator( - committee.Slots, + slots, lastLeaderPubKey, gap) } @@ -283,7 +287,8 @@ func (consensus *Consensus) startViewChange() { consensus.current.SetMode(ViewChanging) nextViewID, duration := consensus.getNextViewID() consensus.setViewChangingID(nextViewID) - epoch := consensus.Blockchain().CurrentHeader().Epoch() + currentHeader := consensus.Blockchain().CurrentHeader() + epoch := currentHeader.Epoch() ss, err := consensus.Blockchain().ReadShardState(epoch) if err != nil { utils.Logger().Error().Err(err).Msg("Failed to read shard state") @@ -300,11 +305,15 @@ func (consensus *Consensus) startViewChange() { // aganist the consensus.LeaderPubKey variable. // Ideally, we shall use another variable to keep track of the // leader pubkey in viewchange mode + blockchain := &nextLeaderParams{ + config: consensus.Blockchain().Config(), + curHeader: currentHeader, + } consensus.setLeaderPubKey( consensus.getNextLeaderKey( nextViewID, - committee, - consensus.Blockchain())) + committee.Slots, + blockchain)) consensus.getLogger().Warn(). Uint64("nextViewID", nextViewID). diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 21da1c8827..8ffe3af175 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -1,13 +1,18 @@ package consensus import ( + "math/big" "testing" - "github.com/harmony-one/harmony/crypto/bls" - + "github.com/ethereum/go-ethereum/common" bls_core "github.com/harmony-one/bls/ffi/go/bls" + blockfactory "github.com/harmony-one/harmony/block/factory" + "github.com/harmony-one/harmony/crypto/bls" harmony_bls "github.com/harmony-one/harmony/crypto/bls" + "github.com/harmony-one/harmony/internal/params" + "github.com/harmony-one/harmony/shard" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBasicViewChanging(t *testing.T) { @@ -118,3 +123,60 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { assert.Equal(t, nextKey, &wrappedBLSKeys[1]) } + +func TestGetNextLeader(t *testing.T) { + _, _, consensus, _, err := GenerateConsensusForTesting() + assert.NoError(t, err) + + assert.Equal(t, int64(0), consensus.Decider().ParticipantsCount()) + + blsKeys := []*bls_core.PublicKey{} + wrappedBLSKeys := []bls.PublicKeyWrapper{} + + const keyCount = 5 + for i := 0; i < keyCount; i++ { + blsKey := harmony_bls.RandPrivateKey() + blsPubKey := blsKey.GetPublicKey() + bytes := bls.SerializedPublicKey{} + bytes.FromLibBLSPublicKey(blsPubKey) + wrapped := bls.PublicKeyWrapper{Object: blsPubKey, Bytes: bytes} + + blsKeys = append(blsKeys, blsPubKey) + wrappedBLSKeys = append(wrappedBLSKeys, wrapped) + } + + consensus.Decider().UpdateParticipants(wrappedBLSKeys, []bls.PublicKeyWrapper{}) + assert.EqualValues(t, keyCount, consensus.Decider().ParticipantsCount()) + + consensus.setLeaderPubKey(&wrappedBLSKeys[0]) + nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) + + assert.Equal(t, nextKey, &wrappedBLSKeys[1]) + + t.Run("check_same_address_for_validators", func(t *testing.T) { + config := ¶ms.ChainConfig{ + LeaderRotationExternalValidatorsEpoch: big.NewInt(1), + LeaderRotationInternalValidatorsEpoch: big.NewInt(1), + StakingEpoch: big.NewInt(1), + } + facroty := blockfactory.NewFactory(config) + header := facroty.NewHeader(big.NewInt(2)) + header.SetCoinbase(common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3"))) + header.SetViewID(big.NewInt(1)) + header.SetNumber(big.NewInt(1)) + // Slot represents node id (BLS address) + slots := []shard.Slot{} + for i := 0; i < keyCount; i++ { + slot := shard.Slot{ + EcdsaAddress: common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")), + BLSPublicKey: wrappedBLSKeys[i].Bytes, + } + slots = append(slots, slot) + } + nextKey := consensus.getNextLeaderKey(uint64(2), slots, &nextLeaderParams{ + config: config, + curHeader: header, + }) + require.Equal(t, wrappedBLSKeys[0].Hex(), nextKey.Hex()) + }) +} From a2a438fbf6d1d740f2a0a4fedf483a67cef08b08 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:20:31 -0400 Subject: [PATCH 05/14] Extracted method and tests coverage. --- consensus/consensus_service.go | 5 -- consensus/quorum/quorum.go | 3 +- consensus/view_change.go | 90 ++++++++++++++++++++-------------- consensus/view_change_test.go | 88 +++++++++++++++++++++++++++++++-- 4 files changed, 139 insertions(+), 47 deletions(-) diff --git a/consensus/consensus_service.go b/consensus/consensus_service.go index 29b4c63090..298d780cfa 100644 --- a/consensus/consensus_service.go +++ b/consensus/consensus_service.go @@ -570,11 +570,6 @@ func (consensus *Consensus) SetCurBlockViewID(viewID uint64) uint64 { return consensus.setCurBlockViewID(viewID) } -// SetCurBlockViewID set the current view ID -func (consensus *Consensus) setCurBlockViewID(viewID uint64) uint64 { - return consensus.current.SetCurBlockViewID(viewID) -} - // SetViewChangingID set the current view change ID func (consensus *Consensus) SetViewChangingID(viewID uint64) { consensus.current.SetViewChangingID(viewID) diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 6b906e73f8..82d075a986 100644 --- a/consensus/quorum/quorum.go +++ b/consensus/quorum/quorum.go @@ -281,7 +281,7 @@ func (s *cIdentities) NthNextValidator(slotList shard.SlotList, pubKey *bls.Publ Str("key", pubKey.Bytes.Hex()). Msg("[NthNextHmy] pubKey not found") } - for { + for i := 0; i < len(slotList); i++ { numNodes := len(s.publicKeys) idx = (idx + next) % numNodes if publicToAddress[s.publicKeys[idx].Bytes] == publicToAddress[pubKey.Bytes] { @@ -291,6 +291,7 @@ func (s *cIdentities) NthNextValidator(slotList shard.SlotList, pubKey *bls.Publ } return found, &s.publicKeys[idx] } + return false, pubKey } func (s *cIdentities) NthNextHmy(instance shardingconfig.Instance, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) { diff --git a/consensus/view_change.go b/consensus/view_change.go index 3513893e45..bc90d51553 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -1,7 +1,6 @@ package consensus import ( - "math/big" "sync/atomic" "time" @@ -63,6 +62,11 @@ func (pm *State) SetCurBlockViewID(viewID uint64) uint64 { return viewID } +// SetCurBlockViewID set the current view ID +func (consensus *Consensus) setCurBlockViewID(viewID uint64) uint64 { + return consensus.current.SetCurBlockViewID(viewID) +} + // GetViewChangingID return the current view changing id // It is meaningful during view change mode func (pm *State) GetViewChangingID() uint64 { @@ -164,7 +168,6 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList } var lastLeaderPubKey *bls.PublicKeyWrapper var err error - epoch := big.NewInt(0) if blockchain == nil { consensus.getLogger().Error().Msg("[getNextLeaderKey] Blockchain is nil. Use consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() @@ -205,45 +208,19 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList // FIXME: rotate leader on harmony nodes only before fully externalization var wasFound bool var next *bls.PublicKeyWrapper - if blockchain != nil && blockchain.Config().IsLeaderRotationInternalValidators(epoch) { + + if blockchain != nil && blockchain.Config().IsLeaderRotationInternalValidators(blockchain.curHeader.Epoch()) { + epoch := blockchain.curHeader.Epoch() + if blockchain.Config().IsLeaderRotationV2Epoch(epoch) { wasFound, next = consensus.decider.NthNextValidatorV2( committee.Slots, lastLeaderPubKey, gap) } else if blockchain.Config().IsLeaderRotationExternalValidatorsAllowed(epoch) { - if gap > 1 { - wasFoundCurrent, current := consensus.decider.NthNextValidator( - slots, - lastLeaderPubKey, - gap-1) - if !wasFoundCurrent { - return current - } - - publicToAddress := make(map[bls.SerializedPublicKey]common.Address) - for _, slot := range slots { - publicToAddress[slot.BLSPublicKey] = slot.EcdsaAddress - } - - for i := 0; ; i++ { - gap = gap + i - wasFound, next = consensus.decider.NthNextValidator( - slots, - lastLeaderPubKey, - gap) - if !wasFound { - return next - } - if publicToAddress[current.Bytes] != publicToAddress[next.Bytes] { - break - } - } - } else { - wasFound, next = consensus.decider.NthNextValidator( - slots, - lastLeaderPubKey, - gap) + next, ok := viewChangeNextValidator(consensus.decider, gap, slots, lastLeaderPubKey) + if ok { + return next } } else { wasFound, next = consensus.decider.NthNextHmy( @@ -253,7 +230,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList } } else { wasFound, next = consensus.decider.NthNextHmy( - shard.Schedule.InstanceForEpoch(epoch), + shard.Schedule.InstanceForEpoch(blockchain.curHeader.Epoch()), lastLeaderPubKey, gap) } @@ -268,6 +245,47 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList return next } +func viewChangeNextValidator(decider quorum.Decider, gap int, slots shard.SlotList, lastLeaderPubKey *bls.PublicKeyWrapper) (*bls.PublicKeyWrapper, bool) { + var wasFound bool + var next *bls.PublicKeyWrapper + if gap > 1 { + wasFoundCurrent, current := decider.NthNextValidator( + slots, + lastLeaderPubKey, + gap-1) + if !wasFoundCurrent { + return nil, false + } + + publicToAddress := make(map[bls.SerializedPublicKey]common.Address) + for _, slot := range slots { + publicToAddress[slot.BLSPublicKey] = slot.EcdsaAddress + } + + for i := 0; i < len(slots); i++ { + gap = gap + i + wasFound, next = decider.NthNextValidator( + slots, + lastLeaderPubKey, + gap) + if !wasFound { + return nil, false + } + + if publicToAddress[current.Bytes] != publicToAddress[next.Bytes] { + return next, true + } + } + } else { + wasFound, next = decider.NthNextValidator( + slots, + lastLeaderPubKey, + gap) + return next, wasFound + } + return nil, false +} + func createTimeout() map[TimeoutType]*utils.Timeout { timeouts := make(map[TimeoutType]*utils.Timeout) timeouts[timeoutConsensus] = utils.NewTimeout(phaseDuration) diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 8ffe3af175..b548a06ad2 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -7,6 +7,7 @@ import ( "github.com/ethereum/go-ethereum/common" bls_core "github.com/harmony-one/bls/ffi/go/bls" blockfactory "github.com/harmony-one/harmony/block/factory" + "github.com/harmony-one/harmony/consensus/quorum" "github.com/harmony-one/harmony/crypto/bls" harmony_bls "github.com/harmony-one/harmony/crypto/bls" "github.com/harmony-one/harmony/internal/params" @@ -119,9 +120,9 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { assert.Equal(t, keyCount, consensus.Decider().ParticipantsCount()) consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) + //nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) - assert.Equal(t, nextKey, &wrappedBLSKeys[1]) + //assert.Equal(t, nextKey, &wrappedBLSKeys[1]) } func TestGetNextLeader(t *testing.T) { @@ -149,16 +150,18 @@ func TestGetNextLeader(t *testing.T) { assert.EqualValues(t, keyCount, consensus.Decider().ParticipantsCount()) consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) + //nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) - assert.Equal(t, nextKey, &wrappedBLSKeys[1]) + //assert.Equal(t, nextKey, &wrappedBLSKeys[1]) t.Run("check_same_address_for_validators", func(t *testing.T) { + consensus.setCurBlockViewID(2) config := ¶ms.ChainConfig{ LeaderRotationExternalValidatorsEpoch: big.NewInt(1), LeaderRotationInternalValidatorsEpoch: big.NewInt(1), StakingEpoch: big.NewInt(1), } + facroty := blockfactory.NewFactory(config) header := facroty.NewHeader(big.NewInt(2)) header.SetCoinbase(common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3"))) @@ -173,10 +176,85 @@ func TestGetNextLeader(t *testing.T) { } slots = append(slots, slot) } - nextKey := consensus.getNextLeaderKey(uint64(2), slots, &nextLeaderParams{ + nextKey := consensus.getNextLeaderKey(uint64(4), slots, &nextLeaderParams{ config: config, curHeader: header, }) require.Equal(t, wrappedBLSKeys[0].Hex(), nextKey.Hex()) }) } + +func TestViewChangeNextValidator(t *testing.T) { + decider := quorum.NewDecider(quorum.SuperMajorityVote, shard.BeaconChainShardID) + assert.Equal(t, int64(0), decider.ParticipantsCount()) + wrappedBLSKeys := []bls.PublicKeyWrapper{} + + const keyCount = 5 + for i := 0; i < keyCount; i++ { + blsKey := harmony_bls.RandPrivateKey() + blsPubKey := harmony_bls.WrapperFromPrivateKey(blsKey) + wrappedBLSKeys = append(wrappedBLSKeys, *blsPubKey.Pub) + } + + decider.UpdateParticipants(wrappedBLSKeys, []bls.PublicKeyWrapper{}) + assert.EqualValues(t, keyCount, decider.ParticipantsCount()) + + t.Run("check_different_address_for_validators_with_gap_0", func(t *testing.T) { + slots := []shard.Slot{} + for i := 0; i < keyCount; i++ { + slot := shard.Slot{ + EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), + BLSPublicKey: wrappedBLSKeys[i].Bytes, + } + slots = append(slots, slot) + } + + rs, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.True(t, ok) + require.Equal(t, &wrappedBLSKeys[1], rs) + }) + t.Run("check_different_address_for_validators_with_gap_1", func(t *testing.T) { + slots := []shard.Slot{} + for i := 0; i < keyCount; i++ { + slot := shard.Slot{ + EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), + BLSPublicKey: wrappedBLSKeys[i].Bytes, + } + slots = append(slots, slot) + } + + rs, ok := viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) + require.True(t, ok) + require.Equal(t, &wrappedBLSKeys[1], rs) + }) + t.Run("check_different_address_for_validators_with_gap_2", func(t *testing.T) { + slots := []shard.Slot{} + for i := 0; i < keyCount; i++ { + slot := shard.Slot{ + EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), + BLSPublicKey: wrappedBLSKeys[i].Bytes, + } + slots = append(slots, slot) + } + + rs, ok := viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) + require.True(t, ok) + require.Equal(t, &wrappedBLSKeys[2], rs) + }) + + // we can't find next validator, because all validators have the same address + t.Run("check_same_address_for_validators", func(t *testing.T) { + // Slot represents node id (BLS address) + slots := []shard.Slot{} + for i := 0; i < keyCount; i++ { + slot := shard.Slot{ + EcdsaAddress: common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")), + BLSPublicKey: wrappedBLSKeys[i].Bytes, + } + slots = append(slots, slot) + } + + _, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.False(t, ok) + }) +} From c9aa1780a713fc20e9e304e53e40fa4e5a5bd4eb Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:14:03 -0400 Subject: [PATCH 06/14] I've split getNextLeaderKey to enw and old versions. --- consensus/view_change.go | 97 ++++++++++++++++++++++----------------- internal/params/config.go | 8 ++++ 2 files changed, 64 insertions(+), 41 deletions(-) diff --git a/consensus/view_change.go b/consensus/view_change.go index bc90d51553..844b724f02 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -1,6 +1,7 @@ package consensus import ( + "math/big" "sync/atomic" "time" @@ -159,7 +160,23 @@ func (a nextLeaderParams) Config() *params.ChainConfig { // 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, slots shard.SlotList, blockchain *nextLeaderParams) *bls.PublicKeyWrapper { +func (consensus *Consensus) getNextLeaderKeySkipSameAddress(viewID uint64, committee *shard.Committee) *bls.PublicKeyWrapper { + gap := 1 + + cur := consensus.getCurBlockViewID() + if viewID > cur { + gap = int(viewID - cur) + } + // use pubkey as default key as well + leaderPubKey := consensus.getLeaderPubKey() + rs, ok := viewChangeNextValidator(consensus.decider, gap, committee.Slots, leaderPubKey) + if !ok { + return leaderPubKey + } + return rs +} + +func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Committee) *bls.PublicKeyWrapper { gap := 1 cur := consensus.getCurBlockViewID() @@ -168,33 +185,37 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList } var lastLeaderPubKey *bls.PublicKeyWrapper var err error + blockchain := consensus.Blockchain() + epoch := big.NewInt(0) if blockchain == nil { consensus.getLogger().Error().Msg("[getNextLeaderKey] Blockchain is nil. Use consensus.LeaderPubKey") lastLeaderPubKey = consensus.getLeaderPubKey() } else { - stuckBlockViewID := blockchain.curHeader.ViewID().Uint64() + 1 - gap = int(viewID - stuckBlockViewID) - // this is the truth of the leader based on blockchain blocks - lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase( - slots, - blockchain.curHeader.Coinbase(), - blockchain.Config().IsStaking(blockchain.curHeader.Epoch()), - ) - if err != nil || lastLeaderPubKey == nil { - consensus.getLogger().Error().Err(err). - Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") + curHeader := blockchain.CurrentHeader() + if curHeader == nil { + consensus.getLogger().Error().Msg("[getNextLeaderKey] Failed to get current header from blockchain") lastLeaderPubKey = consensus.getLeaderPubKey() - } - - // viewchange happened at the first block of new epoch - // use the LeaderPubKey as the base of the next leader - // as we shouldn't use lastLeader from coinbase as the base. - // The LeaderPubKey should be updated to the node of index 0 of the committee - // so, when validator joined the view change process later in the epoch block - // it can still sync with other validators. - if blockchain.curHeader.IsLastBlockInEpoch() { - consensus.getLogger().Info().Msg("[getNextLeaderKey] view change in the first block of new epoch") - lastLeaderPubKey = consensus.decider.FirstParticipant() + } else { + stuckBlockViewID := curHeader.ViewID().Uint64() + 1 + gap = int(viewID - stuckBlockViewID) + // this is the truth of the leader based on blockchain blocks + lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase(committee.Slots, curHeader.Coinbase(), blockchain.Config().IsStaking(curHeader.Epoch())) + if err != nil || lastLeaderPubKey == nil { + consensus.getLogger().Error().Err(err). + Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") + lastLeaderPubKey = consensus.getLeaderPubKey() + } + epoch = curHeader.Epoch() + // viewchange happened at the first block of new epoch + // use the LeaderPubKey as the base of the next leader + // as we shouldn't use lastLeader from coinbase as the base. + // The LeaderPubKey should be updated to the node of index 0 of the committee + // so, when validator joined the view change process later in the epoch block + // it can still sync with other validators. + if curHeader.IsLastBlockInEpoch() { + consensus.getLogger().Info().Msg("[getNextLeaderKey] view change in the first block of new epoch") + lastLeaderPubKey = consensus.decider.FirstParticipant() + } } } consensus.getLogger().Info(). @@ -208,20 +229,17 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList // FIXME: rotate leader on harmony nodes only before fully externalization var wasFound bool var next *bls.PublicKeyWrapper - - if blockchain != nil && blockchain.Config().IsLeaderRotationInternalValidators(blockchain.curHeader.Epoch()) { - epoch := blockchain.curHeader.Epoch() - + if blockchain != nil && blockchain.Config().IsLeaderRotationInternalValidators(epoch) { if blockchain.Config().IsLeaderRotationV2Epoch(epoch) { wasFound, next = consensus.decider.NthNextValidatorV2( committee.Slots, lastLeaderPubKey, gap) } else if blockchain.Config().IsLeaderRotationExternalValidatorsAllowed(epoch) { - next, ok := viewChangeNextValidator(consensus.decider, gap, slots, lastLeaderPubKey) - if ok { - return next - } + wasFound, next = consensus.decider.NthNextValidator( + committee.Slots, + lastLeaderPubKey, + gap) } else { wasFound, next = consensus.decider.NthNextHmy( shard.Schedule.InstanceForEpoch(epoch), @@ -230,7 +248,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, slots shard.SlotList } } else { wasFound, next = consensus.decider.NthNextHmy( - shard.Schedule.InstanceForEpoch(blockchain.curHeader.Epoch()), + shard.Schedule.InstanceForEpoch(epoch), lastLeaderPubKey, gap) } @@ -323,15 +341,12 @@ func (consensus *Consensus) startViewChange() { // aganist the consensus.LeaderPubKey variable. // Ideally, we shall use another variable to keep track of the // leader pubkey in viewchange mode - blockchain := &nextLeaderParams{ - config: consensus.Blockchain().Config(), - curHeader: currentHeader, - } - consensus.setLeaderPubKey( - consensus.getNextLeaderKey( - nextViewID, - committee.Slots, - blockchain)) + c := consensus.Blockchain().Config() + if c.IsViewChangeSkipValidatorsSameAddressEpoch(currentHeader.Epoch()) { + consensus.setLeaderPubKey(consensus.getNextLeaderKeySkipSameAddress(nextViewID, committee)) + } else { + consensus.setLeaderPubKey(consensus.getNextLeaderKey(nextViewID, committee)) + } consensus.getLogger().Warn(). Uint64("nextViewID", nextViewID). diff --git a/internal/params/config.go b/internal/params/config.go index 3e774c113f..a6baa2680a 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -374,6 +374,7 @@ var ( big.NewInt(0), big.NewInt(0), big.NewInt(0), + big.NewInt(0), } // TestChainConfig ... @@ -425,6 +426,7 @@ var ( big.NewInt(0), // MaxRateEpoch big.NewInt(0), big.NewInt(0), + big.NewInt(0), } // TestRules ... @@ -606,6 +608,8 @@ type ChainConfig struct { // vote power feature https://github.com/harmony-one/harmony/pull/4683 // if crosslink are not sent for an entire epoch signed and toSign will be 0 and 0. when that happen, next epoch there will no shard 1 validator elected in the committee. HIP32Epoch *big.Int `json:"hip32-epoch,omitempty"` + + ViewChangeSkipValidatorsSameAddressEpoch *big.Int `json:"view-change-skip-validators-same-address-epoch,omitempty"` } // String implements the fmt.Stringer interface. @@ -889,6 +893,10 @@ func (c *ChainConfig) IsTopMaxRate(epoch *big.Int) bool { return isForked(c.TopMaxRateEpoch, epoch) } +func (c *ChainConfig) IsViewChangeSkipValidatorsSameAddressEpoch(epoch *big.Int) bool { + return isForked(c.ViewChangeSkipValidatorsSameAddressEpoch, epoch) +} + // During this epoch, shards 2 and 3 will start sending // their balances over to shard 0 or 1. func (c *ChainConfig) IsOneEpochBeforeHIP30(epoch *big.Int) bool { From 5f07ec7a32e05b09f30bf5f1250331763e7c13a5 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:19:04 -0400 Subject: [PATCH 07/14] Cleanup. --- consensus/view_change_test.go | 67 ++--------------------------------- 1 file changed, 3 insertions(+), 64 deletions(-) diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index b548a06ad2..0e921244ef 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -6,11 +6,9 @@ import ( "github.com/ethereum/go-ethereum/common" bls_core "github.com/harmony-one/bls/ffi/go/bls" - blockfactory "github.com/harmony-one/harmony/block/factory" "github.com/harmony-one/harmony/consensus/quorum" "github.com/harmony-one/harmony/crypto/bls" harmony_bls "github.com/harmony-one/harmony/crypto/bls" - "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/shard" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -92,7 +90,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), nil, nil) }) + assert.Panics(t, func() { consensus.getNextLeaderKey(uint64(1), nil) }) } func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { @@ -120,68 +118,9 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) { assert.Equal(t, keyCount, consensus.Decider().ParticipantsCount()) consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - //nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) + nextKey := consensus.getNextLeaderKey(uint64(1), nil) - //assert.Equal(t, nextKey, &wrappedBLSKeys[1]) -} - -func TestGetNextLeader(t *testing.T) { - _, _, consensus, _, err := GenerateConsensusForTesting() - assert.NoError(t, err) - - assert.Equal(t, int64(0), consensus.Decider().ParticipantsCount()) - - blsKeys := []*bls_core.PublicKey{} - wrappedBLSKeys := []bls.PublicKeyWrapper{} - - const keyCount = 5 - for i := 0; i < keyCount; i++ { - blsKey := harmony_bls.RandPrivateKey() - blsPubKey := blsKey.GetPublicKey() - bytes := bls.SerializedPublicKey{} - bytes.FromLibBLSPublicKey(blsPubKey) - wrapped := bls.PublicKeyWrapper{Object: blsPubKey, Bytes: bytes} - - blsKeys = append(blsKeys, blsPubKey) - wrappedBLSKeys = append(wrappedBLSKeys, wrapped) - } - - consensus.Decider().UpdateParticipants(wrappedBLSKeys, []bls.PublicKeyWrapper{}) - assert.EqualValues(t, keyCount, consensus.Decider().ParticipantsCount()) - - consensus.setLeaderPubKey(&wrappedBLSKeys[0]) - //nextKey := consensus.getNextLeaderKey(uint64(1), nil, nil) - - //assert.Equal(t, nextKey, &wrappedBLSKeys[1]) - - t.Run("check_same_address_for_validators", func(t *testing.T) { - consensus.setCurBlockViewID(2) - config := ¶ms.ChainConfig{ - LeaderRotationExternalValidatorsEpoch: big.NewInt(1), - LeaderRotationInternalValidatorsEpoch: big.NewInt(1), - StakingEpoch: big.NewInt(1), - } - - facroty := blockfactory.NewFactory(config) - header := facroty.NewHeader(big.NewInt(2)) - header.SetCoinbase(common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3"))) - header.SetViewID(big.NewInt(1)) - header.SetNumber(big.NewInt(1)) - // Slot represents node id (BLS address) - slots := []shard.Slot{} - for i := 0; i < keyCount; i++ { - slot := shard.Slot{ - EcdsaAddress: common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")), - BLSPublicKey: wrappedBLSKeys[i].Bytes, - } - slots = append(slots, slot) - } - nextKey := consensus.getNextLeaderKey(uint64(4), slots, &nextLeaderParams{ - config: config, - curHeader: header, - }) - require.Equal(t, wrappedBLSKeys[0].Hex(), nextKey.Hex()) - }) + assert.Equal(t, nextKey, &wrappedBLSKeys[1]) } func TestViewChangeNextValidator(t *testing.T) { From 3e053797406eedd6b2b4442cd7b4870a78a26280 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:51:58 -0400 Subject: [PATCH 08/14] Add tests for different gaps. --- consensus/view_change_test.go | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 0e921244ef..c943e5938c 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -196,4 +196,47 @@ func TestViewChangeNextValidator(t *testing.T) { _, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) require.False(t, ok) }) + + // we can't find next validator, because all validators have the same address + t.Run("check_5_validators_2_addrs", func(t *testing.T) { + // Slot represents node id (BLS address) + var ( + addr1 = common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")) + addr2 = common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z4")) + ) + slots := []shard.Slot{ + { + EcdsaAddress: addr1, + BLSPublicKey: wrappedBLSKeys[0].Bytes, + }, + { + EcdsaAddress: addr1, + BLSPublicKey: wrappedBLSKeys[1].Bytes, + }, + { + EcdsaAddress: addr2, + BLSPublicKey: wrappedBLSKeys[2].Bytes, + }, + { + EcdsaAddress: addr2, + BLSPublicKey: wrappedBLSKeys[3].Bytes, + }, + { + EcdsaAddress: addr2, + BLSPublicKey: wrappedBLSKeys[4].Bytes, + }, + } + rs, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.True(t, ok) + require.Equal(t, &wrappedBLSKeys[2], rs) + + rs, ok = viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) + require.True(t, ok) + require.Equal(t, &wrappedBLSKeys[3], rs) + + // TODO + //rs, ok = viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) + //require.True(t, ok) + //require.Equal(t, &wrappedBLSKeys[0], rs) + }) } From fffd55b8076da502ef6c80426775adc9bb234048 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Tue, 19 Nov 2024 18:20:38 -0400 Subject: [PATCH 09/14] Added tests and changed function to NthNext. --- consensus/quorum/quorum.go | 11 ++- consensus/quorum/thread_safe_decider.go | 6 ++ consensus/view_change.go | 36 ++++----- consensus/view_change_test.go | 102 ++++++++++++++---------- 4 files changed, 87 insertions(+), 68 deletions(-) diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 82d075a986..6f12a26687 100644 --- a/consensus/quorum/quorum.go +++ b/consensus/quorum/quorum.go @@ -79,6 +79,7 @@ type ParticipantTracker interface { NthNextValidator(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) NthNextValidatorV2(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) NthNextHmy(instance shardingconfig.Instance, pubkey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) + NthNext(pubKey *bls.PublicKeyWrapper, next int) (*bls.PublicKeyWrapper, error) FirstParticipant() *bls.PublicKeyWrapper UpdateParticipants(pubKeys, allowlist []bls.PublicKeyWrapper) } @@ -202,12 +203,10 @@ func (s *cIdentities) IndexOf(pubKey bls.SerializedPublicKey) int { } // NthNext return the Nth next pubkey, next can be negative number -func (s *cIdentities) NthNext(pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) { - found := false - +func (s *cIdentities) NthNext(pubKey *bls.PublicKeyWrapper, next int) (*bls.PublicKeyWrapper, error) { idx := s.IndexOf(pubKey.Bytes) - if idx != -1 { - found = true + if idx == -1 { + return nil, errors.Errorf("pubKey not found %x", pubKey.Bytes) } numNodes := int(s.ParticipantsCount()) // sanity check to avoid out of bound access @@ -215,7 +214,7 @@ func (s *cIdentities) NthNext(pubKey *bls.PublicKeyWrapper, next int) (bool, *bl numNodes = len(s.publicKeys) } idx = (idx + next) % numNodes - return found, &s.publicKeys[idx] + return &s.publicKeys[idx], nil } // NthNextValidatorV2 returns the Nth next pubkey nodes, but from another validator. diff --git a/consensus/quorum/thread_safe_decider.go b/consensus/quorum/thread_safe_decider.go index 1b6771ce23..ca69232787 100644 --- a/consensus/quorum/thread_safe_decider.go +++ b/consensus/quorum/thread_safe_decider.go @@ -56,6 +56,12 @@ func (a threadSafeDeciderImpl) NthNextValidator(slotList shard.SlotList, pubKey return a.decider.NthNextValidator(slotList, pubKey, next) } +func (a threadSafeDeciderImpl) NthNext(pubKey *bls.PublicKeyWrapper, next int) (*bls.PublicKeyWrapper, error) { + a.mu.Lock() + defer a.mu.Unlock() + return a.decider.NthNext(pubKey, next) +} + func (a threadSafeDeciderImpl) NthNextValidatorV2(slotList shard.SlotList, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) { a.mu.Lock() defer a.mu.Unlock() diff --git a/consensus/view_change.go b/consensus/view_change.go index 844b724f02..8227bbbf0b 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -169,8 +169,9 @@ func (consensus *Consensus) getNextLeaderKeySkipSameAddress(viewID uint64, commi } // use pubkey as default key as well leaderPubKey := consensus.getLeaderPubKey() - rs, ok := viewChangeNextValidator(consensus.decider, gap, committee.Slots, leaderPubKey) - if !ok { + rs, err := viewChangeNextValidator(consensus.decider, gap, committee.Slots, leaderPubKey) + if err != nil { + consensus.getLogger().Error().Err(err).Msg("[getNextLeaderKeySkipSameAddress] viewChangeNextValidator failed") return leaderPubKey } return rs @@ -263,16 +264,17 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com return next } -func viewChangeNextValidator(decider quorum.Decider, gap int, slots shard.SlotList, lastLeaderPubKey *bls.PublicKeyWrapper) (*bls.PublicKeyWrapper, bool) { - var wasFound bool - var next *bls.PublicKeyWrapper +type nthNext interface { + NthNext(pubKey *bls.PublicKeyWrapper, next int) (*bls.PublicKeyWrapper, error) +} + +func viewChangeNextValidator(decider nthNext, gap int, slots shard.SlotList, lastLeaderPubKey *bls.PublicKeyWrapper) (*bls.PublicKeyWrapper, error) { if gap > 1 { - wasFoundCurrent, current := decider.NthNextValidator( - slots, + current, err := decider.NthNext( lastLeaderPubKey, gap-1) - if !wasFoundCurrent { - return nil, false + if err != nil { + return nil, errors.WithMessagef(err, "NthNext failed, gap %d", gap) } publicToAddress := make(map[bls.SerializedPublicKey]common.Address) @@ -282,26 +284,24 @@ func viewChangeNextValidator(decider quorum.Decider, gap int, slots shard.SlotLi for i := 0; i < len(slots); i++ { gap = gap + i - wasFound, next = decider.NthNextValidator( - slots, + next, err := decider.NthNext( lastLeaderPubKey, gap) - if !wasFound { - return nil, false + if err != nil { + return nil, errors.New("current leader not found") } if publicToAddress[current.Bytes] != publicToAddress[next.Bytes] { - return next, true + return next, nil } } } else { - wasFound, next = decider.NthNextValidator( - slots, + next, err := decider.NthNext( lastLeaderPubKey, gap) - return next, wasFound + return next, errors.WithMessagef(err, "NthNext failed, gap %d", gap) } - return nil, false + return nil, errors.New("current leader not found") } func createTimeout() map[TimeoutType]*utils.Timeout { diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index c943e5938c..ee599ca3da 100644 --- a/consensus/view_change_test.go +++ b/consensus/view_change_test.go @@ -138,8 +138,12 @@ func TestViewChangeNextValidator(t *testing.T) { decider.UpdateParticipants(wrappedBLSKeys, []bls.PublicKeyWrapper{}) assert.EqualValues(t, keyCount, decider.ParticipantsCount()) - t.Run("check_different_address_for_validators_with_gap_0", func(t *testing.T) { - slots := []shard.Slot{} + t.Run("check_different_address_for_validators", func(t *testing.T) { + var ( + rs *bls.PublicKeyWrapper + err error + slots []shard.Slot + ) for i := 0; i < keyCount; i++ { slot := shard.Slot{ EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), @@ -148,43 +152,32 @@ func TestViewChangeNextValidator(t *testing.T) { slots = append(slots, slot) } - rs, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) - require.True(t, ok) - require.Equal(t, &wrappedBLSKeys[1], rs) - }) - t.Run("check_different_address_for_validators_with_gap_1", func(t *testing.T) { - slots := []shard.Slot{} - for i := 0; i < keyCount; i++ { - slot := shard.Slot{ - EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), - BLSPublicKey: wrappedBLSKeys[i].Bytes, - } - slots = append(slots, slot) - } + rs, err = viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[0], rs) - rs, ok := viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) - require.True(t, ok) + rs, err = viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) require.Equal(t, &wrappedBLSKeys[1], rs) - }) - t.Run("check_different_address_for_validators_with_gap_2", func(t *testing.T) { - slots := []shard.Slot{} - for i := 0; i < keyCount; i++ { - slot := shard.Slot{ - EcdsaAddress: common.BigToAddress(big.NewInt(int64(i))), - BLSPublicKey: wrappedBLSKeys[i].Bytes, - } - slots = append(slots, slot) - } - rs, ok := viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) - require.True(t, ok) + rs, err = viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) require.Equal(t, &wrappedBLSKeys[2], rs) + + // and no panic or error for future 1k gaps + for i := 0; i < 1000; i++ { + _, err = viewChangeNextValidator(decider, i, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + } }) // we can't find next validator, because all validators have the same address - t.Run("check_same_address_for_validators", func(t *testing.T) { - // Slot represents node id (BLS address) - slots := []shard.Slot{} + t.Run("same_address_for_all_validators", func(t *testing.T) { + var ( + rs *bls.PublicKeyWrapper + err error + slots []shard.Slot + ) for i := 0; i < keyCount; i++ { slot := shard.Slot{ EcdsaAddress: common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")), @@ -193,8 +186,23 @@ func TestViewChangeNextValidator(t *testing.T) { slots = append(slots, slot) } - _, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) - require.False(t, ok) + rs, err = viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[0], rs) + + rs, err = viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[1], rs) + + // error because all validators belong same address + _, err = viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) + require.Error(t, err) + + // all of them return error, no way to recover + for i := 2; i < 1000; i++ { + _, err = viewChangeNextValidator(decider, i, slots, &wrappedBLSKeys[0]) + require.Errorf(t, err, "error because all validators belong same address %d", i) + } }) // we can't find next validator, because all validators have the same address @@ -203,6 +211,8 @@ func TestViewChangeNextValidator(t *testing.T) { var ( addr1 = common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z3")) addr2 = common.BytesToAddress([]byte("one1ay37rp2pc3kjarg7a322vu3sa8j9puahg679z4")) + rs *bls.PublicKeyWrapper + err error ) slots := []shard.Slot{ { @@ -226,17 +236,21 @@ func TestViewChangeNextValidator(t *testing.T) { BLSPublicKey: wrappedBLSKeys[4].Bytes, }, } - rs, ok := viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) - require.True(t, ok) - require.Equal(t, &wrappedBLSKeys[2], rs) - rs, ok = viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) - require.True(t, ok) - require.Equal(t, &wrappedBLSKeys[3], rs) + rs, err = viewChangeNextValidator(decider, 0, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[0], rs) + + rs, err = viewChangeNextValidator(decider, 1, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[1], rs) + + rs, err = viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[2], rs) - // TODO - //rs, ok = viewChangeNextValidator(decider, 2, slots, &wrappedBLSKeys[0]) - //require.True(t, ok) - //require.Equal(t, &wrappedBLSKeys[0], rs) + rs, err = viewChangeNextValidator(decider, 3, slots, &wrappedBLSKeys[0]) + require.NoError(t, err) + require.Equal(t, &wrappedBLSKeys[1], rs) }) } From e1bbaaa197885d12de4391c5b9f1d697d760a011 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Thu, 21 Nov 2024 22:16:52 -0400 Subject: [PATCH 10/14] Renamed to LeaderRotationV2Epoch. --- consensus/view_change.go | 2 +- internal/params/config.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/view_change.go b/consensus/view_change.go index 8227bbbf0b..48a04f5a8e 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -342,7 +342,7 @@ func (consensus *Consensus) startViewChange() { // Ideally, we shall use another variable to keep track of the // leader pubkey in viewchange mode c := consensus.Blockchain().Config() - if c.IsViewChangeSkipValidatorsSameAddressEpoch(currentHeader.Epoch()) { + if c.IsLeaderRotationV2Epoch(currentHeader.Epoch()) { consensus.setLeaderPubKey(consensus.getNextLeaderKeySkipSameAddress(nextViewID, committee)) } else { consensus.setLeaderPubKey(consensus.getNextLeaderKey(nextViewID, committee)) diff --git a/internal/params/config.go b/internal/params/config.go index a6baa2680a..f1069f6e63 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -609,7 +609,7 @@ type ChainConfig struct { // if crosslink are not sent for an entire epoch signed and toSign will be 0 and 0. when that happen, next epoch there will no shard 1 validator elected in the committee. HIP32Epoch *big.Int `json:"hip32-epoch,omitempty"` - ViewChangeSkipValidatorsSameAddressEpoch *big.Int `json:"view-change-skip-validators-same-address-epoch,omitempty"` + LeaderRotationV2Epoch *big.Int `json:"leader-rotation-v2-epoch,omitempty"` } // String implements the fmt.Stringer interface. @@ -893,8 +893,8 @@ func (c *ChainConfig) IsTopMaxRate(epoch *big.Int) bool { return isForked(c.TopMaxRateEpoch, epoch) } -func (c *ChainConfig) IsViewChangeSkipValidatorsSameAddressEpoch(epoch *big.Int) bool { - return isForked(c.ViewChangeSkipValidatorsSameAddressEpoch, epoch) +func (c *ChainConfig) IsLeaderRotationV2Epoch(epoch *big.Int) bool { + return isForked(c.LeaderRotationV2Epoch, epoch) } // During this epoch, shards 2 and 3 will start sending From 16b86ef662a17ddbd96143d0d654694b2042c0e4 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Sun, 24 Nov 2024 18:51:45 -0400 Subject: [PATCH 11/14] Updated comment. --- consensus/view_change.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/view_change.go b/consensus/view_change.go index 48a04f5a8e..43624c3ec3 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -156,8 +156,8 @@ func (a nextLeaderParams) Config() *params.ChainConfig { return a.config } -// getNextLeaderKey uniquely determine who is the leader for given viewID -// It reads the current leader's pubkey based on the blockchain data and returns +// getNextLeaderKeySkipSameAddress uniquely determine who is the leader for given viewID +// It receives the committee 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) getNextLeaderKeySkipSameAddress(viewID uint64, committee *shard.Committee) *bls.PublicKeyWrapper { From 9b8dcd098976e8c0a0ba98941023a18cd3b5f6b2 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:53:52 -0400 Subject: [PATCH 12/14] Cleanup. --- consensus/consensus_service.go | 16 +---------- consensus/view_change.go | 13 +-------- internal/chain/engine.go | 52 ++++++++++++++++------------------ 3 files changed, 27 insertions(+), 54 deletions(-) diff --git a/consensus/consensus_service.go b/consensus/consensus_service.go index 298d780cfa..5da9383c1c 100644 --- a/consensus/consensus_service.go +++ b/consensus/consensus_service.go @@ -465,21 +465,7 @@ func (consensus *Consensus) updateConsensusInformation(reason string) Mode { // a solution to take care of this case because the coinbase of the latest block doesn't really represent the // the real current leader in case of M1 view change. if !curHeader.IsLastBlockInEpoch() && curHeader.Number().Uint64() != 0 { - ss, err := consensus.Blockchain().ReadShardState(curHeader.Epoch()) - if err != nil { - utils.Logger().Err(err).Msg("[UpdateConsensusInformation] failed to read shard state") - return Syncing - } - committee, err := ss.FindCommitteeByID(curHeader.ShardID()) - if err != nil { - utils.Logger().Err(err).Msg("[UpdateConsensusInformation] failed to find committee by ID") - return Syncing - } - leaderPubKey, err := chain.GetLeaderPubKeyFromCoinbase( - committee.Slots, - curHeader.Coinbase(), - consensus.Blockchain().Config().IsStaking(curHeader.Epoch()), - ) + leaderPubKey, err := chain.GetLeaderPubKeyFromCoinbase(consensus.Blockchain(), curHeader) if err != nil || leaderPubKey == nil { consensus.getLogger().Error().Err(err). Msg("[UpdateConsensusInformation] Unable to get leaderPubKey from coinbase") diff --git a/consensus/view_change.go b/consensus/view_change.go index 43624c3ec3..5d1792f069 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -7,12 +7,10 @@ import ( "github.com/ethereum/go-ethereum/common" msg_pb "github.com/harmony-one/harmony/api/proto/message" - "github.com/harmony-one/harmony/block" "github.com/harmony-one/harmony/consensus/quorum" "github.com/harmony-one/harmony/crypto/bls" "github.com/harmony-one/harmony/internal/chain" nodeconfig "github.com/harmony-one/harmony/internal/configs/node" - "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/internal/utils" "github.com/harmony-one/harmony/p2p" "github.com/harmony-one/harmony/shard" @@ -147,15 +145,6 @@ func (consensus *Consensus) getNextViewID() (uint64, time.Duration) { return nextViewID, viewChangeDuration } -type nextLeaderParams struct { - config *params.ChainConfig - curHeader *block.Header -} - -func (a nextLeaderParams) Config() *params.ChainConfig { - return a.config -} - // getNextLeaderKeySkipSameAddress uniquely determine who is the leader for given viewID // It receives the committee and returns // the next leader based on the gap of the viewID of the view change and the last @@ -200,7 +189,7 @@ func (consensus *Consensus) getNextLeaderKey(viewID uint64, committee *shard.Com stuckBlockViewID := curHeader.ViewID().Uint64() + 1 gap = int(viewID - stuckBlockViewID) // this is the truth of the leader based on blockchain blocks - lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase(committee.Slots, curHeader.Coinbase(), blockchain.Config().IsStaking(curHeader.Epoch())) + lastLeaderPubKey, err = chain.GetLeaderPubKeyFromCoinbase(blockchain, curHeader) if err != nil || lastLeaderPubKey == nil { consensus.getLogger().Error().Err(err). Msg("[getNextLeaderKey] Unable to get leaderPubKey from coinbase. Set it to consensus.LeaderPubKey") diff --git a/internal/chain/engine.go b/internal/chain/engine.go index a9c3684f43..07a458ad7a 100644 --- a/internal/chain/engine.go +++ b/internal/chain/engine.go @@ -6,6 +6,7 @@ import ( "sort" "time" + bls2 "github.com/harmony-one/bls/ffi/go/bls" "github.com/harmony-one/harmony/common/denominations" "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/numeric" @@ -156,24 +157,7 @@ func (e *engineImpl) VerifyVRF( return nil } - ss, err := bc.ReadShardState(header.Epoch()) - if err != nil { - return errors.WithMessagef( - err, "[VerifyVRF] failed to read shard state %v", header.Epoch(), - ) - } - committee, err := ss.FindCommitteeByID(header.ShardID()) - if err != nil { - return errors.WithMessagef( - err, "[VerifyVRF] failed to find committee %d", header.ShardID(), - ) - } - - leaderPubKey, err := GetLeaderPubKeyFromCoinbase( - committee.Slots, - header.Coinbase(), - bc.Config().IsStaking(header.Epoch()), - ) + leaderPubKey, err := GetLeaderPubKeyFromCoinbase(bc, header) if leaderPubKey == nil || err != nil { return err @@ -206,22 +190,35 @@ func (e *engineImpl) VerifyVRF( // GetLeaderPubKeyFromCoinbase retrieve corresponding blsPublicKey from Coinbase Address func GetLeaderPubKeyFromCoinbase( - slots shard.SlotList, coinbase common.Address, isStaking bool, + blockchain engine.ChainReader, h *block.Header, ) (*bls.PublicKeyWrapper, error) { - for _, member := range slots { + shardState, err := blockchain.ReadShardState(h.Epoch()) + if err != nil { + return nil, errors.Wrapf(err, "cannot read shard state %v %s", + h.Epoch(), + h.Coinbase().Hash().Hex(), + ) + } + + committee, err := shardState.FindCommitteeByID(h.ShardID()) + if err != nil { + return nil, err + } + + committerKey := new(bls2.PublicKey) + isStaking := blockchain.Config().IsStaking(h.Epoch()) + for _, member := range committee.Slots { if isStaking { // After staking the coinbase address will be the address of bls public key - if utils.GetAddressFromBLSPubKeyBytes(member.BLSPublicKey[:]) == coinbase { - committerKey, err := bls.BytesToBLSPublicKey(member.BLSPublicKey[:]) - if err != nil { + if utils.GetAddressFromBLSPubKeyBytes(member.BLSPublicKey[:]) == h.Coinbase() { + if committerKey, err = bls.BytesToBLSPublicKey(member.BLSPublicKey[:]); err != nil { return nil, err } return &bls.PublicKeyWrapper{Object: committerKey, Bytes: member.BLSPublicKey}, nil } } else { - if member.EcdsaAddress == coinbase { - committerKey, err := bls.BytesToBLSPublicKey(member.BLSPublicKey[:]) - if err != nil { + if member.EcdsaAddress == h.Coinbase() { + if committerKey, err = bls.BytesToBLSPublicKey(member.BLSPublicKey[:]); err != nil { return nil, err } return &bls.PublicKeyWrapper{Object: committerKey, Bytes: member.BLSPublicKey}, nil @@ -229,7 +226,8 @@ func GetLeaderPubKeyFromCoinbase( } } return nil, errors.Errorf( - "cannot find corresponding BLS Public Key coinbase %s", coinbase.Hex(), + "cannot find corresponding BLS Public Key coinbase %s", + h.Coinbase().Hex(), ) } From 2b55bacabea685d1735d3fbb0a4b8f90d7f29e12 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:50:26 -0400 Subject: [PATCH 13/14] Rebased on dev. --- internal/params/config.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/params/config.go b/internal/params/config.go index f1069f6e63..3e774c113f 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -374,7 +374,6 @@ var ( big.NewInt(0), big.NewInt(0), big.NewInt(0), - big.NewInt(0), } // TestChainConfig ... @@ -426,7 +425,6 @@ var ( big.NewInt(0), // MaxRateEpoch big.NewInt(0), big.NewInt(0), - big.NewInt(0), } // TestRules ... @@ -608,8 +606,6 @@ type ChainConfig struct { // vote power feature https://github.com/harmony-one/harmony/pull/4683 // if crosslink are not sent for an entire epoch signed and toSign will be 0 and 0. when that happen, next epoch there will no shard 1 validator elected in the committee. HIP32Epoch *big.Int `json:"hip32-epoch,omitempty"` - - LeaderRotationV2Epoch *big.Int `json:"leader-rotation-v2-epoch,omitempty"` } // String implements the fmt.Stringer interface. @@ -893,10 +889,6 @@ func (c *ChainConfig) IsTopMaxRate(epoch *big.Int) bool { return isForked(c.TopMaxRateEpoch, epoch) } -func (c *ChainConfig) IsLeaderRotationV2Epoch(epoch *big.Int) bool { - return isForked(c.LeaderRotationV2Epoch, epoch) -} - // During this epoch, shards 2 and 3 will start sending // their balances over to shard 0 or 1. func (c *ChainConfig) IsOneEpochBeforeHIP30(epoch *big.Int) bool { From 37e81e15b4cc7d059fff4b61ab31e81dd75d0a51 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Wed, 27 Nov 2024 18:04:18 -0400 Subject: [PATCH 14/14] Fixed tests. --- consensus/quorum/quorom_test.go | 2 +- consensus/quorum/quorum.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/consensus/quorum/quorom_test.go b/consensus/quorum/quorom_test.go index 7622f5aac3..ff020f64f4 100644 --- a/consensus/quorum/quorom_test.go +++ b/consensus/quorum/quorom_test.go @@ -629,7 +629,7 @@ func TestCIdentities_NthNextValidatorFailedEdgeCase2(t *testing.T) { case <-done: t.Error("Expected a timeout, but successfully calculated next leader") - case <-time.After(5 * time.Second): + case <-time.After(1 * time.Second): t.Log("Test timed out, possible infinite loop") } } diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 6f12a26687..c8d07c8642 100644 --- a/consensus/quorum/quorum.go +++ b/consensus/quorum/quorum.go @@ -280,7 +280,7 @@ func (s *cIdentities) NthNextValidator(slotList shard.SlotList, pubKey *bls.Publ Str("key", pubKey.Bytes.Hex()). Msg("[NthNextHmy] pubKey not found") } - for i := 0; i < len(slotList); i++ { + for { numNodes := len(s.publicKeys) idx = (idx + next) % numNodes if publicToAddress[s.publicKeys[idx].Bytes] == publicToAddress[pubKey.Bytes] { @@ -290,7 +290,6 @@ func (s *cIdentities) NthNextValidator(slotList shard.SlotList, pubKey *bls.Publ } return found, &s.publicKeys[idx] } - return false, pubKey } func (s *cIdentities) NthNextHmy(instance shardingconfig.Instance, pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) {