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

Avoid generating genesis state in blockchain test runner #536

Merged
merged 1 commit into from
May 29, 2019

Conversation

s1na
Copy link
Contributor

@s1na s1na commented May 29, 2019

blockchain tests don't need to generate the gensis state as they come with their own pre world state. Avoiding this extra generation resulted in the duration of npm run testBlockchain going from 17m down to 2m on my local machine 😁

This is done via a trick: runBlockchain generates the genesis state when current state root doesn't match with the genesis state's root. So I manually manipulate vm._common.genesis().stateRoot to avoid generating the state.

I checked the source code, and this shouldn't have a side-effect (common.genesis().stateRoot doesn't seem to be used elsewhere). But still this is a trick and ideally runBlockchain would accept an option to disable generating genesis state (which would be a breaking change because it changes the parameter ordering, so I didn't go for it).

@holgerd77
Copy link
Member

Ups. 😀 What a great improvement! Phew.

Can you rebase, the branch is outdated?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.992% when pulling 5be8a1d on test/bc-genesis-state into c3311b6 on master.

@s1na
Copy link
Contributor Author

s1na commented May 29, 2019

Updated :)

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good.

@holgerd77 holgerd77 merged commit c2a5cec into master May 29, 2019
@holgerd77 holgerd77 deleted the test/bc-genesis-state branch May 29, 2019 16:08
@s1na s1na mentioned this pull request Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants