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

Add terminal block hash and number to Ropsten genesis file #4026

Merged

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented Jun 28, 2022

Signed-off-by: Fabio Di Fabio fabio.difabio@consensys.net

PR description

Add terminal block hash and number to Ropsten genesis file, and also require that peers on Ropsten must have that block

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 force-pushed the ropsten-terminal-block-hash-number-genesis branch from 9cc615b to 0e7a7d7 Compare June 28, 2022 13:11
@fab-10 fab-10 marked this pull request as ready for review June 28, 2022 13:20
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
validators.add(
new DaoForkPeerValidator(protocolSchedule, metricsSystem, daoBlock.getAsLong()));
}
configOptionsSupplier
Copy link
Contributor

Choose a reason for hiding this comment

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

This is less readable than an if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually find the lambda version more effective to describe what to do in case of something is present, and less error prone, but I can easily read both versions, so I will revert.


for (final Map.Entry<Long, Hash> requiredBlock : requiredBlocks.entrySet()) {
validators.add(
new RequiredBlocksPeerValidator(
protocolSchedule, metricsSystem, requiredBlock.getKey(), requiredBlock.getValue()));
}

configOptionsSupplier
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not as readable as a couple of if blocks. Two if blocks would make it obvious we are adding a config if two config items are persent. get().get().filter(...).ifPresent(...) is much too verbose, especially when one of the ifPresent paths is nested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM

@jflo jflo requested a review from shemnon June 29, 2022 13:49
@fab-10 fab-10 merged commit 27fc468 into hyperledger:main Jul 5, 2022
@fab-10 fab-10 deleted the ropsten-terminal-block-hash-number-genesis branch July 5, 2022 15:32
matkt added a commit to matkt/besu that referenced this pull request Jul 7, 2022
…yperledger#4026)"

This reverts commit 27fc468.

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
fab-10 added a commit to fab-10/besu that referenced this pull request Jul 13, 2022
fab-10 added a commit to fab-10/besu that referenced this pull request Jul 13, 2022
…yperledger#4026)"

This reverts commit 27fc468.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
fab-10 added a commit that referenced this pull request Jul 13, 2022
…4026)" (#4093)

This reverts commit 27fc468.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…er#4026)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* If terminal block is present in the genesis, then peers must have it

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Update unit test

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* address review comments

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…yperledger#4026)" (hyperledger#4093)

This reverts commit 27fc468.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
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