Skip to content

Feature/networking#2

Merged
tkstanczak merged 35 commits into
masterfrom
feature/networking
Feb 26, 2018
Merged

Feature/networking#2
tkstanczak merged 35 commits into
masterfrom
feature/networking

Conversation

@tkstanczak
Copy link
Copy Markdown
Member

merging all the networking work to master so we can follow with the .NET Core transition without too many conflicts

tkstanczak and others added 30 commits January 25, 2018 18:19
@LukaszRozmej LukaszRozmej mentioned this pull request May 7, 2026
6 tasks
@claude claude Bot mentioned this pull request May 7, 2026
14 tasks
LukaszRozmej added a commit to Dyslex7c/nethermind that referenced this pull request May 8, 2026
…leanups

Verified the implementation against the SSZ-REST spec
(execution-apis PR NethermindEth#764, ssz-encoding.md "HTTP status codes" + per-endpoint
condition tables) and corrected drifts:

- Malformed SSZ body returns 400 Bad Request, not 422. The spec is explicit
  ("Bad Request | Malformed SSZ encoding"); 422 is reserved for "Invalid
  payload attributes" which already routes through ErrorCodeToHttpStatus
  on the InvalidPayloadAttributes engine error code. Reverts the round-5 NethermindEth#2
  change which had this backwards.
- TryParsePayloadId now enforces the spec's Bytes8 constraint (exactly
  16 hex chars after stripping "0x"); previously accepted any even-length
  hex. Per spec security considerations: "{payload_id} MUST be validated
  as a well-formed hex-encoded Bytes8 before processing."
- TryParsePayloadId switches to Convert.FromHexString returning
  OperationStatus, removes the try/catch on FormatException, takes
  ReadOnlySpan<char> instead of string.

Other cleanups touched here:
- SszMiddleware.TryRoute walks the path span once with a previous-was-slash
  flag, replacing the separate char-class scan + Contains("//") pass.
- SszMiddleware uses HttpMethods.IsPost / IsGet instead of hand-rolled
  case-insensitive string comparisons.
- SszEndpointHandlerBase: drop the (byte[], int) tuple WriteSszPooledAsync
  overload and the matching Func<T, (byte[], int)> WriteSszResultAsync
  overload — every caller is on the ArrayPoolSpan<byte> path now.
- SszType static constructor → collection-expression initializer for
  BasicTypes (cosmetic).
- HaveCompatibleProgressiveContainerLayout: one walk of left members,
  building two right-side dictionaries up front.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LukaszRozmej added a commit that referenced this pull request May 11, 2026
* feat(merge): add SSZ-REST transport for Engine-API

fix formatting, error in test

reduce repetitions, add declarative routing

address claude review comments

reuse domain types for SSZ

fix failing tests and code lint

introduce SSZ wrapper types for ExecutionPayload

address review comments

address claude review comments

fix SSZ field ordering mismatch

modularize SSZ middleware

fix file naming issue

address claude review comments

revert ExecutionPayload

* address minor review comments

* test deduplication / simplification

* code deduplication / simplification

* performance / allocation

* path to ~zero-alloc on the request path

* remove byte allocation in DecodeGetBlobsRequest

* address claude review comments

* address all review comments

* format whitespace

* address all review comments

* address all review comments

* address round 5 review comments

* address claude review comments

* address review comments: ArrayPoolSpan, rename wiretypes, string allocation, FrozenDictionary

* minimize delegates

* address review comments

* add shadowing test

* address claude review comments

* fix build errors

* address claude review comments

* remove allocation in AsReadOnlyList

* address claude review comments

* address deep review comments

* fix build errors

* address review comments

* address claude review comments

* address review comments

* address deep review comments

* address review comments

* update lock file

* address review comments

* address review comments

* simplify tests

* simplify ForkchoiceUpdatedSszHandler

* refactor: replace IEnumerable with IReadOnlyList in payload bodies/blobs return types

Change return types of `engine_getPayloadBodies{ByHash,ByRange}V{1,2}` and
`engine_getBlobsV{1,2,3}` from `IEnumerable<T?>` to `IReadOnlyList<T?>`. The
inner handlers (`GetPayloadBodiesBy{Hash,Range}V{1,2}Handler`, `GetBlobsHandler`,
`GetBlobsHandlerV2`) now return materialized arrays instead of yield-return
iterators. `BlobsV{1,2}DirectResponse` implement `IReadOnlyList<T?>` so the
streaming JSON path is unchanged. Removes the cast-or-materialize ceremony in
SSZ handlers and lets `SszEndpointHandlerBase.AsReadOnlyList<T>` be deleted.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: descriptor pattern for NewPayload + ForkchoiceUpdated SSZ handlers

Replace the version switches in NewPayloadSszHandler (V1..V5) and
ForkchoiceUpdatedSszHandler (V1..V4) with the static-abstract descriptor
pattern already used by GetPayloadSszHandler. Each version is now one
read-only struct implementing INewPayloadVersion<TWire> /
IForkchoiceUpdatedVersion<TWire>. The handlers are one-line generic
dispatches; the JIT specialises each instantiation. Adding Vn+1 is one
new descriptor + one DI line — no edits to the handler, no edits to
SszCodec, no version-ceiling check.

Removes SszCodec.DecodeNewPayloadRequest and DecodeForkchoiceUpdatedRequest
switches entirely; descriptors decode their own wire types.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: descriptor pattern for remaining versioned SSZ-REST handlers

Migrate GetBlobs, GetPayloadBodiesByHash, GetPayloadBodiesByRange,
Capabilities, and TransitionConfiguration handlers to call IEngineRpcModule
instead of inner domain handlers — closes "always go through engineAPI" review
ask. Each versioned handler now uses the static-abstract descriptor pattern
already established for GetPayload / NewPayload / FCU.

Drops the THandler generic from GetPayloadBodiesByRangeSszHandler. Deletes
all Bridge<IHandler<...>> / Bridge<IAsyncHandler<...>> registrations in
SszMiddlewareConfigurer except the IEngineRpcModule bridge itself.

Replaces hardcoded version literals in descriptors with EngineApiVersions
constants; adds GetBlobs / PayloadBodiesByHash / PayloadBodiesByRange nested
classes plus Latest constants for the new groups (and GetPayload).

Changes engine_exchangeCapabilities return type from IEnumerable<string> to
IReadOnlyList<string> so the SSZ encoder no longer needs the cast-or-materialize
ceremony; ExchangeCapabilitiesHandler materializes once at the source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf: dedup MerkleizeRefType*, stackalloc small chunks, cache enabled-cap list

- Consolidate `MerkleizeRefTypeVector` / `List` / `ProgressiveList` into one
  `MerkleizeRefTypeCore` body switched by an enum; the per-method overloads
  retain their original signatures.
- `MerkleizeProgressiveBytes` now stackallocs the chunk buffer for ≤4 chunks
  (≤128 bytes), the common case for fixed-shape progressive containers.
  Falls back to `ArrayPool<UInt256>` for larger payloads.
- `ExchangeCapabilitiesHandler` caches the enabled-capability list. The
  provider builds its capability dictionary once on first call and the
  enabled subset is stable, so projecting it once amortizes across all
  subsequent fcU/`engine_exchangeCapabilities` invocations. Replaces the
  previous per-call LINQ chain with a direct loop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: extract per-body wire builder for payload bodies encoders

Pull `ToBodyWire` helpers out of `EncodePayloadBodies{V1,V2}Response`. The
encode loops still differ (the wrapper types — `NullablePayloadBodyV{1,2}Wire`,
`PayloadBodiesV{1,2}ResponseWire` — are version-specific by SSZ source-gen
constraint) but the per-element projection no longer duplicates the
Transactions/Withdrawals fields across the two methods.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: convert *ToWire/*FromWire helpers to extension methods

Move all domain↔wire conversions out of `SszCodec` into a dedicated
`internal static class WireConversionExtensions`. Methods are scoped
internal so they only surface inside the assembly and don't pollute
IntelliSense for unrelated generic-collection callsites.

Call sites in `SszCodec` and the per-version descriptors now use the
fluent form: `b.Transactions.ToTxsWire()`, `wire.ParentBeaconBlockRoot`,
`r.BlobsBundle.ToWire()`, `wire.ExpectedBlobVersionedHashes.ToBytesArrays()`,
etc. — replacing the noisier `SszCodec.HashesFromWire(...)` style.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: spec-conform per execution-apis #764 + small cleanups

Verified the implementation against the SSZ-REST spec
(execution-apis PR #764, ssz-encoding.md "HTTP status codes" + per-endpoint
condition tables) and corrected drifts:

- Malformed SSZ body returns 400 Bad Request, not 422. The spec is explicit
  ("Bad Request | Malformed SSZ encoding"); 422 is reserved for "Invalid
  payload attributes" which already routes through ErrorCodeToHttpStatus
  on the InvalidPayloadAttributes engine error code. Reverts the round-5 #2
  change which had this backwards.
- TryParsePayloadId now enforces the spec's Bytes8 constraint (exactly
  16 hex chars after stripping "0x"); previously accepted any even-length
  hex. Per spec security considerations: "{payload_id} MUST be validated
  as a well-formed hex-encoded Bytes8 before processing."
- TryParsePayloadId switches to Convert.FromHexString returning
  OperationStatus, removes the try/catch on FormatException, takes
  ReadOnlySpan<char> instead of string.

Other cleanups touched here:
- SszMiddleware.TryRoute walks the path span once with a previous-was-slash
  flag, replacing the separate char-class scan + Contains("//") pass.
- SszMiddleware uses HttpMethods.IsPost / IsGet instead of hand-rolled
  case-insensitive string comparisons.
- SszEndpointHandlerBase: drop the (byte[], int) tuple WriteSszPooledAsync
  overload and the matching Func<T, (byte[], int)> WriteSszResultAsync
  overload — every caller is on the ArrayPoolSpan<byte> path now.
- SszType static constructor → collection-expression initializer for
  BasicTypes (cosmetic).
- HaveCompatibleProgressiveContainerLayout: one walk of left members,
  building two right-side dictionaries up front.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: factory methods for BasicTypes initializer

Extract two helpers in SszType:

- Primitive<T>() — uses typeof(T) to pull Name/Namespace, derives StaticLength
  from Marshal.SizeOf<T> with a bool override (Marshal returns 4 for bool due
  to native marshaling, but SSZ encodes it as 1). Covers byte/ushort/int/uint/
  long/ulong/bool — 7 entries become 7 one-line calls.
- BytesRef(ns, name, byteLength) — captures the recurring "encode via
  .Bytes.CopyTo, decode via new T(span)" pattern shared by Hash256, Address,
  and Bloom. The decode template embeds the type name via interpolation, so
  adding another fixed-length-byte ref type is one line.

UInt256 (custom encode/decode templates) and SszBytes32/BitArray (no shared
shape) stay as inline initializers. 100+ lines of object-initializer noise
collapsed to 30 lines + two factories.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf: thread ReadOnlyMemory<char> for path 'extra' end-to-end

Change ISszEndpointHandler.HandleAsync from `string extra` to
`ReadOnlyMemory<char> extra`. The middleware now slices the original
Path.Value string via `path.AsMemory(offset)` — zero allocation per
request, regardless of whether the handler uses `extra`. Previously
allocated `extraSpan.ToString()` unconditionally for every request.

TryRoute / TryResolveHandler updated accordingly: routing operates on
ReadOnlySpan internally for FrozenDictionary lookups, returns
ReadOnlyMemory at the boundary. The trace and 404-error interpolations
use `.Span` to skip the ToString() allocation that ROM<char> would
otherwise trigger inside an interpolation handler.

Also: TryParsePayloadId now stackallocs its 8-byte decode buffer so a
malformed hex never allocates. The successful path still allocates one
byte[8] — that crossing into IEngineRpcModule (which JSON-RPC binds via
byte[]) is the unavoidable cost.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* simplify SSZ KeyValueStore

* style: dotnet format whitespace fix for spread operators

CI Check whitespaces job required `..` spread operators in collection
expressions to be followed by a space (`.. SszRestPaths…` instead of
`..SszRestPaths…`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: capabilities HashSet, handler dedup, SszPayloadStatus, lint cleanup

ExchangeCapabilitiesHandler:
- Take HashSet<string> instead of IEnumerable<string> (O(1) Contains
  replaces O(N*M) LINQ Any nested scan).
- Single foreach with deconstruction; nested ifs collapse the missing-check
  into the isEnabled branch.
- Caching the enabled-list build into the same loop (only built on first
  call; later calls just walk for missing-check).
- EngineRpcModule converts IEnumerable -> HashSet at the JSON-RPC boundary
  so the interface stays binding-compatible.

SSZ handler boilerplate:
- GetBlobsV2/BodiesByHash/BodiesByRange/GetPayload now use the existing
  WriteSszResultAsync helper instead of duplicating the
  error/null/encode-and-write cascade.
- WriteSszResultAsync becomes a non-async early-return method (removes the
  single-return await that the C# compiler can't optimize through).

ClientVersionSszHandler:
- Stop discarding the decoded ClientVersionV1; pass it through to
  engine_getClientVersionV1. ClientVersionV1 gains init-only setters and
  a parameterless ctor with field initializers so the wire decode actually
  populates the CL's identification (was previously stuck at the EL's own
  defaults from ProductInfo).
- DecodeClientVersionRequest now reads ClientVersion fields from the wire.

TryParsePayloadId:
- Move from SszEndpointHandlerBase into GetPayloadSszHandler (its sole caller).
- Single Out() projection helper handles both success and failure paths
  (null error = success; non-null = failure) — collapses the
  id-and-err-and-return triple into one-liners.

Renamed the SszStatus* private constants to a named SszPayloadStatus
internal static class.

Lint:
- Drop unused usings flagged by IDE0005 in CapabilitiesSszHandler,
  SszVersionDescriptors, SszMiddlewareTests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: split EngineRpcCapabilitiesProvider, switch expression for WriteSszResultAsync

EngineRpcCapabilitiesProvider:
- Add GetJsonRpcCapabilities() returning only JSON-RPC method names
  (e.g. engine_newPayloadV1).
- Add GetSszRestPaths() returning only SSZ-REST paths
  (e.g. "POST /engine/v1/payloads").
- GetEngineCapabilities() unions the two — kept as before so
  engine_exchangeCapabilities continues to advertise both transports
  in a single response per spec.
- All three results cached lazily on first access.

Tests now read the relevant subset directly instead of filtering the
union with a string-shape heuristic (`s.Contains(' ')`).

WriteSszResultAsync replaced with a switch expression:
- `{ Result.ResultType: not ResultType.Success }` for the failure arm.
- `{ Data: null }` for the 204 arm.
- `{ Data: var data }` extracts non-null Data for the encode arm.

Also: HashSet<string> input to ExchangeCapabilitiesHandler.Handle
takes `[]` collection-expression form in tests (instead of
`new HashSet<string>()`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: replace Contains(' ') heuristic in Should_return_capabilities

Build the JSON-RPC method name set explicitly via reflection over
IEngineRpcModule and intersect with that, instead of inferring "is
JSON-RPC method" from the absence of a space character. Same coverage,
self-documenting filter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: share fork-gating between JSON-RPC and SSZ-REST capability tables

Replace BuildJsonRpcCapabilities/BuildSszRestPaths (which duplicated every
fork gate — `spec.WithdrawalsEnabled`, `spec.IsEip4844Enabled`, etc. —
across two parallel sequences of dictionary inserts) with a single Build
method that registers paired entries via a `Pair(method, path, enabled,
warnIfMissing)` local helper. Each capability is declared once with its
JSON-RPC name, SSZ-REST path, and gating condition together; updates to
the gate stay in sync by construction.

The single SSZ-only entry (`SszRestPaths.PostV1Capabilities` —
engine_exchangeCapabilities is the meta-method itself, not a normal
capability) is added directly.

GetJsonRpcCapabilities/GetSszRestPaths/GetEngineCapabilities surfaces are
unchanged; both per-transport tables and the union are still cached.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: RpcCapabilityOptions flags + provider cleanup

Replace the (bool Enabled, bool WarnIfMissing) tuple value used in
IRpcCapabilitiesProvider with a [Flags] byte enum RpcCapabilityOptions
{ None, Enabled, WarnIfMissing }. One byte per dictionary entry instead
of two bools, and consumers query via fluent extensions
(`options.IsEnabled()`, `options.ShouldWarnIfMissing()`).

EngineRpcCapabilitiesProvider:
- Pair helper renamed to Configure and takes a single RpcCapabilityOptions
  argument instead of two bools (matches the new value type).
- Build is a `static void` with two out parameters (json, ssz) — drops
  the CapabilityTables record.
- Backing storage downgraded to plain Dictionary (the tables are populated
  once on first access and never mutated; concurrent reads are safe).
- Gate(bool) and GateWithWarn(bool) helpers express the two recurring
  fork-gating shapes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: trim unused using + simplify pattern in SszType

- Drop unused `using Nethermind.Core;` from GetPayloadBodiesByHashSszHandler.
- Collapse the `result.Members[0] is { } member && (member.Kind == ...)`
  binding into a single property pattern `result.Members[0] is { Kind: ... }`.
- Drop redundant `.ToString()` on a string-typed conversion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: drop now-unused Nethermind.Core usings

After the WriteSszResultAsync consolidation, GetBlobsSszHandler and
GetPayloadBodiesByRangeSszHandler no longer reference any type from
Nethermind.Core directly. CI Check code lint flagged IDE0005.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address Claude round-11 review findings

- CollectionTypeAnalyzer.SszIgnore matching now uses the SemanticModel
  to compare the resolved attribute type's full name against
  Nethermind.Serialization.Ssz.SszIgnoreAttribute. Eliminates false
  positives for user-defined attributes that happen to share the short
  name. The `SszIgnoreAttributeNames` HashSet is gone; the type is
  referenced via typeof(SszIgnoreAttribute) so the constant is
  nameof-driven (the original review ask).
- Add the missing SszIgnoreAttribute declaration to the generator's
  local Attributes.cs so typeof(...) resolves in the analyzer assembly.
- SszRestPaths.PayloadsGet was a duplicate of Payloads ("payloads"
  vs "payloads") with zero references — removed.
- SszMiddlewareConfigurer no longer constructs a fresh
  JsonRpcUrlCollection. JsonRpcRunner.ConfigureServices already registers
  the shared instance into the MS DI container before configurers run,
  so the bridge would have shadowed it with a duplicate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: drop generator-local SszIgnoreAttribute, use full-name string literal

Remove the duplicate SszIgnoreAttribute declaration from the generator's
local Attributes.cs and replace `typeof(SszIgnoreAttribute).FullName` with
a const string `"Nethermind.Serialization.Ssz.SszIgnoreAttribute"` directly.

The analyzer compares against the resolved attribute symbol's
`ToDisplayString()`, which is name-based regardless of which CLR type
backs the comparison. Adding a generator-local attribute class just to
make `typeof(...)` compile was machinery in service of a single string
constant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf(ssz-rest): encode responses directly into the response PipeWriter

Replaces the ArrayPoolSpan<byte> intermediate with `Func<T, IBufferWriter<byte>, int>`
encoders that GetSpan/Encode/Advance straight into ctx.Response.BodyWriter, then set
ContentLength from the returned byte count before flushing. Saves a pool rent + copy
per response. Streaming-decode (read-side rewrite) is the next PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ssz-rest): abort connection on encode failure; assert length contract

Encoders write directly into the response PipeWriter; if encode throws after
one or more Advance calls, the partial bytes can't be rewound. The middleware's
outer catch then writes a text error into the same pipe, producing a 500 with
[partial-SSZ][text] body. Wrap encode in a try/catch that calls ctx.Abort() so
the CL sees a connection reset instead of a garbled response.

Also Debug.Assert that the encoder's reported length matches pipe.UnflushedBytes
delta — catches drift between T.GetLength and T.Encode in test builds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf(ssz-rest): decode requests directly from PipeReader (Part B)

Middleware now uses PipeReader.ReadAtLeastAsync(ContentLength) and passes the
resulting ReadOnlySequence<byte> through the handler chain. Generator emits a
Decode(ReadOnlySequence<byte>, out T) overload at every container/list/union
emission site: single-segment dispatches to span (zero copy); multi-segment
consolidates once via ArrayPool. Eliminates the prior MemoryStream + ToArray
intermediate copy. SszLib gains primitive sequence overloads as infrastructure
for a future fully recursive sequence-aware decoder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(ssz-rest): cover multi-segment ReadOnlySequence decode paths

Existing SszCodecTests wrap byte[] via the single-segment ReadOnlySequence
constructor — exercises only the IsSingleSegment fast path. Production traffic
from Kestrel arrives in 4 KB pooled blocks, so blob-bearing NewPayload always
runs through the generator's pool-rent + CopyTo branch and SszLib's stack-copy
fallback. New fixture builds explicit multi-segment sequences via a
ReadOnlySequenceSegment chain at boundary sizes 1/3/7/4096, covering the wire
roundtrip path (TransitionConfig, Capabilities, GetBlobs, BodiesByRange,
NewPayloadV3) and the SszLib primitive boundary path (uint, ulong, UInt256).

Also restructures SszMiddleware.InvokeAsync as a chained if/else if/else with
the body-handling work nested in the final else clause so handler/version/extra
stay in scope without scattering early returns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: fix whitespace lint in SszMiddleware else-if chain

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: post-master-merge fallout in StartupTests + bool sequence decode

After merging master, IEngineRpcModule.engine_getBlobsV{1,2,3} returns
IReadOnlyList<BlobAndProof?> (the V{2,3} variant additionally nullable on the
outer wrapper) instead of IEnumerable<BlobAndProof>. Update the NSubstitute
returns in StartupTests and turn on #nullable enable so the annotations parse.

Also: master tightened Decode(ReadOnlySpan<byte>, out bool) to throw on bytes
other than 0/1. Route the sequence overload through ToContiguous so it can't
silently disagree with the span path on invalid input.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf(ssz-rest): pre-filter prefix routes; single-alloc blob V1/V2 encode

Round-14 micro-optimizations.

Routing slow path:
* Build _postPrefixRoutes / _getPrefixRoutes from only resources that have at
  least one AcceptsPathExtra candidate, and only those candidates. Today no
  POST handler accepts a path-extra suffix, so _postPrefixRoutes is empty —
  any failed POST exact-match no longer scans dead routes.
* Drop the redundant pathSpan.Equals(resourceSpan, ...) check in the prefix
  loop (the next length check rules out exact-match) and the per-candidate
  AcceptsPathExtra filter (now enforced at construction).

EncodeGetBlobsV{1,2}Response:
* Two-pass count + single exact-size allocation, replacing the previous
  alloc-full-then-slice-trim pattern. One heap allocation per response
  instead of two when any null entries are dropped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: drop unused Nethermind.Core using in SszMultiSegmentDecodeTests

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(capabilities): use FrozenDictionary including return types

Capability tables are built once on first GetXxx call and never mutated.
Switch the internal storage and the IRpcCapabilitiesProvider return type to
FrozenDictionary<string, RpcCapabilityOptions> — exposes that immutability
in the API and lets callers use FrozenDictionary-specific lookups (e.g.
AlternateLookup<ReadOnlySpan<char>>) without re-freezing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ssz-generator): don't cache empty SszBasicType discovery

If GetTypeByMetadataName for SszBasicTypeAttribute returns null (e.g. during
a parallel-build race where Nethermind.Serialization.Ssz reference resolution
lags), the previous gate cached the failure. Subsequent emits in the same
compilation then short-circuited via the gate with KnownTypes empty and
silently treated [SszBasicType] types like SszBytes8 as plain containers,
producing references to nonexistent SszBytes8.GetLength/Encode/Decode/MerkleizeList
methods.

Only short-circuit when prior discovery actually populated KnownTypes; never
cache the empty/null result. A re-walked namespace tree is far cheaper than
a build that fails with confusing CS0117s.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf(ssz-rest): inline-array SszKzgCommitment for contiguous KZG storage

Closes #11525.

Replace the [SszContainer]+byte[] shape of SszKzgCommitment with an
[InlineArray(48)] basic-type struct (mirrors SszBytes8). SszKzgCommitment[]
is now a single contiguous count*48-byte heap allocation with no
inter-element references — better cache locality during encode and
removes the per-element pointer chase the GC otherwise marks through.

Wire format is identical (raw 48 bytes per element); the existing SSZ
generator already supports basic ref-types with custom encode/decode
templates, so no generator changes were needed. SszBlob (131,072) stays
on byte[] per the issue's explicit non-conversion clause.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* accept empty list

* fix(ssz-rest): align with execution-apis#764 review feedback

- SszCapabilityName: ByteList[64] flat shape (isCollectionItself: true) so
  capability bytes don't get wrapped in a per-element container header.
- PayloadStatus.InvalidBlockHash now folds to INVALID(1) on the SSZ wire;
  drop the SszPayloadStatus byte-constant class and inline the switch.
- SszMiddleware: throw EndOfStreamException on truncated body so the
  inner-catch maps it to 400 instead of falling through to 500.
- Remove TransitionConfiguration SSZ handler / wire types / codec methods /
  path constants; the spec dropped this endpoint. JSON-RPC
  engine_exchangeTransitionConfigurationV1 stays.

Spec-vector tests: ["abc"] capability encodes to 11 bytes;
PayloadStatus.InvalidBlockHash maps to byte 1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(ssz-rest): re-extract EngineStatusToSsz as named string→byte mapper

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: drop unused PayloadStatus.InvalidBlockHash constant

No domain-layer producer references it; the SSZ-codec fold and its test
case are the only remaining users, both removed alongside the constant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(ssz-rest): switch-expression for ToExecutionRequests; truncated-body 400 test

- ToExecutionRequests: convert if/else chain to switch expression with a
  private BuildExecutionRequests helper for the populated case.
- Add Truncated_body_with_overstated_content_length_returns_400
  middleware regression for the EndOfStreamException pre-Slice guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* add benchmark to compare SSZ and JSON payload

* refactor(merge-plugin-benchmark): real JsonRpcProcessor, NSubstitute engine, split codec/round-trip

The previous benchmark compared SSZ-through-real-middleware against a hand-rolled
JSON stub that bypassed JsonRpcProcessor and did its own ad-hoc parse-twice
(JsonDocument + per-payload Deserialize) with a static byte[] response. The two
paths weren't measuring equivalent work, which made the SSZ-vs-JSON comparison
misleading.

JSON path now goes through the production RpcModuleProvider + JsonRpcService +
JsonRpcProcessor with the engine module registered as a SingletonModulePool, so
both paths share the same engine call surface and only the transport + codec
differ.

Other changes:
- Replace 130-line StubEngineModule (mostly NotImplementedException placeholders)
  with NSubstitute on IEngineRpcModule.
- Reuse TestItem.KeccakA-E and TestItem.AddressA-B; drop local consts.
- Cache MediaTypeHeaderValue and the parsed Authorization as static fields;
  return int (status code) from round-trip benchmarks instead of HttpResponseMessage.
- Tighten EncodeSszBody (raw byte[], drops ArrayBufferWriter detour) and
  EncodeJsonBody (Utf8JsonWriter + JsonSerializer.Serialize directly into the
  writer; no JsonDocument injector pattern).
- Single BuildEngineHost helper; SSZ and JSON servers diverge only in
  service registration and the terminal middleware.
- Drop StubProcessExitSource from the JSON server (only SszMiddleware needs it).
- Split into 6 benchmarks: deserialize Ssz/Json, serialize PayloadStatus
  Ssz/Json, full round-trip Ssz/Json. [Params(0,1,3,6)] for blob count.
- Add NSubstitute and Nethermind.Core.Test references to the benchmark csproj.

Validated: 24/24 benchmarks pass. Headline (Blobs=6): SSZ decode 2.4us vs JSON
7.2us (3x); SSZ encode 27ns vs JSON 110ns (4x); round-trip SSZ 78us vs JSON
121us (1.5x).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(merge-plugin-benchmark): rename to NewPayloadSerializationBenchmarks; simplify builders; expand blob params

The class covers more than just an end-to-end round-trip — it also runs
codec-only deserialize and PayloadStatus serialize benchmarks. Rename the file
and class to reflect that. Update the runner's Program.cs reference accordingly.

BuildTransactions: drop the per-tx Random fill — content doesn't affect codec
work or output size, so the random bytes were noise. Just allocate and set the
EIP-1559 type byte.

BuildBlobVersionedHashes: lift the byte buffer out of the loop and use stackalloc
+ Hash256(ReadOnlySpan<byte>) so we don't allocate one byte[32] per blob. With
[Params] now extending up to 72 blobs, the per-iteration allocation is worth
removing.

[Params] expanded from {0, 1, 3, 6} to {0, 1, 3, 6, 12, 24, 36, 72} to surface
how cost scales beyond the spec maximum.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(benchmark-runner): bump auto-gen project build timeout to 10 minutes

BDN's default 120s build timeout for the auto-generated boilerplate trips
when the benchmark project's transitive dep graph is large (Merge.Plugin
pulls in JsonRpc, Synchronization, Init, etc., and a cold restore + build
of all of those takes longer than 2 minutes on a regular machine).

Set ManualConfig.BuildTimeout to 10 minutes — covers a clean build, no
effect once the dep tree is warm.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(merge-plugin-benchmark): realistic heavy-block default; max withdrawals

The previous defaults (50 × 300 B = 15 KB tx data, 2 withdrawals) were nowhere
near a realistic mainnet block. The codec cost the benchmark measured was
mostly fixed-field overhead, not the variable payload weight that dominates in
production.

Bump the tx-data baseline to 250 × 600 B ≈ 150 KB — matches a typical mainnet
block. The constant is a single edit-point if someone wants to stress-test
worst case (e.g. 2500 × 600 B ≈ 1.5 MB observed on mainnet).

Withdrawals go from 2 to 16 (Capella max). BuildWithdrawals fills them from
TestItem.Addresses to keep the payload realistic.

Note: blob count is the right scaling axis for V3 specifically — the envelope
carries 32-byte versioned hashes only, not blob bytes (which travel in a
sidecar). So 72 blobs adds 2.3 KB to the envelope, not 9 MB.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(merge-plugin-benchmark): silence ASP.NET hosting info logs

Host.CreateDefaultBuilder() wires the Microsoft.AspNetCore.Hosting.Diagnostics
logger at Information level by default, which floods stdout with two lines
("Request starting" / "Request finished") per benchmark iteration. Drop all
log providers — BenchmarkDotNet has its own output pipeline and any other
log noise is benchmark-irrelevant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(merge-plugin-benchmark): add GetPayloadV3SerializationBenchmarks; reuse Eip4844Constants

Adds a sibling benchmark targeting engine_getPayloadV3, where the SSZ-vs-JSON
delta is most pronounced — every proposing slot returns ExecutionPayloadV3 +
BlobsBundle (commitments, proofs, ~131 KB blobs). JSON hex-encodes blob bytes,
doubling the wire size; SSZ keeps them as-is.

Layout matches NewPayloadSerializationBenchmarks: SSZ decode wire, JSON parse
JsonDocument (no STJ-constructible target — production CL has its own DOM),
SSZ/JSON encode of the result, full Kestrel round-trip via SszMiddleware /
real JsonRpcProcessor with engine_getPayloadV3 stubbed via NSubstitute.

Block construction uses Build.A.Block with real signed EIP-1559 transactions
(Build.A.Transaction...Signed). Going through ExecutionPayloadV3.TryGetBlock()
on a payload with synthetic byte[] transactions failed RLP decode, so the
benchmark constructs the Block directly.

Reuse existing constants where they exist:
- Eip4844Constants.GasPerBlob (= 131072) for blob byte size and BlobGasUsed.
- Eip4844Constants.BytesPerBlobVersionedHash (= 32) for hash buffer size.
- KzgPolynomialCommitments.KzgBlobHashVersionV1 (= 0x01) for the version byte.

48 (commitment / proof bytes) stays a local const — the canonical source is
Ckzg.BytesPerCommitment / BytesPerProof from CkzgLib, which would require a
new namespace reference for two named constants.

Validated: 48/48 benchmarks pass. At Blobs=72 (worst case): SSZ round-trip
4.1ms / 9.6 MB vs JSON 43.3ms / 115 MB — 10x faster, 12x fewer allocations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* address review comments

* address review comments

* refactor(merge-plugin-benchmark): extract shared EngineBenchmarkHost

NewPayloadSerializationBenchmarks and GetPayloadV3SerializationBenchmarks were
duplicating ~150 lines of identical plumbing: host builder, JSON-RPC processor
wiring, stub URL collection / auth / process-exit, header constants, and the
withdrawal factory.

Extract to EngineBenchmarkHost (internal static): Build(...) for the engine
TestServer scaffolding, BuildJsonServer(engine) for the JSON-RPC pipeline,
BuildWithdrawals(count), shared header values, and the three Stub* classes.
Both benchmark files now keep only their format-specific encoding / decoding
helpers and the SSZ handler registration.

No behavior change. Build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(ssz-generator): make BasicTypes init-only

* refactor(ssz-rest): extract CheckedLong helper; checked cast in DecodeGetPayloadBodiesByRangeRequest

Address benaadams's review on SszExecutionPayload.cs:70 — the 5 inline
"if (value > (ulong)long.MaxValue) throw …" property setters share an
identical shape but no shared code. Extract to a single
SszNumericChecks.CheckedLong helper with the suggested cold-path local
function + [DoesNotReturn, StackTraceHidden] for clean JIT codegen.

While there, fix the unchecked ulong→long cast in
SszCodec.DecodeGetPayloadBodiesByRangeRequest: wire.Start and wire.Count
arrived as long via direct (long)wire.Count, so values in
(long.MaxValue, ulong.MaxValue] wrapped to negative. The handler-side
"count > 32" guard then passed (negative isn't > 32), and the engine's
"count < 1" guard caught it with a generic 400 "must be positive". No
security impact, but the SSZ-spec limit message was being skipped. With
CheckedLong, the same out-of-range condition now produces a proper SSZ
400 "uint64 value … exceeds valid range for long" at decode time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(ssz): remove SszBasicTypeAttribute; inline basic types in generator

The attribute existed so consumers could declare new SSZ basic types
out-of-tree, but the only two users (SszBytes8, SszKzgCommitment) lived
inside Nethermind itself. Per-compilation discovery (DiscoverKnownTypes
walking the namespace tree, attribute symbol lookup, WeakReference cache)
was ~80 lines of generator code in service of two types.

Move SszBytes8 and SszKzgCommitment to Nethermind.Serialization.Ssz so
every project that depends on the SSZ generator can see them, then hardcode
them in SszType.BasicTypes alongside SszBytes32. Generator no longer needs
the discovery pass; the [SszBasicType] attribute and its attribute class
are deleted.

Adding a new fixed-length basic SSZ type now means one entry in
SszType.BasicTypes — same friction as the existing Hash256 / Address / Bloom
entries.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(ssz-rest): drop unused System / interop usings in SszWireTypes

After SszBytes8 and SszKzgCommitment moved to Nethermind.Serialization.Ssz,
the remaining wire types in this file don't use System, InteropServices,
or CompilerServices types — only the SSZ generator attributes plus core/crypto.
Drops three dead usings; resolves IDE0005 warning surfaced by CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Stavros Vlachakis <89769224+svlachakis@users.noreply.github.com>
alexb5dh added a commit that referenced this pull request May 12, 2026
* [C-08] Taiko: restore gas on not-accepted tx

* [C-18] exit RLP `PeekNumberOfItemsRemaining` loop when limit reached

* Use `BlobVersionedHashes` spec limit in `LightTxDecoder` also

* [C-26] validate content lengths against buffer size in `PeekLongPrefixAndContentLength`

Prevents potential `ArgumentOutOfRangeException` in callers

* [C-30] Comment on `PayloadAttributes.GetPayloadId` usage

* [U-12] Improve error on missing JSON parameter constructor

* [U-20] comment explaining asymmetry in `BlobGasUsed`/`ExcessBlobGas` check

* [U-24] slightly better error on invalid IP during discovery

* [U-26] Fix missing `supportTxHash` check in `ReceiptStorageDecoder.GetContentLength`, fix related test

* [U-33] `BloomStructRef.Equals(object)` fix

* [U-32] potential fix for `AuRaSealValidator.ValidateParams` out-of-order calls

* Formatting

* Test fix

* PR feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants