Adding accurateAsOf to the Registrar Actions API
#1484
Conversation
🦋 Changeset detectedLatest commit: d6c42b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Thanks for update. Shared a quick comment 👍
| responseCode: z.literal(RegistrarActionsResponseCodes.Ok), | ||
| registrarActions: z.array(makeNamedRegistrarActionSchema(valueLabel)), | ||
| pageContext: makeResponsePageContextSchema(`${valueLabel}.pageContext`), | ||
| accurateAsOf: makeUnixTimestampSchema(`${valueLabel}.accurateAsOf`), |
There was a problem hiding this comment.
@Goader I note how we are using z.strictObject here. As I understand, this will result in a breaking change.
For example, if we don't carefully coordinate this update with a new release of the ENSAwards site, the ENSAwards site will break.
Could you please suggest:
- What we should do to ensure we coordinate change management effectively so that we won't break the ENSAwards site for more than a few minutes at most?
- Please note how @Y3drk will likely soon start making use of special "snapshot" releases of NPM packages published from the ENSNode repo which are auto-generated from the main branch. Therefore, if we merge this PR as it is now, the ENSAwards client, once building from a "snapshot" release of packages in the ENSNode repo would immediately break, since the ENSApi server it is sending queries to would be out of sync and wouldn't offer this additional field yet.
- How we can refine the zod validations so that they will be more generous to future changes. Ex: introducing an additional field should not cause errors and should just be ignored by older client versions.
One quick idea is to:
- Make this field optional (so that snapshot releases can immediately be used on ENSAwards, even after this PR is merged)
- Change
z.strictObjectto be more generous to future changes so this is less tricky later. - Create a follow-up issue for after the next release to change this field from optional to required.
Please let me know what you think 👍
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Nice update. Thanks! Just 1 more key suggestion.
| * @returns The Unix timestamp of when the data was accurate as of | ||
| * @throws Error if the latest indexed block ref for the chain is null | ||
| */ | ||
| export function getAccurateAsOfTimestamp( |
There was a problem hiding this comment.
The registrar actions API can be used to return data from different chains and is not constrained only to the chainId for the Ethnames subregistry.
Therefore, suggested actions are:
- Remove this utility function.
- Update the logic in
apps/ensapi/src/handlers/registrar-actions-api.tsso that instead of callinggetAccurateAsOfTimestampto get the value for theaccurateAsOftimestamp it usesc.var.indexingStatus.snapshot.slowestChainIndexingCursor - Update the comments for the new
accurateAsOffield added inpackages/ensnode-sdk/src/api/registrar-actions/response.tsso that it cites the following idea: "Represents theslowestChainIndexingCursorfrom the latest omnichain indexing status snapshot captured by ENSApi. The state returned in the response is guaranteed to be accurate as of this timestamp but may be from a timestamp higher than this value."
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Thanks for your updates! Looks good 👍
Registrar Actions: Add accurateAsOf Timestamp
closes: #1424
Reviewer Focus (Read This First)
What reviewers should focus on
get-accurate-as-of-timestamp.ts- follows referrer leaderboard pattern, I think I will extend it further later, when making it stateless for the client, so that it can acommodate both referrer leaderboard and registrar actionsundefinedandError, which should be already done in middlewareProblem & Motivation
Why this exists
What Changed (Concrete)
What actually changed
accurateAsOf: UnixTimestamptoRegistrarActionsResponseOktypeaccurateAsOfgetAccurateAsOfTimestamp()helper - usesgetLatestIndexedBlockRef()for the chainDesign & Planning
How this approach was chosen
Followed exact same pattern as referrer leaderboard cache - uses
getLatestIndexedBlockRef()for the subregistry chain instead of omnichain cursor.Self-Review
What you caught yourself
getLatestIndexedBlockRef()Cross-Codebase Alignment
Related code you checked
accurateAsOf,getLatestIndexedBlockRef,ReferrerLeaderboard,indexingStatusCacheDownstream & Consumer Impact
Who this affects and how
UI can now display "accurate as of" timestamps on registrar actions page and live feed.
accurateAsOfto match existing leaderboard APIsTesting Evidence
How this was validated
curlmultiple scenarios, including when ENS Indexer is available or not, and the indexing status cache was populated or notScope Reductions
What you intentionally didn't do
Did not move the logic from referrer leaderboard cache to the helper function yet. I intend and think it will be best to do it when I deal with stateful logic.
Risk Analysis
How this could go wrong
Low risk - additive change only, middleware handles edge cases gracefully via SWR cache.
Pre-Review Checklist (Blocking)