-
Notifications
You must be signed in to change notification settings - Fork 428
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
Set head according to fcuState even when attributes are incorrect #6144
Conversation
d0dc381
to
7a2f91f
Compare
ValidateAttributes(payloadAttributes, version) ?? | ||
payloadUpdateResult ?? | ||
StartBuildingPayload(newHeadBlock!, forkchoiceState, payloadAttributes) | ||
); |
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.
This looks wrong to me; the wrong version is called and we are doing everything and only returning an error at the end. Sounds wrong. Did you consider that the hive test is poorly constructed and should be amended? Did you discuss it with Hive test guys? Did you discuss it as spec? |
Wrong version of what? It returns result if presents, result can be an error or SYNCING/VALID, not just error. Let's call?
|
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.
I hate the spec and would like to change it, but the PR looks good
Fixes Closes Resolves hive test
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?