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

weak subjecitivity checks #3391

Merged
merged 5 commits into from
Nov 4, 2021
Merged

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Oct 25, 2021

Motivation

The aim of the PR is to rectify weak subjectivity checks:

  • while intializing if DB's finalized is ahead of the provided anchor states finalized (though wssStateFile), use the db's finalized as anchor state instead of again starting from the wss checkpoint
  • add a simpler check on wss's subjectivity period of the anchor state w.r.t. the clockSlot while initializing the chain
    Description

UPDATE:
previous resolution of making the wss check on the head when chain syncs to synced state for the first time was deemed overkill, so reverted that and replaced with the initialization time clockEpoch check.

example error of providing an checkpoint wss subjectivity period behind the clockEpoch
image

Closes #issue_number
partially closes #3385
Steps to test or reproduce

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2021

CLA assistant check
All committers have signed the CLA.

@codeclimate
Copy link

codeclimate bot commented Oct 25, 2021

Code Climate has analyzed commit f068176 and detected 0 issues on this pull request.

View more on Code Climate.

@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #3391 (f068176) into master (aafc5d7) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3391   +/-   ##
=======================================
  Coverage   38.36%   38.36%           
=======================================
  Files         303      303           
  Lines        7738     7738           
  Branches     1157     1157           
=======================================
  Hits         2969     2969           
  Misses       4628     4628           
  Partials      141      141           

@g11tech g11tech force-pushed the wssyncchecks branch 3 times, most recently from 5f0ca18 to 57af146 Compare October 26, 2021 07:24
packages/lodestar/src/chain/eventHandlers.ts Outdated Show resolved Hide resolved
packages/lodestar/test/utils/mocks/chain/chain.ts Outdated Show resolved Hide resolved
packages/cli/src/cmds/beacon/initBeaconState.ts Outdated Show resolved Hide resolved
packages/lodestar/src/chain/emitter.ts Outdated Show resolved Hide resolved
packages/lodestar/src/chain/eventHandlers.ts Outdated Show resolved Hide resolved
packages/lodestar/src/chain/eventHandlers.ts Outdated Show resolved Hide resolved
wemeetagain
wemeetagain previously approved these changes Nov 2, 2021
twoeths
twoeths previously approved these changes Nov 3, 2021
@wemeetagain wemeetagain merged commit 70a534d into ChainSafe:master Nov 4, 2021
@g11tech g11tech deleted the wssyncchecks branch November 4, 2021 17:23
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.

investigate: weak subjectivity checkpoint improper use
5 participants