Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Fix BlockHeader.prototype.validate() bug #49

Merged
merged 1 commit into from
Aug 8, 2018
Merged

Conversation

vpulim
Copy link
Contributor

@vpulim vpulim commented Jul 28, 2018

Fixes a this instead of self bug in BlockHeader.prototype.validate().

Needs tests, but I wanted to submit a PR to highlight the fix.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.234% when pulling 22729b8 on validate-fix into 16aba3e on master.

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

This looks good to me. The code is within a callback, where the outer scope's this wouldn't be valid.

@vpulim
Copy link
Contributor Author

vpulim commented Jul 30, 2018

Thanks @axic. Once this is merged, can we do a 2.1.0 release so that ethereumjs/ethereumjs-blockchain#59 can be merged?

@holgerd77
Copy link
Member

@vpulim Hi Vinay, just had a short look, does this really justify a 2.1.0 release or is 2.0.1 enough (I have not strong opinion)?

@axic
Copy link
Member

axic commented Aug 7, 2018

Definitely 2.0.1, 2.1.0 would mean a breaking change in semver and thus users wouldn't get the bugfix.

@vpulim
Copy link
Contributor Author

vpulim commented Aug 7, 2018

@axic, @holgerd77 Yes, 2.0.1 makes more sense since it’s just a bug fix. Thanks.

@holgerd77 holgerd77 merged commit daf6c8d into master Aug 8, 2018
@holgerd77 holgerd77 deleted the validate-fix branch August 8, 2018 09:01
@holgerd77
Copy link
Member

Ok, just released this as v2.0.1: https://github.com/ethereumjs/ethereumjs-block/releases/tag/v2.0.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants