-
Notifications
You must be signed in to change notification settings - Fork 15
feat(ensindexer): introduce initial efp plugin demo
#771
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?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: a35833c The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
This is a plugin demonstrating ENSNode's capability to index the Ethereum Follow Protocol data.
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.
@tk-o Awesome to see this! Reviewed and shared a few suggestions 👍
|
BugBot run |
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.
Bug: Type Mismatch Between Functions
The getDatasources function's return type was changed from Datasource[] to Record<DatasourceName, Datasource>, but the getIndexedChainIds function still expects Datasource[] as input. This type mismatch causes a runtime error when getIndexedChainIds(getDatasources(...)) is called, as getIndexedChainIds attempts to call .map() on the object returned by getDatasources.
apps/ensindexer/src/lib/plugin-helpers.ts#L116-L143
ensnode/apps/ensindexer/src/lib/plugin-helpers.ts
Lines 116 to 143 in 57ef002
| */ | |
| export function getDatasources( | |
| config: Pick<ENSIndexerConfig, "ensDeploymentChain" | "plugins">, | |
| ): Record<DatasourceName, Datasource> { | |
| const requiredDatasourceNames = getRequiredDatasourceNames(config.plugins); | |
| const ensDeployment = getENSDeployment(config.ensDeploymentChain); | |
| const ensDeploymentDatasources = Object.entries(ensDeployment) as Array< | |
| [DatasourceName, Datasource] | |
| >; | |
| const datasources = {} as Record<DatasourceName, Datasource>; | |
| for (let [datasourceName, datasource] of ensDeploymentDatasources) { | |
| if (requiredDatasourceNames.includes(datasourceName)) { | |
| datasources[datasourceName] = datasource; | |
| } | |
| } | |
| return datasources; | |
| } | |
| /** | |
| * Get a list of unique indexed chain IDs for selected plugin names. | |
| */ | |
| export function getIndexedChainIds(datasources: Datasource[]): number[] { | |
| const indexedChainIds = datasources.map((datasource) => datasource.chain.id); | |
| return uniq(indexedChainIds); | |
| } |
BugBot free trial expires on June 9, 2025
You have used $0.00 of your $5.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
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.
@tk-o Hey super updates! Reviewed and shared some additional suggestions 👍
efp pluginefp plugin demo
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.
@tk-o Good to see the improvements here 👍 Thanks!
Reviewed and shared feedback.
| Lineanames: "lineanames", | ||
| ThreeDNSOptimism: "threedns-optimism", | ||
| ThreeDNSBase: "threedns-base", | ||
| EFPRoot: "efp-root", |
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.
| EFPRoot: "efp-root", | |
| EFPRoot: "efproot", |
Goal: enhance alignment with the ENSRoot.
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.
Perhaps we should change the value for ENSRoot to be ens-root so we enhance alignment across all DatasourceNames entries?
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.
@tk-o sure I'm good with that 👍
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.
@tk-o Thanks for your updates 👍 Reviewed and shared feedback.
| "EVM chain ID of the chain where the EFP list records are stored, an `EFPDeploymentChainId` value.", | ||
| listRecordsAddress: | ||
| "Contract address on chainId where the EFP list records are stored (always lowercase).", | ||
| slot: "A unique identifier within the List Storage Location, distinguishes between multiple EFP lists stored in the same `EFPListRecords` smart contract by serving as the key in mappings for list operations and metadata.", |
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.
| slot: "A unique identifier within the List Storage Location, distinguishes between multiple EFP lists stored in the same `EFPListRecords` smart contract by serving as the key in mappings for list operations and metadata.", | |
| slot: "A unique identifier distinguishing between multiple EFP lists stored in the same `EFPListRecords` smart contract.", |
Is that fair?
| lslId: | ||
| "Value of `EncodedLsl` type (optional, lowercase if present). Stores the ID of the List Storage Location. If the List Storage Location was never created or not in a recognized format, this field value will be `null`.", | ||
| }), | ||
| ...generateTypeDocSetWithTypeName("efp_listStorageLocation", "EFP List Storage Location", { |
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.
| ...generateTypeDocSetWithTypeName("efp_listStorageLocation", "EFP List Storage Location", { | |
| ...generateTypeDocSetWithTypeName("efp_listStorageLocation", "EFP List Storage Locations with recognized formatting", { |
Is that fair?
| /** | ||
| * The following is documentation for packages/ensnode-schema/src/efp.schema.ts | ||
| */ | ||
| ...generateTypeDocSetWithTypeName("efp_listToken", "EFP List Token", { |
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.
| ...generateTypeDocSetWithTypeName("efp_listToken", "EFP List Token", { | |
| ...generateTypeDocSetWithTypeName("efp_listToken", "EFP List Tokens", { |
| * The following is documentation for packages/ensnode-schema/src/efp.schema.ts | ||
| */ | ||
| ...generateTypeDocSetWithTypeName("efp_listToken", "EFP List Token", { | ||
| id: "Unique token ID for an EFP List Token", |
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.
| id: "Unique token ID for an EFP List Token", | |
| id: "Unique token ID for the ERC-721A NFT representing the EFP List Token", |
| slot: "The 32-byte value that specifies the storage slot of the EFP list records within the listRecordsAddress contract. This disambiguates multiple lists stored within the same contract and de-couples it from the EFP List NFT token id which is stored on the EFP deployment root chain and inaccessible on other chains.", | ||
| listTokenId: "Unique identifier for this EFP list token", | ||
| listToken: "A reference to the related ListToken entity", | ||
| id: "ListStorageLocation ID, an `EncodedLsl` value (always lowercase).", |
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.
| id: "ListStorageLocation ID, an `EncodedLsl` value (always lowercase).", | |
| recognizedLsl: "ListStorageLocation with a recognized valid `EncodedLsl` value (always lowercase).", |
What do you think?
I find it nice to avoid introducing additional terminology for "lsl id". Instead, in my current understanding, we have lsl, and a subset of lsl we index will be "recognized". All lsl we index will be found in the efp_listToken table, while only the recognized ones will be found in the efp_listStorageLocation table.
| @@ -0,0 +1,56 @@ | |||
| /** | |||
| * This Hono application enables the ENSIndexer API to handle requests related to the Ethereum Follow Protocol. | |||
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.
| * This Hono application enables the ENSIndexer API to handle requests related to the Ethereum Follow Protocol. | |
| * This Hono application demonstrates a draft proof-of-concept for how custom API handlers can be defined to operate on indexed Ethereum Follow Protocol data. |
| import { type ListTokenId, parseListTokenId } from "./utils"; | ||
|
|
||
| interface HonoEFP { | ||
| db: ReadonlyDrizzle<Record<string, unknown>>; |
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 not so familiar with Drizzle. Are we supposed to pass in the overall schema we define in another file here to help give Drizzle full type system knowledge?
Appreciate your advice.
apps/ensindexer/src/api/index.ts
Outdated
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.
| // Use Hono EFP application for handling EFP-related requests | |
| // Activate the draft proof-of-concept Hono app for handling EFP-related API requests |
| listRecordsAddress: lslContract.listRecordsAddress, | ||
| slot: lslContract.slot, | ||
| }) | ||
| .onConflictDoNothing(); |
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 my other comments about suggested data model changes.
I think we want to remove this onConflictDoNothing and change this from an insert operation to an upsert operation.
| }) | ||
| .onConflictDoNothing(); | ||
| } catch { | ||
| // The `encodedLsl` value could not be decoded. |
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 my other comments about suggested data model changes.
Based on those suggestions, I believe here we want to perform a delete operation in the efp_listStorageLocation table to handle the case that the previous LSL for the List Token was recognized but the new value isn't.
This PR introduces an initial ENSNode plugin demo for indexing the Ethereum Follow Protocol.
Suggested review order:
packages/datasources/src/*&packages/ensnode-sdk/src/utils/types.tspackages/ensnode-schema/src/efp.schema.ts&apps/ensindexer/src/lib/api-documentation.tsapps/ensindexer/src/plugins/efp/efp.plugin.tsapps/ensindexer/src/plugins/efp/lib/utils.tsapps/ensindexer/src/plugins/efp/lib/chains.tsapps/ensindexer/src/plugins/efp/lib/lsl.tsapps/ensindexer/src/plugins/efp/lib/api.tsapps/ensindexer/src/plugins/efp/handlers/EFPListRegistry.tsMost important features:
Transferevent (handled event variants: mint, ownership change, burn)UpdateListStorageLocationeventefp_listStorageLocationtableefp_unrecognizedListStorageLocationtableGET /efp/list/:listTokenIdroute returns basic information about a requested List Token (including its encoded LSL, if available)Most important changes:
EFPBasedatasource added to themainnetENS Namespace object and to thesepoliaENS Namespace object;EFPListRegistrycontract on, respectively, Base and Base Sepolia chains.efpplugin created;EFPRootdatasource only (as theEFPListRegistrycontract has only been present on Base chain)packages/ensnode-schema/src/efp.schema.tsfile),event.args.fromis thezeroAddress),UpdateListStorageLocationevent,listStorageLocationvalue cannot be parsed successfully — in this case, we store it in its original encoded form on the relevant List Token entity/efppath