-
Notifications
You must be signed in to change notification settings - Fork 19
feat(cross-subname-indexing): allow activating multiple plugins together #21
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
Changes from all commits
618e8d3
7d8f978
76754e4
0ffa78c
b118dfb
c71479f
ceb03cc
fa57a34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,32 @@ | ||
| import { ACTIVE_PLUGIN } from "./src/lib/plugin-helpers"; | ||
| import { deepMergeRecursive } from "./src/lib/helpers"; | ||
| import { type IntersectionOf, getActivePlugins } from "./src/lib/plugin-helpers"; | ||
| import * as baseEthPlugin from "./src/plugins/base.eth/ponder.config"; | ||
| import * as ethPlugin from "./src/plugins/eth/ponder.config"; | ||
| import * as lineaEthPlugin from "./src/plugins/linea.eth/ponder.config"; | ||
|
|
||
| // list of all available plugins | ||
| // any of them can be activated in the runtime | ||
| const plugins = [baseEthPlugin, ethPlugin, lineaEthPlugin] as const; | ||
|
|
||
| // here we export only a single 'plugin's config, by type it as every config | ||
| // this makes all of the mapping types happy at typecheck-time, but only the | ||
| // relevant config is run at runtime | ||
| export default ((): AllConfigs => { | ||
| const pluginToActivate = plugins.find((p) => p.ownedName === ACTIVE_PLUGIN); | ||
| // intersection of all available plugin configs to support correct typechecking | ||
| // of the indexing handlers | ||
| type AllPluginsConfig = IntersectionOf<(typeof plugins)[number]["config"]>; | ||
|
|
||
| if (!pluginToActivate) { | ||
| throw new Error(`Unsupported ACTIVE_PLUGIN: ${ACTIVE_PLUGIN}`); | ||
| } | ||
| // Activates the indexing handlers included in selected active plugins and | ||
| // returns and intersection of their combined config. | ||
| function getActivePluginsConfig(): AllPluginsConfig { | ||
| const activePlugins = getActivePlugins(plugins); | ||
|
|
||
| pluginToActivate.activate(); | ||
| // load indexing handlers from the active plugins into the runtime | ||
| activePlugins.forEach((plugin) => plugin.activate()); | ||
|
|
||
| return pluginToActivate.config as AllConfigs; | ||
| })(); | ||
| const config = activePlugins | ||
| .map((plugin) => plugin.config) | ||
| .reduce((acc, val) => deepMergeRecursive(acc, val), {} as AllPluginsConfig); | ||
|
|
||
| // Helper type to get the intersection of all config types | ||
| type IntersectionOf<T> = (T extends any ? (x: T) => void : never) extends (x: infer R) => void | ||
| ? R | ||
| : never; | ||
| // The type of the exported default is the intersection of all plugin configs to | ||
| // each plugin can be correctly typechecked | ||
| type AllConfigs = IntersectionOf<(typeof plugins)[number]["config"]>; | ||
| return config as AllPluginsConfig; | ||
| } | ||
|
|
||
| // The type of the default export is the intersection of all available plugin | ||
| // configs to each plugin can be correctly typechecked | ||
| export default getActivePluginsConfig(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ export const makeRegistrarHandlers = (ownedName: `${string}eth`) => { | |
| if (domain.labelName !== name) { | ||
| await context.db | ||
| .update(domains, { id: node }) | ||
| .set({ labelName: name, name: `${name}${ownedName}` }); | ||
| .set({ labelName: name, name: `${name}.${ownedName}` }); | ||
|
Member
Author
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. This was a bug, that existed since we dropped
Member
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. Good catch, thanks. |
||
| } | ||
|
|
||
| await context.db.update(registrations, { id: label }).set({ labelName: name, cost }); | ||
|
|
@@ -79,7 +79,7 @@ export const makeRegistrarHandlers = (ownedName: `${string}eth`) => { | |
| context: Context; | ||
| args: { name: string; label: Hex; cost: bigint }; | ||
| }) { | ||
| return await setNamePreimage(context, name, label, cost); | ||
| await setNamePreimage(context, name, label, cost); | ||
| }, | ||
|
|
||
| async handleNameRenewedByController({ | ||
|
|
@@ -89,7 +89,7 @@ export const makeRegistrarHandlers = (ownedName: `${string}eth`) => { | |
| context: Context; | ||
| args: { name: string; label: Hex; cost: bigint }; | ||
| }) { | ||
| return await setNamePreimage(context, name, label, cost); | ||
| await setNamePreimage(context, name, label, cost); | ||
| }, | ||
|
|
||
| async handleNameRenewed({ | ||
|
|
@@ -117,7 +117,7 @@ export const makeRegistrarHandlers = (ownedName: `${string}eth`) => { | |
|
|
||
| async handleNameTransferred({ | ||
| context, | ||
| args: { tokenId, from, to }, | ||
| args: { tokenId, to }, | ||
| }: { | ||
| context: Context; | ||
| args: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,12 +15,15 @@ export async function setupRootNode({ context }: { context: Context }) { | |
| await upsertAccount(context, zeroAddress); | ||
|
|
||
| // initialize the ENS root to be owned by the zeroAddress and not migrated | ||
| await context.db.insert(domains).values({ | ||
| id: ROOT_NODE, | ||
| ownerId: zeroAddress, | ||
| createdAt: 0n, | ||
| isMigrated: false, | ||
| }); | ||
| await context.db | ||
| .insert(domains) | ||
| .values({ | ||
| id: ROOT_NODE, | ||
| ownerId: zeroAddress, | ||
| createdAt: 0n, | ||
| isMigrated: false, | ||
| }) | ||
| .onConflictDoNothing(); | ||
|
Member
Author
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. I had to introduce
Member
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. @tk-o Hey really appreciate your PR comment on this! Could you please also document this idea on each place in the code where this idea applies? This way the relevant documentation will live on in our code in an easy to find place after this PR is merged. Taking a step-back, I'm a bit confused on why there might be conflicts caused by other plugins here. I suppose the conflict might be caused by how we are enabling the base.eth or linea.eth plugins to be activated without the .eth plugin being activated. In other words, we don't have any concept of plugin dependencies implemented, such that the activation of plugin X also requires the activation of plugin Y. Do I understand that correctly? Appreciate your advice. I'm ok to not introduce plugin dependencies yet. I think we can come back to this as a separate goal in the future. It would add complexity and I'm not sure if its even the best idea. For now just trying to understand the current situation better. Thanks 👍
Member
Author
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. Each plugin requires a root node to be included in the |
||
| } | ||
|
|
||
| function isDomainEmpty(domain: typeof domains.$inferSelect) { | ||
|
|
@@ -89,36 +92,38 @@ export const handleNewOwner = | |
| }) => { | ||
| const { label, node, owner } = event.args; | ||
|
|
||
| const parent = await context.db.find(domains, { id: node }); | ||
| const subnode = makeSubnodeNamehash(node, label); | ||
|
|
||
| const incrementSubdomainCountOnParent = async () => | ||
| // only increment subdomainCount | ||
| Boolean( | ||
| // if the parent domain exists and | ||
| parent !== null && | ||
| // the subdomain doesn't exist yet | ||
| (await context.db.find(domains, { id: subnode })), | ||
| ); | ||
|
|
||
| // ensure owner | ||
| await upsertAccount(context, owner); | ||
|
|
||
| // note that we set isMigrated so that if this domain is being interacted with on the new registry, its migration status is set here | ||
| let domain = await context.db.find(domains, { id: subnode }); | ||
| if (domain) { | ||
| // if the domain already exists, this is just an update of the owner record (& isMigrated) | ||
| await context.db.update(domains, { id: domain.id }).set({ ownerId: owner, isMigrated }); | ||
| } else { | ||
| // otherwise create the domain | ||
| domain = await context.db.insert(domains).values({ | ||
| const domain = await context.db | ||
| .insert(domains) | ||
| .values({ | ||
| id: subnode, | ||
| ownerId: owner, | ||
| parentId: node, | ||
| createdAt: event.block.timestamp, | ||
| isMigrated, | ||
| }); | ||
|
|
||
| // and increment parent subdomainCount | ||
| await context.db | ||
| .update(domains, { id: node }) | ||
| .set((row) => ({ subdomainCount: row.subdomainCount + 1 })); | ||
| } | ||
| }) | ||
| .onConflictDoUpdate(() => ({ | ||
| // if the domain already exists, this is just an update of the owner record (& isMigrated) | ||
| ownerId: owner, | ||
| isMigrated, | ||
| })); | ||
|
|
||
| // if the domain doesn't yet have a name, construct it here | ||
| if (!domain.name) { | ||
| const parent = await context.db.find(domains, { id: node }); | ||
|
|
||
| // TODO: implement sync rainbow table lookups | ||
| // https://github.com/ensdomains/ens-subgraph/blob/master/src/ensRegistry.ts#L111 | ||
| const labelName = encodeLabelhash(label); | ||
|
|
@@ -127,6 +132,12 @@ export const handleNewOwner = | |
| await context.db.update(domains, { id: domain.id }).set({ name, labelName }); | ||
| } | ||
|
|
||
| if (await incrementSubdomainCountOnParent()) { | ||
| await context.db | ||
| .update(domains, { id: parent!.id }) | ||
| .set((row) => ({ subdomainCount: row.subdomainCount + 1 })); | ||
| } | ||
|
Comment on lines
+135
to
+139
Member
Author
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. @shrugs, I hope you'll like this change. It allows not updating the parent domain entry if it does not exist. Useful during partial block-range indexing, and neutral to full block-range indexing.
Member
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. @tk-o Hey thanks for your PR comment above. I see that comment is specific to lines 135-139. What about all the other changes being proposed to While we're working on our "V1" milestone I'm cautious about changes to indexing logic that are difficult for me to easily review. Open to making them if there's a good reason 👍 |
||
|
|
||
| // garbage collect newly 'empty' domain iff necessary | ||
| if (owner === zeroAddress) { | ||
| await recursivelyRemoveEmptyDomainFromParentSubdomainCount(context, domain.id); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,3 +16,55 @@ export const blockConfig = ( | |
| startBlock: Math.min(Math.max(start || 0, startBlock), end || Number.MAX_SAFE_INTEGER), | ||
| endBlock: end, | ||
| }); | ||
|
|
||
| type AnyObject = { [key: string]: any }; | ||
|
|
||
| /** | ||
| * Deep merge two objects recursively. | ||
| * @param target The target object to merge into. | ||
| * @param source The source object to merge from. | ||
| * @returns The merged object. | ||
| * @see https://stackoverflow.com/a/48218209 | ||
| * @example | ||
| * const obj1 = { a: 1, b: 2, c: { d: 3 } }; | ||
| * const obj2 = { a: 4, c: { e: 5 } }; | ||
| * const obj3 = deepMergeRecursive(obj1, obj2); | ||
| * // { a: 4, b: 2, c: { d: 3, e: 5 } } | ||
| */ | ||
| export function deepMergeRecursive<T extends AnyObject, U extends AnyObject>( | ||
|
Member
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. i'd actually prefer to depend on a package for this, what do you think? no dependency is nice but this looks complex enough and generic enough that a well-tested dependency seems a-ok to me. i found this one for example https://github.com/voodoocreation/ts-deepmerge
Member
Author
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. We could use lib, I tried
Member
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. @tk-o Thanks for proposing this. Agreed with @shrugs that a well-tested library for this sounds attractive. My head hurts thinking for code review of testing on this. Appreciate if it might take some time to figure out how to get one of those libraries to work. If we can find a solution to that in this PR, then great 👍 If it needs more time, then please feel welcome to create a new GitHub Issue for us to track this goal and we can come back to it in a separate PR. |
||
| target: T, | ||
| source: U, | ||
| ): T & U { | ||
| const output = { ...target } as T & U; | ||
|
|
||
| function isObject(item: any): item is AnyObject { | ||
| return item && typeof item === "object" && !Array.isArray(item); | ||
| } | ||
|
|
||
| if (isObject(target) && isObject(source)) { | ||
| Object.keys(source).forEach((key) => { | ||
| if (isObject(source[key])) { | ||
| if (!(key in target)) { | ||
| Object.assign(output, { [key]: source[key] }); | ||
| } else { | ||
| (output as AnyObject)[key] = deepMergeRecursive( | ||
| (target as AnyObject)[key], | ||
| (source as AnyObject)[key], | ||
| ); | ||
| } | ||
| } else if (Array.isArray(source[key])) { | ||
| if (!(key in target)) { | ||
| Object.assign(output, { [key]: source[key] }); | ||
| } else { | ||
| (output as AnyObject)[key] = Array.isArray((target as AnyObject)[key]) | ||
| ? [...(target as AnyObject)[key], ...source[key]] | ||
| : source[key]; | ||
| } | ||
| } else { | ||
| Object.assign(output, { [key]: source[key] }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return output; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| # Indexer plugins | ||
|
|
||
| This directory contains plugins which allow defining subname-specific processing of blockchain events. | ||
| Only one plugin can be active at a time. Use the `ACTIVE_PLUGIN` env variable to select the active plugin, for example: | ||
| Only one plugin can be active at a time. Use the `ACTIVE_PLUGINS` env variable to select the active plugin, for example: | ||
|
|
||
| ``` | ||
| ACTIVE_PLUGIN=base.eth | ||
| ACTIVE_PLUGINS=base.eth | ||
| ``` |
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.
Suggesting this to help us give more distinct naming to all available plugins vs all activated plugins.