From cd853dfd794ac3d7c4d35a5a3fec77068d9094e1 Mon Sep 17 00:00:00 2001 From: calmbeing Date: Tue, 26 Apr 2022 17:41:45 +0800 Subject: [PATCH] resolved comments --- core/types/vote.go | 22 +++++++++++++ core/vote/vote_manager.go | 66 +++++++------------------------------ core/vote/vote_pool.go | 49 ++++++++++++++------------- core/vote/vote_pool_test.go | 2 +- core/vote/vote_signer.go | 64 ++++++++++++++++++++++------------- 5 files changed, 102 insertions(+), 101 deletions(-) diff --git a/core/types/vote.go b/core/types/vote.go index 8be8e11158..2586b16a71 100644 --- a/core/types/vote.go +++ b/core/types/vote.go @@ -3,6 +3,9 @@ package types import ( "sync/atomic" + "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/crypto/bls" + "github.com/ethereum/go-ethereum/common" ) @@ -61,3 +64,22 @@ func (v *VoteEnvelope) calcVoteHash() common.Hash { } func (b BLSPublicKey) Bytes() []byte { return b[:] } + +// Verify vote using BLS. +func (vote *VoteEnvelope) Verify() error { + blsPubKey, err := bls.PublicKeyFromBytes(vote.VoteAddress[:]) + if err != nil { + return errors.Wrap(err, "convert public key from bytes to bls failed") + } + + sig, err := bls.SignatureFromBytes(vote.Signature[:]) + if err != nil { + return errors.Wrap(err, "invalid signature") + } + + voteDataHash := vote.Data.Hash() + if !sig.Verify(blsPubKey, voteDataHash[:]) { + return errors.New("verify bls signature failed.") + } + return nil +} diff --git a/core/vote/vote_manager.go b/core/vote/vote_manager.go index 840f97f507..3f99aa19ca 100644 --- a/core/vote/vote_manager.go +++ b/core/vote/vote_manager.go @@ -1,12 +1,7 @@ package vote import ( - "context" "fmt" - "io/ioutil" - - "github.com/prysmaticlabs/prysm/validator/accounts/iface" - "github.com/prysmaticlabs/prysm/validator/accounts/wallet" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus" @@ -20,7 +15,7 @@ import ( ) type ( - getHighestJustifiedHeader func(chain consensus.ChainHeaderReader, header *types.Header) *types.Header + getHighestJustifiedHeaderFunc func(chain consensus.ChainHeaderReader, header *types.Header) *types.Header ) // VoteManager will handle the vote produced by self. @@ -39,10 +34,10 @@ type VoteManager struct { engine consensus.PoSA - get getHighestJustifiedHeader + getHighestJustifiedHeader getHighestJustifiedHeaderFunc } -func NewVoteManager(mux *event.TypeMux, chainconfig *params.ChainConfig, chain *core.BlockChain, pool *VotePool, journalPath, blsPasswordPath, blsWalletPath string, engine consensus.PoSA, get getHighestJustifiedHeader) (*VoteManager, error) { +func NewVoteManager(mux *event.TypeMux, chainconfig *params.ChainConfig, chain *core.BlockChain, pool *VotePool, journalPath, blsPasswordPath, blsWalletPath string, engine consensus.PoSA, getHighestJustifiedHeader getHighestJustifiedHeaderFunc) (*VoteManager, error) { voteManager := &VoteManager{ mux: mux, @@ -53,45 +48,11 @@ func NewVoteManager(mux *event.TypeMux, chainconfig *params.ChainConfig, chain * pool: pool, engine: engine, - get: get, - } - - dirExists, err := wallet.Exists(blsWalletPath) - if err != nil { - log.Error("Check BLS wallet exists error: %v.", err) - return nil, err - } - if !dirExists { - log.Error("BLS wallet did not exists.") - return nil, fmt.Errorf("BLS wallet did not exists.") - } - - walletPassword, err := ioutil.ReadFile(blsPasswordPath) - if err != nil { - log.Error("Read BLS wallet password error: %v.", err) - return nil, err - } - log.Info("Read BLS wallet password successfully") - - w, err := wallet.OpenWallet(context.Background(), &wallet.Config{ - WalletDir: blsWalletPath, - WalletPassword: string(walletPassword), - }) - if err != nil { - log.Error("Open BLS wallet failed: %v.", err) - return nil, err - } - log.Info("Open BLS wallet successfully") - - km, err := w.InitializeKeymanager(context.Background(), iface.InitKeymanagerConfig{ListenForChanges: false}) - if err != nil { - log.Error("Initialize key manager failed: %v.", err) - return nil, err + getHighestJustifiedHeader: getHighestJustifiedHeader, } - log.Info("Initialized keymanager successfully") - // Create voteSigner - voteSigner, err := NewVoteSigner(&km) + // Create voteSigner. + voteSigner, err := NewVoteSigner(blsPasswordPath, blsWalletPath) if err != nil { return nil, err } @@ -194,7 +155,7 @@ func (voteManager *VoteManager) loop() { // A validator must not vote within the span of its other votes . (Rule 2) // Validators always vote for their canonical chain’s latest block. (Rule 3) func (voteManager *VoteManager) UnderRules(header *types.Header) (bool, uint64, common.Hash) { - justifiedHeader := voteManager.get(voteManager.chain, header) + justifiedHeader := voteManager.getHighestJustifiedHeader(voteManager.chain, header) if justifiedHeader == nil { log.Error("highestJustifiedHeader is nil") return false, 0, common.Hash{} @@ -219,21 +180,16 @@ func (voteManager *VoteManager) UnderRules(header *types.Header) (bool, uint64, return false, 0, common.Hash{} } - journalLatestVote, err := journal.ReadVote(lastIndex) - if err != nil { - return false, 0, common.Hash{} - } - if journalLatestVote == nil { - // Indicate there's no vote before in local node, so it must be under rules. - return true, sourceNumber, sourceHash - } - for index := lastIndex; index >= firstIndex; index-- { vote, err := journal.ReadVote(index) if err != nil { return false, 0, common.Hash{} } if vote == nil { + // Indicate there's no vote in local journal, so it must be under rules. + if index == 0 { + return true, sourceNumber, sourceHash + } log.Error("vote is nil") return false, 0, common.Hash{} } diff --git a/core/vote/vote_pool.go b/core/vote/vote_pool.go index bcd46d1982..725cb0d1ce 100644 --- a/core/vote/vote_pool.go +++ b/core/vote/vote_pool.go @@ -139,17 +139,6 @@ func (pool *VotePool) putIntoVotePool(vote *types.VoteEnvelope) bool { votesPq = pool.futureVotesPq isFutureVote = true } else { - // Verify if the vote comes from valid validators based on voteAddress (BLSPublicKey). - if posa, ok := pool.engine.(consensus.PoSA); ok { - if !posa.VerifyVote(pool.chain, vote) { - return false - } - } - // Verify bls signature. - if err := VerifyVoteWithBLS(vote); err != nil { - log.Error("Failed to verify voteMessage", "err", err) - return false - } votes = pool.curVotes votesPq = pool.curVotesPq } @@ -159,14 +148,20 @@ func (pool *VotePool) putIntoVotePool(vote *types.VoteEnvelope) bool { return false } - pool.putVote(votes, votesPq, vote, voteData, voteHash, isFutureVote) - if !isFutureVote { + // Verify if the vote comes from valid validators based on voteAddress (BLSPublicKey), only verify curVotes here, will verify futureVotes in transfer process. + if posa, ok := pool.engine.(consensus.PoSA); ok { + if !posa.VerifyVote(pool.chain, vote) { + return false + } + } // Send vote for handler usage of broadcasting to peers. voteEv := core.NewVoteEvent{Vote: vote} pool.votesFeed.Send(voteEv) } + pool.putVote(votes, votesPq, vote, voteData, voteHash, isFutureVote) + return true } @@ -259,13 +254,11 @@ func (pool *VotePool) transfer(blockHash common.Hash) { continue } } - // Verify the vote from futureVotes. - if err := VerifyVoteWithBLS(vote); err == nil { - // In the process of transfer, send valid vote to votes channel for handler usage - voteEv := core.NewVoteEvent{Vote: vote} - pool.votesFeed.Send(voteEv) - validVotes = append(validVotes, vote) - } + + // In the process of transfer, send valid vote to votes channel for handler usage + voteEv := core.NewVoteEvent{Vote: vote} + pool.votesFeed.Send(voteEv) + validVotes = append(validVotes, vote) } voteData := heap.Pop(futurePq) @@ -349,15 +342,25 @@ func (pool *VotePool) basicVerify(vote *types.VoteEnvelope, headNumber uint64, m log.Warn("BlockNumber of vote is outside the range of header-256~header+13") return false } - // To prevent DOS attacks, make sure no more than 50 votes for the same blockHash if it's futureVotes - // No more than 21 votes per blockHash if not futureVotes and no more than 50 votes per blockHash if futureVotes + + // To prevent DOS attacks, make sure no more than 21 votes per blockHash if not futureVotes + // and no more than 50 votes per blockHash if futureVotes. maxVoteAmountPerBlock := maxCurVoteAmountPerBlock if isFutureVote { maxVoteAmountPerBlock = maxFutureVoteAmountPerBlock } if voteBox, ok := m[targetHash]; ok { - return len(voteBox.voteMessages) <= maxVoteAmountPerBlock + if len(voteBox.voteMessages) > maxVoteAmountPerBlock { + return false + } } + + // Verify bls signature. + if err := vote.Verify(); err != nil { + log.Error("Failed to verify voteMessage", "err", err) + return false + } + return true } diff --git a/core/vote/vote_pool_test.go b/core/vote/vote_pool_test.go index 3e6d2b2bc0..a8d2d97e8f 100644 --- a/core/vote/vote_pool_test.go +++ b/core/vote/vote_pool_test.go @@ -170,7 +170,7 @@ func testVotePool(t *testing.T, inValidRules bool) { file.Close() os.Remove(journal) - var ruleFunc getHighestJustifiedHeader + var ruleFunc getHighestJustifiedHeaderFunc if inValidRules { ruleFunc = getHighestJustifiedHeaderForValid } else { diff --git a/core/vote/vote_signer.go b/core/vote/vote_signer.go index 356bbdda11..d062ad4467 100644 --- a/core/vote/vote_signer.go +++ b/core/vote/vote_signer.go @@ -3,16 +3,20 @@ package vote import ( "context" "fmt" + "io/ioutil" "time" "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/crypto/bls" validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client" + "github.com/prysmaticlabs/prysm/validator/accounts/iface" + "github.com/prysmaticlabs/prysm/validator/accounts/wallet" "github.com/prysmaticlabs/prysm/validator/keymanager" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" ) @@ -25,16 +29,51 @@ type VoteSigner struct { pubKey [48]byte } -func NewVoteSigner(km *keymanager.IKeymanager) (*VoteSigner, error) { +func NewVoteSigner(blsPasswordPath, blsWalletPath string) (*VoteSigner, error) { + dirExists, err := wallet.Exists(blsWalletPath) + if err != nil { + log.Error("Check BLS wallet exists error: %v.", err) + return nil, err + } + if !dirExists { + log.Error("BLS wallet did not exists.") + return nil, fmt.Errorf("BLS wallet did not exists.") + } + + walletPassword, err := ioutil.ReadFile(blsPasswordPath) + if err != nil { + log.Error("Read BLS wallet password error: %v.", err) + return nil, err + } + log.Info("Read BLS wallet password successfully") + + w, err := wallet.OpenWallet(context.Background(), &wallet.Config{ + WalletDir: blsWalletPath, + WalletPassword: string(walletPassword), + }) + if err != nil { + log.Error("Open BLS wallet failed: %v.", err) + return nil, err + } + log.Info("Open BLS wallet successfully") + + km, err := w.InitializeKeymanager(context.Background(), iface.InitKeymanagerConfig{ListenForChanges: false}) + if err != nil { + log.Error("Initialize key manager failed: %v.", err) + return nil, err + } + log.Info("Initialized keymanager successfully") + ctx, cancel := context.WithTimeout(context.Background(), voteSignerTimeout) defer cancel() - pubKeys, err := (*km).FetchValidatingPublicKeys(ctx) + pubKeys, err := km.FetchValidatingPublicKeys(ctx) if err != nil { return nil, errors.Wrap(err, "could not fetch validating public keys") } + return &VoteSigner{ - km: km, + km: &km, pubKey: pubKeys[0], }, nil } @@ -65,25 +104,6 @@ func (signer *VoteSigner) SignVote(vote *types.VoteEnvelope) error { return nil } -// VerifyVoteWithBLS using BLS. -func VerifyVoteWithBLS(vote *types.VoteEnvelope) error { - blsPubKey, err := bls.PublicKeyFromBytes(vote.VoteAddress[:]) - if err != nil { - return errors.Wrap(err, "convert public key from bytes to bls failed") - } - - sig, err := bls.SignatureFromBytes(vote.Signature[:]) - if err != nil { - return errors.Wrap(err, "invalid signature") - } - - voteDataHash := vote.Data.Hash() - if !sig.Verify(blsPubKey, voteDataHash[:]) { - return errors.New("verify bls signature failed.") - } - return nil -} - // Metrics to indicate if there's any failed signing. func votesSigningErrorMetric(blockNumber uint64, blockHash common.Hash) metrics.Gauge { return metrics.GetOrRegisterGauge(fmt.Sprintf("voteSigning/blockNumber/%d/blockHash/%s", blockNumber, blockHash), nil)