-
Notifications
You must be signed in to change notification settings - Fork 879
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
add Xchain-pruning-blocks-retained must be >= epochlength (#7963) #8140
base: main
Are you sure you want to change the base?
Conversation
e4a731f
to
86764b9
Compare
…dation if it does(hyperledger#7963) Signed-off-by: philosup <[email protected]>
Signed-off-by: philosup <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution.
Just one small suggestion to check other consensus mechanism that also have an epoch.
this.commandLine, | ||
"--Xchain-pruning-blocks-retained must be >= " | ||
+ unstableChainPruningOptions.getChainDataPruningBlocksRetainedLimit()); | ||
} else if (genesisConfigOptions.isQbft() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ibft and Clique also have epochs and will have the same issue. Suggest using isPoa()
here instead.
Signed-off-by: philosup <[email protected]>
Signed-off-by: philosup <[email protected]>
+ unstableChainPruningOptions.getChainDataPruningBlocksRetainedLimit()); | ||
} else if (genesisConfigOptions.isPoa() | ||
&& unstableChainPruningOptions.getChainDataPruningBlocksRetained() | ||
< genesisConfigOptions.getQbftConfigOptions().getEpochLength()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also need to check the epoch length for clique and ibft2 not just qbft.
Doing something this should work
`< (genesisConfigOptions.getBftConfigOptions().getEpochLength() || genesisConfigOptions.getCliqueConfigOptions.getEpochLength())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified to handle epochLength per consensus mechanism, as genesisConfigOptions.getCliqueConfigOptions().getEpochLength() returns a default value of 30000 even when Clique is not configured.
Signed-off-by: philosup <[email protected]>
Signed-off-by: philosup <[email protected]>
PR description
Fixed Issue(s)
fixes #7963
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests