-
Notifications
You must be signed in to change notification settings - Fork 15
feat(ensanalytics): referrer details endpoint #1318
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
Conversation
🦋 Changeset detectedLatest commit: 3a3fc72 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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.
|
| referrers: new Map([ | ||
| [ | ||
| "0x538e35b2888ed5b[c58cf2825d76cf6265aa4e31e", | ||
| "0x538e35b2888ed5bc58cf2825d76cf6265aa4e31e", |
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.
probably a bug from earlier
lightwalker-eth
left a comment
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.
@Goader Hey overall looking really good. Shared a few suggestions 👍
| * Extends {@link AwardedReferrerMetrics} but with rank set to null to represent | ||
| * a referrer who is not on the leaderboard (has zero referrals). | ||
| */ | ||
| export interface UnrankedReferrerMetrics |
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.
Nicely done 👍
| rank: null; | ||
|
|
||
| /** | ||
| * Always false for unranked referrers since they don't qualify for awards. |
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.
| * Always false for unranked referrers since they don't qualify for awards. | |
| * Always false for unranked referrers. |
| export interface UnrankedReferrerMetrics | ||
| extends Omit<AwardedReferrerMetrics, "rank" | "isQualified"> { | ||
| /** | ||
| * The referrer's rank on the leaderboard is null because they are not on the leaderboard. |
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.
| * The referrer's rank on the leaderboard is null because they are not on the leaderboard. | |
| * The referrer is not on the leaderboard and therefore has no rank. |
|
|
||
| /** | ||
| * Extends {@link AwardedReferrerMetrics} but with rank set to null to represent | ||
| * a referrer who is not on the leaderboard (has zero referrals). |
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.
| * a referrer who is not on the leaderboard (has zero referrals). | |
| * a referrer who is not on the leaderboard (has zero referrals within the rules associated with the leaderboard). |
| * | ||
| * @see {@link AwardedReferrerMetrics} | ||
| */ | ||
| export interface ReferrerDetailData { |
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.
| export interface ReferrerDetailData { | |
| export interface ReferrerDetailRanked { |
|
|
||
| // If referrer not found, create an unranked zero-score referrer record | ||
| // with rank: null and isQualified: false | ||
| const unrankedReferrerMetrics = buildUnrankedReferrerMetrics(referrer); |
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.
I'm thinking perhaps this idea should move directly into the leaderboard. Ex: you ask the leaderboard to get the details of referrer X and the leaderboard then internally gives you back a ranked or unranked referrer as appropriate.
| }); | ||
|
|
||
| // Get referrer detail for a specific address | ||
| app.get("/referrer/:referrer", validate("param", referrerAddressSchema), async (c) => { |
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.
| app.get("/referrer/:referrer", validate("param", referrerAddressSchema), async (c) => { | |
| app.get("/referrers/:referrer", validate("param", referrerAddressSchema), async (c) => { |
Goal: Align with the existing API endpoint. One is for paginating through all referrers, then if you add this extra detail in the path it ideally should give the details of one specific referrer.
| } satisfies ReferrerDetailResponse), | ||
| ); | ||
| } catch (error) { | ||
| logger.error({ error }, "Error in /ensanalytics/referrer/:referrer endpoint"); |
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.
See feedback above on path
packages/ensnode-sdk/src/client.ts
Outdated
| * }); | ||
| * if (response.responseCode === ReferrerDetailResponseCodes.Ok) { | ||
| * const { referrer } = response.data; | ||
| * if (referrer.rank !== null) { |
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.
Please see related feedback about adding a type field which we could use to more explicitly distinguish between ranked vs unranked.
|
@Goader Hey, as part of this PR could you please also look for other places in our existing code where terminology might be refined. For example I was just looking through our existing code and saw this: https://github.com/namehash/ensnode/blob/main/packages/ensnode-sdk/src/client.ts#L366-L418 Here terminology improvements can include:
|
lightwalker-eth
left a comment
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.
@Goader Hey great updates 🚀
I shared a few small suggestions.
Could you please also add changesets as necessary for this PR?
After that, please take the lead to merge this PR 🚀 Thanks!
| * Request parameters for a referrer leaderboard page query. | ||
| */ | ||
| export interface ReferrerLeaderboardPaginationRequest extends ReferrerLeaderboardPaginationParams {} | ||
| export interface ReferrerLeaderboardPageRequest extends ReferrerLeaderboardPaginationParams {} |
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.
| export interface ReferrerLeaderboardPageRequest extends ReferrerLeaderboardPaginationParams {} | |
| export interface ReferrerLeaderboardPageRequest extends ReferrerLeaderboardPageParams {} |
| error: "Internal Server Error", | ||
| errorMessage: "Failed to load referrer leaderboard data.", | ||
| } satisfies ReferrerDetailResponse), | ||
| 500, |
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.
| error: "Internal Server Error", | |
| errorMessage: "Failed to load referrer leaderboard data.", | |
| } satisfies ReferrerDetailResponse), | |
| 500, | |
| error: "Service Unavailable", | |
| errorMessage: "Referrer leaderboard data has not been successfully cached yet.", | |
| } satisfies ReferrerDetailResponse), | |
| 503, |
| * @param referrer - The referrer address | ||
| * @returns An {@link UnrankedReferrerMetrics} with zero values for all metrics and null rank | ||
| */ | ||
| export const buildUnrankedReferrerMetrics = (referrer: Address): UnrankedReferrerMetrics => { |
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.
I assume we aren't using this anywhere anymore based on the new logic in getReferrerDetail?
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.
It is used in getReferrerDetail, and I think it should stay that way to avoid mixing logic. I also added a verification function for buildUnrankedReferrerMetrics similarly to other build... functions.
closes: #1262
This PR introduces a new referrer detail endpoint API that allows querying individual referrer information, whether they are ranked on the leaderboard or not.
Changes:
@namehash/ens-referrals package:
getReferrerDetail()function to retrieve detail for a specific referrer addressReferrerDetaildiscriminated union type (ReferrerDetailRanked|ReferrerDetailUnranked)UnrankedReferrerMetricsinterface for referrers not on the leaderboard (rank: null, isQualified: false)buildUnrankedReferrerMetrics()helper to create zero-score referrer recordsvalidateUnrankedReferrerMetrics()validation functionReferrerLeaderboardPaginationParams→ReferrerLeaderboardPageParams)@ensnode/ensnode-sdk package:
getReferrerDetail()method toENSNodeClientclassReferrerDetailRequestandReferrerDetailResponsetypesReferrerDetailResponseCodesenum (Ok, Error)makeReferrerDetailResponseSchema,makeReferrerDetailRankedSchema,makeReferrerDetailUnrankedSchema)serializeReferrerDetailResponse,deserializeReferrerDetailResponse)ensapi package:
GET /ensanalytics/referrers/:referrer