feat(test-specs): add container definitions for ssz-engine api#2
feat(test-specs): add container definitions for ssz-engine api#2RazorClient wants to merge 6 commits into
Conversation
3efbb2f to
9ea9046
Compare
There was a problem hiding this comment.
this file does the work of https://github.com/ethereum/consensus-specs/blob/master/tests/core/pyspec/eth_consensus_specs/debug/random_value.py and is needed to have the static ssz tests with https://github.com/ethereum/consensus-specs/blob/master/tests/core/pyspec/eth_consensus_specs/test/phase0/ssz_static/test_ssz_static.py in the next prs
2dfeb53 to
aaa910b
Compare
aaa910b to
351d603
Compare
| BuiltPayloadAmsterdam, | ||
| BuiltPayloadCancun, | ||
| BuiltPayloadOsaka, | ||
| BuiltPayloadParis, | ||
| BuiltPayloadPrague, | ||
| BuiltPayloadShanghai, |
There was a problem hiding this comment.
Should the BuiltPayload containers really exist? Will clients have this?
| from .constants import ( | ||
| BYTES_PER_BLOB, | ||
| BYTES_PER_CELL, | ||
| CELLS_PER_EXT_BLOB, | ||
| MAX_BAL_BYTES, | ||
| MAX_BLOB_COMMITMENTS_PER_BLOCK, | ||
| MAX_BLOBS_REQUEST, | ||
| MAX_BODIES_REQUEST, | ||
| MAX_BYTES_PER_EXECUTION_REQUEST, | ||
| MAX_BYTES_PER_TX, |
There was a problem hiding this comment.
Here, I would actually suggest doing from .constants import *. Maybe EELS will disagree. I expect everything from that file will always be imported/used.
There was a problem hiding this comment.
star exporting would work if we use all the exports. If not, then ruff marks every use as F405 (it can't trace names through import *)
| def envelope_bytes( | ||
| fork: str, | ||
| *, | ||
| parent_hash: bytes, | ||
| fee_recipient: bytes, | ||
| state_root: bytes, | ||
| receipts_root: bytes, | ||
| logs_bloom: bytes, |
There was a problem hiding this comment.
envelope_bytes is intentionally a plain-values builder: callers pass ordinary bytes/int, and it wraps them into the SSZ containers internally, so plain parameter types are the arguments
| """ | ||
| The Osaka execution payload. | ||
| """ |
There was a problem hiding this comment.
Docstrings like these are not useful. I would suggest removing docstrings from all containers in this file.
There was a problem hiding this comment.
The repo has pydocstyle D101 globally enabled, so every public class must have a docstring.
SSZ container definitions and helpers for the REST+SSZ Engine API refactor execution-apis PR.
This is foundational plumbing for the upcoming REST+SSZ blockchain-test support;
it does not add any fillable test cases yet.
🔗 Related Issues or PRs
Tracks execution-apis PR
✅ Checklist
uvx tox -e static
Cute Animal Picture