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

EIP2384 - Ice Age Adustment around Istanbul #211

Merged
merged 10 commits into from
Dec 4, 2019

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Nov 21, 2019

Add support for EIP2384

  • Current placeholder is "EIP2384Block" because Icestanbul seemed a
    little too silly.
  • Add difficulty calculator tests from the reference tests

(ethereum/EIPs#2384)

Signed-off-by: Danno Ferrin <[email protected]>

Add support for EIP2384
* Current placeholder is "EIP2384Block" because Icestanbul seemed a
  little too silly.
* Add difficulty calculator tests from the reference tests

Signed-off-by: Danno Ferrin <[email protected]>
* update to new name
* add fork block numbers
* update reference tests

Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: Danno Ferrin <[email protected]>

# Conflicts:
#	config/src/test-support/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java
@shemnon shemnon marked this pull request as ready for review December 2, 2019 22:22
@RatanRSur
Copy link
Contributor

Icestanbul is an amazing name.

Signed-off-by: Danno Ferrin <[email protected]>
https: //gitter.im/ethereum/AllCoreDevs?at=5de7b410d64a052fb6b3fa8d
Signed-off-by: Danno Ferrin <[email protected]>
(time, parent, protocolContext) ->
calculateThawedDifficulty(time, parent, MUIR_GLACIER_FAKE_BLOCK_OFFSET);

private static BigInteger calculateThawedDifficulty(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be calculateMuirGlacierDifficulty? As it is it seems like it reduces code discoverability if you're searching for instances of the hard fork, like you could have when it was Byzantium?

Copy link
Contributor Author

@shemnon shemnon Dec 4, 2019

Choose a reason for hiding this comment

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

It's used in 3 different places, Byzantium, Constantinople, and now MuirGlacier. Before it was calculageByzantiumDifficulty.

}

@Test
public void testDifficultyCalculation() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this test not be run in isolation? Getting NullPointerExceptions when running this alone through intellij. Is there a gradle task that downloads resources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How many, if its 3 NPEs You need to update your submodules, if it's more you need to run it via gradle or have IntelliJ add the reference tests resoureces to the classpath when running.

@shemnon shemnon merged commit fc7338f into hyperledger:master Dec 4, 2019
@shemnon shemnon deleted the icestanbul branch December 4, 2019 21:44
edwardmack pushed a commit to ChainSafe/besu that referenced this pull request Feb 4, 2020
Add support for MuirGlacier
* Add block numbers for Ropsten and Mainnet
* Add reference tests for difficulty calculations
* Update reference tests for new MuirGlacier difficulty tests

Signed-off-by: Danno Ferrin <[email protected]>
edwardmack pushed a commit to ChainSafe/besu that referenced this pull request Feb 4, 2020
Add support for MuirGlacier
* Add block numbers for Ropsten and Mainnet
* Add reference tests for difficulty calculations
* Update reference tests for new MuirGlacier difficulty tests

Signed-off-by: Danno Ferrin <[email protected]>
Signed-off-by: edwardmack <[email protected]>
siladu pushed a commit to siladu/besu that referenced this pull request Oct 28, 2024
…ibutes

Engine API: respond with error if payload attributes are invalid
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.

2 participants