Skip to content

finish consensus if header is received earlier #7583

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

Merged
AdoAdoAdo merged 3 commits into from
Dec 30, 2025
Merged

finish consensus if header is received earlier #7583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
619ddf1
finish consensus if header is received earlier
sstanculeanu Dec 24, 2025
9d5a869
Merge branch 'fix-multiple-headers-not-being-referenced' of https://g…
sstanculeanu Dec 29, 2025
e263fc6
Merge branch 'feat/supernova-async-exec' into fix-round-canceled-when…
sstanculeanu Dec 30, 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
8 changes: 8 additions & 0 deletions consensus/spos/bls/v2/subroundBlock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,10 @@ func (sr *subroundBlock) receivedBlockBody(ctx context.Context, cnsDta *consensu
return false
}

if sr.IsSubroundFinished(sr.Current()) {
return false
}

node := string(cnsDta.PubKey)

if !sr.IsNodeLeaderInCurrentRound(node) { // is NOT this node leader in current round?
Expand Down Expand Up @@ -556,6 +560,10 @@ func (sr *subroundBlock) receivedBlockHeader(headerHandler data.HeaderHandler) {
return
}

if sr.IsSubroundFinished(sr.Current()) {
return
}

isHeaderForCurrentConsensus := sr.isHeaderForCurrentConsensus(headerHandler)
if !isHeaderForCurrentConsensus {
log.Debug("subroundBlock.receivedBlockHeader - header is not for current consensus")
Expand Down
13 changes: 13 additions & 0 deletions consensus/spos/bls/v2/subroundStartRound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/multiversx/mx-chain-core-go/core/check"
"github.com/multiversx/mx-chain-core-go/data"
outportcore "github.com/multiversx/mx-chain-core-go/data/outport"
"github.com/multiversx/mx-chain-go/consensus/spos/bls"

"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/consensus/spos"
Expand Down Expand Up @@ -131,6 +132,18 @@ func (sr *subroundStartRound) initCurrentRound() bool {
return false
}

currentHeader := sr.Blockchain().GetCurrentBlockHeader()
if !check.IfNil(currentHeader) && int64(currentHeader.GetRound()) == sr.RoundHandler().Index() {
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The comparison between currentHeader.GetRound() (which returns uint64) and sr.RoundHandler().Index() (which returns int64) involves casting uint64 to int64. This can cause issues if the round number exceeds the maximum value of int64, leading to incorrect comparisons due to overflow. Consider comparing both values as uint64 to avoid potential overflow issues.

Copilot uses AI. Check for mistakes.
log.Debug("initCurrentRound: header for current consensus already committed, setting all subrounds as finished")

sr.SetStatus(sr.Current(), spos.SsFinished)
sr.SetStatus(bls.SrBlock, spos.SsFinished)
sr.SetStatus(bls.SrSignature, spos.SsFinished)
sr.SetStatus(bls.SrEndRound, spos.SsFinished)

return true
}
Comment on lines +135 to +145
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

This early exit logic for finishing consensus when a header is already committed lacks test coverage. The new behavior introduced in lines 135-145 should be tested to ensure it correctly handles the case where the current block header's round matches the consensus round, and that all subrounds are properly marked as finished. Consider adding a test case to verify this scenario.

Copilot uses AI. Check for mistakes.
Comment on lines +135 to +145
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

There's a potential race condition in the early exit logic. Between checking if the current header's round matches the consensus round (line 136) and setting the subround statuses (lines 139-142), the blockchain state could change due to concurrent operations. If another goroutine commits a different block for the same round during this window, the consensus state might become inconsistent. Consider using synchronization mechanisms to ensure atomicity of this check-and-set operation, or verify that existing locks already protect this critical section.

Copilot uses AI. Check for mistakes.

sr.AppStatusHandler().SetStringValue(common.MetricConsensusRoundState, "")

err := sr.generateNextConsensusGroup(sr.RoundHandler().Index())
Expand Down
Loading