From 0ef30140253a2c9e8cfb0e593143b6d950625521 Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 7 Dec 2023 17:01:46 +0800 Subject: [PATCH 01/11] Refine code and comments --- consensus/istanbul/backend/engine.go | 6 ++--- consensus/istanbul/backend/randao.go | 31 +++++++++++++++----------- consensus/istanbul/backend/snapshot.go | 22 +++++------------- params/config.go | 9 -------- 4 files changed, 26 insertions(+), 42 deletions(-) diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go index c465d9fb23..8873b71c8a 100644 --- a/consensus/istanbul/backend/engine.go +++ b/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 } diff --git a/consensus/istanbul/backend/randao.go b/consensus/istanbul/backend/randao.go index ece8f16cad..d7a62a8f97 100644 --- a/consensus/istanbul/backend/randao.go +++ b/consensus/istanbul/backend/randao.go @@ -36,10 +36,10 @@ func newChainBlsPubkeyProvider() *ChainBlsPubkeyProvider { } } -// The default implementation for BlsPubkeyFunc. +// GetBlsPubkey is the default implementation for BlsPubkeyFunc. // 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 +54,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) { +// getBlsInfosv 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,12 +102,15 @@ 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 prevMixHash == nil { + prevMixHash = params.ZeroMixHash + } if len(prevMixHash) != 32 { logger.Error("invalid prevMixHash", "number", number.Uint64(), "prevMixHash", hexutil.Encode(prevMixHash)) return nil, nil, errInvalidRandaoFields @@ -123,10 +128,19 @@ func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []by return randomReveal, mixHash, nil } +// VerifyRandao verifies whether header.RandomReveal is the same as expected. 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 header.RandomReveal == nil || header.MixHash == nil { + return errInvalidRandaoFields + } + // The following condition is only true when header's block number is the Randao hardfork block number. + // Because of the above condition, prevMixHash cannot be nil after Randao hardfork block. + if prevMixHash == nil { + prevMixHash = params.ZeroMixHash + } proposer, err := sb.Author(header) if err != nil { @@ -173,12 +187,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 - } -} diff --git a/consensus/istanbul/backend/snapshot.go b/consensus/istanbul/backend/snapshot.go index ef0f64771d..ec4e3777b8 100644 --- a/consensus/istanbul/backend/snapshot.go +++ b/consensus/istanbul/backend/snapshot.go @@ -23,7 +23,6 @@ package backend import ( "bytes" "encoding/json" - "math/big" "github.com/klaytn/klaytn/consensus" @@ -220,22 +219,13 @@ 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) + snap.ValSet.SetMixHash(lastHeader.MixHash) if writable { gov.SetTotalVotingPower(snap.ValSet.TotalVotingPower()) diff --git a/params/config.go b/params/config.go index 0a8235de79..c947adb0f6 100644 --- a/params/config.go +++ b/params/config.go @@ -400,15 +400,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 { From 8a69068cfc6b84f46633e4389080bac1de23358e Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 7 Dec 2023 17:18:37 +0800 Subject: [PATCH 02/11] Additional comments for Randao HF prerequisites --- params/config.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/params/config.go b/params/config.go index c947adb0f6..29fd9c712e 100644 --- a/params/config.go +++ b/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 Rando 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 From b800f95d2178f8ca99516d94d8972f899ddb0c46 Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 7 Dec 2023 18:15:49 +0800 Subject: [PATCH 03/11] fix test failure --- consensus/istanbul/backend/randao.go | 20 +++++++++++--------- consensus/istanbul/backend/snapshot.go | 5 ++++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/consensus/istanbul/backend/randao.go b/consensus/istanbul/backend/randao.go index d7a62a8f97..19944f424c 100644 --- a/consensus/istanbul/backend/randao.go +++ b/consensus/istanbul/backend/randao.go @@ -108,11 +108,12 @@ func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []by if sb.blsSecretKey == nil { return nil, nil, errNoBlsKey } - if prevMixHash == nil { - prevMixHash = params.ZeroMixHash + parentMixHash := prevMixHash + if parentMixHash == nil { + parentMixHash = params.ZeroMixHash } - if len(prevMixHash) != 32 { - logger.Error("invalid prevMixHash", "number", number.Uint64(), "prevMixHash", hexutil.Encode(prevMixHash)) + if len(parentMixHash) != 32 { + logger.Error("invalid prevMixHash", "number", number.Uint64(), "prevMixHash", hexutil.Encode(parentMixHash)) return nil, nil, errInvalidRandaoFields } @@ -123,7 +124,7 @@ 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 } @@ -136,10 +137,11 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade if header.RandomReveal == nil || header.MixHash == nil { return errInvalidRandaoFields } - // The following condition is only true when header's block number is the Randao hardfork block number. + // The following if condition is only true when header's block number is the Randao hardfork block number. // Because of the above condition, prevMixHash cannot be nil after Randao hardfork block. - if prevMixHash == nil { - prevMixHash = params.ZeroMixHash + parentMixHash := prevMixHash + if parentMixHash == nil { + parentMixHash = params.ZeroMixHash } proposer, err := sb.Author(header) @@ -165,7 +167,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 } diff --git a/consensus/istanbul/backend/snapshot.go b/consensus/istanbul/backend/snapshot.go index ec4e3777b8..b194592330 100644 --- a/consensus/istanbul/backend/snapshot.go +++ b/consensus/istanbul/backend/snapshot.go @@ -225,7 +225,10 @@ func (s *Snapshot) apply(headers []*types.Header, gov governance.Engine, addr co snap.ValSet.SetBlockNum(snap.Number) snap.ValSet.SetSubGroupSize(snap.CommitteeSize) - snap.ValSet.SetMixHash(lastHeader.MixHash) + + if chain.Config().IsRandaoForkEnabled(lastHeader.Number) { + snap.ValSet.SetMixHash(lastHeader.MixHash) + } if writable { gov.SetTotalVotingPower(snap.ValSet.TotalVotingPower()) From c51ac91e35974ca01c56b33306002317ea171d61 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 09:55:44 +0800 Subject: [PATCH 04/11] handle nil mixHash in weightedRandomProposer func, and fix typo --- consensus/istanbul/validator/weighted.go | 12 ++++++------ params/config.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/consensus/istanbul/validator/weighted.go b/consensus/istanbul/validator/weighted.go index db48ce77e5..35444f0a44 100644 --- a/consensus/istanbul/validator/weighted.go +++ b/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 mixHash == nil { + 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))] } @@ -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) diff --git a/params/config.go b/params/config.go index 29fd9c712e..3d64100f3f 100644 --- a/params/config.go +++ b/params/config.go @@ -201,7 +201,7 @@ type ChainConfig struct { // Randao is an optional hardfork // RandaoCompatibleBlock, RandaoRegistryRecords and RandaoRegistryOwner all must be specified to enable Randao. - // Since Rando also enables KIP113 (BLS registry) simultaneously, the followings should be done before the hardfork. + // 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 From f14b7463d7bb28cd1a55361758e7b6a722113943 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 11:22:07 +0800 Subject: [PATCH 05/11] handle nil mixHash in SubListWithProposer and update a test case --- consensus/istanbul/validator/weighted.go | 8 ++++---- .../istanbul/validator/weighted_random_test.go | 15 ++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/consensus/istanbul/validator/weighted.go b/consensus/istanbul/validator/weighted.go index 35444f0a44..4bb7ab96e7 100644 --- a/consensus/istanbul/validator/weighted.go +++ b/consensus/istanbul/validator/weighted.go @@ -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 mixHash == nil { + 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 diff --git a/consensus/istanbul/validator/weighted_random_test.go b/consensus/istanbul/validator/weighted_random_test.go index 282038144d..4dcad5fb5a 100644 --- a/consensus/istanbul/validator/weighted_random_test.go +++ b/consensus/istanbul/validator/weighted_random_test.go @@ -580,13 +580,14 @@ 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)" - round: 0, - committeeSize: valSize - 1, - mixHash: nil, - expectedProposer: validators[0], // fall back to roundRobinProposer - expectedCommittee: nil, // SubList fails. + { // nil MixHash (this case should not happen) + blockNum: forkNum + 10, // expect log "no mixHash number=(forkNum+10)" + round: 0, + committeeSize: valSize - 1, + mixHash: nil, + // fall back to calculate proposer with zeroMixHash + expectedProposer: SelectRandaoCommittee(validators, valSize-1, params.ZeroMixHash)[0], + expectedCommittee: SelectRandaoCommittee(validators, valSize-1, params.ZeroMixHash), }, { // IsSubset() == true blockNum: forkNum, From 680495ed9ce43f40b3d73929fb3f2011c7832977 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 11:30:33 +0800 Subject: [PATCH 06/11] update comments for a test case --- consensus/istanbul/validator/weighted_random_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/consensus/istanbul/validator/weighted_random_test.go b/consensus/istanbul/validator/weighted_random_test.go index 4dcad5fb5a..09a74b2007 100644 --- a/consensus/istanbul/validator/weighted_random_test.go +++ b/consensus/istanbul/validator/weighted_random_test.go @@ -580,12 +580,11 @@ func TestWeightedCouncil_Randao(t *testing.T) { expectedProposer: SelectRandaoCommittee(validators, valSize+1, testMixHash)[0], expectedCommittee: validators, }, - { // nil MixHash (this case should not happen) - blockNum: forkNum + 10, // expect log "no mixHash number=(forkNum+10)" - round: 0, - committeeSize: valSize - 1, - mixHash: nil, - // fall back to calculate proposer with zeroMixHash + { // nil MixHash (expected MixHash value at fork block number) + blockNum: forkNum, + round: 0, + committeeSize: valSize - 1, + 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), }, From da4cbeb53da2505db33adfa05cd2ff9f69ac8d38 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 11:43:44 +0800 Subject: [PATCH 07/11] update comments --- consensus/istanbul/backend/randao.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/consensus/istanbul/backend/randao.go b/consensus/istanbul/backend/randao.go index 19944f424c..07688d8788 100644 --- a/consensus/istanbul/backend/randao.go +++ b/consensus/istanbul/backend/randao.go @@ -17,7 +17,7 @@ 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. @@ -25,6 +25,7 @@ type BlsPubkeyProvider interface { ResetBlsCache() } +// ChainBlsPubkeyProvider is the default implementation for BlsPubkeyProvider. type ChainBlsPubkeyProvider struct { cache *lru.ARCCache // Cached BlsPublicKeyInfos } @@ -36,7 +37,7 @@ func newChainBlsPubkeyProvider() *ChainBlsPubkeyProvider { } } -// GetBlsPubkey is 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.getBlsInfos(chain, num) @@ -54,7 +55,7 @@ func (p *ChainBlsPubkeyProvider) GetBlsPubkey(chain consensus.ChainReader, propo return bls.PublicKeyFromBytes(info.PublicKey) } -// getBlsInfosv returns all registered BLS info at the given block number. +// 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 { @@ -129,7 +130,7 @@ func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []by return randomReveal, mixHash, nil } -// VerifyRandao verifies whether header.RandomReveal is the same as expected. +// 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 @@ -137,8 +138,9 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade if header.RandomReveal == nil || header.MixHash == nil { return errInvalidRandaoFields } - // The following if condition is only true when header's block number is the Randao hardfork block number. - // Because of the above condition, prevMixHash cannot be nil after Randao hardfork block. + // 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 parentMixHash == nil { parentMixHash = params.ZeroMixHash From 0df72fd707667d74054403c755137e200e094bbf Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 12:14:35 +0800 Subject: [PATCH 08/11] Change nil header.mixHash checking mothod to check the length --- blockchain/evm.go | 2 +- consensus/istanbul/backend/randao.go | 12 +++++++----- consensus/istanbul/validator/weighted.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/blockchain/evm.go b/blockchain/evm.go index a0d27f23fd..b8e922c85e 100644 --- a/blockchain/evm.go +++ b/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 diff --git a/consensus/istanbul/backend/randao.go b/consensus/istanbul/backend/randao.go index 07688d8788..fb933560d0 100644 --- a/consensus/istanbul/backend/randao.go +++ b/consensus/istanbul/backend/randao.go @@ -110,7 +110,7 @@ func (sb *backend) CalcRandao(number *big.Int, prevMixHash []byte) ([]byte, []by return nil, nil, errNoBlsKey } parentMixHash := prevMixHash - if parentMixHash == nil { + if len(parentMixHash) == 0 { parentMixHash = params.ZeroMixHash } if len(parentMixHash) != 32 { @@ -135,17 +135,19 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade if header.Number.Sign() == 0 { return nil // Do not verify genesis block } - if header.RandomReveal == nil || header.MixHash == nil { - 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 parentMixHash == nil { + if len(parentMixHash) == 0 { parentMixHash = params.ZeroMixHash } + if len(header.RandomReveal) != 96 || len(header.MixHash) != 32 { + return errInvalidRandaoFields + } + proposer, err := sb.Author(header) if err != nil { return err diff --git a/consensus/istanbul/validator/weighted.go b/consensus/istanbul/validator/weighted.go index 4bb7ab96e7..3dadbfb4c4 100644 --- a/consensus/istanbul/validator/weighted.go +++ b/consensus/istanbul/validator/weighted.go @@ -277,7 +277,7 @@ func weightedRandomProposer(valSet istanbul.ValidatorSet, lastProposer common.Ad // After Randao: Select one from ValidatorSet using MixHash as a seed. if rules.IsRandao { mixHash := weightedCouncil.mixHash - if mixHash == nil { + if len(mixHash) == 0 { mixHash = params.ZeroMixHash } // def proposer_selector(validators, committee_size, round, seed): @@ -380,7 +380,7 @@ func (valSet *weightedCouncil) SubListWithProposer(prevHash common.Hash, propose // This committee must include proposers for all rounds because // the proposer is picked from the this committee. See weightedRandomProposer(). mixHash := valSet.mixHash - if mixHash == nil { + if len(mixHash) == 0 { mixHash = params.ZeroMixHash } // def select_committee_KIP146(validators, committee_size, seed): From 37b74adcf3636c3cd58d98a482088ae521375f0b Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 13:23:34 +0800 Subject: [PATCH 09/11] minor code adjustment --- consensus/istanbul/backend/randao.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/istanbul/backend/randao.go b/consensus/istanbul/backend/randao.go index fb933560d0..c199a112db 100644 --- a/consensus/istanbul/backend/randao.go +++ b/consensus/istanbul/backend/randao.go @@ -136,6 +136,10 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade 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. @@ -144,10 +148,6 @@ func (sb *backend) VerifyRandao(chain consensus.ChainReader, header *types.Heade parentMixHash = params.ZeroMixHash } - if len(header.RandomReveal) != 96 || len(header.MixHash) != 32 { - return errInvalidRandaoFields - } - proposer, err := sb.Author(header) if err != nil { return err From 740f92fe9c8db1d60089dfc232d2d7f46b1aec5a Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 13:27:02 +0800 Subject: [PATCH 10/11] minor refinement --- consensus/istanbul/backend/snapshot.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/istanbul/backend/snapshot.go b/consensus/istanbul/backend/snapshot.go index b194592330..30e525d45e 100644 --- a/consensus/istanbul/backend/snapshot.go +++ b/consensus/istanbul/backend/snapshot.go @@ -226,7 +226,7 @@ func (s *Snapshot) apply(headers []*types.Header, gov governance.Engine, addr co snap.ValSet.SetBlockNum(snap.Number) snap.ValSet.SetSubGroupSize(snap.CommitteeSize) - if chain.Config().IsRandaoForkEnabled(lastHeader.Number) { + if len(lastHeader.MixHash) != 0 { // After Rando HF snap.ValSet.SetMixHash(lastHeader.MixHash) } From eb99234c237f5bcfaa3370ca3d912322fd597820 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 8 Dec 2023 13:59:22 +0800 Subject: [PATCH 11/11] Safe mixHash assigment for nil value --- api/api_ethereum.go | 6 ++++-- api/api_public_blockchain.go | 6 ++++-- consensus/istanbul/backend/engine.go | 4 +++- consensus/istanbul/backend/snapshot.go | 4 +++- consensus/istanbul/validator/weighted.go | 10 +++++++--- node/cn/filters/api.go | 4 +++- 6 files changed, 24 insertions(+), 10 deletions(-) diff --git a/api/api_ethereum.go b/api/api_ethereum.go index ea07327eeb..8f004360e4 100644 --- a/api/api_ethereum.go +++ b/api/api_ethereum.go @@ -1253,9 +1253,11 @@ func (api *EthereumAPI) rpcMarshalHeader(head *types.Header, inclMiner bool) (ma result["baseFeePerGas"] = (*hexutil.Big)(head.BaseFee) } } - if b.ChainConfig().IsRandaoForkEnabled(head.Number) { + if len(head.RandomReveal) > 0 { result["randomReveal"] = hexutil.Bytes(head.RandomReveal) - result["mixHash"] = hexutil.Bytes(head.MixHash) + } + if len(head.MixHash) > 0 { + result["mixhash"] = hexutil.Bytes(head.MixHash) } return result, nil } diff --git a/api/api_public_blockchain.go b/api/api_public_blockchain.go index d79ca6e051..1dc3639151 100644 --- a/api/api_public_blockchain.go +++ b/api/api_public_blockchain.go @@ -582,9 +582,11 @@ func RpcOutputBlock(b *types.Block, td *big.Int, inclTx bool, fullTx bool, rules fields["baseFeePerGas"] = (*hexutil.Big)(head.BaseFee) } } - if rules.IsRandao { + if len(head.RandomReveal) > 0 { fields["randomReveal"] = hexutil.Bytes(head.RandomReveal) - fields["mixHash"] = hexutil.Bytes(head.MixHash) + } + if len(head.MixHash) > 0 { + fields["mixhash"] = hexutil.Bytes(head.MixHash) } return fields, nil diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go index 8873b71c8a..9a758f7b86 100644 --- a/consensus/istanbul/backend/engine.go +++ b/consensus/istanbul/backend/engine.go @@ -732,7 +732,9 @@ func (sb *backend) initSnapshot(chain consensus.ChainReader) (*Snapshot, error) valSet := validator.NewValidatorSet(istanbulExtra.Validators, nil, istanbul.ProposerPolicy(pset.Policy()), pset.CommitteeSize(), chain) - valSet.SetMixHash(genesis.MixHash) + if len(genesis.MixHash) != 0 { + valSet.SetMixHash(genesis.MixHash) + } snap := newSnapshot(sb.governance, 0, genesis.Hash(), valSet, chain.Config()) if err := snap.store(sb.db); err != nil { diff --git a/consensus/istanbul/backend/snapshot.go b/consensus/istanbul/backend/snapshot.go index 30e525d45e..ffa62b2834 100644 --- a/consensus/istanbul/backend/snapshot.go +++ b/consensus/istanbul/backend/snapshot.go @@ -359,7 +359,9 @@ func (s *Snapshot) UnmarshalJSON(b []byte) error { if j.Policy == istanbul.WeightedRandom { s.ValSet = validator.NewWeightedCouncil(j.Validators, j.DemotedValidators, j.RewardAddrs, j.VotingPowers, j.Weights, j.Policy, j.SubGroupSize, j.Number, j.ProposersBlockNum, nil) validator.RecoverWeightedCouncilProposer(s.ValSet, j.Proposers) - s.ValSet.SetMixHash(j.MixHash) + if len(j.MixHash) != 0 { + s.ValSet.SetMixHash(j.MixHash) + } } else { s.ValSet = validator.NewSubSet(j.Validators, j.Policy, j.SubGroupSize) } diff --git a/consensus/istanbul/validator/weighted.go b/consensus/istanbul/validator/weighted.go index 3dadbfb4c4..1d7447e607 100644 --- a/consensus/istanbul/validator/weighted.go +++ b/consensus/istanbul/validator/weighted.go @@ -259,7 +259,9 @@ func GetWeightedCouncilData(valSet istanbul.ValidatorSet) (validators []common.A proposers[i] = proposer.Address() } proposersBlockNum = weightedCouncil.proposersBlockNum - mixHash = weightedCouncil.mixHash + if len(mixHash) != 0 { + mixHash = weightedCouncil.mixHash + } } else { logger.Error("invalid proposer policy for weightedCouncil") } @@ -628,8 +630,10 @@ func (valSet *weightedCouncil) Copy() istanbul.ValidatorSet { newWeightedCouncil.proposers = make([]istanbul.Validator, len(valSet.proposers)) copy(newWeightedCouncil.proposers, valSet.proposers) - newWeightedCouncil.mixHash = make([]byte, len(valSet.mixHash)) - copy(newWeightedCouncil.mixHash, valSet.mixHash) + if valSet.mixHash != nil { // mixHash is nil before Randao HF + newWeightedCouncil.mixHash = make([]byte, len(valSet.mixHash)) + copy(newWeightedCouncil.mixHash, valSet.mixHash) + } return &newWeightedCouncil } diff --git a/node/cn/filters/api.go b/node/cn/filters/api.go index 5d670be90c..451ef0705a 100644 --- a/node/cn/filters/api.go +++ b/node/cn/filters/api.go @@ -251,8 +251,10 @@ func RPCMarshalHeader(head *types.Header, rules params.Rules) map[string]interfa result["baseFeePerGas"] = (*hexutil.Big)(head.BaseFee) } } - if rules.IsRandao { + if len(head.RandomReveal) > 0 { result["randomReveal"] = hexutil.Bytes(head.RandomReveal) + } + if len(head.MixHash) > 0 { result["mixhash"] = hexutil.Bytes(head.MixHash) }