Fix Post Submit E2E: change getblockroot REST api optimistic check #15015
Conversation
james-prysm
added
Bug
| b32Root := bytesutil.ToBytes32(root) | ||
| isOptimistic, err := s.OptimisticModeFetcher.IsOptimisticForRoot(ctx, b32Root) | ||
| isOptimistic, err := s.OptimisticModeFetcher.IsOptimistic(ctx) |
There was a problem hiding this comment.
I am assuming here that the gRPC implementation is correct in this case. if not then the evaluator may not be correct either
There was a problem hiding this comment.
I am missing something. The input request is a root. When we query optimistic status, without specifying it, how do you know if the root is optimistic?
There was a problem hiding this comment.
@terencechain The response is a root, the request can be any of "head" (canonical head in node's view), "genesis", "finalized", , <hex encoded blockRoot with 0x prefix>. and b32Root is the root that was calculated from the input (by fetching the head block from the DB, or the block at the passed in slot etc)
There was a problem hiding this comment.
Sure. All I am saying is say I am syncing two branches. Branch A is optimistic and branch B is non-optimistic
Given the input, I could select branch A or branch B. But without IsOptimisticForRoot the ForRoot, how do you know if it needs to select branch A or B?
james-prysm
changed the title
| @@ -0,0 +1,3 @@ | |||
| ### Fixed | |||
|
|
|||
| - fixes e2e introduced by PR#14997 by changing the optimistic check in getblockroot for rest APIs. | |||
There was a problem hiding this comment.
I would change this as this actually fixes two existing bugs. The failing test must be wrong or have a bad assumption on head. The Sync committee production is a bad bug as well, needs to be commented here with the fix.
|
Discussed on huddle, change to block root API is incorrect, @potuz will fix in a followup PR, I'll open a new one to add the optimistic check to the beacon endpoint. |
|
Follow up is done in #15021 |
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
#14997 happened to break E2E post submit for REST API tests. upon investigation the issue was caused by
validator_sync_participation_0failing fromlevel=error msg="Could not get sync committee contribution" error="failed to get sync message block root: the node is currently optimistic and cannot serve validators"This is caused by a difference between gRPC and REST optimistic checks, and for some reason PR14997 changed this.gRPC implementation : https://github.com/prysmaticlabs/prysm/blob/014dbd5c3af3dde8de6dafab9d2e15e523e97167/beacon-chain/rpc/prysm/v1alpha1/validator/sync_committee.go#L16
REST implementation:
https://github.com/prysmaticlabs/prysm/blob/6b84f8c6b1e73254d23cca9ac31a567723b0bddc/beacon-chain/rpc/eth/beacon/handlers.go#L1163
Which issues(s) does this PR fix?
Fixes #
Other notes for review
Acknowledgements