refactor(ensnode-sdk): improve Indexing Status data model#1615
Open
refactor(ensnode-sdk): improve Indexing Status data model#1615
Conversation
The goal of this change is to split deserialization from validation, so it's possible to: a) deserialize a serialized value, including validation; b) validate an unvalidated business-layer value.
🦋 Changeset detectedLatest commit: f02c31c The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe indexing status module in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts
Fixed
Show fixed
Hide fixed
Contributor
Greptile OverviewGreptile SummaryThis PR refactors the Indexing Status data model in ENSNode SDK by introducing a clean separation of concerns across three layers: schema (Zod validation), validation, and deserialization. The refactoring enables a crucial new capability: validating existing business-layer objects without requiring serialized representations, which unlocks client-side use cases. Key Changes:
Architecture Benefits:
Files Changed: 43 files with ~3000 lines added and ~2700 lines removed, primarily reorganizing existing code into the new layered structure Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Consumer as Consumer/Client
participant Deserialize as Deserialize Layer
participant Validate as Validate Layer
participant Schema as Schema Layer (Zod)
participant Types as Business Types
Note over Consumer,Types: New Architecture: Separation of Concerns
rect rgb(200, 220, 255)
Note over Consumer,Types: Use Case 1: Deserialize Serialized Data
Consumer->>Deserialize: deserializeChainIndexingStatusSnapshot(serialized)
Deserialize->>Schema: makeChainIndexingStatusSnapshotSchema()
Schema->>Schema: Define structure + invariants
Deserialize->>Schema: schema.safeParse(serialized)
Schema->>Schema: Validate structure
Schema->>Schema: Check invariants (e.g., startBlock <= endBlock)
Schema-->>Deserialize: Validated data or error
Deserialize-->>Consumer: ChainIndexingStatusSnapshot
end
rect rgb(220, 255, 200)
Note over Consumer,Types: Use Case 2: Validate Existing Object (NEW!)
Consumer->>Validate: validateChainIndexingStatusSnapshot(unvalidated)
Validate->>Schema: makeChainIndexingStatusSnapshotSchema()
Schema->>Schema: Define structure + invariants
Validate->>Schema: schema.safeParse(unvalidated)
Schema->>Schema: Validate structure
Schema->>Schema: Check invariants
Schema-->>Validate: Validated data or error
Validate-->>Consumer: ChainIndexingStatusSnapshot
end
rect rgb(255, 220, 200)
Note over Consumer,Types: Serialization Flow
Consumer->>Types: serializeChainIndexingSnapshots(chains)
Types-->>Consumer: SerializedChainIndexingStatusSnapshot
end
Note over Schema: Schema layer contains:<br/>- Zod schema builders<br/>- Invariant validation functions<br/>- Reusable across layers
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the ENSNode SDK’s ensindexer/indexing-status module into separated layers (business types, serialize/deserialize, and validation) to enable validation of already-shaped objects and to prepare for upcoming indexing-status model divergence (#1405), while keeping existing consumer-facing behavior consistent.
Changes:
- Split the former monolithic
ensindexer/indexing-statusimplementation into granular modules (*-snapshottypes,serialize/*,deserialize/*,validate/*). - Updated API indexing-status parsing to validate serialized responses first, then preprocess into business-layer shapes before validation.
- Updated/added tests and mocks for the new module structure.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/internal.ts | Removes re-export of now-deleted indexing-status zod schema module. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/index.ts | Replaces broad barrel exports with explicit exports for new layers. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts | Introduces chain snapshot business types + helpers moved from previous helpers/types. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/cross-chain-indexing-status-snapshot.ts | Introduces cross-chain snapshot types + getLatestIndexedBlockRef. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.ts | Introduces omnichain snapshot types + derived status/cursor helpers. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/realtime-indexing-status-projection.ts | Introduces projection type + creation helper (moved from projection.ts). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/block-refs.mock.ts | Adds shared BlockRef fixtures for tests. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.mocks.ts | Adds shared chain snapshot fixtures for tests. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts | Adds validation + Zod schema builder + invariants for chain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts | Adds validation + Zod schema builder + invariants for omnichain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts | Adds validation + Zod schema builder + invariants for cross-chain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/validate/realtime-indexing-status-projection.ts | Adds validation + Zod schema builder + invariants for projections. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/chain-indexing-status-snapshot.ts | Adds serializers and serialized-type aliases for chain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/omnichain-indexing-status-snapshot.ts | Adds serializers + serialized types for omnichain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/cross-chain-indexing-status-snapshot.ts | Adds serializers + serialized types for cross-chain snapshots. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/realtime-indexing-status-projection.ts | Adds serializers + serialized types for projections. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/chain-indexing-status-snapshot.ts | Adds deserializer for chain snapshots (direct validation). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/omnichain-indexing-status-snapshot.ts | Adds deserializer with preprocess building Map-based shapes before validation. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/cross-chain-indexing-status-snapshot.ts | Adds deserializer with preprocess into business-layer shapes before validation. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/realtime-indexing-status-projection.ts | Adds deserializer with preprocess before validation. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/chain-indexing-status-snapshot.test.ts | Updates tests to use new module layout and mocks. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/conversions.test.ts | Updates imports to new serialize/deserialize module locations. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/realtime-indexing-status-projection.test.ts | Updates imports to new module locations. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.test.ts | Adds/updates tests for omnichain status/cursor helpers. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.test.ts | Adds tests for createIndexingConfig. |
| packages/ensnode-sdk/src/api/indexing-status/zod-schemas.ts | Adds schemas for serialized responses and wires projection serialized schema. |
| packages/ensnode-sdk/src/api/indexing-status/deserialize.ts | Changes to two-step parse: validate serialized -> preprocess -> validate business model. |
| apps/ensindexer/src/lib/indexing-status/ponder-metadata/validations.ts | Removes invariant relying on SDK indexing-status helpers; keeps Ponder metrics validation only. |
| packages/ensnode-sdk/src/ensindexer/indexing-status/zod-schemas.ts | Deleted (replaced by validate/* modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/validations.ts | Deleted (replaced by validate/* modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/types.ts | Deleted (split into multiple business-type modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialized-types.ts | Deleted (replaced by serialize/* modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/serialize.ts | Deleted (split into serialize/* modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize.ts | Deleted (split into deserialize/* modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/helpers.ts | Deleted (logic moved into business-type modules and validate modules). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/helpers.test.ts | Deleted (replaced by new test files). |
| packages/ensnode-sdk/src/ensindexer/indexing-status/projection.ts | Deleted (logic moved into realtime-indexing-status-projection.ts). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...s/ensnode-sdk/src/ensindexer/indexing-status/validate/realtime-indexing-status-projection.ts
Outdated
Show resolved
Hide resolved
...ensnode-sdk/src/ensindexer/indexing-status/deserialize/omnichain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.test.ts
Outdated
Show resolved
Hide resolved
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...es/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
… data model. These functions enable new use cases on consumers side.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🤖 Fix all issues with AI agents
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.test.ts`:
- Line 29: The test description is misleading: the it(...) string reads "returns
'indefinite' indexer config if the endBlock exists" while the test actually
passes endBlock = null; update the test description to match the test scenario
(e.g., "returns 'indefinite' indexer config if the endBlock is null" or "if
endBlock does not exist") so it accurately reflects the variable endBlock used
in the test block; locate the failing spec by the it(...) string in
chain-indexing-status-snapshot.test.ts and change only the description text,
leaving the test logic and the endBlock variable as-is.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts`:
- Around line 314-322: The function getTimestampForLowestOmnichainStartBlock
currently uses Math.min(...array) which yields Infinity for empty input and can
blow the stack for huge arrays; change it to compute the minimum via a guarded
reduce (e.g. reduce over chains to compute minTimestamp starting at Infinity)
and then if the result is Infinity return a sensible default UnixTimestamp (e.g.
0) or throw—update getTimestampForLowestOmnichainStartBlock to use reduce on
chains.map(c => c.config.startBlock.timestamp) and handle the empty-case by
returning 0 (or throwing) instead of returning Infinity.
- Around line 375-385: The function sortChainStatusesByStartBlockAsc currently
calls Array.prototype.sort on the incoming chains array which mutates the
caller's input; change it to perform a non-mutating sort by sorting a shallow
copy (e.g., use [...chains] or chains.slice() before sort) or, if available, use
chains.toSorted(...) and keep the same comparator that uses
chainA.config.startBlock.timestamp - chainB.config.startBlock.timestamp; ensure
the original chains parameter is not modified and return the new sorted array
from sortChainStatusesByStartBlockAsc.
- Around line 337-369: getTimestampForHighestOmnichainKnownBlock can call
Math.max on an empty latestKnownBlockTimestamps array which yields -Infinity;
update the function (keeping the same signature) to check if
latestKnownBlockTimestamps is empty and return a safe fallback (e.g., 0 as a
UnixTimestamp) when no timestamps were collected, otherwise return
Math.max(...latestKnownBlockTimestamps); reference
getTimestampForHighestOmnichainKnownBlock and latestKnownBlockTimestamps to
locate the change.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/cross-chain-indexing-status-snapshot.ts`:
- Around line 34-51: Update the JSDoc `@link` references that mistakenly use the
type name CrossChainIndexingStrategyId to the correct const object name
CrossChainIndexingStrategyIds (e.g., change {`@link`
CrossChainIndexingStrategyId.Omnichain} to {`@link`
CrossChainIndexingStrategyIds.Omnichain}) in the cross-chain snapshot comments
so the links point to the actual identifier used at runtime (refer to the
occurrences around the
CrossChainIndexingStrategyId/CrossChainIndexingStrategyIds mentions and the
Omnichain reference in this file).
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/cross-chain-indexing-status-snapshot.ts`:
- Around line 62-70: The factory
makeSerializedCrossChainIndexingStatusSnapshotSchema currently uses
z.object(...) causing inconsistent validation; change it to use
z.strictObject(...) so the schema is strict like its siblings—update the call in
makeSerializedCrossChainIndexingStatusSnapshotSchema to z.strictObject({
strategy: z.enum(CrossChainIndexingStrategyIds), slowestChainIndexingCursor:
makeUnixTimestampSchema(valueLabel), snapshotTime:
makeUnixTimestampSchema(valueLabel), omnichainSnapshot:
makeSerializedOmnichainIndexingStatusSnapshotSchema(valueLabel) }) to match the
other deserializers and ensure strict shape validation.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/omnichain-indexing-status-snapshot.ts`:
- Around line 107-109: The JSDoc references a non-existent symbol
makeChainIndexingStatusesSchema; update the doc link to the correct validator
name (likely makeSerializedChainIndexingStatusSnapshotsSchema) or to
makeChainIndexingStatusSnapshotsSchema if you meant the runtime schema from
validate/omnichain-indexing-status-snapshot.ts; locate the JSDoc above the
builder (in omnichain-indexing-status-snapshot.ts) and replace {`@link`
makeChainIndexingStatusesSchema} with the intended symbol name so the link
points to an existing function.
- Around line 111-125: Update the JSDoc above
buildUnvalidatedChainIndexingStatuses to use the correct link target: replace
the incorrect {`@link` makeChainIndexingStatusesSchema} with {`@link`
makeChainIndexingStatusSnapshotSchema} so the documentation points to the actual
schema function; verify the JSDoc is adjacent to the
buildUnvalidatedChainIndexingStatuses declaration and that the symbol name
matches exactly makeChainIndexingStatusSnapshotSchema.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/deserialize/realtime-indexing-status-projection.ts`:
- Line 1: The import from "zod/v4" is inconsistent with other deserializers:
replace the named-import style "import { prettifyError, z } from 'zod/v4'" with
the default-import style "import z, { prettifyError } from 'zod/v4'" so that the
symbols z and prettifyError match the rest of the codebase (look for usages of z
and prettifyError in the realtime-indexing-status-projection deserializer).
- Around line 57-64: The default valueLabel "Value" in
makeSerializedRealtimeIndexingStatusProjectionSchema is too generic; change its
default to a more descriptive string like "Realtime Indexing Status Projection"
so validation errors are clearer. Locate the function
makeSerializedRealtimeIndexingStatusProjectionSchema and update the default
parameter valueLabel to "Realtime Indexing Status Projection", keeping the rest
of the schema construction (snapshot, projectedAt, worstCaseDistance) unchanged.
Ensure this mirrors the descriptive style used by the sibling factory
makeSerializedCrossChainIndexingStatusSnapshotSchema.
In `@packages/ensnode-sdk/src/ensindexer/indexing-status/index.ts`:
- Around line 1-16: The barrel currently uses export * from 16 modules (e.g.,
"serialize/realtime-indexing-status-projection",
"deserialize/chain-indexing-status-snapshot",
"validate/omnichain-indexing-status-snapshot",
"omnichain-indexing-status-snapshot", "realtime-indexing-status-projection",
etc.), which risks name collisions; change this to either (A) scoped namespace
re-exports per layer like export * as deserialize from "./deserialize"; export *
as serialize from "./serialize"; export * as validate from "./validate"; and
keep top-level domain exports (omnichain-indexing-status-snapshot,
realtime-indexing-status-projection, chain-indexing-status-snapshot,
cross-chain-indexing-status-snapshot) explicit, or (B) replace wildcard exports
with explicit named re-exports (export { X, Y } from
"./serialize/chain-indexing-status-snapshot") for each module so the public API
is explicit and collisions are prevented—update the current export lines (those
referencing serialize/, deserialize/, validate/, omnichain-, realtime-, chain-,
cross-chain- modules) accordingly.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.test.ts`:
- Line 65: Fix the typo in the test description string inside the it(...) call
in omnichain-indexing-status-snapshot.test.ts by adding the missing closing
single quote so the description reads correctly (e.g., change "can correctly
derive 'following' status if at least one chain is 'following" to include the
trailing single quote). Locate the it(...) test case and update its string
literal; no other code changes are needed.
- Around line 172-180: The Following snapshot test fixture sets
latestIndexedBlock > latestKnownBlock which violates the chain-level validation
invariant; update the test data in the omnichain-indexing-status-snapshot.test
(the object using ChainIndexingStatusIds.Following and typed as
ChainIndexingStatusSnapshotFollowing) so that latestKnownBlock.timestamp >=
latestIndexedBlock.timestamp — e.g., swap evenLaterBlockRef and laterBlockRef or
replace latestKnownBlock with a block ref whose timestamp is >
evenLaterBlockRef.timestamp; ensure the fields latestIndexedBlock and
latestKnownBlock are consistent with the
serialize/chain-indexing-status-snapshot.test.ts validation rules.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.ts`:
- Around line 253-257: The code redundantly calls
getOmnichainIndexingStatus(chains) inside getOmnichainIndexingCursor causing
duplicate traversal; change getOmnichainIndexingCursor to accept an optional
precomputed status parameter (e.g., status?: OmnichainIndexingStatusIds), use
that status when provided and only call getOmnichainIndexingStatus(chains) if
status is undefined, and update the check that compares to
OmnichainIndexingStatusIds.Unstarted to use the local status variable; adjust
all callers to pass the already-computed status where available (otherwise leave
behavior unchanged).
- Around line 246-273: The filter in getOmnichainIndexingCursor does not narrow
the discriminated union so accessing chain.latestIndexedBlock is unsafe; update
the .filter(...) that currently checks chain.chainStatus !==
ChainIndexingStatusIds.Queued to use a type predicate that narrows
ChainIndexingStatusSnapshot to the non-Queued variant (e.g., (chain): chain is
Exclude<ChainIndexingStatusSnapshot, ChainIndexingStatusSnapshotQueued> =>
chain.chainStatus !== ChainIndexingStatusIds.Queued) so TypeScript knows each
remaining item has latestIndexedBlock and the subsequent .map(...) is type-safe.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/serialize/realtime-indexing-status-projection.ts`:
- Around line 27-35: serializeRealtimeIndexingStatusProjection currently always
uses serializeCrossChainIndexingStatusSnapshotOmnichain even though its input
type RealtimeIndexingStatusProjection can represent other
CrossChainIndexingStrategy variants; update the function to either (A) narrow
the input type to only the Omnichain-specific projection type, or (B) implement
a strategy-based dispatch: inspect indexingProjection.snapshot.strategy (or the
discriminant on snapshot), switch on CrossChainIndexingStrategy cases, call the
corresponding serializer for each strategy (e.g.,
serializeCrossChainIndexingStatusSnapshotOmnichain for Omnichain), and add an
exhaustive default that throws if an unknown strategy is encountered so future
strategies won't silently produce incorrect output; ensure you update imports
and types accordingly and preserve the returned shape
SerializedRealtimeIndexingStatusProjection.
- Around line 11-14: Remove the unused and inconsistently named interface
SerializedCurrentIndexingProjectionOmnichain: locate its declaration (extends
Omit<RealtimeIndexingStatusProjection, "snapshot"> with snapshot:
SerializedOmnichainIndexingStatusSnapshot) and delete it, ensuring there are no
remaining references; keep SerializedRealtimeIndexingStatusProjection as the
return type from the serialization function and update any exports or barrel
files if they were exporting the removed interface.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts`:
- Around line 59-94: The invariant_snapshotTimeIsTheHighestKnownBlockTimestamp
function uses .filter().map() to access variant-specific properties (endBlock,
backfillEndBlock, latestKnownBlock) without TypeScript type predicates; update
the filter callbacks to be proper type guards that narrow the union types—e.g. a
predicate for chain.config that asserts config.configType ===
ChainIndexingConfigTypeIds.Definite so .map can safely read endBlock.timestamp,
a predicate for chain.chainStatus === ChainIndexingStatusIds.Backfill so .map
can read backfillEndBlock.timestamp, and a predicate for
ChainIndexingStatusIds.Following so .map can read latestKnownBlock.timestamp;
ensure the predicates return a type predicate (chain is DefiniteConfigChain or
BackfillStatusChain etc.) so the compiler enforces correctness and prevents
future regressions.
- Around line 36-53: The function
invariant_slowestChainEqualsToOmnichainSnapshotTime currently contains a
leftover console.log that emits debug output; remove the console.log call (the
one logging slowestChainIndexingCursor and omnichainIndexingCursor) so the
invariant only reports failures by pushing to ctx.issues (keep the existing
ctx.issues.push logic and surrounding variable usage of
slowestChainIndexingCursor, omnichainSnapshot, and omnichainIndexingCursor
intact).
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts`:
- Around line 85-93: The function
checkChainIndexingStatusSnapshotsForOmnichainStatusSnapshotFollowing currently
returns the same input type and uses .some(), which is both a no-op type
predicate and misnamed; change its return type to narrow to the specific
following subtype (e.g., chains is ChainIndexingStatusSnapshotFollowing[]) and
replace .some() with .every() to ensure all chains have chainStatus ===
ChainIndexingStatusIds.Following; also rename the boolean variable from
allChainsHaveValidStatuses to something accurate like allChainsAreFollowing to
reflect the universal check and return that value.
- Around line 385-390: The Following variant schema
makeOmnichainIndexingStatusSnapshotFollowingSchema currently omits the
object-level invariant; attach the exported invariant
invariant_omnichainStatusSnapshotFollowingHasValidChains to the returned
z.strictObject so the full
ParsePayload<OmnichainIndexingStatusSnapshotFollowing> is validated (rather than
trying to add a chains-level check). In practice, wrap the existing
z.strictObject({...}) with a
.check(invariant_omnichainStatusSnapshotFollowingHasValidChains) so the
OmnichainIndexingStatusIds.Following schema enforces the same chain consistency
rules as the other variants (using makeChainIndexingStatusSnapshotsSchema and
makeUnixTimestampSchema as before).
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/realtime-indexing-status-projection.ts`:
- Around line 70-80: The snapshot field uses
makeCrossChainIndexingStatusSnapshotSchema which performs deep, per-call
validation of all chain snapshots and can be expensive on hot paths; update
makeRealtimeIndexingStatusProjectionSchema to reference a lighter-weight
snapshot validator (e.g., create and use a new
makeCrossChainIndexingStatusSnapshotSummarySchema or a cached/z.lazy shallow
schema) instead of the full deep schema, or add a parameter/flag to reuse a
prevalidated snapshot schema; ensure you preserve the existing projectedAt and
worstCaseDistance fields and still apply the two invariants
invariant_realtimeIndexingStatusProjectionProjectedAtIsAfterOrEqualToSnapshotTime
and invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect when
swapping the snapshot schema.
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.test.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/cross-chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...es/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...es/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...s/ensnode-sdk/src/ensindexer/indexing-status/validate/realtime-indexing-status-projection.ts
Outdated
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...snode-sdk/src/ensindexer/indexing-status/deserialize/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@greptile review |
tk-o
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 43 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...es/ensnode-sdk/src/ensindexer/indexing-status/schema/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/schema/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
...es/ensnode-sdk/src/ensindexer/indexing-status/schema/cross-chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@packages/ensnode-sdk/src/api/indexing-status/deserialize.ts`:
- Around line 38-40: The current chain uses z.preprocess inside .pipe which is
less idiomatic for Zod v4; replace the preprocess usage by applying
.transform(buildUnvalidatedIndexingStatusResponse) on
makeSerializedIndexingStatusResponseSchema() and then
.pipe(makeIndexingStatusResponseSchema()) so the flow becomes
makeSerializedIndexingStatusResponseSchema().transform(buildUnvalidatedIndexingStatusResponse).pipe(makeIndexingStatusResponseSchema()).safeParse(maybeResponse),
preserving the same semantics but providing better typing for the transform
callback and clearer composition (update the code that references
makeSerializedIndexingStatusResponseSchema,
buildUnvalidatedIndexingStatusResponse, makeIndexingStatusResponseSchema, and
safeParse).
In `@packages/ensnode-sdk/src/api/indexing-status/zod-schemas.ts`:
- Around line 32-38: The new schema factory functions need explicit TypeScript
return type annotations to lock the public API and prevent signature drift;
update makeSerializedIndexingStatusResponseOkSchema (and the other factory at
makeSerializedIndexingStatusResponseSchema / the factory around lines 62-68) to
declare a concrete Zod return type (for example z.ZodObject or z.ZodType with
the appropriate inferred shape) instead of relying on contextual typing so
callers see the schema type in the signature and compiler/type hints remain
stable.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts`:
- Around line 349-358: The code redundantly checks chain.config.endBlock after
narrowing by chain.config.configType === ChainIndexingConfigTypeIds.Definite;
since that type is a ChainIndexingConfigDefinite and always includes a
non-optional endBlock, remove the extra truthiness check and directly push
chain.config.endBlock.timestamp (i.e., replace the condition `&&
chain.config.endBlock` with no additional check) in the loop where
latestKnownBlockTimestamps.push is called.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.ts`:
- Around line 264-272: The variable allChainsHaveValidStatuses in
checkChainIndexingStatusSnapshotsForOmnichainStatusSnapshotFollowing is
misleading because it uses .some() (checks at least one) not .every(); either
change the logic to use .every() if you intended to verify all chains are
Following, or (preferred) rename the variable to anyChainIsFollowing (or
atLeastOneChainFollowing) and return that variable so the name matches the
actual .some() semantics.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/chain-indexing-status-snapshot.test.ts`:
- Around line 22-23: formatParseError currently uses a non-null assertion on
zodParseError.error which will throw if called with a successful parse; update
the helper (formatParseError) to guard on zodParseError.success (or check
zodParseError.error) and either throw an explicit informative error when success
is true or return a sensible fallback, ensuring prettifyError is only called
when an actual parse error exists; reference the ZodSafeParseResult<T> parameter
to locate and change the helper in tests.
- Around line 276-297: The test data currently violates two invariants
(config.startBlock > config.endBlock and config.startBlock >
latestIndexedBlock); update the serialized fixture in the test so it only
violates the intended one (config.startBlock after latestIndexedBlock).
Concretely, in the test that constructs the ChainIndexingStatusSnapshotCompleted
object used with makeChainIndexingStatusSnapshotSchema(), set config.startBlock
to laterBlockRef, set config.endBlock to a value >= that startBlock (e.g.,
laterBlockRef), and set latestIndexedBlock to latestBlockRef so only the
`config.startBlock` vs `latestIndexedBlock` constraint fails.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/cross-chain-indexing-status-snapshot.ts`:
- Around line 43-78: The invariant
invariant_snapshotTimeIsTheHighestKnownBlockTimestamp can call Math.max over
empty arrays (via omnichainSnapshot.chains being empty) producing -Infinity; add
an explicit guard at the start of the function to handle an empty chains
collection (e.g., if Array.from(omnichainSnapshot.chains.values()).length === 0)
and either return early or push a validation issue, so you never call Math.max
with no operands and the assumption that chains are non-empty is made explicit.
- Around line 14-18: Rename the mis-typed file
omnichian-indexing-status-snapshot.ts to omnichain-indexing-status-snapshot.ts
and update the imports that reference it: replace the import in
cross-chain-indexing-status-snapshot.ts (the import of
buildUnvalidatedOmnichainIndexingStatusSnapshot,
makeOmnichainIndexingStatusSnapshotSchema,
makeSerializedOmnichainIndexingStatusSnapshotSchema) and likewise update the
import paths in deserialize/omnichain-indexing-status-snapshot.ts and
validate/omnichain-indexing-status-snapshot.ts so they point to the corrected
omnichain-indexing-status-snapshot module name.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/omnichian-indexing-status-snapshot.ts`:
- Around line 419-466: The switch in
buildUnvalidatedOmnichainIndexingStatusSnapshot over omnichainStatus is not
exhaustive and can implicitly return undefined for future variants; add a
default branch that performs a never-exhaustive type check (e.g., call an
assertNever/unreachable helper or throw an Error using the omnichainStatus as
payload) so the compiler errors if a new OmnichainIndexingStatusIds value is
added and to ensure the function always returns a value for every case.
- Around line 37-52: The call to getOmnichainIndexingStatus inside
invariant_omnichainSnapshotStatusIsConsistentWithChainSnapshot can throw and
escape Zod's issue collection; wrap the call in a try/catch, and if it throws
push a ctx.issues entry (e.g., code: "custom", input: snapshot) with a message
that includes the caught error message, then return so the exception doesn't
propagate; otherwise continue comparing expectedOmnichainStatus to
snapshot.omnichainStatus as before.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/realtime-indexing-status-projection.ts`:
- Around line 6-7: The import paths are inconsistent: change the import of
makeCrossChainIndexingStatusSnapshotSchema to use the same local-relative style
as makeSerializedCrossChainIndexingStatusSnapshotSchema (i.e., import from
"./cross-chain-indexing-status-snapshot") so both
makeCrossChainIndexingStatusSnapshotSchema and
makeSerializedCrossChainIndexingStatusSnapshotSchema use the same "./" sibling
module path.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts`:
- Around line 1-21: Extract a generic validation helper (e.g.
validateWithSchema) that accepts a Zod schema, an unvalidated value and a type
name, calls schema.safeParse, throws a new Error with prettifyError on failure,
and returns parsed.data; then replace the body of
validateChainIndexingStatusSnapshot (which currently calls
makeChainIndexingStatusSnapshotSchema, schema.safeParse, prettifyError and
returns parsed.data) to call this helper instead; do the same refactor for
validateCrossChainIndexingStatusSnapshot and
validateRealtimeIndexingStatusProjection so they all delegate to
validateWithSchema and remove the duplicated parsing/throwing logic.
- Around line 6-8: The JSDoc summary uses ambiguous wording "maybe" for the
validated type; update the comment for the validator (the JSDoc above
validateChainIndexingStatusSnapshot) to mirror sibling validators by rephrasing
it to something like "Validate a {`@link` ChainIndexingStatusSnapshot} object." so
the doc clearly describes the function purpose and matches other validators'
phrasing.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts`:
- Around line 6-8: Update the JSDoc comment that currently reads "Validate an
{`@link` CrossChainIndexingStatusSnapshot} object." to use the correct indefinite
article by changing "an" to "a" so it reads "Validate a {`@link`
CrossChainIndexingStatusSnapshot} object."; locate the comment above the
validate function or the CrossChainIndexingStatusSnapshot-related declaration in
cross-chain-indexing-status-snapshot.ts and make this single-word correction in
the JSDoc block.
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts`:
- Line 4: Rename the schema file from omnichian-indexing-status-snapshot.ts to
omnichain-indexing-status-snapshot.ts and update the three imports that
reference the misspelled module so they import the corrected filename;
specifically, change the import that brings in
makeOmnichainIndexingStatusSnapshotSchema to reference the corrected path/name
(so the symbol remains makeOmnichainIndexingStatusSnapshotSchema but the module
path uses "omnichain-indexing-status-snapshot"), and apply the same fix to the
other two places that import the schema so all three imports point to the
renamed file.
packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/omnichain-indexing-status-snapshot.ts
Show resolved
Hide resolved
...ges/ensnode-sdk/src/ensindexer/indexing-status/schema/chain-indexing-status-snapshot.test.ts
Show resolved
Hide resolved
...ges/ensnode-sdk/src/ensindexer/indexing-status/schema/realtime-indexing-status-projection.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
.../ensnode-sdk/src/ensindexer/indexing-status/validate/cross-chain-indexing-status-snapshot.ts
Show resolved
Hide resolved
...es/ensnode-sdk/src/ensindexer/indexing-status/validate/omnichain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
...ages/ensnode-sdk/src/ensindexer/indexing-status/schema/omnichain-indexing-status-snapshot.ts
Show resolved
Hide resolved
Contributor
Author
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/realtime-indexing-status-projection.ts`:
- Around line 36-52: The invariant function
invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect currently
computes expectedWorstCaseDistance using
snapshot.omnichainSnapshot.omnichainIndexingCursor; change it to use
snapshot.slowestChainIndexingCursor instead so the check matches the
RealtimeIndexingStatusProjection contract: compute expectedWorstCaseDistance =
projectedAt - snapshot.slowestChainIndexingCursor and keep the rest of the
comparison and ctx.issues push unchanged (refer to the projection, projectedAt,
snapshot, worstCaseDistance symbols in the function).
Comment on lines
+36
to
+52
| export function invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect( | ||
| ctx: ParsePayload<RealtimeIndexingStatusProjection>, | ||
| ) { | ||
| const projection = ctx.value; | ||
| const { projectedAt, snapshot, worstCaseDistance } = projection; | ||
| const { omnichainSnapshot } = snapshot; | ||
| const expectedWorstCaseDistance = projectedAt - omnichainSnapshot.omnichainIndexingCursor; | ||
|
|
||
| if (worstCaseDistance !== expectedWorstCaseDistance) { | ||
| ctx.issues.push({ | ||
| code: "custom", | ||
| input: projection, | ||
| message: | ||
| "`worstCaseDistance` must be the exact difference between `projectedAt` and `snapshot.omnichainIndexingCursor`.", | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
worstCaseDistance invariant uses omnichainSnapshot.omnichainIndexingCursor instead of snapshot.slowestChainIndexingCursor.
The RealtimeIndexingStatusProjection interface (in realtime-indexing-status-projection.ts) documents worstCaseDistance as "The distance between projectedAt and snapshot.slowestChainIndexingCursor." However, this invariant computes it against omnichainSnapshot.omnichainIndexingCursor.
While these are guaranteed equal by invariant_slowestChainEqualsToOmnichainSnapshotTime, using snapshot.slowestChainIndexingCursor would be more self-documenting and directly match the interface contract. It would also remain correct independently of the cross-chain invariant.
Suggested change
const projection = ctx.value;
const { projectedAt, snapshot, worstCaseDistance } = projection;
- const { omnichainSnapshot } = snapshot;
- const expectedWorstCaseDistance = projectedAt - omnichainSnapshot.omnichainIndexingCursor;
+ const expectedWorstCaseDistance = projectedAt - snapshot.slowestChainIndexingCursor;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect( | |
| ctx: ParsePayload<RealtimeIndexingStatusProjection>, | |
| ) { | |
| const projection = ctx.value; | |
| const { projectedAt, snapshot, worstCaseDistance } = projection; | |
| const { omnichainSnapshot } = snapshot; | |
| const expectedWorstCaseDistance = projectedAt - omnichainSnapshot.omnichainIndexingCursor; | |
| if (worstCaseDistance !== expectedWorstCaseDistance) { | |
| ctx.issues.push({ | |
| code: "custom", | |
| input: projection, | |
| message: | |
| "`worstCaseDistance` must be the exact difference between `projectedAt` and `snapshot.omnichainIndexingCursor`.", | |
| }); | |
| } | |
| } | |
| export function invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect( | |
| ctx: ParsePayload<RealtimeIndexingStatusProjection>, | |
| ) { | |
| const projection = ctx.value; | |
| const { projectedAt, snapshot, worstCaseDistance } = projection; | |
| const expectedWorstCaseDistance = projectedAt - snapshot.slowestChainIndexingCursor; | |
| if (worstCaseDistance !== expectedWorstCaseDistance) { | |
| ctx.issues.push({ | |
| code: "custom", | |
| input: projection, | |
| message: | |
| "`worstCaseDistance` must be the exact difference between `projectedAt` and `snapshot.omnichainIndexingCursor`.", | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@packages/ensnode-sdk/src/ensindexer/indexing-status/schema/realtime-indexing-status-projection.ts`
around lines 36 - 52, The invariant function
invariant_realtimeIndexingStatusProjectionWorstCaseDistanceIsCorrect currently
computes expectedWorstCaseDistance using
snapshot.omnichainSnapshot.omnichainIndexingCursor; change it to use
snapshot.slowestChainIndexingCursor instead so the check matches the
RealtimeIndexingStatusProjection contract: compute expectedWorstCaseDistance =
projectedAt - snapshot.slowestChainIndexingCursor and keep the rest of the
comparison and ctx.issues push unchanged (refer to the projection, projectedAt,
snapshot, worstCaseDistance symbols in the function).
Contributor
Author
There was a problem hiding this comment.
@lightwalker-eth do you think this suggestion make sense?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
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
1 participant
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.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
a. Deserialize a serialized representation of business-layer data model, including validation.
b. Validate an unvalidated object with shape matching business-layer data model.
Why
/api/configinto/api/indexing-status#1405, Indexing Status API will soon be different for ENSApi and ENSIndexer. Changes in this PR enable re-using ENSIndexer data model in the future ENSApi data model for Indexing Status.Testing
/api/indexing-statuson local ENSIndexer instance./api/indexing-statuson local ENSApi instance connected to ENSIndexer Alpha in the Green env.Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)