feat(ensindexer): leverage SWR caches to achieve goals related to LocalPonderClient#1652
feat(ensindexer): leverage SWR caches to achieve goals related to LocalPonderClient#1652tk-o wants to merge 12 commits intofeat/indexing-status-builder-3from
LocalPonderClient#1652Conversation
Allow setting "context" object for cache. This object can be accessed from function that fetches data into cache. The "context" object may include values, such as, API clients, configuration, etc.
This will not be needed as we will use SWRCache with proactive initialization to achieve similar result in a much cleaner way.
For data coherence, it is optimal to fetch from `/status` and `/metrics` endpoints of Ponder app at the same time. Therefore, we will use a single SWRCache to load Ponder Status and Metrics.
With this cachange, the LocalPonderClient reads external data only from SWR caches. This allows managing possible network request failuers easily within each individual cache implementation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
There was a problem hiding this comment.
Pull request overview
This PR refactors ENSIndexer’s LocalPonderClient initialization to rely on SWR caches (with proactive revalidation) and extends the shared SWRCache utility to support a context object for cache loaders.
Changes:
- Extend
SWRCacheto accept an optionalcontextparameter infn()and add asetContext()API. - Refactor
LocalPonderClientto use new SWR-backed caches for Ponder indexing metrics/status (dynamic) and derived immutable chain metadata. - Update the Ponder API handler to use a singleton
LocalPonderClientinstance (no per-request initialization).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/cache/swr-cache.ts | Adds context support and proactive revalidation stop API to the shared SWR cache implementation. |
| packages/ensnode-sdk/src/shared/cache/swr-cache.test.ts | Updates tests for the new (cachedResult, context) callback signature and adds context coverage. |
| apps/ensindexer/src/lib/ponder-api-client.ts | Switches singleton getter to synchronous construction and injects SWR caches into LocalPonderClient. |
| apps/ensindexer/ponder/src/api/lib/local-ponder-client.ts | Reworks LocalPonderClient to extend PonderClient and source metadata via SWR caches. |
| apps/ensindexer/ponder/src/api/lib/chains-indexing-metadata-immutable.ts | Introduces builder for immutable chain indexing metadata. |
| apps/ensindexer/ponder/src/api/lib/chains-indexing-metadata-dynamic.ts | Introduces builder for dynamic chain indexing metadata from metrics/status. |
| apps/ensindexer/ponder/src/api/lib/cache/ponder-client.cache.ts | Adds SWR cache for Ponder metrics/status with proactive revalidation. |
| apps/ensindexer/ponder/src/api/lib/cache/chains-indexing-metadata-immutable.cache.ts | Adds SWR cache for immutable chain metadata and stops revalidation after success. |
| apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts | Uses a module-level singleton LocalPonderClient for request handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| indexingStatus: chainIndexingStatus, | ||
| } satisfies ChainIndexingMetadataDynamic; | ||
|
|
||
| // Cache the dynamic metadata for this chain ID |
There was a problem hiding this comment.
Comment says "Cache the dynamic metadata for this chain ID", but this function is only building and returning a Map (no caching side effects). Please adjust the comment to avoid implying additional behavior.
| // Cache the dynamic metadata for this chain ID | |
| // Store the dynamic metadata for this chain ID in the map |
| this.#chainsIndexingMetadataImmutableCache = chainsIndexingMetadataImmutableCache; | ||
| this.#chainsIndexingMetadataImmutableCache.setContext({ | ||
| indexedChainIds: this.indexedChainIds, | ||
| chainsConfigBlockrange: this.chainsConfigBlockrange, | ||
| publicClients: this.publicClients, | ||
| ponderClientCache: this.#ponderClientCache, | ||
| }); |
There was a problem hiding this comment.
The comment/typing suggests these are "Lazily initialized" (#chainsConfigBlockrange?, #publicClients?), but the constructor immediately forces initialization by reading this.chainsConfigBlockrange and this.publicClients to set the cache context. Either update the comment/field optionality to reflect eager initialization, or defer computing these values until needed (e.g., pass suppliers/getters in the cache context).
| * @param publicClients A map of chain ID to its corresponding public client, | ||
| * used to fetch block references for chain's blockrange. | ||
| * @param ponderClient The Ponder client used to fetch indexing metrics and status. |
There was a problem hiding this comment.
The JSDoc for buildChainsIndexingMetadataImmutable lists a ponderClient param (and describes fetching metrics/status), but the function signature now takes ponderIndexingMetrics directly and doesn’t accept a client. Please update the @param docs to match the current parameters/behavior.
| * @param publicClients A map of chain ID to its corresponding public client, | |
| * used to fetch block references for chain's blockrange. | |
| * @param ponderClient The Ponder client used to fetch indexing metrics and status. | |
| * @param indexedChainIds Set of chain IDs that are being indexed. | |
| * @param chainsConfigBlockrange Map of chain ID to its configured blockrange. | |
| * @param publicClients Map of chain ID to its corresponding public client, | |
| * used to fetch block references for the chain's blockrange. | |
| * @param ponderIndexingMetrics Indexing metrics for all chains, used to derive | |
| * historical/backfill ranges for each indexed chain. |
| public destroy(): void { | ||
| this.stopProactiveRevalidation(); | ||
| } |
There was a problem hiding this comment.
destroy() calls stopProactiveRevalidation() even though it returns a Promise (because the method is async). This creates a floating Promise and will likely trip Biome's suspicious/noFloatingPromises, and it also makes destroy() appear synchronous while doing async work. Consider making stopProactiveRevalidation() synchronous (no async, return void) since it only clears an interval, or make destroy() async and properly await/handle the Promise.
| * @throws Error if the client fails to initialize after | ||
| * the specified number of retries. |
There was a problem hiding this comment.
The JSDoc still mentions retries ("fails to initialize after the specified number of retries"), but the retry logic was removed and this function no longer performs async initialization. Please update/remove this @throws text so it matches the current behavior.
| * @throws Error if the client fails to initialize after | |
| * the specified number of retries. | |
| * @throws Error if the client cannot be instantiated. |
| }, | ||
| ttl: Number.POSITIVE_INFINITY, | ||
| proactiveRevalidationInterval: 5, // indexing metrics and status can change frequently, so proactively revalidate every 5 seconds to ensure data is fresh | ||
| proactivelyInitialize: true, |
There was a problem hiding this comment.
This cache is proactivelyInitialize: true but fn hard-requires context to be set. Because the cache instance is created at module load time, the first proactive revalidation will run before LocalPonderClient calls setContext(), caching an Error until the next interval tick. To avoid a guaranteed initial failure (and up-to-5s delay after context is set), consider setting proactivelyInitialize: false and triggering an initial read() after setContext(), and/or configuring a finite errorTtl so reads can also schedule retries.
| proactivelyInitialize: true, | |
| errorTtl: 5, // on errors, allow retries after 5 seconds instead of caching failures indefinitely | |
| proactivelyInitialize: false, |
|
|
||
| // Stop the proactive revalidation of this cache since we have | ||
| // successfully loaded the data and initialized the client state. | ||
| chainsIndexingMetadataImmutableCache.destroy(); |
There was a problem hiding this comment.
After successfully loading immutable data, this calls chainsIndexingMetadataImmutableCache.destroy(). Now that SWRCache has stopProactiveRevalidation(), using that method here would better express the intent (stop the interval) without implying the cache is being torn down entirely.
| chainsIndexingMetadataImmutableCache.destroy(); | |
| chainsIndexingMetadataImmutableCache.stopProactiveRevalidation(); |
tk-o
left a comment
There was a problem hiding this comment.
Self-review completed.
| * such as API clients or configuration. | ||
| */ | ||
| fn: (cachedResult?: CachedResult<ValueType>) => Promise<ValueType>; | ||
| fn: (cachedResult?: CachedResult<ValueType>, context?: ContextType) => Promise<ValueType>; |
There was a problem hiding this comment.
In my opinion, this has been a missing feature. Without that, all dependencies must be linked directly in the file containing the SWR cache definition. However, with the context variable being available for the fn function, we can model data loading functionality in a flexible way.
| * such as when caching immutable data. | ||
| */ | ||
| public destroy(): void { | ||
| public async stopProactiveRevalidation(): Promise<void> { |
There was a problem hiding this comment.
Adding this function as it reads less scary than .destroy(). This function is handy to stop proactive validation for a given cache programatically.
| // the singleton client instance on app startup. | ||
| // This ensures that the client is ready to use when handling requests, | ||
| // and allows us to catch initialization errors early. | ||
| getLocalPonderClient(); |
There was a problem hiding this comment.
There should be need to call getLocalPonderClient in order to initialize the singleton client instance. We'll manage that initialization via SWR caches which LocalPonderClient will use directly.
| @@ -0,0 +1,57 @@ | |||
| import { SWRCache } from "@ensnode/ensnode-sdk"; | |||
There was a problem hiding this comment.
I initially thought that creating separate cache for Ponder Indexing Metrics and Ponder Indexing Status was a good idea. Then, I figured that fetching those two data objects is optimal when done at the same time. The reason is that both data points will describe states that were captured at the same moment of indexing progress.
| @@ -0,0 +1,55 @@ | |||
| import { SWRCache } from "@ensnode/ensnode-sdk"; | |||
There was a problem hiding this comment.
I initially thought that creating separate cache for Ponder Indexing Metrics and Ponder Indexing Status was a good idea. Then, I figured that fetching those two data objects is optimal when done at the same time. The reason is that both data points will describe states that were captured at the same moment of indexing progress.
| @@ -0,0 +1,80 @@ | |||
| import type { PublicClient } from "viem"; | |||
There was a problem hiding this comment.
We need to fetch ChainIndexingMetadataImmutable object successfully just once, and then keep using it in the application use cases.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
SWRCacheto allow reading "cache context" from the data loader function. Also, introducesstopProactiveRevalidation()method so that proactive validation could be turned off for a given cache instance after some condition is met.PonderClient, and other for fetching data from RPCs (viapublicClients).LocalPonderClientto read data from aforementioned SWR caches to simplify how possible data loading failures are handled.getLocalPonderClient()function that returns aLocalPonderClientsingleton instance. Now, there's no need to await anything.Why
LocalPonderClientshould leverage it.Testing
/api/indexing-statusendpoint (including logs it produces, see below)Logs
Notes for Reviewer (Optional)
LocalPonderClientidea in a meaningful way.LocalPonderClient"knows" about Ponder APIs and configs, and allows caching data that can be later used to handle Indexing Status API requests.Pre-Review Checklist (Blocking)