-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Don't mark blocks as invalid on context deadlines #14838
Conversation
0935b35
to
365c245
Compare
When processing state transition, if the error is because of a context deadline, do not mark it as invalid.
365c245
to
5d9c3d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont mark the block as invalid due to context error, it's being handle on the cache level. I think it's better to handle this on the cache level...
prysm/beacon-chain/sync/validate_beacon_blocks.go
Lines 413 to 415 in bc69ab8
if ctx.Err() != nil { // Do not mark block as bad if it was due to context error. | |
return | |
} |
@@ -468,6 +468,9 @@ func (s *Service) validateStateTransition(ctx context.Context, preState state.Be | |||
stateTransitionStartTime := time.Now() | |||
postState, err := transition.ExecuteStateTransition(ctx, preState, signed) | |||
if err != nil { | |||
if ctx.Err() == context.DeadlineExceeded { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not handle this for all context error? is deadline the only one? what about timeout?
@@ -468,6 +468,9 @@ func (s *Service) validateStateTransition(ctx context.Context, preState state.Be | |||
stateTransitionStartTime := time.Now() | |||
postState, err := transition.ExecuteStateTransition(ctx, preState, signed) | |||
if err != nil { | |||
if ctx.Err() != nil { | |||
return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we not need a log or error wrapper for context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be logged by the p2p package.
you need to add a changelog |
When processing state transition, if the error is because of a context deadline, do not mark it as invalid.