Forkchoice: expose if no tip is viable #11153
Conversation
This PR changes the behavior on when the node is considered optimistic. A call to `blockchain.IsOptimistic()` relies solely on forkchoice, if all tips are invalid, then it's optimistic. If the current headroot is not in forkchoice then it's optimistic. A call to `blockchain.IsOptimisticForRoot()` will return true if the requested root is headroot and it's not found in forkchoice
| headRoot, err := s.HeadRoot(ctx) | ||
| if err != nil { | ||
| return true, err | ||
| } | ||
| if bytes.Equal(headRoot, root[:]) { | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
This is confusing. I think often readers will miss the fact that we only end up here if the root is not in fork choice store
There was a problem hiding this comment.
I'll update the comment
| ss, err := s.cfg.BeaconDB.StateSummary(ctx, root) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| if ss == nil { |
There was a problem hiding this comment.
Why not do this so we don't have to worry about s?HeadRoot at all
| if ss == nil { | |
| if ss == nil { | |
| if s.cfg.ForkChoiceStore.AllTipsAreInvalid() { | |
| return true, nil | |
| } |
There was a problem hiding this comment.
because forkchoice's AllTipsAreInvalid may fail if there was no call to Head() between the prunning and the call here.
There was a problem hiding this comment.
But I agree that it's such a long shot that someone requests by root (instead of just IsOptimistic) exactly at this same time. I'll let you decide, if you want me to switch this to avoid the comparison I'm happy with it... this is not really a hot path
There was a problem hiding this comment.
I'm just thinking future proof. What if we were to remove HeadRoot from blockchain's chaininfo one day and default everything to the fork choice store's head?
There was a problem hiding this comment.
Ah but the problem of doing the suggestion is that someone may actually be requesting by root an old block, we should reply correctly instead of "true" if forkchoice is in an incoherent state
This PR changes the behavior on when the node is considered optimistic.
A call to
blockchain.IsOptimistic()relies solely on forkchoice, ifall tips are invalid, then it's optimistic. If the current headroot is
not in forkchoice then it's optimistic.
A call to
blockchain.IsOptimisticForRoot()will return true if therequested root is headroot and it's not found in forkchoice