cl/cltypes, cl/phase1/execution_client: dedupe signed-container SSZ and engine API call boilerplate#22176
Draft
yperbasis wants to merge 3 commits into
Draft
cl/cltypes, cl/phase1/execution_client: dedupe signed-container SSZ and engine API call boilerplate#22176yperbasis wants to merge 3 commits into
yperbasis wants to merge 3 commits into
Conversation
Pure refactor, no behavior change; existing tests and the build are the safety net. All 23 EngineAPIRPCClient methods shared the same shape: allocate result, CallContext, return. Replace with generic call/callValue helpers so each method is a one-liner. getAssembledBlockV4/V5/V6 differed only in the engine method and error string; fold them into getAssembledBlockFromEngine taking the method value and name (error strings unchanged).
Pin the exact EncodeSSZ bytes, HashSSZ root, and EncodingSizeSSZ of every
{Message, Signature} container (SignedVoluntaryExit, SignedBeaconBlockHeader,
SignedBLSToExecutionChange, SignedContributionAndProof,
SignedAggregateAndProof, SignedProposerPreferences,
SignedExecutionPayloadBid, SignedExecutionPayloadEnvelope) for
deterministically-filled instances, plus a decode round-trip through a fresh
instance. These types cross the network; the goldens guard the upcoming
SSZ-method unification byte-for-byte.
Note: SignedExecutionPayloadEnvelope's pinned size (981) is 12 bytes below
its encoded length (993) because ExecutionRequests.EncodingSizeSSZ does not
count its three field offsets - a pre-existing quirk pinned as-is, not fixed
here.
Route the SSZ quartet (encode/decode/size/hash) of the eight
{Message, Signature} container types through encodeSigned/decodeSigned/
sizeSigned/hashSigned instead of each type restating the schema.
sizeSigned derives the 96-byte signature plus, for dynamic messages, the
4-byte offset from Message.Static(), replacing the per-type constants
(96+x, x+96, 100+x, 4+x+96) that all encoded the same rule.
Behavior is byte-identical - pinned by the golden SSZ tests added in the
previous commit (encoding, hash tree root, and encoding size per type).
Per-type decode initialization (always-new vs nil-check, nested
Contribution init, beaconCfg-aware envelope constructor) and the
SignedProposerPreferences nil-Message size guard are kept verbatim.
SignedBeaconBlock/SignedBlindedBeaconBlock are intentionally not unified:
their Block requires per-fork construction and their decode does not
initialize the message.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in Caplin by centralizing (1) Engine API JSON-RPC boilerplate and (2) SSZ encode/decode/size/hash logic shared across multiple {Message, Signature} “signed container” types, while adding golden tests to pin byte-identical SSZ behavior during refactors.
Changes:
- Collapses many near-identical Engine API
CallContextwrappers into genericcall[T]/callValue[T]helpers, and unifiesgetAssembledBlockV4/V5/V6into a single parameterized helper. - Deduplicates SSZ encode/decode/size/hash implementations for multiple signed-container CL types via shared helpers (
encodeSigned,decodeSigned,sizeSigned,hashSigned). - Adds SSZ golden tests to lock down exact encodings, hash roots, and decode/encode round-trips for the signed-container types.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cl/phase1/execution_client/execution_client_engine.go | Replaces per-version assembled-block getters (V4–V6) with a single helper that takes the Engine API method value. |
| cl/phase1/execution_client/engine_api_rpc_client.go | Introduces generic RPC wrappers to remove repetitive CallContext boilerplate across Engine API methods. |
| cl/cltypes/signed_ssz.go | Adds shared SSZ helper functions for signed-container {Message, Signature} types. |
| cl/cltypes/validator.go | Switches SignedVoluntaryExit SSZ methods to use shared signed-container helpers. |
| cl/cltypes/epbs_proposer.go | Switches SignedProposerPreferences SSZ methods/size calculation to shared helpers (preserving nil guard). |
| cl/cltypes/epbs_payload.go | Switches SignedExecutionPayloadBid / SignedExecutionPayloadEnvelope SSZ methods/size calculation to shared helpers. |
| cl/cltypes/contribution.go | Switches SignedContributionAndProof SSZ methods/size calculation to shared helpers. |
| cl/cltypes/bls_to_execution_change.go | Switches SignedBLSToExecutionChange SSZ methods/size calculation to shared helpers. |
| cl/cltypes/beacon_header.go | Switches SignedBeaconBlockHeader SSZ methods/size calculation to shared helpers. |
| cl/cltypes/aggregate.go | Switches SignedAggregateAndProof SSZ methods/size calculation to shared helpers. |
| cl/cltypes/signed_ssz_golden_test.go | Adds golden tests pinning SSZ encoding/hash/size and round-trip decode for signed-container types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+52
| func requireGoldenSSZ(t *testing.T, obj signedContainerSSZ, wantEncHex, wantRootHex string, wantSize int) []byte { | ||
| t.Helper() | ||
| enc, err := obj.EncodeSSZ(nil) | ||
| require.NoError(t, err) | ||
| root, err := obj.HashSSZ() | ||
| require.NoError(t, err) | ||
| require.Equal(t, wantEncHex, hex.EncodeToString(enc)) | ||
| require.Equal(t, wantRootHex, hex.EncodeToString(root[:])) | ||
| require.Equal(t, wantSize, obj.EncodingSizeSSZ()) | ||
| return enc | ||
| } |
Member
Author
|
Filed the follow-up for the pre-existing size bug found while pinning goldens here: #22181 (ExecutionRequests.EncodingSizeSSZ omits its field offsets). The fix will need to update the one pinned EncodingSizeSSZ golden for SignedExecutionPayloadEnvelope. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two consolidations in Caplin, three commits:
Engine API client: 23 methods in
engine_api_rpc_client.gowere identicalCallContextwrappers; they collapse onto genericcall[T]/callValue[T]one-liners (the second needed for slice-returning methods).getAssembledBlockV4/V5/V6fold into one version-parameterized function taking theGetPayloadVnmethod value; V3 keeps its extra nil-payload logic. Interface unchanged; transport-only, no consensus logic. Net −91 lines.Signed containers: eight
{Message, Signature}types (SignedVoluntaryExit,SignedBeaconBlockHeader,SignedBLSToExecutionChange,SignedContributionAndProof,SignedAggregateAndProof,SignedProposerPreferences,SignedExecutionPayloadBid,SignedExecutionPayloadEnvelope) repeated the same SSZ quartet. Shared free helpers (encodeSigned/decodeSigned/sizeSigned/hashSigned) now carry it;sizeSignedstates the actual rule once (96 + msgSize + 4 iff the message is dynamic), replacing the ad-hoc per-type constants (96+x,x+96,4+x+96) — all verified equivalent. Exported structs, JSON tags, and per-type decode inits are untouched.SignedBeaconBlock/SignedBlindedBeaconBlockdeliberately skipped (per-fork construction; different nil-guards).Safety net, committed green before the refactor:
signed_ssz_golden_test.gopins, per type, the fullEncodeSSZhex,EncodingSizeSSZ,HashSSZroot, and a decode round-trip on deterministically-filled instances (+299 lines). A mutation check (sizeSigned+1) flips all eight red, so the harness genuinely guards the helpers.Pre-existing bug found while pinning, preserved as-is and worth a follow-up issue:
ExecutionRequests.EncodingSizeSSZ(cl/cltypes/execution_requests.go:43) omits its three 4-byte field offsets, soSignedExecutionPayloadEnvelope.EncodingSizeSSZunder-reports by 12 (981 vs 993 encoded). Same class as theBlindedBeaconBodysize bug fixed in #22165.Verified: full
go test ./cl/...green, goldens byte-identical pre/post, scopedgolangci-lintclean (×2),make erigon. (cl/spectestvectors not present locally; CI covers.)Part of a dedup series; siblings #22165–#22175.