Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Remove difficulty bomb for Atlantis difficulty calculation #69

Merged
merged 3 commits into from
Jun 18, 2019

Conversation

austinabell
Copy link
Contributor

Remove difficulty bomb interactions as ETC had removed their difficulty bomb.

#68 #67

@austinabell austinabell requested a review from noot as a code owner June 17, 2019 22:40
@austinabell
Copy link
Contributor Author

Circle CI doesn't pull the ethereum tests submodule but now basically all of the difficulty tests are failing since it assumes the bomb delay remains. I will think of the best way to fix these tests and generate new tests to keep the same quantity of test.

@austinabell
Copy link
Contributor Author

To be able to use difficulty tests from the ETH submodule for EIP 100, tests have to be skipped at and after 3_200_000 because that is when the ETH tests submodule difficulty bomb logic kicks in:

	if config.IsAtlantis(parentNumber) && parent.Number.Cmp(big.NewInt(3199999)) >= 0 {
		return nil
	}

let me know if you guys think I should handle these tests differently or if there exists somewhere we can test the classic difficulty to ensure sync with other clients. I think these tests and the knowledge that all tests pass before removing difficulty bomb logic is sufficient, but we should discuss and make sure this is correct before merging in. Should also make sure we are compliant with any potential bugs referenced in #68 and try to improve the existing testing (because ETC does not test very meaningful things)

@austinabell austinabell changed the title Removed difficulty bomb for atlantis difficulty calculation Remove difficulty bomb for Atlantis difficulty calculation Jun 17, 2019
@noot noot merged commit d8c1243 into development Jun 18, 2019
@noot noot deleted the austin/difficultybombfix branch June 18, 2019 14:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants