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

Remove consensus group size chain simulator #6248

Merged
merged 7 commits into from
Jun 12, 2024

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Jun 10, 2024

Reasoning behind the pull request

  • Removed the option from the config structure from the chain simulator to set the consensus group size.
    The chain simulator works only with size 1 of the meta/shard consensus group.

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 Jun 10, 2024
iulianpascalau
iulianpascalau previously approved these changes Jun 10, 2024
@axenteoctavian axenteoctavian self-requested a review June 10, 2024 09:54
axenteoctavian
axenteoctavian previously approved these changes Jun 10, 2024
}

type ArgsBaseChainSimulator struct {
ArgsChainSimulator
ConsensusGroupSize uint32
Copy link
Contributor

Choose a reason for hiding this comment

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

Useless, the chain simulator does not work with something else other than 1 here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This helps for Sovereign Chain Simulator

AlterConfigsFunction func(cfg *config.Configs)
}

type ArgsBaseChainSimulator struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing comment, useless struct, anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

axenteoctavian
axenteoctavian previously approved these changes Jun 11, 2024
mariusmihaic
mariusmihaic previously approved these changes Jun 11, 2024
@miiu96 miiu96 dismissed stale reviews from mariusmihaic and axenteoctavian via 1a92d80 June 11, 2024 13:12
Copy link
Contributor

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

Normal allin test: 1.7.12-2a6916fbce -> remove-consensus-group-siz-1a92d80213

--- Specific errors ---

block hash does not match 1784
wrong nonce in block 836
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0

/------/

--- Statistics ---

Nr. of all ERRORS: 0
Nr. of all WARNS: 368
Nr. of new ERRORS: 0
Nr. of new WARNS: 4
Nr. of PANICS: 0

/------/

--- ERRORS ---

/------/

--- WARNINGS ---

/------/

@miiu96 miiu96 merged commit 1fb4786 into rc/v1.7.next1 Jun 12, 2024
7 checks passed
@miiu96 miiu96 deleted the remove-consensus-group-size-chain-simulator branch June 12, 2024 08:57
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.

5 participants