Skip to content

Conversation

@AdoAdoAdo
Copy link
Contributor

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

Base automatically changed from refactor-v2-subround-signature to feat/equivalent-messages October 17, 2024 08:42
@AdoAdoAdo AdoAdoAdo changed the title cleanup and adapting cleanup subround-endround Oct 21, 2024
@AdoAdoAdo AdoAdoAdo changed the title cleanup subround-endround cleanup subround end round Oct 21, 2024
@AdoAdoAdo AdoAdoAdo changed the title cleanup subround end round cleanup consensus v2 subround end round Oct 21, 2024
sentSignatureTracker: sentSignatureTracker,
worker: worker,
signatureThrottler: signatureThrottler,
blockCommitted: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is never set to true.. is it needed?


messageSender := string(cnsDta.PubKey)

if sr.IsJobDone(sr.SelfPubKey(), sr.Current()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if sr.IsJobDone(sr.SelfPubKey(), sr.Current()) {
if sr.IsSelfJobDone(sr.Current()) {

this one also covers multikey mode

// TODO[cleanup cns finality]: remove the leader check
isEquivalentMessagesFlagEnabled := sr.EnableEpochsHandler().IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, sr.GetHeader().GetEpoch())
if !sr.IsSelfLeader() && !isEquivalentMessagesFlagEnabled {
if !sr.IsSelfLeader() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't thinks this would be ok, as the invalid signers should be broadcast by the one who also sent the proof, not by the leader

}
}

func (sposWorkerMock *SposWorkerMock) AddReceivedProofHandler(handler func(proofHandler consensus.ProofHandler)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing comment

@AdoAdoAdo AdoAdoAdo marked this pull request as ready for review December 9, 2024 12:34
@ssd04 ssd04 merged commit 47279c4 into feat/equivalent-messages Dec 9, 2024
5 checks passed
@ssd04 ssd04 deleted the refactor-v2-subround-endround branch December 9, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants