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

REST: fix timing check for bellatrix epoch. #3807

Merged
merged 1 commit into from
Jun 28, 2022
Merged

REST: fix timing check for bellatrix epoch. #3807

merged 1 commit into from
Jun 28, 2022

Conversation

cheatfate
Copy link
Contributor

No description provided.

@cheatfate cheatfate requested a review from tersec June 27, 2022 11:24
@@ -253,7 +267,15 @@ proc initFullNode(
finalizedEpochRef.eth1_data,
finalizedEpochRef.eth1_deposit_index)
node.updateLightClientFromDag()
eventBus.finalQueue.emit(data)
# TODO (cheatfate): Proper implementation required
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this PR, would this not have been accomplished? Of course, maybe what will be viewed as a proper implementation will change, but I'm not sure what this is standing in for (likewise the other TODO (cheatfate): Proper implementation required comments in similar place).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it always is either none or some(false) -- is it never intended to be some(true)?

What is fixed here is that the optimistic field is included as soon as wall clock passes past bellatrix, instead of this depending on the locally synced DAG head slot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment, yes, it's never intended to be some(true). #3793 might change this, but Nimbus unstable is never an optimistic node, nor does it have optimistically imported blocks.

@github-actions
Copy link

Unit Test Results

     12 files  ±0     857 suites  ±0   1h 14m 48s ⏱️ + 19m 36s
1 711 tests ±0  1 659 ✔️ ±0    52 💤 ±0  0 ±0 
9 940 runs  ±0  9 812 ✔️ ±0  128 💤 ±0  0 ±0 

Results for commit cae3b66. ± Comparison against base commit 91d5434.

@tersec tersec merged commit d1581a2 into unstable Jun 28, 2022
@tersec tersec deleted the timecheck branch June 28, 2022 10:21
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.

None yet

3 participants