Implement weak subjectivity safety checks
Issue Addressed
Closes #7273
Proposed Changes
https://github.com/ethereum/consensus-specs/pull/4179
hey @dapplion when you have a chance, could you take a look at what I have so far?
The general flow is
- user sets the weak subjectivity period checkpoint via the existing
wss-checkpointflag - on startup, we make the weak subjectivity calculation, using the user provided ws checkpoint, the head snapshot and the most recent finalized checkpoint
If the user doesn't set a weak subjectivity checkpoint, what are we supposed to do here?
If the user doesn't set a weak subjectivity checkpoint, what are we supposed to do here?
We should assert that the initial state is within the weak subjectivity period on any type of start.
We should also offer a flag such that
- If flag set and starting from outside weak subjectivity period: big warn log, continue start
- If flag not set and starting from outside weak subjectivity period: error, abort start
Thanks for all the help Lion. I've made changes based on your feedback and added additional test coverage
This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏
Some required checks have failed. Could you please take a look @eserilev? 🙏
Some required checks have failed. Could you please take a look @eserilev? 🙏
There are some tests failures. Maybe we could use a small non-zero constant pre-Electra to smooth over this issue, or we could update these tests to only run post-Electra.
Yeah I'm going to fix theses tests to run post Electra
InvalidPayloadRig wasnt using the fork_from_env stuff when constructing spec so we weren't actually running related tests against RECENT_FORKS in some cases.
revert_minority_fork_on_resume was an annoying test to try to get right when setting the WS calculation to 0 pre-electra. Setting the calculation to a min of 5 fixes the test.
basic sim and fallback sim were also failing and I havent had a chance to look at why. I've ramped up the pre electra ws calc to 256 to fix this (5 was too small of a number apparently).