Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

parlia: fix verifyVoteAttestation when verify a batch of headers #2121

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ type PoSA interface {
IsSystemContract(to *common.Address) bool
EnoughDistance(chain ChainReader, header *types.Header) bool
IsLocalBlock(header *types.Header) bool
GetJustifiedNumberAndHash(chain ChainHeaderReader, header *types.Header) (uint64, common.Hash, error)
GetJustifiedNumberAndHash(chain ChainHeaderReader, headers []*types.Header) (uint64, common.Hash, error)
GetFinalizedHeader(chain ChainHeaderReader, header *types.Header) *types.Header
VerifyVote(chain ChainHeaderReader, vote *types.VoteEnvelope) error
IsActiveValidatorAt(chain ChainHeaderReader, header *types.Header, checkVoteKeyFn func(bLSPublicKey *types.BLSPublicKey) bool) bool
Expand Down
24 changes: 15 additions & 9 deletions consensus/parlia/parlia.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,11 @@ func (p *Parlia) verifyVoteAttestation(chain consensus.ChainHeaderReader, header
// The source block should be the highest justified block.
sourceNumber := attestation.Data.SourceNumber
sourceHash := attestation.Data.SourceHash
justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, parent)
headers := []*types.Header{parent}
if len(parents) > 0 {
headers = parents
}
justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, headers)
if err != nil {
return fmt.Errorf("unexpected error when getting the highest justified number and hash")
}
Expand Down Expand Up @@ -871,7 +875,7 @@ func (p *Parlia) assembleVoteAttestation(chain consensus.ChainHeaderReader, head

// Prepare vote attestation
// Prepare vote data
justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, parent)
justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, []*types.Header{parent})
if err != nil {
return fmt.Errorf("unexpected error when getting the highest justified number and hash")
}
Expand Down Expand Up @@ -1266,7 +1270,7 @@ func (p *Parlia) VerifyVote(chain consensus.ChainHeaderReader, vote *types.VoteE
return fmt.Errorf("target number mismatch")
}

justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, header)
justifiedBlockNumber, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(chain, []*types.Header{header})
if err != nil {
log.Error("failed to get the highest justified number and hash", "headerNumber", header.Number, "headerHash", header.Hash())
return fmt.Errorf("unexpected error when getting the highest justified number and hash")
Expand Down Expand Up @@ -1751,20 +1755,22 @@ func (p *Parlia) applyTransaction(
return nil
}

// GetJustifiedNumberAndHash returns the highest justified block's number and hash on the branch including and before `header`
func (p *Parlia) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, header *types.Header) (uint64, common.Hash, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is better to use a similar function signature as p.snapshot?

func (p *Parlia) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, header *types.Header, parents []*types.Header) (uint64, common.Hash, error) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p.snapshot defination is bad, we define it by following clique,
it's params parents has different meaning with all other interfaces,
image

if chain == nil || header == nil {
// GetJustifiedNumberAndHash retrieves the number and hash of the highest justified block
// within the branch including `headers` and utilizing the latest element as the head.
func (p *Parlia) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, headers []*types.Header) (uint64, common.Hash, error) {
if chain == nil || len(headers) == 0 || headers[len(headers)-1] == nil {
return 0, common.Hash{}, fmt.Errorf("illegal chain or header")
}
snap, err := p.snapshot(chain, header.Number.Uint64(), header.Hash(), nil)
head := headers[len(headers)-1]
snap, err := p.snapshot(chain, head.Number.Uint64(), head.Hash(), headers)
if err != nil {
log.Error("Unexpected error when getting snapshot",
"error", err, "blockNumber", header.Number.Uint64(), "blockHash", header.Hash())
"error", err, "blockNumber", head.Number.Uint64(), "blockHash", head.Hash())
return 0, common.Hash{}, err
}

if snap.Attestation == nil {
if p.chainConfig.IsLuban(header.Number) {
if p.chainConfig.IsLuban(head.Number) {
log.Debug("once one attestation generated, attestation of snap would not be nil forever basically")
}
return 0, chain.GetHeaderByNumber(0).Hash(), nil
Expand Down
2 changes: 1 addition & 1 deletion core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ func (bc *BlockChain) empty() bool {
// GetJustifiedNumber returns the highest justified blockNumber on the branch including and before `header`.
func (bc *BlockChain) GetJustifiedNumber(header *types.Header) uint64 {
if p, ok := bc.engine.(consensus.PoSA); ok {
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(bc, header)
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(bc, []*types.Header{header})
if err == nil {
return justifiedBlockNumber
}
Expand Down
2 changes: 1 addition & 1 deletion core/blockchain_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (bc *BlockChain) CurrentSafeBlock() *types.Header {
if currentHeader == nil {
return nil
}
_, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(bc, currentHeader)
_, justifiedBlockHash, err := p.GetJustifiedNumberAndHash(bc, []*types.Header{currentHeader})
if err == nil {
return bc.GetHeaderByHash(justifiedBlockHash)
}
Expand Down
2 changes: 1 addition & 1 deletion core/headerchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func NewHeaderChain(chainDb ethdb.Database, config *params.ChainConfig, engine c
// GetJustifiedNumber returns the highest justified blockNumber on the branch including and before `header`.
func (hc *HeaderChain) GetJustifiedNumber(header *types.Header) uint64 {
if p, ok := hc.engine.(consensus.PoSA); ok {
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(hc, header)
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(hc, []*types.Header{header})
if err == nil {
return justifiedBlockNumber
}
Expand Down
2 changes: 1 addition & 1 deletion core/vote/vote_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,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) {
sourceNumber, sourceHash, err := voteManager.engine.GetJustifiedNumberAndHash(voteManager.chain, header)
sourceNumber, sourceHash, err := voteManager.engine.GetJustifiedNumberAndHash(voteManager.chain, []*types.Header{header})
if err != nil {
log.Error("failed to get the highest justified number and hash at cur header", "curHeader's BlockNumber", header.Number, "curHeader's BlockHash", header.Hash())
return false, 0, common.Hash{}
Expand Down
6 changes: 3 additions & 3 deletions core/vote/vote_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ func newTestBackend() *testBackend {
func (b *testBackend) IsMining() bool { return true }
func (b *testBackend) EventMux() *event.TypeMux { return b.eventMux }

func (p *mockPOSA) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, header *types.Header) (uint64, common.Hash, error) {
parentHeader := chain.GetHeaderByHash(header.ParentHash)
func (p *mockPOSA) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, headers []*types.Header) (uint64, common.Hash, error) {
parentHeader := chain.GetHeaderByHash(headers[len(headers)-1].ParentHash)
if parentHeader == nil {
return 0, common.Hash{}, fmt.Errorf("unexpected error")
}
return parentHeader.Number.Uint64(), parentHeader.Hash(), nil
}

func (p *mockInvalidPOSA) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, header *types.Header) (uint64, common.Hash, error) {
func (p *mockInvalidPOSA) GetJustifiedNumberAndHash(chain consensus.ChainHeaderReader, headers []*types.Header) (uint64, common.Hash, error) {
return 0, common.Hash{}, fmt.Errorf("not supported")
}

Expand Down
2 changes: 1 addition & 1 deletion light/lightchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ func (lc *LightChain) CurrentHeader() *types.Header {
// GetJustifiedNumber returns the highest justified blockNumber on the branch including and before `header`
func (lc *LightChain) GetJustifiedNumber(header *types.Header) uint64 {
if p, ok := lc.engine.(consensus.PoSA); ok {
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(lc.hc, header)
justifiedBlockNumber, _, err := p.GetJustifiedNumberAndHash(lc.hc, []*types.Header{header})
if err == nil {
return justifiedBlockNumber
}
Expand Down