-
Notifications
You must be signed in to change notification settings - Fork 114
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
Unit tests for the Blockstore #1129
Conversation
if (hash == null) { | ||
return null; | ||
return blockFamily; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change will let AionBlockChainImpl.isValid() throw null exception. Please have a check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the null
check in AionBlockchainImpl.isValid
to exit if the parent block is null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool.
if (hash == null) { | ||
return null; | ||
return blockFamily; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will let AionBlockChainImpl.createNewMiningBlockInternal() throw NPE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would already throw an NPE because the blockFamily is accessed without a null check. The reason for the NPE differs but it will be thown in either case. If that happens I think we should allows the failure and fix the cause when we find one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So We should just throw NPE if the input hash equal to null. instead of return values (or return null)
Same as the other method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I made the update.
ac137bc
to
3cdd7e3
Compare
The method was incorrectly returning all blocks flagged as main chain.
3cdd7e3
to
5fb5e03
Compare
Type of change