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.

Sign up for GitHub

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

Fix transition block #6724

Merged
merged 9 commits into from
Jan 22, 2025
+59 −17
Merged

Fix transition block #6724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9ea746a
deps update
sstanculeanu Jan 22, 2025
fa83110
updated core go
sstanculeanu Jan 22, 2025
6329556
updated flag check after review
sstanculeanu Jan 22, 2025
e7c6067
added proper setProbableHighestNonce
sstanculeanu Jan 22, 2025
3cb0133
Merge branch 'equivalent-proofs-stabilization-2' of https://github.co…
sstanculeanu Jan 22, 2025
903e4cf
rollback invalid unlocks
sstanculeanu Jan 22, 2025
ca7c322
reverted `updated flag check after review` commit
sstanculeanu Jan 22, 2025
245cb71
further fixes on notarization
sstanculeanu Jan 22, 2025
ebf0fb5
Merge branch 'equivalent-proofs-stabilization-2' of https://github.co…
sstanculeanu Jan 22, 2025
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
6 changes: 3 additions & 3 deletions common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ func IsEpochChangeBlockForFlagActivation(header data.HeaderHandler, enableEpochs
return isStartOfEpochBlock && isBlockInActivationEpoch
}

// IsFlagEnabledAfterEpochsStartBlock returns true if the flag is enabled for the header, but it is not the epoch start block
func IsFlagEnabledAfterEpochsStartBlock(header data.HeaderHandler, enableEpochsHandler EnableEpochsHandler, flag core.EnableEpochFlag) bool {
// isFlagEnabledAfterEpochsStartBlock returns true if the flag is enabled for the header, but it is not the epoch start block
func isFlagEnabledAfterEpochsStartBlock(header data.HeaderHandler, enableEpochsHandler EnableEpochsHandler, flag core.EnableEpochFlag) bool {
isFlagEnabled := enableEpochsHandler.IsFlagEnabledInEpoch(flag, header.GetEpoch())
isEpochStartBlock := IsEpochChangeBlockForFlagActivation(header, enableEpochsHandler, flag)
return isFlagEnabled && !isEpochStartBlock
}

// ShouldBlockHavePrevProof returns true if the block should have a proof
func ShouldBlockHavePrevProof(header data.HeaderHandler, enableEpochsHandler EnableEpochsHandler, flag core.EnableEpochFlag) bool {
return IsFlagEnabledAfterEpochsStartBlock(header, enableEpochsHandler, flag) && header.GetNonce() > 1
return isFlagEnabledAfterEpochsStartBlock(header, enableEpochsHandler, flag) && header.GetNonce() > 1
}

// VerifyProofAgainstHeader verifies the fields on the proof match the ones on the header
Expand Down
1 change: 1 addition & 0 deletions consensus/chronology/chronology.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func (chr *chronology) RemoveAllSubrounds() {

chr.subrounds = make(map[int]int)
chr.subroundHandlers = make([]consensus.SubroundHandler, 0)
chr.subroundId = srBeforeStartRound

chr.mutSubrounds.Unlock()
}
Expand Down
2 changes: 1 addition & 1 deletion consensus/spos/bls/v2/subroundBlock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func (sr *subroundBlock) saveProofForPreviousHeaderIfNeeded(header data.HeaderHa
proof := header.GetPreviousProof()
err := common.VerifyProofAgainstHeader(proof, prevHeader)
if err != nil {
log.Debug("saveProofForPreviousHeaderIfNeeded: invalid proof, %w", err)
log.Debug("saveProofForPreviousHeaderIfNeeded: invalid proof, %s", err.Error())
return
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/klauspost/cpuid/v2 v2.2.5
github.com/mitchellh/mapstructure v1.5.0
github.com/multiversx/mx-chain-communication-go v1.0.15-0.20240508074652-e128a1c05c8e
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250120115427-6f987e761979
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250122100317-798de8d35458
github.com/multiversx/mx-chain-crypto-go v1.2.12-0.20240508074452-cc21c1b505df
github.com/multiversx/mx-chain-es-indexer-go v1.7.2-0.20240619122842-05143459c554
github.com/multiversx/mx-chain-logger-go v1.0.15-0.20240508072523-3f00a726af57
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ github.com/multiversx/concurrent-map v0.1.4 h1:hdnbM8VE4b0KYJaGY5yJS2aNIW9TFFsUY
github.com/multiversx/concurrent-map v0.1.4/go.mod h1:8cWFRJDOrWHOTNSqgYCUvwT7c7eFQ4U2vKMOp4A/9+o=
github.com/multiversx/mx-chain-communication-go v1.0.15-0.20240508074652-e128a1c05c8e h1:Tsmwhu+UleE+l3buPuqXSKTqfu5FbPmzQ4MjMoUvCWA=
github.com/multiversx/mx-chain-communication-go v1.0.15-0.20240508074652-e128a1c05c8e/go.mod h1:2yXl18wUbuV3cRZr7VHxM1xo73kTaC1WUcu2kx8R034=
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250120115427-6f987e761979 h1:x17xl7BbRwR48BGWq0ij2G7UlXUkVOdRiQFtUNJckSU=
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250120115427-6f987e761979/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE=
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250122100317-798de8d35458 h1:lMzqNUv+UbtcvwL98uf52z0hfXlA/ASrWAKAh8lyt0E=
github.com/multiversx/mx-chain-core-go v1.2.21-0.20250122100317-798de8d35458/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE=
github.com/multiversx/mx-chain-crypto-go v1.2.12-0.20240508074452-cc21c1b505df h1:clihfi78bMEOWk/qw6WA4uQbCM2e2NGliqswLAvw19k=
github.com/multiversx/mx-chain-crypto-go v1.2.12-0.20240508074452-cc21c1b505df/go.mod h1:gtJYB4rR21KBSqJlazn+2z6f9gFSqQP3KvAgL7Qgxw4=
github.com/multiversx/mx-chain-es-indexer-go v1.7.2-0.20240619122842-05143459c554 h1:Fv8BfzJSzdovmoh9Jh/by++0uGsOVBlMP3XiN5Svkn4=
Expand Down
2 changes: 0 additions & 2 deletions process/block/baseProcess.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,6 @@ func (bp *baseProcessor) computeHeadersForCurrentBlock(usedInBlock bool) (map[ui
}

if bp.hasMissingProof(headerInfo, hdrHash) {
bp.hdrsForCurrBlock.mutHdrsForBlock.RUnlock()
return nil, fmt.Errorf("%w for header with hash %s", process.ErrMissingHeaderProof, hex.EncodeToString([]byte(hdrHash)))
}

Expand All @@ -652,7 +651,6 @@ func (bp *baseProcessor) computeHeadersForCurrentBlockInfo(usedInBlock bool) (ma
}

if bp.hasMissingProof(headerInfo, metaBlockHash) {
bp.hdrsForCurrBlock.mutHdrsForBlock.RUnlock()
return nil, fmt.Errorf("%w for header with hash %s", process.ErrMissingHeaderProof, hex.EncodeToString([]byte(metaBlockHash)))
}

Expand Down
35 changes: 28 additions & 7 deletions process/block/shardblock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,15 +578,14 @@ func (sp *shardProcessor) checkMetaHdrFinality(header data.HeaderHandler) error
return process.ErrNilBlockHeader
}

if common.IsFlagEnabledAfterEpochsStartBlock(header, sp.enableEpochsHandler, common.EquivalentMessagesFlag) {
marshalledHeader, err := sp.marshalizer.Marshal(header)
if err != nil {
return err
if sp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, header.GetEpoch()) {
hash, errHash := sp.getHeaderHash(header)
if errHash != nil {
return errHash
}

headerHash := sp.hasher.Compute(string(marshalledHeader))
if !sp.proofsPool.HasProof(header.GetShardID(), headerHash) {
return fmt.Errorf("%w, missing proof for header %s", process.ErrHeaderNotFinal, hex.EncodeToString(headerHash))
if !sp.proofsPool.HasProof(header.GetShardID(), hash) {
return fmt.Errorf("%w, missing proof for header %s", process.ErrHeaderNotFinal, hex.EncodeToString(hash))
}

return nil
Expand Down Expand Up @@ -614,6 +613,19 @@ func (sp *shardProcessor) checkMetaHdrFinality(header data.HeaderHandler) error
continue
}

if sp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, metaHdr.GetEpoch()) {
hash, errHash := sp.getHeaderHash(metaHdr)
if errHash != nil {
return errHash
}

if sp.proofsPool.HasProof(core.MetachainShardId, hash) {
return nil
}

return process.ErrHeaderNotFinal
}

lastVerifiedHdr = metaHdr
nextBlocksVerified += 1
}
Expand All @@ -628,6 +640,15 @@ func (sp *shardProcessor) checkMetaHdrFinality(header data.HeaderHandler) error
return nil
}

func (sp *shardProcessor) getHeaderHash(header data.HeaderHandler) ([]byte, error) {
marshalledHeader, errMarshal := sp.marshalizer.Marshal(header)
if errMarshal != nil {
return nil, errMarshal
}

return sp.hasher.Compute(string(marshalledHeader)), nil
}

func (sp *shardProcessor) checkAndRequestIfMetaHeadersMissing() {
orderedMetaBlocks, _ := sp.blockTracker.GetTrackedHeaders(core.MetachainShardId)

Expand Down
12 changes: 12 additions & 0 deletions process/sync/baseForkDetector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,18 @@ func (bfd *baseForkDetector) processReceivedProof(proof data.HeaderProofHandler)

_ = bfd.append(hInfo)

probableHighestNonce := bfd.computeProbableHighestNonce()
bfd.setProbableHighestNonce(probableHighestNonce)

log.Debug("forkDetector.processReceivedProof",
"round", hInfo.round,
"nonce", hInfo.nonce,
"hash", hInfo.hash,
"state", hInfo.state,
"probable highest nonce", bfd.probableHighestNonce(),
"last checkpoint nonce", bfd.lastCheckpoint().nonce,
"final checkpoint nonce", bfd.finalCheckpoint().nonce,
"has proof", hInfo.hasProof)
}

func (bfd *baseForkDetector) processReceivedBlock(
Expand Down
12 changes: 11 additions & 1 deletion process/track/blockProcessor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func (bp *blockProcessor) checkHeaderFinality(
return process.ErrNilBlockHeader
}

if common.IsFlagEnabledAfterEpochsStartBlock(header, bp.enableEpochsHandler, common.EquivalentMessagesFlag) {
if bp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, header.GetEpoch()) {
// the index in argument is for the next block after header
if bp.proofsPool.HasProof(header.GetShardID(), sortedHeadersHashes[index-1]) {
return nil
Expand All @@ -336,6 +336,16 @@ func (bp *blockProcessor) checkHeaderFinality(
continue
}

// if the currentHeader(the one that should confirm the finality of the prev)
Copy link
Contributor

Choose a reason for hiding this comment

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

probably not needed if we handle inside the block tracker the check that there is a proof.
Will take the change of block tracker in a separate PR.

Can you add a check somewhere on the processing (for both leader and other nodes - in create block for leader & process block for others) that every used block has proof

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should already be covered:

Copy link
Contributor

Choose a reason for hiding this comment

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

when verifying the proof for the header not the prev proof, we should not use common.IsFlagEnabledAfterEpochsStartBlock(header, sp.enableEpochsHandler, common.EquivalentMessagesFlag)

so I think func (sp *shardProcessor) checkMetaHdrFinality(header data.HeaderHandler) error has the wrong check

// is the epoch start block of equivalent messages, we must check for its proof as well
if bp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, currHeader.GetEpoch()) {
if bp.proofsPool.HasProof(currHeader.GetShardID(), sortedHeadersHashes[index]) {
return nil
}

return process.ErrHeaderNotFinal
}

prevHeader = currHeader
numFinalityAttestingHeaders += 1
}
Expand Down
Loading