Skip to content

Conversation

@mkalinin
Copy link
Contributor

Proposes to extend an optimistic node definition with the following condition:

  • If fork choice state of a node ended up without viable branches in a block tree because blocks from every such branch were transitioned from NOT_VALIDATED to INVALIDATED

The motivation of this change is to prevent different and potentially dangerous behaviours handling this situation. For instance, CL implementation may revert its justified checkpoint in the store back to the previous one which is prone to surround voting. By stating that node must stay optimistic in this case we protect from such cases.

This exact statement attempts to discern the optimistic sync nature of no viable branch state from any other ways in which a node may come to the same state. In cases when a node comes to a state without viable branches in a way that is not related to optimistic sync, a node should not be an optimistic node.

cc @paulhauner @potuz @ajsutton

@mkalinin
Copy link
Contributor Author

Suppose CL is receiving the following statuses from EL:

[0: C', VALID] <- [1: VALID] <- ... <- [24: VALID] <- [31: Justifies C', SYNCING] <- [0: C'', SYNCING]

And eventually SYNCING resolves as follows:

[0: C', VALID] <- [1: VALID] <- ... <- [24: VALID] <- [31: Justifies C', INVALID] <- [0: C'', INVALID]

CL removes 31 and 0: C'' from its block tree leaving store.justified_checkpoint == C' and no branch satisfying it. In this case get_head returns C'.root as per the spec making honest validators propose atop and attest to C'.root instead of [24: VALID] (or a descendant of this block).

This PR suggest to keep a node optimistic if it came into this state via optimistic sync (as in the above scenario).

Alternatively, we may keep a node syncing (not optimistically) if it came into this state despite of a way the node did it. The idea is to prevent such a node from serving its validator client until it either recovers automatically by pulling the other viable chain from the network and switching to it or does the same after restart.

Co-authored-by: terencechain <terence@prysmaticlabs.com>
@mkalinin
Copy link
Contributor Author

Summoning @tersec to get his input on this proposal

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.

3 participants