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

Validate that blockperiodseconds is set to a positive integer #3186

Merged
merged 36 commits into from
Jan 5, 2022

Conversation

georgep9
Copy link
Contributor

@georgep9 georgep9 commented Dec 16, 2021

PR description

Implemented getPositiveInt function in JsonUtil to validate a JSON positive number value. Using this function, the blockperiodseconds should now be validated whenever retrieved from the genensis config.

Fixed Issue(s)

Fixes #3093

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…lues

Signed-off-by: George Patterson <g-patt@outlook.com>
…nds property

Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
@georgep9 georgep9 force-pushed the validate_blockperiodseconds branch from 4646893 to 5b3e5fc Compare December 20, 2021 04:35
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…in JsonUtil

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
@georgep9 georgep9 marked this pull request as ready for review December 21, 2021 23:59
Signed-off-by: George Patterson <g-patt@outlook.com>
Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

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

Minor nitpicks and suggestions in the comments.

I am going to be annoying though and ask that you make the same change for BftFork.getBlockPeriodSeconds please :)
It is possible to use a 'transition' to change the blockperiodseconds of an existing network so it needs the same validation constraints applied to it.
See: https://besu.hyperledger.org/en/stable/HowTo/Configure/Consensus-Protocols/IBFT/#configure-block-time-on-an-existing-network-deployment

Note this transition also applies to QBFT

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
…ts. Changes applied for BftFork.

Signed-off-by: George Patterson <g-patt@outlook.com>
Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

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

Code looks good 👍 .
I want to have a play with it locally before I approve which will probably be next year :)

…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
…kperiodseconds

Signed-off-by: George Patterson <g-patt@outlook.com>
Signed-off-by: George Patterson <g-patt@outlook.com>
Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

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

LGTM!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@siladu siladu merged commit c25483c into hyperledger:main Jan 5, 2022
matkt pushed a commit to matkt/besu that referenced this pull request Jan 5, 2022
…edger#3186)

Implemented getPositiveInt function in JsonUtil to validate a JSON positive number value. Using this function, the blockperiodseconds should now be validated whenever retrieved from the genesis config, including transitions.

Signed-off-by: George Patterson <g-patt@outlook.com>
@georgep9 georgep9 deleted the validate_blockperiodseconds branch January 14, 2022 02:35
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…edger#3186)

Implemented getPositiveInt function in JsonUtil to validate a JSON positive number value. Using this function, the blockperiodseconds should now be validated whenever retrieved from the genesis config, including transitions.

Signed-off-by: George Patterson <g-patt@outlook.com>
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.

Validate that blockperiodseconds is set to a positive integer
3 participants