feat: add beacon client connector - #132
Merged
Merged
Conversation
RaekwonIII
previously approved these changes
May 28, 2026
Chris-ssvlabs
requested changes
May 29, 2026
Chris-ssvlabs
left a comment
Chris-ssvlabs
left a comment
Contributor
There was a problem hiding this comment.
- Batch reads use GET with repeated id= params — URL length ceiling.
getBeaconValidatorsURL (src/api/beacon/index.ts:1121) builds ?id=x&id=y&.... For large
validator sets (hundreds+), this URL blows past common proxy/node limits (nginx ~8KB,
Alchemy similar). The standard Beacon API supports POST
/eth/v1/beacon/states/{state_id}/validators with {ids:[...]} body precisely for this.
"Verified against Alchemy" almost certainly only exercised small batches. Either document a
practical cap or switch large batches to POST.
Minor / non-blocking
-
Inconsistent missing-endpoint behavior. getBeaconValidatorStates (:1224) early-returns
[] for empty input before any endpoint check, while single reads throw "Beacon endpoint is
not configured." A misconfigured SDK silently returns [] for an empty-arg batch.
Intentional-looking (matches the getBeaconValidators guard), just asymmetric. -
waitForBeaconValidatorActivation treats not-found as keep-waiting. A permanently-wrong
validatorId polls until timeout rather than failing fast. Fine for not-yet-seen validators,
but no fast-fail path. Also polls head (reorg-able) — acceptable for activation. -
isConfig now requires 'beacon' in props (src/config/create.ts:49). All in-repo config
builders updated (mock/config, create). Only matters for external consumers hand-building a
config object — low risk, worth a mental note. -
Type accuracy nit. BeaconValidator.index is typed string but the code (and a test)
handle null. Raw type is optimistic; runtime guards cover it, so harmless.
Nice catch in the diff
src/mock/api.ts drops the as unknown as ConfigReturnType['api'] escape hatch and returns a
structurally-typed object — surfaced the previously-missing checkOperatorDKGEnabled /
getDaoValues mocks. Net improvement.
sumbat-ssvlabs
previously approved these changes
Jun 1, 2026
martinssvlabs
dismissed stale reviews from sumbat-ssvlabs and RaekwonIII
via
June 9, 2026 12:44
e16626b
RaekwonIII
previously approved these changes
Jun 10, 2026
…ation waits - throw on batch 404s instead of silently returning [], so a misconfigured endpoint no longer masquerades as "no validators" - bound each waitForBeaconValidatorActivation attempt by the remaining deadline and retry transient fetch errors instead of aborting the whole wait on a single blip - reject an empty/blank validatorId in getBeaconValidator instead of silently degrading to the unfiltered validator list - align the getBeaconValidatorStates mock with the real index-aligned response shape - replace the tautological waitForBeaconValidatorActivation mock assertion with surface-shape checks - treat a present-but-non-object beacon config as an incomplete prebuilt config, not just a missing one Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…alidate numeric fields - cap GET validator batches at 64 deduplicated ids per the beacon-APIs spec (maxItems: 64, documented 414 above that), falling back to POST otherwise; reject blank ids before requesting - bound each activation-wait attempt by requestTimeoutMs (defaults to pollIntervalMs) instead of the full remaining budget, so a single stalled request can no longer consume the entire timeoutMs with zero retries - classify activation-wait errors: retry network errors, our own attempt-timeout abort, 408/429/5xx; throw non-retryable 4xx and response-validation failures immediately via new BeaconHttpError/ BeaconValidationError types; preserve the last retryable error through to the final timeout - require canonical non-negative decimal strings and enforce the uint64 range before parsing beacon numeric fields with BigInt, and split index (no far-future sentinel) from epoch fields (sentinel -> null) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
parseBeaconJSON wrapped every error from response.json() as a BeaconValidationError, which isRetryableActivationError treats as permanent. But response.json() also rejects when the body read itself fails (connection drop, or our own attempt-timeout abort firing mid-stream) — not only for genuinely malformed JSON content — so a transient body-read failure stopped activation polling after a single attempt instead of retrying. Only wrap SyntaxError (thrown once the body has been read in full but fails to parse); let any other error propagate so the default retryable classification applies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t timeout
- assertBeaconData only rejected undefined data, so a {"data": null}
body (malformed response or proxy error) passed through as a
legitimate null result, indistinguishable from a genuine 404 —
causing waitForBeaconValidatorActivation to poll the full timeout
instead of failing fast. Now rejects null too.
- requestTimeoutMs defaulted to pollIntervalMs, so a short poll
interval against a healthy endpoint that's merely slower than that
interval aborted every attempt and could never succeed. Decoupled
the default to a fixed 10s budget, independent of poll frequency.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, address adversarial review - hasIncompletePrebuiltConfigShape only ever protected the beacon field; the identical silent-config-loss bug existed for every other ConfigReturnType field (subgraph, rest, contractAddresses, etc.) since the check required ALL of them present before even looking at beacon. Generalized to looksPrebuilt && !isConfig(props), reusing isConfig's existing per-field validation symmetrically instead of duplicating it. - getBeaconValidators/getBeaconValidatorStates now accept an optional AbortSignal, matching the single-item functions. - getBeaconValidator/getBeaconValidators gained a doc comment (mirrored in the README) clarifying they only validate the response envelope, not each validator's field-level shape. - cleanup: removed a dead unreachable branch in getBeaconValidatorsURL, wired the orphaned RawBeaconValidatorStatus type into rawStatus, de-duplicated isNonNullObject across config/create.ts and sdk.ts, hoisted a recomputed MAX_SAFE_INTEGER constant, widened BeaconValidationError to consistently cover caller-input preconditions (not just response validation), added a digit-count cap to the canonical-integer regex. - added test coverage for: the generalized config-shape guard, mapBeaconValidatorState's top-level shape checks, assertPositiveInteger's bounds, a genuine fetch() rejection (not just an HTTP error response), batch signal passthrough, and a stronger multi-length mock-alignment check. - README: documented requestTimeoutMs, failOnNotFound, BeaconHttpError/BeaconValidationError retry semantics, the raw-accessor envelope-only caveat, and clarified extendedConfig.beacon is optional. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ear real lint errors
- isConfig accepted arrays wherever a config field is expected to be a
plain object (typeof [] === 'object' in JS), defeating
hasIncompletePrebuiltConfigShape for e.g. beacon: []. isNonNullObject
now excludes arrays explicitly.
- mapBeaconValidatorState only checked validator.pubkey was a string,
not that it matched the Beacon API's actual pubkey format (0x +
96 hex chars, per the beacon-APIs Pubkey type). Added
assertBeaconPubkey. Replaced the too-short test fixtures ('0xabc',
'0xaaa', '0xbbb') with valid 48-byte hex keys everywhere normalized
validation is exercised; verified against a live endpoint that a
real validator pubkey still passes.
- fixed all 119 real ESLint errors in PR-touched files (115 prettier,
3 unused-vars, 1 no-constant-condition), confirmed by actually
running eslint without --fix. Replaced while(true) with for(;;) in
the activation-wait loop; added a small typed omitKey test helper
instead of unused destructured bindings.
- added negative tests for array-shaped beacon/rest/publicClient/api
fields, and pubkey validation tests (missing, empty, non-hex,
wrong-length, valid mixed-case).
Deeper isConfig value validation (endpoint types, contractAddresses
shape, callable API member checks) is intentionally out of scope --
the array exclusion above covers the realistic silent-data-loss bug
class; further hardening would target inputs that already violate the
TS contract outright.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…onicalize ids - Build request URLs by mutating a URL's pathname instead of the shared url-join util, which silently clobbered any endpoint already carrying a query string (e.g. ?apiKey=...). - Cap pollIntervalMs/requestTimeoutMs at setTimeout's int32 delay limit, since they reach real timers; timeoutMs is left uncapped since it's only ever used for deadline arithmetic. - Canonicalize plain-decimal validator-id lookups so a caller-supplied id like '0012' matches the canonical '12' key from a batch response.
isConfig checks each field's presence and top-level shape, not nested primitive values (e.g. beacon.endpoint's type) — the comment claimed full symmetric validation, which isn't accurate.
Non-2xx responses threw BeaconHttpError without consuming or cancelling response.body, leaving the connection unreleased until GC. This accumulates across waitForBeaconValidatorActivation's retry loop against an unhealthy endpoint. Cancel the body before throwing, on both the single and batch read paths.
The prior connection-release fix covered the !response.ok throws but missed this branch, which is the higher-frequency one in practice: without failOnNotFound, waitForBeaconValidatorActivation takes this exact path on every poll until a validator appears, accumulating uncancelled bodies across a normal, successful wait.
… timeout buildBeaconURL throws a plain TypeError for a malformed endpoint, which isRetryableActivationError treats as retryable by default, so waitForBeaconValidatorActivation silently retried a failure that could never succeed for the entire timeoutMs. Validate the endpoint once up front and fail immediately with a BeaconValidationError instead.
A syntactically valid URL with a scheme fetch can't use (ftp:, mailto:, etc.) passed the existing endpoint preflight, then failed with a plain TypeError from fetch itself — hitting the same default-retryable gap as a malformed URL and retrying until timeout instead of failing fast.
…endpoints and blocked ports Two gaps in the last two commits' hardening: - response.body?.cancel() was awaited unguarded; a rejecting cancel() (e.g. an already-errored stream) would mask the real result — a BeaconHttpError or a 404's null — with the cancellation's own error, which then hits the default-retryable path and gets retried until timeout instead of surfacing the real failure once. Made cancellation best-effort so it can never replace the meaningful result. - The endpoint preflight caught malformed URLs and unsupported schemes but not credentialed URLs (fetch refuses to construct a Request from one) or Fetch-spec-blocked ports (only observable by attempting the request) — both hit the same "permanent config error retried until timeout" bug this preflight exists to prevent. Credentials are now rejected in the preflight; blocked ports are classified non-retryable via undici's specific failure cause, without treating every fetch-thrown TypeError as non-retryable (genuine transient network failures must keep retrying).
…t errors The endpoint preflight interpolated the raw endpoint into BeaconValidationError's message unconditionally, so a credentialed URL or a query-string API key that failed validation for any reason (not just its own credentials check) leaked the secret into the thrown error, and from there into logs and error trackers. Report url.origin + pathname once the endpoint has parsed successfully; fall back to the raw string only when it never parsed at all, since there's nothing to redact in that case.
…edentials A string that fails new URL() entirely can still visibly contain a credential (e.g. 'https://user:secret@' is invalid because the host is missing, not because the credential syntax is malformed), so the raw fallback in the last redaction fix could still leak a password or API key for this specific class of input. Report a generic placeholder instead of any part of the raw string until the endpoint has actually parsed into a URL we can redact structurally.
…eads getBeaconValidator/getBeaconValidators had no endpoint preflight at all, so a credentialed or unfetchable endpoint reached fetch() directly — and Node's own TypeError for a credentialed URL embeds it verbatim, leaking the credential outside the activation-wait path this protection was originally scoped to. Extracted the shared check into assertFetchableBeaconEndpoint, used by all three entry points, and redact to origin + our own fixed API path (not url.pathname) so a provider's path-embedded API key (e.g. Infura's /v3/<key>) can't leak either.
An opaque-path scheme (a typo like 'htttps:', or a non-special scheme like 'mailto:'/'admin:') serializes url.origin as the literal string 'null', which would otherwise produce a message like 'null/eth/v1/beacon/states/head/validators'. Not a leak — the fixed BEACON_VALIDATORS_PATH constant this concatenates with never carries caller input — but confusing. Keep the generic placeholder for anything without a real origin to report.
'0xmock-beacon-validator' fails the SDK's own enforced 0x-prefixed 96-hex-char BLS pubkey contract, so any test piping the mock's normalized state into a pubkey-consuming path exercises a shape production would reject. Tightened the corresponding assertion from expect.any(String) to a regex match so a regression here is actually caught.
A validatorId of '.' or '..' survives encodeURIComponent unchanged (dots aren't escaped), and the URL pathname setter's standard dot-segment normalization then silently redirects the request to the unfiltered validators collection or an unrelated parent endpoint instead of the id simply not matching anything, which is the spec's own defined behavior for an unmatched id. Reject only all-dots ids specifically — any other non-matching string (e.g. a fake id used in not-found tests) still passes through untouched, matching how the real API actually behaves.
A JS caller bypassing this SDK's own TS types can pass a non-string validatorId, and even a well-typed string can contain an unpaired UTF-16 surrogate that throws URIError from encodeURIComponent — both are plain TypeError/URIError, isRetryableActivationError's default, so left unchecked waitForBeaconValidatorActivation retried a permanent input error for the entire timeoutMs instead of failing fast.
RaekwonIII
approved these changes
Aug 10, 2026
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
4 participants
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.
Summary
Adds a beacon client connector to the SDK with normalized validator reads and activation polling.
Changes
extendedConfig.beacon.endpointNotes
Implemented against the standard Ethereum Beacon API.
Verified against a live Alchemy beacon endpoint and aligned with standard Beacon API provider docs.