-
Notifications
You must be signed in to change notification settings - Fork 16
WIP feat(ensindexer): introduce Indexing Status Builder module
#1613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,121 @@ | ||||||||||||||||||||||||||||||
| import type { PublicClient } from "viem"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import { bigIntToNumber, deserializeBlockRef } from "@ensnode/ensnode-sdk"; | ||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||
| BlockNumber, | ||||||||||||||||||||||||||||||
| BlockRef, | ||||||||||||||||||||||||||||||
| Blockrange, | ||||||||||||||||||||||||||||||
| ChainId, | ||||||||||||||||||||||||||||||
| ChainIndexingMetrics, | ||||||||||||||||||||||||||||||
| } from "@ensnode/ponder-sdk"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Fetch block ref from RPC. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * @param publicClient for a chain | ||||||||||||||||||||||||||||||
| * @param blockNumber | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * @throws error if data validation fails. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| async function fetchBlockRef( | ||||||||||||||||||||||||||||||
| publicClient: PublicClient, | ||||||||||||||||||||||||||||||
| blockNumber: BlockNumber, | ||||||||||||||||||||||||||||||
| ): Promise<BlockRef> { | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| const block = await publicClient.getBlock({ blockNumber: BigInt(blockNumber) }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return deserializeBlockRef({ | ||||||||||||||||||||||||||||||
| number: bigIntToNumber(block.number), | ||||||||||||||||||||||||||||||
| timestamp: bigIntToNumber(block.timestamp), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||
| const errorMessage = error instanceof Error ? error.message : "Unknown error"; | ||||||||||||||||||||||||||||||
| throw new Error(`Failed to fetch block ref for block number ${blockNumber}: ${errorMessage}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Chain Block Refs | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * Represents information about indexing scope for an indexed chain. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| export interface ChainBlockRefs { | ||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Based on Ponder Configuration | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| config: { | ||||||||||||||||||||||||||||||
| startBlock: BlockRef; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| endBlock: BlockRef | null; | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Based on Ponder runtime metrics | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| backfillEndBlock: BlockRef; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Get {@link IndexedChainBlockRefs} for indexed chains. | ||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||
| * Guaranteed to include {@link ChainBlockRefs} for each indexed chain. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| export async function getChainsBlockRefs( | ||||||||||||||||||||||||||||||
| chainIds: ChainId[], | ||||||||||||||||||||||||||||||
| chainsConfigBlockrange: Record<ChainId, Blockrange>, | ||||||||||||||||||||||||||||||
| chainsIndexingMetrics: Map<ChainId, ChainIndexingMetrics>, | ||||||||||||||||||||||||||||||
| publicClients: Record<ChainId, PublicClient>, | ||||||||||||||||||||||||||||||
| ): Promise<Map<ChainId, ChainBlockRefs>> { | ||||||||||||||||||||||||||||||
| const chainsBlockRefs = new Map<ChainId, ChainBlockRefs>(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| for (const chainId of chainIds) { | ||||||||||||||||||||||||||||||
| const blockrange = chainsConfigBlockrange[chainId]; | ||||||||||||||||||||||||||||||
| const startBlock = blockrange?.startBlock; | ||||||||||||||||||||||||||||||
| const endBlock = blockrange?.endBlock; | ||||||||||||||||||||||||||||||
| const publicClient = publicClients[chainId]; | ||||||||||||||||||||||||||||||
| const indexingMetrics = chainsIndexingMetrics.get(chainId); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (typeof startBlock !== "number") { | ||||||||||||||||||||||||||||||
| throw new Error(`startBlock not found for chain ${chainId}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (typeof publicClient === "undefined") { | ||||||||||||||||||||||||||||||
| throw new Error(`publicClient not found for chain ${chainId}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (typeof indexingMetrics === "undefined") { | ||||||||||||||||||||||||||||||
| throw new Error(`indexingMetrics not found for chain ${chainId}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const historicalTotalBlocks = indexingMetrics.backfillSyncBlocksTotal; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (typeof historicalTotalBlocks !== "number") { | ||||||||||||||||||||||||||||||
| throw new Error(`No historical total blocks metric found for chain ${chainId}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const backfillEndBlock = startBlock + historicalTotalBlocks - 1; | ||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate that The check at Line 92 only verifies the value is a 🛡️ Proposed fix const historicalTotalBlocks = indexingMetrics.backfillSyncBlocksTotal;
- if (typeof historicalTotalBlocks !== "number") {
+ if (typeof historicalTotalBlocks !== "number" || historicalTotalBlocks < 1) {
throw new Error(`No historical total blocks metric found for chain ${chainId}`);
}Based on learnings: "backfillSyncBlocksTotal field in Ponder Indexing Metrics must always be a positive integer (at least 1). Zero blocks for backfill indicates an error condition or misconfiguration, not a valid state." 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| // fetch relevant block refs using RPC | ||||||||||||||||||||||||||||||
| const [startBlockRef, endBlockRef, backfillEndBlockRef] = await Promise.all([ | ||||||||||||||||||||||||||||||
| fetchBlockRef(publicClient, startBlock), | ||||||||||||||||||||||||||||||
| endBlock ? fetchBlockRef(publicClient, endBlock) : null, | ||||||||||||||||||||||||||||||
| fetchBlockRef(publicClient, backfillEndBlock), | ||||||||||||||||||||||||||||||
| ]); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const chainBlockRef = { | ||||||||||||||||||||||||||||||
| config: { | ||||||||||||||||||||||||||||||
| startBlock: startBlockRef, | ||||||||||||||||||||||||||||||
| endBlock: endBlockRef, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| backfillEndBlock: backfillEndBlockRef, | ||||||||||||||||||||||||||||||
| } satisfies ChainBlockRefs; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| chainsBlockRefs.set(chainId, chainBlockRef); | ||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||
| throw new Error(`Could not get BlockRefs for chain ${chainId}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+114
to
+117
|
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Chains are processed sequentially; consider parallelizing across chains. The ♻️ Sketch: parallel processing across chains- const chainsBlockRefs = new Map<ChainIdString, ChainBlockRefs>();
-
- for (const chainId of chainIds) {
- // ... validation and fetch per chain ...
- chainsBlockRefs.set(chainId, chainBlockRef);
- }
-
- return chainsBlockRefs;
+ const entries = await Promise.all(
+ chainIds.map(async (chainId) => {
+ // ... validation and fetch per chain ...
+ return [chainId, chainBlockRef] as const;
+ }),
+ );
+
+ return new Map(entries);🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return chainsBlockRefs; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,154 @@ | ||||||||||||||
| import { | ||||||||||||||
| ChainIndexingConfigTypeIds, | ||||||||||||||
| ChainIndexingStatusIds, | ||||||||||||||
| type ChainIndexingStatusSnapshot, | ||||||||||||||
| type ChainIndexingStatusSnapshotBackfill, | ||||||||||||||
| type ChainIndexingStatusSnapshotCompleted, | ||||||||||||||
| type ChainIndexingStatusSnapshotFollowing, | ||||||||||||||
| type ChainIndexingStatusSnapshotQueued, | ||||||||||||||
| createIndexingConfig, | ||||||||||||||
| } from "@ensnode/ensnode-sdk"; | ||||||||||||||
| import { | ||||||||||||||
| type ChainId, | ||||||||||||||
| type ChainIndexingMetrics, | ||||||||||||||
| type ChainIndexingStatus, | ||||||||||||||
| isBlockRefEqualTo, | ||||||||||||||
| } from "@ensnode/ponder-sdk"; | ||||||||||||||
|
|
||||||||||||||
| import type { ChainBlockRefs } from "./chain-block-refs"; | ||||||||||||||
| import { validateChainIndexingStatusSnapshot } from "./validate/chain-indexing-status-snapshot"; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Build Chain Indexing Status Snapshot | ||||||||||||||
| * | ||||||||||||||
| * Builds {@link ChainIndexingStatusSnapshot} for a chain based on: | ||||||||||||||
| * - block refs based on chain configuration and RPC data, | ||||||||||||||
| * - current indexing status, | ||||||||||||||
| * - current indexing metrics. | ||||||||||||||
| */ | ||||||||||||||
| export function buildChainIndexingStatusSnapshot( | ||||||||||||||
| chainId: ChainId, | ||||||||||||||
| chainBlockRefs: ChainBlockRefs, | ||||||||||||||
| chainIndexingMetrics: ChainIndexingMetrics, | ||||||||||||||
| chainIndexingStatus: ChainIndexingStatus, | ||||||||||||||
| ): ChainIndexingStatusSnapshot { | ||||||||||||||
| const { checkpointBlock } = chainIndexingStatus; | ||||||||||||||
|
Comment on lines
+29
to
+35
|
||||||||||||||
| const config = createIndexingConfig( | ||||||||||||||
| chainBlockRefs.config.startBlock, | ||||||||||||||
| chainBlockRefs.config.endBlock, | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| // TODO: Use `ChainIndexingMetrics` data model from PR #1612. | ||||||||||||||
| // This updated data model includes `type` field to distinguish | ||||||||||||||
| // between different chain indexing phases, for example: | ||||||||||||||
| // Queued, Backfill, Realtime, Completed. | ||||||||||||||
|
|
||||||||||||||
| // In omnichain ordering, if the startBlock is the same as the | ||||||||||||||
| // status block, the chain has not started yet. | ||||||||||||||
| if (isBlockRefEqualTo(chainBlockRefs.config.startBlock, checkpointBlock)) { | ||||||||||||||
|
Comment on lines
+46
to
+48
|
||||||||||||||
| // In omnichain ordering, if the startBlock is the same as the | |
| // status block, the chain has not started yet. | |
| if (isBlockRefEqualTo(chainBlockRefs.config.startBlock, checkpointBlock)) { | |
| // In omnichain ordering, if the start block number is the same as the | |
| // checkpoint block number, the chain has not started yet. | |
| if (chainBlockRefs.config.startBlock.number === checkpointBlock.number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Status determination order: Queued check may mask contradictory states.
If checkpointBlock equals startBlock and indexingCompleted is simultaneously true (a contradictory state), the Queued branch on line 48 fires first and the inconsistency is silently swallowed. I see the TODO on lines 41-44 acknowledges the need for a proper type discriminator from PR #1612, which should address this. Until then, consider adding a defensive log/warning when the Queued condition is met but metrics indicate completion or realtime, so contradictory states don't go unnoticed.
🤖 Prompt for AI Agents
In
`@apps/ensindexer/src/lib/indexing-status-builder/chain-indexing-status-snapshot.ts`
around lines 46 - 94, The Queued branch (when
isBlockRefEqualTo(chainBlockRefs.config.startBlock, checkpointBlock) is true)
can mask contradictory states if chainIndexingMetrics.indexingCompleted or
indexingRealtime are also true; update the queued branch in
chain-indexing-status-snapshot to emit a defensive warning (e.g., logger.warn)
including chainId, checkpointBlock and which metric(s) are set before returning
the Queued snapshot so these inconsistencies are visible in logs; keep the
existing return value (validateChainIndexingStatusSnapshot with
ChainIndexingStatusIds.Queued) but do not change downstream logic.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| type CrossChainIndexingStatusSnapshotOmnichain, | ||
| CrossChainIndexingStrategyIds, | ||
| type OmnichainIndexingStatusSnapshot, | ||
| } from "@ensnode/ensnode-sdk"; | ||
| import type { UnixTimestamp } from "@ensnode/ponder-sdk"; | ||
|
Comment on lines
+1
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check the actual filename in the repository
find . -type f -name "*cross*chain*indexing*" -o -name "*corss*chain*indexing*" | grep -E "\.(ts|tsx|js)$"
# Check for any imports referencing either spelling
rg "from.*corss-chain-indexing-status-snapshot" --type ts
rg "from.*cross-chain-indexing-status-snapshot" --type ts
# Check if the file exists at the path mentioned in the review
ls -la "apps/ensindexer/src/lib/indexing-status-builder/" 2>/dev/null | grep -i "chain"Repository: namehash/ensnode Length of output: 477 🏁 Script executed: # Check for barrel/index exports in the directory
cat "apps/ensindexer/src/lib/indexing-status-builder/index.ts" 2>/dev/null
# Search for imports of the specific function name
rg "buildCrossChainIndexingStatusSnapshotOmnichain" --type ts
# Check for wildcard imports from this directory
rg "from.*indexing-status-builder['\"]" --type tsRepository: namehash/ensnode Length of output: 211 Filename typo: The file is named 🤖 Prompt for AI Agents |
||
|
|
||
| export function buildCrossChainIndexingStatusSnapshotOmnichain( | ||
| omnichainSnapshot: OmnichainIndexingStatusSnapshot, | ||
| snapshotTime: UnixTimestamp, | ||
| ): CrossChainIndexingStatusSnapshotOmnichain { | ||
| return { | ||
| strategy: CrossChainIndexingStrategyIds.Omnichain, | ||
| slowestChainIndexingCursor: omnichainSnapshot.omnichainIndexingCursor, | ||
| snapshotTime, | ||
| omnichainSnapshot, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { | ||
| type ChainIndexingStatusSnapshotBackfill, | ||
| type ChainIndexingStatusSnapshotCompleted, | ||
| type ChainIndexingStatusSnapshotQueued, | ||
| getOmnichainIndexingCursor, | ||
| getOmnichainIndexingStatus, | ||
| OmnichainIndexingStatusIds, | ||
| type OmnichainIndexingStatusSnapshot, | ||
| type OmnichainIndexingStatusSnapshotBackfill, | ||
| type OmnichainIndexingStatusSnapshotCompleted, | ||
| type OmnichainIndexingStatusSnapshotFollowing, | ||
| type OmnichainIndexingStatusSnapshotUnstarted, | ||
| } from "@ensnode/ensnode-sdk"; | ||
| import type { ChainId, PonderIndexingMetrics, PonderIndexingStatus } from "@ensnode/ponder-sdk"; | ||
|
|
||
| import type { ChainBlockRefs } from "./chain-block-refs"; | ||
| import { buildChainIndexingStatusSnapshots } from "./chain-indexing-status-snapshot"; | ||
| import { validateOmnichainIndexingStatusSnapshot } from "./validate/omnichain-indexing-status-snapshot"; | ||
|
|
||
| export function buildOmnichainIndexingStatusSnapshot( | ||
| indexedChainIds: ChainId[], | ||
| chainsBlockRefs: Map<ChainId, ChainBlockRefs>, | ||
| ponderIndexingMetrics: PonderIndexingMetrics, | ||
| ponderIndexingStatus: PonderIndexingStatus, | ||
| ): OmnichainIndexingStatusSnapshot { | ||
| const chainStatusSnapshots = buildChainIndexingStatusSnapshots( | ||
| indexedChainIds, | ||
| chainsBlockRefs, | ||
| ponderIndexingMetrics.chains, | ||
| ponderIndexingStatus.chains, | ||
| ); | ||
| const chains = Array.from(chainStatusSnapshots.values()); | ||
| const omnichainStatus = getOmnichainIndexingStatus(chains); | ||
| const omnichainIndexingCursor = getOmnichainIndexingCursor(chains); | ||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| switch (omnichainStatus) { | ||
| case OmnichainIndexingStatusIds.Unstarted: { | ||
| return validateOmnichainIndexingStatusSnapshot({ | ||
| omnichainStatus: OmnichainIndexingStatusIds.Unstarted, | ||
| chains: chainStatusSnapshots as Map<ChainId, ChainIndexingStatusSnapshotQueued>, // narrowing the type here, will be validated in the following 'check' step | ||
| omnichainIndexingCursor, | ||
| } satisfies OmnichainIndexingStatusSnapshotUnstarted); | ||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| case OmnichainIndexingStatusIds.Backfill: { | ||
| return validateOmnichainIndexingStatusSnapshot({ | ||
| omnichainStatus: OmnichainIndexingStatusIds.Backfill, | ||
| chains: chainStatusSnapshots as Map<ChainId, ChainIndexingStatusSnapshotBackfill>, // narrowing the type here, will be validated in the following 'check' step | ||
| omnichainIndexingCursor, | ||
| } satisfies OmnichainIndexingStatusSnapshotBackfill); | ||
| } | ||
|
|
||
| case OmnichainIndexingStatusIds.Completed: { | ||
| return validateOmnichainIndexingStatusSnapshot({ | ||
| omnichainStatus: OmnichainIndexingStatusIds.Completed, | ||
| chains: chainStatusSnapshots as Map<ChainId, ChainIndexingStatusSnapshotCompleted>, // narrowing the type here, will be validated in the following 'check' step | ||
| omnichainIndexingCursor, | ||
| } satisfies OmnichainIndexingStatusSnapshotCompleted); | ||
| } | ||
|
|
||
| case OmnichainIndexingStatusIds.Following: | ||
| return validateOmnichainIndexingStatusSnapshot({ | ||
| omnichainStatus: OmnichainIndexingStatusIds.Following, | ||
| chains: chainStatusSnapshots, | ||
| omnichainIndexingCursor, | ||
| } satisfies OmnichainIndexingStatusSnapshotFollowing); | ||
| } | ||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,7 @@ | ||||||||||||||||||
| import type { ChainIndexingStatusSnapshot } from "@ensnode/ensnode-sdk"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export function validateChainIndexingStatusSnapshot( | ||||||||||||||||||
| unvalidatedSnapshot: ChainIndexingStatusSnapshot, | ||||||||||||||||||
| ): ChainIndexingStatusSnapshot { | ||||||||||||||||||
|
||||||||||||||||||
| ): ChainIndexingStatusSnapshot { | |
| ): ChainIndexingStatusSnapshot { | |
| if (unvalidatedSnapshot === null || typeof unvalidatedSnapshot !== "object") { | |
| throw new TypeError("Invalid ChainIndexingStatusSnapshot: expected a non-null object."); | |
| } | |
| // Additional structural validation can be added here if the shape of | |
| // ChainIndexingStatusSnapshot is available at runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
No-op validator provides false confidence to callers.
validateChainIndexingStatusSnapshot performs no validation—it returns the input unchanged. Callers will assume the snapshot has been validated. Since ChainIndexingStatusSnapshot is a discriminated union of 4 variants (Queued, Backfill, Following, Completed), consider either:
- Implementing actual invariant checks (e.g., verifying the discriminant and variant-specific fields).
- Adding a clear
TODOcomment so this doesn't ship to production unnoticed.
🤖 Prompt for AI Agents
In
`@apps/ensindexer/src/lib/indexing-status-builder/validate/chain-indexing-status-snapshot.ts`
around lines 1 - 7, validateChainIndexingStatusSnapshot currently returns its
input without checks; replace the no-op with real validation of the
discriminated union ChainIndexingStatusSnapshot by switching on the discriminant
(the variant tag) and asserting each variant's required fields and types (e.g.,
Queued, Backfill, Following, Completed-specific invariants such as required
numeric offsets, timestamps, or nullable fields), throwing a descriptive Error
on failure; if you cannot implement full checks now, replace the body with a
clear TODO comment mentioning validateChainIndexingStatusSnapshot and
short-circuit with a runtime assertion (e.g., throw new Error("TODO:
validateChainIndexingStatusSnapshot not implemented")) so callers don't get
silent false confidence.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import type { OmnichainIndexingStatusSnapshot } from "@ensnode/ensnode-sdk"; | ||
|
|
||
| export function validateOmnichainIndexingStatusSnapshot( | ||
| unvalidatedSnapshot: OmnichainIndexingStatusSnapshot, | ||
| ): OmnichainIndexingStatusSnapshot { | ||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return unvalidatedSnapshot; | ||
| } | ||
tk-o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Preserve the original error as
causefor proper error chaining.The message is extracted, but the full stack trace and error identity are discarded. Using the
causeoption preserves the entire chain for debuggability while still providing your contextual message.♻️ Proposed fix
} catch (error) { - const errorMessage = error instanceof Error ? error.message : "Unknown error"; - throw new Error(`Failed to fetch block ref for block number ${blockNumber}: ${errorMessage}`); + throw new Error(`Failed to fetch block ref for block number ${blockNumber}`, { cause: error }); }🤖 Prompt for AI Agents