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

Update Validity Conditions to Match Spec #1611

Merged
merged 4 commits into from
Feb 15, 2019
Merged

Update Validity Conditions to Match Spec #1611

merged 4 commits into from
Feb 15, 2019

Conversation

rauljordan
Copy link
Contributor

@rauljordan rauljordan commented Feb 15, 2019

No tracking issue


Description

Write why you are making the changes in this pull request
Currently, our block validity conditions had 4 conditions even though the spec on master has 3. One of them was wrong, as we previously had that:

Verify time.Now() > state.genesis_time + block.slot* SLOT_DURATION.

but instead we should have

Verify time.Now() > state.genesis_time + (block.slot-GENESIS_SLOT)* SLOT_DURATION.

Additionally, we were checking that the DepositRoot of the ETH1 data existed in the powchain, but instead we should be checking if BlockHash points to a block.

@codecov
Copy link

codecov bot commented Feb 15, 2019

Codecov Report

Merging #1611 into master will decrease coverage by 0.16%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master    #1611      +/-   ##
==========================================
- Coverage   71.73%   71.56%   -0.17%     
==========================================
  Files          94       94              
  Lines        6630     6465     -165     
==========================================
- Hits         4756     4627     -129     
+ Misses       1465     1433      -32     
+ Partials      409      405       -4

@rauljordan rauljordan merged commit d174c4e into master Feb 15, 2019
@rauljordan rauljordan deleted the update-validity branch February 15, 2019 19:49
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