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

Handing pending atts if they dont have the state #4904

Merged
merged 4 commits into from Feb 19, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Feb 19, 2020

Here's the problem, when a node receives an attestation and it doesn't have the block. It requests the block. It will then go ahead to validate the signature in sync without waiting for state to be processed. Breaking this down into time line:

1.)

[2020-02-18 09:23:15]  INFO blockchain: Executing state transition on block root=0x6b34ab0d... slot=286016

2.)

[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist
[2020-02-18 09:23:16] ERROR blockchain: Failed to validate attestation error=pre state of target block 286016 does not exist

3.)

[2020-02-18 09:23:16]  INFO blockchain: Starting next epoch epoch=8938 finalizedEpoch=8934 justifiedEpoch=8936 previousJustifiedEpoch=8936
[2020-02-18 09:23:16]  INFO blockchain: Validator registry information activeValidators=41660 averageBalance=3.17437 ETH totalValidators=41667
[2020-02-18 09:23:16]  INFO blockchain: Finished applying state transition attestations=1 deposits=0 slot=286016

3 needs to happen before 2. This PR adds the HasState check around pending attestation queue to ensure the signature validation has the state in the DB

@terencechain terencechain changed the title Hold pending atts if they dont have the state Handing pending atts if they dont have the state Feb 19, 2020
@terencechain terencechain self-assigned this Feb 19, 2020
@terencechain terencechain added the Bug Something isn't working label Feb 19, 2020
@nisdas nisdas added the Priority: Critical Highest, immediate priority item label Feb 19, 2020
@codecov
Copy link

codecov bot commented Feb 19, 2020

Codecov Report

Merging #4904 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4904   +/-   ##
=======================================
  Coverage   29.13%   29.13%           
=======================================
  Files         206      206           
  Lines       15674    15674           
=======================================
  Hits         4566     4566           
  Misses      10308    10308           
  Partials      800      800           

@terencechain terencechain merged commit 731cc0b into master Feb 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the hold-atts-without-state branch February 19, 2020 14:46
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Priority: Critical Highest, immediate priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants