chore(pytest): temporary consume engine tweak for nimbul-el #1425
Conversation
spencer-tb
added
scope:pytest
|
Assuming this gets approved I will create an issue to revert this change once nimbus updates. |
There was a problem hiding this comment.
I'm hesitant to add this check to all clients since it is out of spec.
I would incline to add a context-sensitive validator that checks if the client is the nimbus-el, and only then allow this out-of-spec behavior.
I added context to the rpc response validation (for exceptions not client-specific validations) in this PR: https://github.com/ethereum/execution-spec-tests/pull/1416/files
We could cherry-pick the context changes into this PR and then apply these relaxed validation only to nimbus-el.
| @@ -84,14 +84,40 @@ class PayloadStatus(CamelModel): | |||
|
|
|||
| status: PayloadStatusEnum | |||
| latest_valid_hash: Hash | None | |||
| validation_error: str | None | |||
| # TODO: Temporarily optional to allow nimbus-el to run `consume-engine` | |||
| validation_error: Optional[str] = None | |||
There was a problem hiding this comment.
Was there an error with str | None = None?
There was a problem hiding this comment.
Ahh will try this
| # Allows us to run `consume-engine` for nimbus-el. | ||
| @model_validator(mode="before") | ||
| @classmethod | ||
| def ensure_validation_error(cls, data): |
There was a problem hiding this comment.
Couldn't this validator be added to PayloadStatus?
There was a problem hiding this comment.
Let me try this as well thanks
🗒️ Description
Currently
consume-engineis not compatible withnimbus-el. These are due to the following points:For each
forkchoiceUpdateandnewPayloadrequest we send, each response should contain thePayloadStatusV1within it. For nimbusPayloadStatusV1is missing thevalidationErrorfield for VALID responses (note it exists for INVALID).For nimbus the
forkchoiceUpdateresponse doesn't include thepayloadId.Our pydantic models error out due to the latter. This PR adds a temporary workaround that allows us to run
consume-enginefor nimbus, without breaking other clients.🔗 Related Issues
N/A
✅ Checklist
All: Added an entry to CHANGELOG.md.