feat(ensv2): HCA-aware Event.sender + filter + docs#2014
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 21e87e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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 |
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR introduces HCA-aware event support to the Omnigraph API and ENS indexer. It adds a new Changes
Sequence DiagramsequenceDiagram
participant ENSv2Event as ENSv2 Event
participant Handler as Event Handler
participant AcctHelper as ensureAccount
participant EventHelper as ensureEvent
participant Database as Database
participant GraphQLAPI as GraphQL API
participant Client as Client
ENSv2Event->>Handler: Event triggered (e.g., Transfer)
Handler->>Handler: Extract HCA-aware sender<br/>(from event args or tx.from)
Handler->>AcctHelper: ensureAccount(address)
AcctHelper->>Database: INSERT account if needed
AcctHelper-->>Handler: NormalizedAddress or null
Handler->>EventHelper: ensureEvent(event, sender)
EventHelper->>Database: INSERT event with sender override
EventHelper-->>Handler: Confirmation
Client->>GraphQLAPI: Query events (sender filter)
GraphQLAPI->>Database: SELECT from events WHERE sender = ?
Database-->>GraphQLAPI: Events matching HCA sender
GraphQLAPI-->>Client: Return Event with sender field
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 55 minutes and 48 seconds.Comment |
Contributor
Greptile SummaryThis PR surfaces the pre-existing Confidence Score: 5/5Safe to merge; only P2 style findings, no logic or security issues. All handler call sites pass the right HCA-candidate arg, the No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Chain as On-Chain Event
participant Handler as ENSv2 Handler
participant EA as ensureAccount()
participant EE as ensureEvent()
participant DB as events table
Chain->>Handler: e.g. LabelRegistered(sender, ...)
Handler->>EA: ensureAccount(context, sender)
EA-->>Handler: senderId (NormalizedAddress | null)
Handler->>EE: ensureEvent(context, event, senderId)
EE->>EE: sender = senderId ?? tx.from
EE->>DB: INSERT events SET sender=...
DB-->>EE: eventId
EE-->>Handler: eventId
note over DB: bySender index on events.sender
participant API as GraphQL API
participant Resolver as eventsWhereConditions()
API->>Resolver: EventsWhereInput { sender: "0x..." }
Resolver->>DB: WHERE events.sender = ?
DB-->>API: matching events
note over API: Account.events uses sender=parent.id (HCA-aware)
Reviews (6): Last reviewed commit: "fix: address Copilot/Vercel review notes..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
# Conflicts: # docker/docker-compose.devnet.yml # docker/envs/.env.docker.devnet
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/lib/ensv2/event-db-helpers.ts (1)
23-27: 🛠️ Refactor suggestion | 🟠 MajorRemove redundant
@returnsJSDoc line.Line 26 restates the summary and should be dropped for consistency.
Suggested cleanup
/** * Ensures that an Event entity exists for the given `context` and `event`, returning the Event's * unique id. - * - * `@returns` event.id */As per coding guidelines:
**/*.{ts,tsx}: Do not add JSDoc@returnstags that merely restate the method summary; remove such redundancy during PR review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/ensv2/event-db-helpers.ts` around lines 23 - 27, Remove the redundant JSDoc "@returns event.id" line from the comment block that begins "Ensures that an Event entity exists for the given `context` and `event`" (the JSDoc describing the function that returns the Event's unique id); keep the summary and any meaningful tags but delete the `@returns` tag that merely restates the summary so the JSDoc follows the project's guideline against redundant return tags.
🤖 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/lib/ensv2/event-db-helpers.ts`:
- Around line 23-27: Remove the redundant JSDoc "@returns event.id" line from
the comment block that begins "Ensures that an Event entity exists for the given
`context` and `event`" (the JSDoc describing the function that returns the
Event's unique id); keep the summary and any meaningful tags but delete the
`@returns` tag that merely restates the summary so the JSDoc follows the project's
guideline against redundant return tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b0814f5-f20c-4286-99b8-9816b57990a1
📒 Files selected for processing (16)
.changeset/hca-aware-events.mdapps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.tsapps/ensapi/src/omnigraph-api/schema/account.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/schema/event.tsapps/ensapi/src/omnigraph-api/schema/permissions.tsapps/ensapi/src/omnigraph-api/schema/registration.tsapps/ensapi/src/omnigraph-api/schema/registry-permissions-user.tsapps/ensapi/src/omnigraph-api/schema/resolver-permissions-user.tsapps/ensindexer/src/lib/ensv2/account-db-helpers.tsapps/ensindexer/src/lib/ensv2/event-db-helpers.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.tsdocker/docker-compose.devnet.ymldocker/envs/.env.docker.devnetpackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts
💤 Files with no reviewable changes (1)
- docker/envs/.env.docker.devnet
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes ENSv2 event indexing and Omnigraph querying HCA-aware by introducing/using an events.sender actor field (distinct from tx.from) and switching Account.events to filter by that actor.
Changes:
- Adds an
events.sendercolumn + index and documents HCA-aware semantics across ENSv2 schema fields. - Plumbs “sender override” through ENSv2 indexer handlers so
Event.senderreflects event-level actor args when available. - Exposes
Event.sender+ asenderfilter in Omnigraph, and flipsAccount.eventsto filter bysenderinstead offrom.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts | Adds events.sender + bySender index; updates address typing/docs to emphasize HCA-aware vs tx.from. |
| apps/ensindexer/src/lib/ensv2/event-db-helpers.ts | Extends ensureEvent to persist the new sender field with override/fallback behavior. |
| apps/ensindexer/src/lib/ensv2/account-db-helpers.ts | Updates ensureAccount to return the inserted account id for reuse by handlers. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.ts | Passes registrant-derived sender override into ensureEvent for NameRegistered. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts | Passes registrant/unregistrant/sender/operator overrides into ensureEvent for key ENSv2 events. |
| apps/ensapi/src/omnigraph-api/schema/event.ts | Adds Event.sender, updates docs for Event.from, adds sender filter, updates AccountEventsWhereInput semantics. |
| apps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.ts | Implements SQL condition for where.sender. |
| apps/ensapi/src/omnigraph-api/schema/account.ts | Flips Account.events resolver to filter by sender instead of from. |
| apps/ensapi/src/omnigraph-api/schema/domain.ts | Updates Domain.owner description for ENSv1 vs ENSv2 HCA semantics. |
| apps/ensapi/src/omnigraph-api/schema/registration.ts | Documents ENSv2 registrant/unregistrant HCA-aware semantics. |
| apps/ensapi/src/omnigraph-api/schema/permissions.ts | Documents PermissionsUser.user as HCA-aware when applicable. |
| apps/ensapi/src/omnigraph-api/schema/registry-permissions-user.ts | Updates user field docs for HCA awareness. |
| apps/ensapi/src/omnigraph-api/schema/resolver-permissions-user.ts | Updates user field docs for HCA awareness. |
| docker/envs/.env.docker.devnet | Removes label-set/schema version env vars from this env file. |
| docker/docker-compose.devnet.yml | Moves label-set/schema version configuration into compose env blocks; removes RPC_URL_1 for devnet services. |
| .changeset/hca-aware-events.md | Changeset describing new Omnigraph Event.sender and Account.events behavior change. |
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/ensv2/event-db-helpers.ts:66
ensureEventnow persistsevent.sender(and already persistedfrom/to/address) into columns typed/documented asNormalizedAddress, but the values are written directly fromevent.transaction.from/event.transaction.to/event.log.addresswithout any normalization or assertion. If Ponder ever provides checksummed/mixed-case addresses here, Postgres equality comparisons will become case-sensitive mismatches (notably GraphQL’sAddressscalar parses inputs to lowercase), andsender/from/addressfilters will silently return no results.
Recommend normalizing (or at least asserting normalization) for sender, from, to, and address at the write boundary in ensureEvent to guarantee the NormalizedAddress invariant.
.values({
id: event.id,
// sender override if provided, otherwise transaction.from
sender: sender ?? event.transaction.from,
// chain
chainId: context.chain.id,
// block
blockNumber: event.block.number,
blockHash: event.block.hash,
timestamp: event.block.timestamp,
// transaction
transactionHash: event.transaction.hash,
transactionIndex: event.transaction.transactionIndex,
from: event.transaction.from,
to: event.transaction.to,
// log
address: event.log.address,
logIndex: event.log.logIndex,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…dAddress Update HCA-aware Address descriptions across schema and GraphQL to a unified pattern: "the HCA account address if used, otherwise Transaction.from". Tighten interpretAddress and ensureAccount inputs to NormalizedAddress. Regenerate omnigraph schema.graphql and introspection.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (2)
apps/ensapi/src/omnigraph-api/schema/permissions.ts:268
PermissionsUser.userdescription says it falls back toTransaction.from, but this field is the account being granted roles (from the permissions event args) and is not tied to the tx submitter. This wording will mislead API consumers when tx.from is a relayer or when roles are granted to an address other than the submitter. Suggest rewording to describe it as the role grantee address (HCA address if applicable) without referencingTransaction.from.
user: t.field({
description:
"The User for whom these Roles are granted: the HCA account address if used, otherwise Transaction.from.",
type: AccountRef,
nullable: false,
resolve: (parent) => parent.user,
}),
apps/ensapi/src/omnigraph-api/schema/event.ts:224
AccountEventsWhereInputis documented as "like EventsWhereInput but without sender", but it currently also omits thefromfilter. WithAccount.eventsnow implyingsender, it may be useful (and less surprising) to allowfrominAccountEventsWhereInputso callers can still filter bytx.fromwithin an account’s sender-scoped events; otherwise, consider updating the docstring/description to explicitly state which filters are available.
/**
* Shared filter for events connections. Used by Domain.events, Resolver.events, Permissions.events,
* and Account.events (which excludes `sender` since it's implied).
*/
export const EventsWhereInput = builder.inputType("EventsWhereInput", {
description: "Filter conditions for an events connection.",
fields: (t) => ({
selector_in: t.field({
type: ["Hex"],
description:
"Filter to events whose selector (event signature) is one of the provided values.",
}),
timestamp_gte: t.field({
type: "BigInt",
description: "Filter to events at or after this UnixTimestamp.",
}),
timestamp_lte: t.field({
type: "BigInt",
description: "Filter to events at or before this UnixTimestamp.",
}),
from: t.field({
type: "Address",
description:
"Filter to events whose `tx.from` matches. Not HCA-aware — use `sender` to filter by the HCA account address.",
}),
sender: t.field({
type: "Address",
description:
"Filter to events whose `sender` matches: the HCA account address if used, otherwise Transaction.from.",
}),
}),
});
/**
* Like EventsWhereInput but without `sender` (used where `sender` is implied, e.g. Account.events).
*/
export const AccountEventsWhereInput = builder.inputType("AccountEventsWhereInput", {
description: "Filter conditions for Account.events (where `sender` is implied by the Account).",
fields: (t) => ({
selector_in: t.field({
type: ["Hex"],
description:
"Filter to events whose selector (event signature) is one of the provided values.",
}),
timestamp_gte: t.field({
type: "BigInt",
description: "Filter to events at or after this UnixTimestamp.",
}),
timestamp_lte: t.field({
type: "BigInt",
description: "Filter to events at or before this UnixTimestamp.",
}),
}),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Member
Author
|
@greptile review |
Reword HCA-aware Address descriptions for fields that aren't actually populated from tx.from (Domain.owner, Registration.registrant, Registration.unregistrant, PermissionsUser.user). These read from protocol event args (TransferSingle.to, LabelRegistered.sender, EACRolesChanged.account), not Transaction.from. Keep "otherwise Transaction.from" only on Event.sender and the sender filter, where it is literally accurate. Update Account.events integration test to assert event.sender (now HCA-aware) instead of event.from. Add sender to the shared EventFragment and EventResult type. Regenerate omnigraph schema.graphql. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@greptile review |
lightwalker-eth
approved these changes
Apr 29, 2026
Member
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Nice 🚀 1 small comment please merge when ready 👍
- events: add sender column row, update from description to flag never HCA-aware, add sender to indexes list - permissions_user_events: document join table (was missing from initial ENSDb docs in #2007) - domains.ownerId: polymorphic ENSv1 effective-owner / ENSv2 on-chain owner (HCA-aware) wording - registrations.registrantId / unregistrantId: ENSv2 HCA-aware note - permissions_users.user: HCA-aware grantee description Per #2014 (comment) review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per Copilot review: the description says "without sender" but the input omitted both sender and from. Add a from filter so callers can narrow by tx.from while sender is implied by the Account. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1813
Reviewer Focus (Read This First)
primary review surface:
Account.eventssemantics flip (apps/ensapi/src/omnigraph-api/schema/account.ts): now filters bysender(HCA-aware) instead offrom(tx.from). this is a quiet behavior change — anyone relying on the old "all txs this EOA submitted" semantics will see different results once HCA traffic exists.apps/ensindexer/src/plugins/ensv2/handlers/ensv2/*,apps/ensindexer/src/lib/ensv2/event-db-helpers.ts): the override path is what makesEvent.senderHCA-aware. confirm each ENSv2 ensureEvent call site passes the right HCA-candidate arg (registrant / unregistrant / sender / operator) and not, e.g., the wrong field.events.sendercolumn (NOT NULL) andevents_sender_index. requires reindex.Problem & Motivation
ENSv2 contracts route writes through HCAs (Hybrid Custody Accounts / smart accounts) via
IHCAFactoryBasic. on-chain events emit an explicitsender/owner/accountargument that is HCA-aware — the HCA address rather than the controlling EOA. previously the indexedeventstable only storedtx.from, which is the EOA/relayer that submitted the transaction and is never HCA-aware. omnigraph consumers asking "which events did this account trigger" had no way to query by the HCA actor.a
sendercolumn already existed on the events table andensureEventalready accepted an optional override, but the handlers weren't passing it through, the GraphQL layer didn't expose it, no filter existed, and the schema didn't document HCA semantics anywhere.What Changed (Concrete)
packages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts: documented HCA-awareness onevent.sender,event.from(explicit "never HCA-aware"),domain.ownerId,registration.registrantId/unregistrantId,permissionsUser.user. addedbySenderindex on the events table.apps/ensindexer/src/lib/ensv2/event-db-helpers.ts:ensureEvent(context, event, sender?)already accepted an HCA-aware override; verified handler audit covered all call sites.ETHRegistrar.ts,ENSv2Registry.ts): pass HCA-candidate arg as the sender override onNameRegistered(owner),LabelRegistered/Reserved(sender),LabelUnregistered(sender),ExpiryUpdated(sender),SubregistryUpdated(sender),TransferSingle/Batch(operator). gaps (no actor in args, falls back totx.from):EACRolesChangedgranter,NameRenewed,TokenRegenerated, all sharedResolver:*events. accepted.apps/ensapi/src/omnigraph-api/schema/event.ts: exposedEvent.senderfield; updatedEvent.fromdescription to call out non-HCA semantics; addedsenderfilter toEventsWhereInput; updatedfromfilter description; updatedAccountEventsWhereInputto marksenderas implied.apps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.ts: extendedEventsWhereinterface andeventsWhereConditionsto apply the newsenderfilter.apps/ensapi/src/omnigraph-api/schema/account.ts:Account.eventsnow resolves withsender = parent.id(HCA-aware) instead offrom = parent.id.Domain.owner(polymorphic ENSv1/v2 wording),Registration.registrant/unregistrant(HCA-aware in v2),PermissionsUser.user,RegistryPermissionsUser.user,ResolverPermissionsUser.user.Design & Planning
spec'd inline with @shrugs over two passes (initial proposal + re-proposal after the user clarified that
event.senderalready existed as an override mechanism and that they wanted it surfaced via filter). no separate design doc.alternatives considered:
exposing only the schema column and skipping the GraphQL filter — rejected because consumers need to query by HCA actor for this to be useful.
keeping
Account.eventsonfromand adding a parallelAccount.txsfor thetx.fromview — rejected as YAGNI; the HCA-awaresenderis the more useful default andfromfilter remains available viaEventsWhereInput.reverse: rename
event.sender→event.actorto avoid confusion withtx.fromcolloquially being "the sender" — rejected to minimize churn; docs clarify the distinction.Planning artifacts: conversation transcript only.
Reviewed / approved by: @shrugs (live).
Self-Review
reviewed every changed file end-to-end after implementation.
..., the HCA account address, if used.for non-polymorphic fields and the explicit ENSv1/ENSv2 two-clause wording forDomain.ownerandRegistration.registrant/unregistrant.Event.fromdescription explicitly disclaims HCA-awareness;Event.senderdescribes fallback semantics; filter docs cross-reference each other.Cross-Codebase Alignment
HCA,IHCAFactoryBasic,ensureEvent,event.sender,event.from,_msgSender,sender:.apps/ensindexer/src/plugins/ensv2/handlers/ensv1/(intentionally not HCA-aware per project decision);ENSv1Domain.rootRegistryOwner;Event.addressandEvent.to;AccountId.address; registry/registrar contract address columns.Downstream & Consumer Impact
Event.sender: Address!field and asenderfilter onEventsWhereInput.Account.eventssemantics changes fromtx.from-based tosender-based filtering.senderis overloaded —tx.fromis colloquially "the sender" but in this schema "sender" is reserved for the HCA-aware actor. descriptions on bothEvent.fromandEvent.senderexplicitly cross-link to disambiguate.Testing Evidence
pnpm typecheckclean across all 24 workspace projects.feat/hca-aware-eventsbranch) reindexed against the running anvil; verified theevents.sendercolumn is populated for all 762 indexed events.sender != from— exactly the Registry events routed through ETHRegistrar (LabelRegistered,ExpiryUpdated,TransferSingle). spot-checked tx0x1c8a1a5c…b0viacast tx:tx.from = 0x7099…79C8(user EOA),tx.to = 0x4C4a…5584(ETHRegistrar), event emitted from registry0x8f86…fe4cf, indexedevent.sender = 0x4C4a…5584(ETHRegistrar — the registry's_msgSender()).events_sender_indexexists on the table after reindex.Account.eventsreturning HCA-aware results.TransferSingleusesoperatorrather thanfrom/to).Scope Reductions
HCAFactoryand route at least one register/transfer through it for a true HCA-vs-EOA differentiation test.Account.eventsHCA-aware filtering.EACRolesChanged.Risk Analysis
Account.eventsbehavior change is silent. callers expecting old semantics will see different results once HCA traffic exists. today the devnet has no HCAs so the practical delta is zero, but staging/prod consumers should be informed.events.sendercolumn requires reindex. existing deployments will need to drop and rebuild the events table (or run a backfill that defaultssendertofrom).events_sender_indexis a write-path cost; negligible relative to existing indexes on the same table.Account.eventsresolver reverts tofrom = parent.id, GraphQL schema removes one field and one filter argument.Pre-Review Checklist (Blocking)
🤖 Generated with Claude Code