Replace ponder.on with addOnchainEventListener#1839
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 5fca5f5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces direct Changes
Sequence Diagram(s)sequenceDiagram
participant Ponder as ponder.on
participant Wrapper as addOnchainEventListener
participant Handler as Event Handler
participant DB as ensDb (ensIndexerSchema)
rect rgba(200,200,255,0.5)
Ponder->>Wrapper: register(eventName, wrappedCallback)
end
rect rgba(200,255,200,0.5)
Note right of Wrapper: on event, wrapper builds\nIndexingEngineContext { ... , ensDb: context.db }
Wrapper->>Handler: invoke({ context: IndexingEngineContext, event })
end
rect rgba(255,200,200,0.5)
Handler->>DB: read/write using ensIndexerSchema
DB-->>Handler: result
end
Handler-->>Wrapper: (return / promise)
Wrapper-->>Ponder: (propagate return / unsubscribe handle)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Introduces a new addOnchainEventListener helper (wrapping ponder.on) and migrates the indexer’s event handler registrations to use it, enabling future control logic around when indexing starts.
Changes:
- Added
addOnchainEventListenerwrapper utility and a Vitest unit test for it. - Replaced direct
ponder.on(...)registrations across plugins/handlers withaddOnchainEventListener(...). - Added a changeset to bump
ensindexerfor the API-level migration.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts | Switches TokenScope 3DNS ERC1155 listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/tokenscope/handlers/Seaport.ts | Switches Seaport sale listener registration to addOnchainEventListener. |
| apps/ensindexer/src/plugins/tokenscope/handlers/NameWrapper.ts | Switches NameWrapper ERC1155 listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/tokenscope/handlers/BaseRegistrars.ts | Switches BaseRegistrar transfer listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/shared-handlers/multi-chain/Resolver.ts | Switches shared multichain resolver listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/threedns/handlers/ThreeDNSToken.ts | Switches 3DNS subgraph token listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/threedns/handlers/ThreeDNSResolver.ts | Switches 3DNS subgraph resolver listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registry.ts | Switches ENSv1 registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registrar.ts | Switches subgraph registrar/controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/NameWrapper.ts | Switches subgraph NameWrapper listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/Registry.ts | Switches Lineanames registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/Registrar.ts | Switches Lineanames registrar/controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/NameWrapper.ts | Switches Lineanames NameWrapper listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/basenames/handlers/Registry.ts | Switches Basenames registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/subgraph/plugins/basenames/handlers/Registrar.ts | Switches Basenames registrar/controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/lineanames/handlers/Lineanames_RegistrarController.ts | Switches Lineanames registrar-controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/lineanames/handlers/Lineanames_Registrar.ts | Switches Lineanames registrar listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_UniversalRegistrarRenewalWithReferrer.ts | Switches Ethnames universal registrar renewal listener to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_RegistrarController.ts | Switches Ethnames registrar-controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_Registrar.ts | Switches Ethnames registrar listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/basenames/handlers/Basenames_RegistrarController.ts | Switches Basenames registrar-controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/registrars/basenames/handlers/Basenames_Registrar.ts | Switches Basenames registrar listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/ThreeDNSToken.ts | Switches Protocol Acceleration 3DNS token listener to addOnchainEventListener. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/StandaloneReverseRegistrar.ts | Switches reverse registrar listener to addOnchainEventListener. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/Resolver.ts | Switches Protocol Acceleration resolver listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv2Registry.ts | Switches Protocol Acceleration ENSv2 registry listener to addOnchainEventListener. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv1Registry.ts | Switches Protocol Acceleration ENSv1 registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/shared/Resolver.ts | Switches ENSv2 shared resolver listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/EnhancedAccessControl.ts | Switches ENSv2 EAC listener to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.ts | Switches ENSv2 ETH registrar listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts | Switches ENSv2 registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.ts | Switches ENSv1 registrar controller listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.ts | Switches ENSv1 NameWrapper listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts | Switches ENSv1 registry listeners to addOnchainEventListener. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/BaseRegistrar.ts | Switches ENSv1 BaseRegistrar listeners to addOnchainEventListener. |
| apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.ts | Adds the ponder.on wrapper used across the codebase. |
| apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.test.ts | Adds unit test asserting wrapper delegates to ponder.on. |
| .changeset/bold-bananas-jump.md | Adds changeset describing the migration to addOnchainEventListener. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.ts
Outdated
Show resolved
Hide resolved
Greptile SummaryThis PR introduces Key changes:
Confidence Score: 5/5Safe to merge — purely mechanical refactoring with a well-tested new abstraction layer and no logic changes. All 62 changed files are mechanical No files require special attention — Important Files Changed
Sequence DiagramsequenceDiagram
participant PonderRuntime as Ponder Runtime
participant Wrapper as addOnchainEventListener
participant Builder as buildIndexingEngineContext
participant Handler as Event Handler
Note over Wrapper: Called once at startup (registration)
Wrapper->>PonderRuntime: ponder.on(eventName, callback)
Note over PonderRuntime: On each matched onchain event
PonderRuntime->>Wrapper: callback({ context, event })
Wrapper->>Builder: buildIndexingEngineContext(context)
Builder-->>Wrapper: IndexingEngineContext { ...context, ensDb: context.db }
Wrapper->>Handler: eventHandler({ context: IndexingEngineContext, event })
Handler-->>PonderRuntime: void | Promise<void>
Reviews (3): Last reviewed commit: "Apply AI feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.test.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts (1)
115-206: 🧹 Nitpick | 🔵 TrivialMinor inconsistency in commented-out code:
addEventListenervsaddOnchainEventListener.The commented-out handlers use
addEventListener(lines 115, 138, 173) while the active handlers useaddOnchainEventListener. If these handlers are re-enabled later, they would need to be updated to use the correct function name.🔧 Suggested fix for consistency
- // addEventListener( + // addOnchainEventListener( // namespaceContract(pluginName, "ThreeDNSToken:RegistrationCreated"),Apply similar changes at lines 138 and 173.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts` around lines 115 - 206, Commented-out 3DNS handlers use the old helper name addEventListener which is inconsistent with the active handlers that call addOnchainEventListener; update the three commented blocks that register "ThreeDNSToken:RegistrationCreated", "ThreeDNSToken:RegistrationTransferred", and "ThreeDNSToken:RegistrationBurned" to call addOnchainEventListener instead of addEventListener so they match the active pattern and will work if re-enabled (search for the commented addEventListener(...) blocks and replace the function name).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts`:
- Around line 115-206: Commented-out 3DNS handlers use the old helper name
addEventListener which is inconsistent with the active handlers that call
addOnchainEventListener; update the three commented blocks that register
"ThreeDNSToken:RegistrationCreated", "ThreeDNSToken:RegistrationTransferred",
and "ThreeDNSToken:RegistrationBurned" to call addOnchainEventListener instead
of addEventListener so they match the active pattern and will work if re-enabled
(search for the commented addEventListener(...) blocks and replace the function
name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5abd6553-c93f-4a9a-b32c-20c69f3f5123
📒 Files selected for processing (38)
.changeset/bold-bananas-jump.mdapps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.test.tsapps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/BaseRegistrar.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/EnhancedAccessControl.tsapps/ensindexer/src/plugins/ensv2/handlers/shared/Resolver.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv1Registry.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv2Registry.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/Resolver.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/StandaloneReverseRegistrar.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/ThreeDNSToken.tsapps/ensindexer/src/plugins/registrars/basenames/handlers/Basenames_Registrar.tsapps/ensindexer/src/plugins/registrars/basenames/handlers/Basenames_RegistrarController.tsapps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_Registrar.tsapps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_RegistrarController.tsapps/ensindexer/src/plugins/registrars/ethnames/handlers/Ethnames_UniversalRegistrarRenewalWithReferrer.tsapps/ensindexer/src/plugins/registrars/lineanames/handlers/Lineanames_Registrar.tsapps/ensindexer/src/plugins/registrars/lineanames/handlers/Lineanames_RegistrarController.tsapps/ensindexer/src/plugins/subgraph/plugins/basenames/handlers/Registrar.tsapps/ensindexer/src/plugins/subgraph/plugins/basenames/handlers/Registry.tsapps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/NameWrapper.tsapps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/Registrar.tsapps/ensindexer/src/plugins/subgraph/plugins/lineanames/handlers/Registry.tsapps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/NameWrapper.tsapps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registrar.tsapps/ensindexer/src/plugins/subgraph/plugins/subgraph/handlers/Registry.tsapps/ensindexer/src/plugins/subgraph/plugins/threedns/handlers/ThreeDNSResolver.tsapps/ensindexer/src/plugins/subgraph/plugins/threedns/handlers/ThreeDNSToken.tsapps/ensindexer/src/plugins/subgraph/shared-handlers/multi-chain/Resolver.tsapps/ensindexer/src/plugins/tokenscope/handlers/BaseRegistrars.tsapps/ensindexer/src/plugins/tokenscope/handlers/NameWrapper.tsapps/ensindexer/src/plugins/tokenscope/handlers/Seaport.tsapps/ensindexer/src/plugins/tokenscope/handlers/ThreeDNSToken.ts
tk-o
force-pushed
the
feat/add-onchain-event-listener
branch
from
edd2e68 to
eb3cd6b
Compare
tk-o
force-pushed
the
feat/add-onchain-event-listener
branch
from
eb3cd6b to
daf7f59
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.ts
Outdated
Show resolved
Hide resolved
Works as a thin wrapper over `ponder.on` function. This enabled adding some control logic while indexing onchain events.
Mechanical updates only.
tk-o
force-pushed
the
feat/add-onchain-event-listener
branch
from
daf7f59 to
e7bf838
Compare
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Looks nice! Shared a small suggestion please feel welcome to merge when ready 👍
apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/src/lib/onchain-events/add-onchain-event-listener.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 62 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tk-o
force-pushed
the
feat/add-onchain-event-listener
branch
from
8364aa3 to
9993dae
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/lib/indexing-engines/ponder.ts`:
- Around line 48-69: The JSDoc for addOnchainEventListener has an extra blank
line before the closing */ which breaks formatting; remove the stray empty line
inside the comment so the block closes directly after the final sentence. Locate
the comment above the addOnchainEventListener function (the block referencing
ponder.on and buildIndexingEngineContext) and delete the blank line immediately
before the */ so the JSDoc is compact and correctly formatted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e821c4f8-3556-4bbf-9f62-b65a57ef90c0
📒 Files selected for processing (2)
apps/ensindexer/src/lib/indexing-engines/ponder.test.tsapps/ensindexer/src/lib/indexing-engines/ponder.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 62 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Updates look nice! Thank you 🙌 Shared a few small suggestions please feel welcome to merge when ready 👍
| @@ -1,16 +1,20 @@ | |||
| import type { Context } from "ponder:registry"; | |||
| import schema from "ponder:schema"; | |||
| import ensIndexerSchema from "ponder:schema"; | |||
There was a problem hiding this comment.
Would it make sense to make it so that @/lib/indexing-engines/ponder had an export named ensIndexerSchema and then all the event handlers would import ensIndexerSchema from that file too?
Goals include: no direct imports from Ponder in our event handlers.
| * {@link addOnchainEventListener}. | ||
| */ | ||
| export interface IndexingEngineContext extends PonderIndexingContext<EventNames> { | ||
| ensDb: PonderIndexingContext<EventNames>["db"]; |
There was a problem hiding this comment.
Suggest to add some JSdoc on this ensDb field to explain what this field is.
You might find some inspiration for how to talk about this field from how we documented a similar field in ensdb-sdk but perhaps for this context with Ponder there's other special ideas to note here.
| /** | ||
| * A thin wrapper around `ponder.on` that allows us to: | ||
| * - Provide custom context to event handlers. | ||
| * - Execute additional logic before or after the event handler, if needed. |
There was a problem hiding this comment.
| * - Execute additional logic before or after the event handler, if needed. | |
| * - Execute additional logic before or after event handlers, if needed. |
Separates Ponder modules from being directly tied to event handlers.
tk-o
force-pushed
the
feat/add-onchain-event-listener
branch
from
0e86b04 to
5fca5f5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 62 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vi.mock("ponder:schema", () => ({ | ||
| ensIndexerSchema: {}, | ||
| })); |
There was a problem hiding this comment.
The mock for ponder:schema doesn’t match how ensIndexerSchema is re-exported in ponder.ts (export * as ensIndexerSchema from "ponder:schema" expects ponder:schema to export tables directly). Returning { ensIndexerSchema: {} } makes the namespace shape ensIndexerSchema.ensIndexerSchema, which can mask issues if future tests start accessing schema tables. Prefer mocking ponder:schema as {} (if unused) or exporting the expected table bindings directly.
| vi.mock("ponder:schema", () => ({ | |
| ensIndexerSchema: {}, | |
| })); | |
| vi.mock("ponder:schema", () => ({})); |
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
addOnchainEventListeneras a thin wrapper overponder.onfunction.apps/ensindexer/src/lib/indexing-engines/ponder.tsponder.oncalls withaddOnchainEventListener.context.dbwithcontext.ensDbschemawithensIndexerSchemaWhy
initIndexingstrategy, which would initially require ENSRainbow instance to be ready before the onchain events indexing could start.Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)