Skip to content

Skip signer indices indexing#6862

Merged
sstanculeanu merged 16 commits intofeat/andromeda-fixesfrom
extend-block-andromeda
Mar 12, 2025
Merged

Skip signer indices indexing#6862
sstanculeanu merged 16 commits intofeat/andromeda-fixesfrom
extend-block-andromeda

Conversation

@miiu96
Copy link
Copy Markdown
Contributor

@miiu96 miiu96 commented Mar 6, 2025

Reasoning behind the pull request

  • Extend the OutportBlock structure with leaderIndex and leaderBlsKey
  • Skip the population of signerIndices field when EquivalentMessage flag is active ( we no longer need to put this field in OutportBlock because all validators are in the consensus group for every block)

Proposed changes

Testing procedure

  • Check signer indices will no longer appear on blocks after EquivalentMessage flag activation
  • Also, check signer indices will no longer appear in rounds index after EquivalentMessage flag activation

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?

@miiu96 miiu96 self-assigned this Mar 6, 2025
}

signersIndexes, err := sr.NodesCoordinator().GetValidatorsIndexes(pubKeys, epoch)
signersIndexes := make([]uint64, 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the flag check can be removed from here, as the /v2/subroundStartRound.go will be used for Equivalent messages, and /v1/subroundStartRound.go otherwise

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

refactored


signersIndexes, err := sr.NodesCoordinator().GetValidatorsIndexes(pubKeys, epoch)
signersIndexes := make([]uint64, 0)
if !sr.EnableEpochsHandler().IsFlagEnabled(common.EquivalentMessagesFlag) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this can be removed from here, as the /v2/subroundStartRound.go will be used for Equivalent messages, and /v1/subroundStartRound.go otherwise

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

Comment thread outport/process/outportDataProvider.go Outdated

signersIndexes := make([]uint64, 0)
// when EquivalentMessages flag is enabled signer indices can be empty because all validators are in consensus group
if odp.enableEpochsHandler.IsFlagEnabled(common.EquivalentMessagesFlag) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if odp.enableEpochsHandler.IsFlagEnabled(common.EquivalentMessagesFlag) {
if odp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, header.GetEpoch()) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

AdoAdoAdo
AdoAdoAdo previously approved these changes Mar 10, 2025
sstanculeanu
sstanculeanu previously approved these changes Mar 11, 2025
AdoAdoAdo
AdoAdoAdo previously approved these changes Mar 11, 2025
@miiu96 miiu96 dismissed stale reviews from AdoAdoAdo and sstanculeanu via 9924706 March 11, 2025 12:50
@sstanculeanu sstanculeanu merged commit 6dd6536 into feat/andromeda-fixes Mar 12, 2025
4 checks passed
@sstanculeanu sstanculeanu deleted the extend-block-andromeda branch March 12, 2025 08:58
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.

3 participants