Code and comments cleaning for Randao and BLS

blockchain/evm.go

@@ -69,7 +69,7 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
 		baseFee = new(big.Int).SetUint64(params.ZeroBaseFee)
 	}
 
-	if header.MixHash != nil {
+	if len(header.MixHash) != 0 {
 		random = common.BytesToHash(header.MixHash)
 	} else { // Before Randao hardfork, RANDOM (44) returns last block hash
 		random = header.ParentHash

consensus/istanbul/backend/engine.go

@@ -254,8 +254,7 @@ func (sb *backend) verifyCascadingFields(chain consensus.ChainReader, header *ty
 
 	// VerifyRandao must be after verifySigner because it needs the signer (proposer) address
 	if chain.Config().IsRandaoForkEnabled(header.Number) {
-		prevMixHash := headerMixHash(chain, parent)
-		if err := sb.VerifyRandao(chain, header, prevMixHash); err != nil {
+		if err := sb.VerifyRandao(chain, header, parent.MixHash); err != nil {
 			return err
 		}
 	} else if header.RandomReveal != nil || header.MixHash != nil {
@@ -438,8 +437,7 @@ func (sb *backend) Prepare(chain consensus.ChainReader, header *types.Header) er
 	}
 
 	if chain.Config().IsRandaoForkEnabled(header.Number) {
-		prevMixHash := headerMixHash(chain, parent)
-		randomReveal, mixHash, err := sb.CalcRandao(header.Number, prevMixHash)
+		randomReveal, mixHash, err := sb.CalcRandao(header.Number, parent.MixHash)
 		if err != nil {
 			return err
 		}

consensus/istanbul/backend/randao.go

@@ -17,14 +17,15 @@ import (
 	"github.com/klaytn/klaytn/params"
 )
 
-// For testing without KIP-113 contract setup
+// BlsPubkeyProvider allows introducing a ChainBlsPubkeyProvider mock to be used for testing
 type BlsPubkeyProvider interface {
 	// num should be the header number of the block to be verified.
 	// Thus, since the state of num does not exist, the state of num-1 must be used.
 	GetBlsPubkey(chain consensus.ChainReader, proposer common.Address, num *big.Int) (bls.PublicKey, error)
 	ResetBlsCache()
 }
 
+// ChainBlsPubkeyProvider is the default implementation for BlsPubkeyProvider.
 type ChainBlsPubkeyProvider struct {
 	cache *lru.ARCCache // Cached BlsPublicKeyInfos
 }
@@ -36,10 +37,10 @@ func newChainBlsPubkeyProvider() *ChainBlsPubkeyProvider {
 	}
 }
 
-// The default implementation for BlsPubkeyFunc.
+// GetBlsPubkey retrieves a BLS public key stored in blockchain.
 // Queries KIP-113 contract and verifies the PoP.
 func (p *ChainBlsPubkeyProvider) GetBlsPubkey(chain consensus.ChainReader, proposer common.Address, num *big.Int) (bls.PublicKey, error) {
-	infos, err := p.getAllCached(chain, num)
+	infos, err := p.getBlsInfos(chain, num)
 	if err != nil {
 		return nil, err
 	}
@@ -54,7 +55,9 @@ func (p *ChainBlsPubkeyProvider) GetBlsPubkey(chain consensus.ChainReader, propo
 	return bls.PublicKeyFromBytes(info.PublicKey)
 }
 
-func (p *ChainBlsPubkeyProvider) getAllCached(chain consensus.ChainReader, num *big.Int) (system.BlsPublicKeyInfos, error) {
+// getBlsInfos returns all registered BLS info at the given block number.
+// It retrieves cache first, and then retrieves the storage of KIP113 contract.
+func (p *ChainBlsPubkeyProvider) getBlsInfos(chain consensus.ChainReader, num *big.Int) (system.BlsPublicKeyInfos, error) {
 	if item, ok := p.cache.Get(num.Uint64()); ok {
 		logger.Trace("BlsPublicKeyInfos cache hit", "number", num.Uint64())
 		return item.(system.BlsPublicKeyInfos), nil
@@ -100,14 +103,18 @@ func (p *ChainBlsPubkeyProvider) ResetBlsCache() {
 	p.cache.Purge()
 }
 
-// Calculate KIP-114 Randao header fields
+// CalcRandao calculates Randao-related header values specified in KIP-114.
 // https://github.com/klaytn/kips/blob/kip114/KIPs/kip-114.md
 func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []byte, error) {
 	if sb.blsSecretKey == nil {
 		return nil, nil, errNoBlsKey
 	}
-	if len(prevMixHash) != 32 {
-		logger.Error("invalid prevMixHash", "number", number.Uint64(), "prevMixHash", hexutil.Encode(prevMixHash))
+	parentMixHash := prevMixHash
+	if len(parentMixHash) == 0 {
+		parentMixHash = params.ZeroMixHash
+	}
+	if len(parentMixHash) != 32 {
+		logger.Error("invalid prevMixHash", "number", number.Uint64(), "prevMixHash", hexutil.Encode(parentMixHash))
 		return nil, nil, errInvalidRandaoFields
 	}
 
@@ -118,16 +125,29 @@ func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []by
 	randomReveal := bls.Sign(sb.blsSecretKey, msg[:]).Marshal()
 
 	// calc_mix_hash() = xor(prevMixHash, keccak256(randomReveal))
-	mixHash := calcMixHash(randomReveal, prevMixHash)
+	mixHash := calcMixHash(randomReveal, parentMixHash)
 
 	return randomReveal, mixHash, nil
 }
 
+// VerifyRandao verifies whether Randao-related header values conform to KIP-114.
 func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Header, prevMixHash []byte) error {
 	if header.Number.Sign() == 0 {
 		return nil // Do not verify genesis block
 	}
 
+	if len(header.RandomReveal) != 96 || len(header.MixHash) != 32 {
+		return errInvalidRandaoFields
+	}
+
+	// The VerifyRandao is invoked only since Randao hardfork block number.
+	// Since Randao hardfork, the header fields are cannot be nil because of the check above (header.RandomReveal == nil || header.MixHash == nil).
+	// Therefore it's safe to assume that if prevMixHash == nil, then the given header is exactly Randao hardfork block number.
+	parentMixHash := prevMixHash
+	if len(parentMixHash) == 0 {
+		parentMixHash = params.ZeroMixHash
+	}
+
 	proposer, err := sb.Author(header)
 	if err != nil {
 		return err
@@ -151,7 +171,7 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade
 	}
 
 	// if not newHeader.mixHash == calc_mix_hash(prevMixHash, newHeader.randomReveal): return False
-	mixHash := calcMixHash(header.RandomReveal, prevMixHash)
+	mixHash := calcMixHash(header.RandomReveal, parentMixHash)
 	if !bytes.Equal(header.MixHash, mixHash) {
 		return errInvalidRandaoFields
 	}
@@ -173,12 +193,3 @@ func calcMixHash(randomReveal, prevMixHash []byte) []byte {
 	}
 	return mixHash
 }
-
-// At the fork block's parent, pretend that prevMixHash is ZeroMixHash.
-func headerMixHash(chain consensus.ChainReader, header *types.Header) []byte {
-	if chain.Config().IsRandaoForkBlockParent(header.Number) {
-		return params.ZeroMixHash
-	} else {
-		return header.MixHash
-	}
-}

consensus/istanbul/backend/snapshot.go

@@ -23,7 +23,6 @@ package backend
 import (
 	"bytes"
 	"encoding/json"
-	"math/big"
 
 	"github.com/klaytn/klaytn/consensus"
 
@@ -220,23 +219,17 @@ func (s *Snapshot) apply(headers []*types.Header, gov governance.Engine, addr co
 			}
 		}
 	}
-	snap.Number += uint64(len(headers))
-	snap.Hash = headers[len(headers)-1].Hash()
-
-	if snap.ValSet.Policy() == istanbul.WeightedRandom {
-		snap.ValSet.SetBlockNum(snap.Number)
-
-		bigNum := new(big.Int).SetUint64(snap.Number)
-		if chain.Config().IsRandaoForkBlockParent(bigNum) {
-			// The ForkBlock must select proposers using MixHash but (ForkBlock - 1) has no MixHash. Using ZeroMixHash instead.
-			snap.ValSet.SetMixHash(params.ZeroMixHash)
-		} else if chain.Config().IsRandaoForkEnabled(bigNum) {
-			// Feed parent MixHash
-			snap.ValSet.SetMixHash(headers[len(headers)-1].MixHash)
-		}
-	}
+	lastHeader := headers[len(headers)-1]
+	snap.Number = lastHeader.Number.Uint64()
+	snap.Hash = lastHeader.Hash()
+
+	snap.ValSet.SetBlockNum(snap.Number)
 	snap.ValSet.SetSubGroupSize(snap.CommitteeSize)
 
+	if len(lastHeader.MixHash) != 0 { // After Rando HF
+		snap.ValSet.SetMixHash(lastHeader.MixHash)
+	}
+
 	if writable {
 		gov.SetTotalVotingPower(snap.ValSet.TotalVotingPower())
 		gov.SetMyVotingPower(snap.getMyVotingPower(addr))

consensus/istanbul/validator/weighted.go

@@ -276,13 +276,13 @@ func weightedRandomProposer(valSet istanbul.ValidatorSet, lastProposer common.Ad
 	rules := fork.Rules(new(big.Int).SetUint64(weightedCouncil.blockNum + 1))
 	// After Randao: Select one from ValidatorSet using MixHash as a seed.
 	if rules.IsRandao {
-		if weightedCouncil.mixHash == nil {
-			logger.Error("no mixHash", "number", weightedCouncil.blockNum)
-			return nil
+		mixHash := weightedCouncil.mixHash
+		if len(mixHash) == 0 {
+			mixHash = params.ZeroMixHash
 		}
 		// def proposer_selector(validators, committee_size, round, seed):
 		// select_committee_KIP146(validators, committee_size, seed)[round % len(validators)]
-		committee := SelectRandaoCommittee(weightedCouncil.List(), weightedCouncil.subSize, weightedCouncil.mixHash)
+		committee := SelectRandaoCommittee(weightedCouncil.List(), weightedCouncil.subSize, mixHash)
 		return committee[round%uint64(len(committee))]
 	}
 
@@ -379,14 +379,14 @@ func (valSet *weightedCouncil) SubListWithProposer(prevHash common.Hash, propose
 	if fork.Rules(view.Sequence).IsRandao {
 		// This committee must include proposers for all rounds because
 		// the proposer is picked from the this committee. See weightedRandomProposer().
-		if valSet.mixHash == nil {
-			logger.Error("no mixHash", "number", valSet.blockNum)
-			return nil
+		mixHash := valSet.mixHash
+		if len(mixHash) == 0 {
+			mixHash = params.ZeroMixHash
 		}
 		// def select_committee_KIP146(validators, committee_size, seed):
 		// shuffled = shuffle_validators_KIP146(validators, seed)
 		// return shuffled[:min(committee_size, len(validators))]
-		return SelectRandaoCommittee(validators, committeeSize, valSet.mixHash)
+		return SelectRandaoCommittee(validators, committeeSize, mixHash)
 	}
 
 	// Before Randao: SelectRandomCommittee, but the first two members are proposer and next proposer
@@ -783,8 +783,8 @@ func filterValidators(isSingleMode bool, govNodeAddr common.Address, weightedVal
 
 // getStakingAmountsOfValidators calculates stakingAmounts of validators.
 // If validators have multiple staking contracts, stakingAmounts will be a sum of stakingAmounts with the same rewardAddress.
-//  - []*weightedValidator : a list of validators which type is converted to weightedValidator
-//  - []float64 : a list of stakingAmounts.
+//   - []*weightedValidator : a list of validators which type is converted to weightedValidator
+//   - []float64 : a list of stakingAmounts.
 func getStakingAmountsOfValidators(validators istanbul.Validators, stakingInfo *reward.StakingInfo) ([]*weightedValidator, []float64, error) {
 	nVals := len(validators)
 	weightedVals := make([]*weightedValidator, nVals)

consensus/istanbul/validator/weighted_random_test.go

@@ -580,13 +580,13 @@ func TestWeightedCouncil_Randao(t *testing.T) {
 			expectedProposer:  SelectRandaoCommittee(validators, valSize+1, testMixHash)[0],
 			expectedCommittee: validators,
 		},
-		{ // nil MixHash
-			blockNum:          forkNum + 10, // expect log "no mixHash  number=(forkNum+10)"
+		{ // nil MixHash (expected MixHash value at fork block number)
+			blockNum:          forkNum,
 			round:             0,
 			committeeSize:     valSize - 1,
-			mixHash:           nil,
-			expectedProposer:  validators[0], // fall back to roundRobinProposer
-			expectedCommittee: nil,           // SubList fails.
+			mixHash:           nil, // If rules.IsRandao && mixHash == nil, then ZeroMixHash is used
+			expectedProposer:  SelectRandaoCommittee(validators, valSize-1, params.ZeroMixHash)[0],
+			expectedCommittee: SelectRandaoCommittee(validators, valSize-1, params.ZeroMixHash),
 		},
 		{ // IsSubset() == true
 			blockNum:          forkNum,

params/config.go

@@ -200,7 +200,11 @@ type ChainConfig struct {
 	Kip103ContractAddress common.Address `json:"kip103ContractAddress,omitempty"` // Kip103 contract address already deployed on the network
 
 	// Randao is an optional hardfork
-	// RandaoCompatibleBlock, RandaoRegistryRecords and RandaoRegistryOwner all must be specified to enable Randao
+	// RandaoCompatibleBlock, RandaoRegistryRecords and RandaoRegistryOwner all must be specified to enable Randao.
+	// Since Randao also enables KIP113 (BLS registry) simultaneously, the followings should be done before the hardfork.
+	//   - BLS contract (KIP113) should be deployed
+	//   - Validators information should be registered on the BLS contract
+	//   - Randao registry should be specified with the KIP113 contract address
 	RandaoCompatibleBlock *big.Int        `json:"randaoCompatibleBlock,omitempty"` // RandaoCompatible activate block (nil = no fork)
 	RandaoRegistry        *RegistryConfig `json:"randaoRegistry,omitempty"`        // Registry initial states
 
@@ -400,15 +404,6 @@ func (c *ChainConfig) IsKIP103ForkBlock(num *big.Int) bool {
 	return c.Kip103CompatibleBlock.Cmp(num) == 0
 }
 
-// IsRandaoForkBlockParent returns whethere num is one block before the randao block.
-func (c *ChainConfig) IsRandaoForkBlockParent(num *big.Int) bool {
-	if c.RandaoCompatibleBlock == nil || num == nil {
-		return false
-	}
-	nextNum := new(big.Int).Add(num, common.Big1)
-	return c.RandaoCompatibleBlock.Cmp(nextNum) == 0 // randao == num + 1
-}
-
 // IsRandaoForkBlock returns whether num is equal to the randao block.
 func (c *ChainConfig) IsRandaoForkBlock(num *big.Int) bool {
 	if c.RandaoCompatibleBlock == nil || num == nil {