Refactor event registration to allow multiple handlers per event - #1469
Conversation
Registering the same event more than once no longer errors or composes handlers. Each `indexer.onEvent` becomes its own registration; a `contractRegister` merges into a matching handler registration (either registration order, matched on the resolved `where` and wildcard flag), otherwise it stands on its own. Wildcard registrations are no longer capped at one per signature. The routing, buffer dedup (keyed on registration index), and query selection layers already fanned one log out to every matching registration, so this only reshapes the registration layer: - HandlerRegister resolves each call eagerly into a process-global per-chain store (survives an import-cached re-registration cycle; one config per isolate). `finishRegistration` just backfills raw-event-only regs, drops where-empty EVM regs, and assigns the chain-scoped index. The raw persistent slot, per-call resolved store, and the merge/throw paths are gone. - The duplicate-event guard moves from a runtime check to config parse (`Contract::new`), keyed on sighash + indexed-topic count for EVM and the discriminator for SVM. The wildcard-interference guard is dropped. - simulate fans a simulated event out to every registration. raw_events keeps one row per fetched item, so a log matched by N registrations writes N rows (no dedup, by design). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
A `where` resolving to no topic selections already contributes no query terms, so the registration fetches nothing on its own — the explicit drop in finishRegistration wasn't needed here. Keeping the registration also removes its awkward ordering dependency with the raw-event backfill (a filtered-out event still counts as registered, so no bare raw-event reg is added for it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
|
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:
📝 WalkthroughWalkthroughThe PR replaces incremental event registration with chain-independent intents resolved per chain, supports multiple compatible or distinct registrations, fans simulation events across matches, deduplicates raw events, and adds contract-level duplicate dispatch-key validation. ChangesRegistration routing and dispatch validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HandlerRegister
participant EnvioGlobal
participant Config
participant SimulateItems
participant PgStorage
HandlerRegister->>EnvioGlobal: append pending onEvent intent
HandlerRegister->>Config: validate intent options across matching chains
HandlerRegister->>Config: resolve and merge registrations per chain
SimulateItems->>HandlerRegister: getSimulateOnEventRegistrations
HandlerRegister-->>SimulateItems: matching onEvent registrations
SimulateItems->>PgStorage: emit event items for registrations
PgStorage->>PgStorage: deduplicate raw events by log coordinate
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f0a8e391d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let onEventRegistrationsByChainId = | ||
| EnvioGlobal.value.onEventRegistrationsByChainId->( | ||
| Utils.magic: dict<unknown> => dict<array<Internal.onEventRegistration>> |
There was a problem hiding this comment.
This per-chain global cache is only populated while handler modules execute, but HandlerLoader.registerAllHandlers is called repeatedly with the active config and Node will not re-run already-imported handler modules. In the TestIndexer path the helper narrows chainMap to the chains in each process() call before registering handlers, so if the first run materializes chain 1 and a later run materializes chain 1337, finishRegistration has no config-independent handler intents left to rebuild chain 1337 registrations and the later run falls back to bare/no handlers. Keep the cached state independent of chain/config, or replay/materialize it for each startRegistration config.
Useful? React with 👍 / 👎.
| let matching = | ||
| getChainOnEventRegistrations(~chainId)->Array.filter(reg => | ||
| reg.eventConfig.contractName === eventConfig.contractName && | ||
| reg.eventConfig.name === eventConfig.name | ||
| ) |
There was a problem hiding this comment.
When an EVM where callback returns false, LogSelection represents that as resolvedWhere.topicSelections = []; the previous finishRegistration explicitly dropped those registrations before assigning indexes. This new path returns every matching registration by event name, and finishRegistration also indexes builtRegs without the empty-DNF filter, so simulate can fan out to a handler that opted out of the current chain and normal fetching can keep useless address-dependent partitions. Filter EVM registrations with empty topicSelections before exposing or indexing them.
Useful? React with 👍 / 👎.
| .clone() | ||
| .unwrap_or_else(|| "none".to_string()), | ||
| ), | ||
| EventKind::Fuel(_) => None, |
There was a problem hiding this comment.
This branch opts Fuel events out of the new parser-level collision check even though the runtime dispatch identity for Fuel is still event.sighash (logId for LogData, or fixed values like mint/burn/transfer). Since this commit removed HandlerRegister.validateEventIdOrThrow, a Fuel contract can now define the same logId or fixed receipt kind twice, and the Fuel router will match one receipt to both indistinguishable event definitions. Use the Fuel sighash as the dispatch key here as well.
Useful? React with 👍 / 👎.
Registration reverts to storing chain-independent handler/contractRegister intents, resolved per-config at finishRegistration, instead of a per-chain resolved cache. The cache was only populated while handler modules ran, but those are import-cached and registerAllHandlers is called repeatedly with a narrowed config (TestIndexer narrows chainMap per process() call) — so chains absent when handlers first ran got no registrations. Intents are config-independent, so finishRegistration materializes registrations for whatever chains the current config has, and repeated registration is idempotent. contractRegister→handler merge and the where-empty drop now happen at finishRegistration. Also from PR review: - Fuel events participate in the parser-level duplicate-dispatch-key check, keyed on sighash (logId / mint/burn/transfer/call), matching the router. - Restore dropping EVM registrations whose where resolves to no topic selections (per-chain opt-out) at finishRegistration. - Remove the intentional duplicate-handler fixtures (and their two composition tests) that only existed to exercise the removed handler composition; they otherwise inflate every downstream fetch/query count assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 021b096191
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| HandlerRegister.getSimulateOnEventRegistrations( | ||
| ~config, | ||
| ~chainId, | ||
| ~eventConfig, | ||
| ) | ||
| // Append into the registration array that the chain state will own and | ||
| // put that same registration object directly on the simulated item. | ||
| let onEventRegistrationIndex = onEventRegistrations->Array.length | ||
| let onEventRegistration = {...onEventRegistration, index: onEventRegistrationIndex} | ||
| onEventRegistrations->Array.push(onEventRegistration)->ignore | ||
|
|
||
| items | ||
| ->Array.push( | ||
| Internal.Event({ | ||
| onEventRegistration, | ||
| chain, | ||
| blockNumber, | ||
| logIndex, | ||
| // Simulate keeps the transaction inline on the payload, so the store | ||
| // key is unused. | ||
| transactionIndex: 0, | ||
| payload: ( | ||
| { | ||
| contractName: eventConfig.contractName, | ||
| eventName: eventConfig.name, | ||
| params, | ||
| chainId, | ||
| srcAddress, | ||
| logIndex, | ||
| transaction, | ||
| block, | ||
| }: Evm.payload | ||
| )->Evm.fromPayload, | ||
| }), | ||
| ) | ||
| ->ignore | ||
| )->Array.forEach(reg => { |
There was a problem hiding this comment.
When this expands one simulate input into one item per registration, SimulateSource.getItemsOrThrow still decides whether a query matches by collecting only eventConfig.id from the query selection (packages/envio/src/sources/SimulateSource.res:40-52). With two registrations for the same event but different filters or address-dependency, a query that selected only registration A will also return the prebuilt item for registration B because their event ids are identical, so handlers can run in simulate even though their registration was not part of the source query that routed the log. The simulate source needs to match the item's registration index, not just the event id, once this fan-out exists.
Useful? React with 👍 / 👎.
| pendingOnEventRegistrations->Array.some(p => | ||
| p.contractName === contractName && | ||
| p.eventName === eventName && | ||
| p.eventOptions->Option.flatMap(v => v.wildcard)->Option.getOr(false) |
There was a problem hiding this comment.
When the same event has both wildcard and non-wildcard registrations and a simulate item omits srcAddress, SimulateItems.deriveSrcAddress calls this helper once before fanning out. Because it now returns true if any matching registration is wildcard, the whole simulated log gets the dummy source address; the non-wildcard registration then fails its generated clientAddressFilter against the configured contract addresses and its handler is skipped. The source address fallback needs to be chosen per fanned-out registration, or non-wildcard registrations should still get the configured contract address.
Useful? React with 👍 / 👎.
| ~duplicateMsg="Cannot register a second handler with different options. Make sure all handlers for the same event use identical options (wildcard, where)", | ||
| ~logger, | ||
| ) | ||
| pendingOnEventRegistrations |
There was a problem hiding this comment.
This path now only queues the raw event options, so invalid where objects/callback results are no longer resolved while indexer.onEvent (or pre-registered replay) is executing. The lifecycle test still expects a bad indexed parameter to throw from setHandler, but this returns successfully and only fails later in finishRegistration after the rest of the handler module has run, losing the precise registration call-site error and allowing side effects after an invalid registration. Keep an eager validation/materialization pass for the active config before accepting the intent.
Useful? React with 👍 / 👎.
| m.contractRegister->Option.isSome && | ||
| sameEventAndFilter(m, reg, ~config) | ||
| ) { | ||
| | -1 => merged := merged.contents->Array.concat([reg]) |
There was a problem hiding this comment.
When two handlers are now allowed for the same contract/event and only one uses where.block.number._gte, both registrations can share the same address partition start because IndexingAddresses.makeContractConfigs collapses a contract's registration start blocks to the minimum, while the EVM client registration payload only sends topic selections (packages/envio/src/sources/HyperSyncClient.res:271-300) and not the per-registration startBlock. As a result, logs before the later _gte are still routed to that handler whenever another registration for the same contract starts earlier; the block start needs to be enforced per registration, not only as a contract-level minimum.
Useful? React with 👍 / 👎.
- Restore call-site `where` validation: setHandler/setContractRegister resolve the intent against the active config before storing it, so an invalid `where` throws at the registration call site again (not deferred to finishRegistration). - Remove the intentional duplicate-registration fixtures that only exercised the removed handler composition / mismatched-options throw: a second Gravatar.FactoryEvent handler+contractRegister pair (captured-add test, whose property is already covered by throwOnHangingRegistration) and a CustomSelection wildcard re-registration. These added extra registrations to the Gravatar contract, inflating fetch/query counts across the E2E and rollback suites. Drop the now-orphaned captured-add test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
![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)
packages/cli/src/config_parsing/system_config.rs (1)
1953-1956: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReport duplicate dispatch keys, not duplicate signatures.
Fuel and SVM conflicts are not necessarily signature conflicts, and the EVM key also includes indexed-topic count. Change the message to say the events “share the same dispatch key” so configuration authors receive an accurate diagnosis.
Proposed wording
- "Duplicate event detected on contract {name}: {existing} and {} share the \ - same signature, so they can't be told apart while indexing. Remove the \ + "Duplicate event detected on contract {name}: {existing} and {} share the \ + same dispatch key, so they can't be told apart while indexing. Remove the \ duplicate event.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/config_parsing/system_config.rs` around lines 1953 - 1956, Update the duplicate-event error constructed in the event validation flow to report that {existing} and {name} share the same dispatch key, replacing the inaccurate “same signature” wording while preserving the rest of the diagnostic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/cli/src/config_parsing/system_config.rs`:
- Around line 1953-1956: Update the duplicate-event error constructed in the
event validation flow to report that {existing} and {name} share the same
dispatch key, replacing the inaccurate “same signature” wording while preserving
the rest of the diagnostic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efe35edb-87ae-40e8-8afa-7fd61b18dcee
📒 Files selected for processing (6)
packages/cli/src/config_parsing/system_config.rspackages/envio-tests/test/HandlerRegister_test.respackages/envio/src/EnvioGlobal.respackages/envio/src/HandlerRegister.resscenarios/test_codegen/src/handlers/EventHandlers.tsscenarios/test_codegen/test/EventHandler.test.ts
💤 Files with no reviewable changes (2)
- scenarios/test_codegen/src/handlers/EventHandlers.ts
- scenarios/test_codegen/test/EventHandler.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/envio-tests/test/HandlerRegister_test.res
The per-contract collision key is a dispatch key (EVM sighash + indexed count, Fuel logId/receipt kind, SVM discriminator), not strictly a signature, so the diagnostic wording is more accurate this way. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 089b24a77b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let where = intent.eventOptions->Option.flatMap(v => v.where) | ||
| config.chainMap | ||
| ->ChainMap.values | ||
| ->Array.some(chainConfig => |
There was a problem hiding this comment.
The current fix still short-circuits at the first matching chain: Array.some stops as soon as buildOnEventRegistrationWith succeeds. In a multi-chain config where a where callback returns false or a valid filter on the first matching chain but an invalid params/block filter on a later matching chain, indexer.onEvent accepts and stores the intent, and the error is deferred until finishRegistration after the rest of the handler module has run. Use a full iteration over all matching chains for this eager validation before pushing the intent.
Useful? React with 👍 / 👎.
| }), | ||
| ) | ||
| ->ignore | ||
| )->Array.forEach(reg => { |
There was a problem hiding this comment.
When one simulate input matches multiple registrations, this loop now pushes multiple internal source.simulateItems, but SimulateDeadInputTracker.makeFromConfig still reports array positions from that internal fanned-out list as “index in each chain's simulate array”. If a single user input at index 0 is filtered out for two registrations (for example an explicit non-indexed srcAddress), the failure reports indices 0, 1 even though the user's simulate array only had index 0; carry the original itemIndex through fan-out or dedupe tracking by the raw input index.
Useful? React with 👍 / 👎.
| if !isDroppedByWhere(~config, reg) { | ||
| onEventRegistrations | ||
| ->Array.push({...reg, index: onEventRegistrations->Array.length}) | ||
| ->ignore |
There was a problem hiding this comment.
When raw_events is enabled and a single log matches two registrations for the same contract/event (for example two indexer.onEvent handlers), this loop now keeps both registrations so the source returns one Internal.Event per handler. PgStorage.writeBatch converts every event item into a raw_events row, and that table uses a serial primary key rather than (chain_id,event_id), so one on-chain log is persisted multiple times as identical raw events; dedupe raw-event insertion by the log coordinate or generate raw events before handler fan-out.
Useful? React with 👍 / 👎.
`validateIntentWhere` used `Array.some`, stopping at the first chain that defines the event. A `where` that resolves fine on the first chain but is structurally invalid on a later one slipped past the call-site check and only threw at `finishRegistration`. Iterate every matching chain so the error surfaces at the registration call site regardless of which chain's resolution is invalid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
- finishRegistration: when raw events are enabled, an event whose `where` opts out of a chain (empty topic selections) is no longer fully dropped — its handler still doesn't run there, but a bare (handler-less) registration is added so the event's logs are captured for `raw_events`. The backfill now keys on surviving registrations; a `where: false` event still isn't reported as handler-less (tracked via intentKeys). - PgStorage.writeBatch: a single log fans out to one item per matching registration, but `raw_events` records the log, so dedupe rows by log coordinate (chain, block, logIndex) — one row per log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scenarios/test_codegen/test/EventFilters_test.res`:
- Around line 489-497: Update handlerPresence to inspect all registrations named
"WithExcessField" rather than using Array.find, and assert the complete matching
set for each chain. Verify the registrations’ exact count and raw-event
registration type through the public registration API, preserving the expected
enabled result for chain "137" and absent result for chain "100".
🪄 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: CHILL
Plan: Pro
Run ID: c9981235-40cf-45bf-a483-08e5d6ecc722
📒 Files selected for processing (4)
packages/envio-tests/test/HandlerRegister_test.respackages/envio/src/HandlerRegister.respackages/envio/src/PgStorage.resscenarios/test_codegen/test/EventFilters_test.res
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/envio-tests/test/HandlerRegister_test.res
- packages/envio/src/HandlerRegister.res
| let handlerPresence = chainId => | ||
| switch registrationsByChainId->Dict.get(chainId) { | ||
| | Some({HandlerRegister.onEventRegistrations: regs}) => | ||
| regs->Array.some(reg => reg.eventConfig.name === "WithExcessField") | ||
| | None => false | ||
| regs | ||
| ->Array.find(reg => reg.eventConfig.name === "WithExcessField") | ||
| ->Option.map(reg => reg.handler->Option.isSome) | ||
| | None => None | ||
| } | ||
| t.expect((hasEvent("137"), hasEvent("100"))).toEqual((true, false)) | ||
| t.expect((handlerPresence("137"), handlerPresence("100"))).toEqual((Some(true), Some(false))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete raw-event registration set.
Array.find checks only the first matching registration, and checking handler->Option.isSome does not prove that the registration is specifically the bare raw-events registration. With multiple registrations now supported, this can pass while another matching handler registration remains. Filter all WithExcessField registrations and assert their exact count/type through the public registration API where possible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scenarios/test_codegen/test/EventFilters_test.res` around lines 489 - 497,
Update handlerPresence to inspect all registrations named "WithExcessField"
rather than using Array.find, and assert the complete matching set for each
chain. Verify the registrations’ exact count and raw-event registration type
through the public registration API, preserving the expected enabled result for
chain "137" and absent result for chain "100".
Correct the raw-events behavior: a `where: false` event has a handler that opted out of the chain, so it gets no registration there at all — not even a raw-events one. Event configs with no explicit handler still get a bare raw-events registration when raw events are enabled. This reverts the finishRegistration change from the previous commit back to keying the backfill on the resolved registrations; the raw-events row dedup (PgStorage) stays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb6080627
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if contract.name === intent.contractName { | ||
| switch contract.events->Array.find(e => e.name === intent.eventName) { | ||
| | Some(eventConfig) => | ||
| let _ = buildOnEventRegistrationWith( |
There was a problem hiding this comment.
For EVM registrations whose where option is a callback, this validation call already builds the registration and invokes LogSelection.parseWhereOrThrow; finishRegistration later replays the same intent through resolveChainRegistrations, so the same callback runs a second time during normal startup even though LogSelection.res documents it as invoked exactly once per chain. If a callback closes over mutable state, reads time/randomness, or has side effects, validation can accept one filter while the indexer actually registers another. Cache/materialize the resolved registration for the active config instead of discarding it here.
Useful? React with 👍 / 👎.
| svm.discriminator | ||
| .clone() | ||
| .unwrap_or_else(|| "none".to_string()), |
There was a problem hiding this comment.
When two SVM instructions use the same discriminator bytes with different hex casing, such as 0x0f and 0x0F, this string key treats them as distinct and parsing accepts both. The SVM router decodes discriminator strings to bytes before matching, so a matching instruction then fans out to both indistinguishable registrations even though this parser check is meant to reject dispatch collisions. Normalize/lowercase the discriminator or key on decoded bytes plus length before inserting.
Useful? React with 👍 / 👎.
| // Validate before storing so a broken `where` never leaves a poisoned intent | ||
| // in the global store. | ||
| validateIntentWhere(~config=registration.config, intent) | ||
| pendingOnEventRegistrations->Array.push(intent)->ignore |
There was a problem hiding this comment.
When a handler module is imported before startRegistration, withRegistration queues callbacks in preRegistered and startRegistration replays them with pop(), so this append records the last top-level registration first. Since the new pendingOnEventRegistrations order is now the dispatch order for multiple handlers on the same event, preloaded handler modules run those handlers in reverse. Replay preRegistered FIFO or otherwise preserve source order before appending intents.
Useful? React with 👍 / 👎.
Reword the same-contract duplicate-event error to drop the "dispatch key" jargon. Normalize SVM discriminators (lowercase) before keying so hex-casing variants collide, matching the router. Replace the two Rust `#[test]` cases with parseYaml tests in the ReScript suite, plus an SVM casing-collision case. Replay pre-registered handlers in FIFO source order so multi-handler dispatch order matches registration order, with a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
Cache each registration intent's per-chain resolution on the intent, so the user's `where` callback runs exactly once per chain instead of once at the registration call site and again at every `finishRegistration`/simulate. This restores the "invoked exactly once per chain" invariant. Add an end-to-end test: two handlers on one event (Gravatar.MultiHandlerOrder) both run, ordered by (blockNumber, logIndex, registration index), plus a unit test asserting a `where` callback is invoked once per chain across repeated finishes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
The new Gravatar.MultiHandlerOrder event shifts the generated contract ABI, which `Indexer_test`'s full chain-config deep-equal asserts on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcf9df7f66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let coordinate = `${item | ||
| ->Internal.getItemChainId | ||
| ->Int.toString}-${item | ||
| ->Internal.getItemBlockNumber | ||
| ->Int.toString}-${item->Internal.getItemLogIndex->Int.toString}` |
There was a problem hiding this comment.
In Fuel raw-events runs, this coordinate can collide for different receipts: the Fuel source preserves both txId and receiptIndex, and receiptIndex is not block-scoped. If two transactions in the same block each emit a matching receipt at index 0, the first adds the key <chain>-<block>-0 and the second is dropped before toRawEvent, losing a real raw event. Include the Fuel transaction id, or otherwise use an ecosystem-specific unique log coordinate, while still collapsing fan-out duplicates.
Useful? React with 👍 / 👎.
| registeredKeys->Utils.Set.has( | ||
| getKey(~contractName=contract.name, ~eventName=eventConfig.name), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
When raw_events is enabled and an EVM handler's where resolves to false on this chain, builtRegs still adds the event to registeredKeys, but the registration is later removed by isDroppedByWhere before it reaches onEventRegistrations. This branch therefore skips creating the bare raw-events registration too, so the configured event is not fetched or stored at all even though handler-less events are normally backfilled for raw events. Base this check on surviving registrations, or add a raw registration when every matching handler registration was dropped by where.
Useful? React with 👍 / 👎.
| ->Array.forEach(chainConfig => | ||
| resolveIntentForChain(~config=registration.config, ~chainConfig, intent)->ignore | ||
| ) | ||
| pendingOnEventRegistrations->Array.push(intent)->ignore |
There was a problem hiding this comment.
When two handlers for the same event live in different auto-loaded handler files, this append makes their registration index depend on whichever module's top-level indexer.onEvent runs first. HandlerLoader.autoLoadFromSrcHandlers starts all dynamic imports together with Promise.all, so independent files, especially with top-level await or different dependency graphs, can register in nondeterministic order; this refactor then uses that order to sort same-log fan-out. Load/record handler files in a stable sequence before assigning registration indexes.
Useful? React with 👍 / 👎.
Replace the chain-independent intent array + per-intent resolved-where cache with a single persistent `activeRegistration` that handlers resolve into at `onEvent`/`onBlock` call time (the `where` callback runs once per chain there, at the call site). `startRegistration` is idempotent, so the import-cached handlers register once and every later `finishRegistration` reuses the result; `finishRegistration` reads the store and builds a fresh per-config output (merge, raw-events backfill, index) without mutating it, so a run appending a simulate/mock source stays isolated. onEvent + onBlock now share the `chainRegistrations` type in the session store. onBlock validation runs against the config start block (registration sees the full, un-narrowed config); the persistence-derived resume block overrides downstream without re-validation. MockIndexer registers once against the full config and narrows per run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
Base changed `expectParseError` to compare the full parse-error string exactly, so the duplicate-event assertions need the complete message (including the `Config parse error:` prefix and, for the EVM case, the `Failed parsing globally defined contract` context) instead of a substring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
* Run createTestIndexer in-process instead of worker threads (#1467) * Run createTestIndexer in-process instead of worker threads Replace the per-chain worker + storage message-proxy with an in-process run against an in-memory Persistence.storage, removing the dominant cost (re-evaluating the envio module graph in a fresh isolate per process()). - IndexerState gets an injectable ~onExit; ExitOnCaughtUp resolves it instead of process.exit, so a caught-up run in-process resolves a promise rather than killing the test runner. Production default unchanged. - TestIndexer builds a per-instance in-memory storage (config-derived initial state, never a real DB) and drives IndexerState/IndexerLoop directly; runs bypass Persistence.init and stop the loop on completion. - Registrations are captured once and cloned per run (patchConfig appends a simulate source), so independent createTestIndexer instances run in parallel without shared mutable registration state. - Handlers run inside an AsyncLocalStorage scope so a handler calling indexer.onEvent throws (as in production) without finishing the global registration the test itself uses. - Delete TestIndexerWorker and the proxy's message-channel machinery. - Restore vitest to envio devDependencies (needed to resolve the Vitest binding for local scenario test runs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh * Drop JSON round-trip from TestIndexer in-memory storage The worker model serialized entities to JSON to cross the thread boundary. In-process there is no boundary, so store and load entities decoded: - handleLoad filters decoded entities with the already-typed filter and returns them directly (no serialize/parse, no rowsSchema round-trip). - handleWriteBatch takes Persistence.updatedEntity and stores the decoded entities as-is instead of encoding then re-parsing them. - Delete TestIndexerProxyStorage entirely — its serializable types were the only remaining use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh * Isolate test indexer entity store with defensive copies (#1472) * Harden in-process TestIndexer: isolate store, drop dead resume plumbing - Copy entities on the set/get boundary so user code mutating a returned entity (or an object passed to `set`) can't corrupt the in-memory store; add a regression test. - Remove the unused `resumeInitialState`/`currentInitialState` injection — the runner sets `storageStatus = Ready(...)` directly, so it was never reached; make `resumeInitialState` a throwing stub like the other unused storage methods. - Await the in-flight write fiber in cleanup instead of busy-polling it. - Document the loss of per-test isolation (shared module/effect-cache state) in the indexer-testing skill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DAwNk4gUT3QV6dh7uBtef1 * Replace AsyncLocalStorage guard with plain finished-registration flag The in-process runner used an AsyncLocalStorage scope so a handler calling `indexer.onEvent` at runtime would throw, while still letting tests call the registration API after a run (which required clearing the finished flag). The only thing depending on the API staying callable post-registration was two type-surface tests; those are compile-time only and don't need to run after registration. - Move the `indexer.onEvent`/`contractRegister` type-surface checks into EventHandlers.ts under `if (0)` (type-checked, never executed), and drop the two runtime `it(...)` cases. - Delete the AsyncLocalStorage scope, `runInHandlerScope`/`isInHandlerScope`, and `clearActiveRegistration`; restore `throwIfFinishedRegistration` to the plain `finished` check. A handler registering at runtime now throws via the finished flag, as in production. - getRegistrations no longer reopens the registration API after capturing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DAwNk4gUT3QV6dh7uBtef1 * Tighten test-isolation note in indexer-testing skill Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DAwNk4gUT3QV6dh7uBtef1 --------- Co-authored-by: Claude <noreply@anthropic.com> * Collapse makeCreateTestIndexer factory into createTestIndexer The (~config) => unit => t currying was a dependency-injection seam (it once also threaded ~workerPath); nothing injects a custom config and its only caller invoked it immediately. Fold it into a single createTestIndexer() that loads the memoized config internally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh * Test handler errors and improve TestIndexer error messages (#1474) * Fix stale TestIndexer comments and drop dead chainId tuple element - copyEntity comment no longer claims all entity fields are immutable scalars; shallow copy still shares array-valued fields. - Rollback storage stubs no longer advise setting rollbackOnReorg to false — the runner already forces it off, making them unreachable. - Drop the unused chainId element from chainEntries (validation kept). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCpLSmKU3MZaAyGRjmcAVc * Add regression test for handler-throw propagation A synchronous throw in an event handler body must reject the process() promise at the call site, carrying the handler's original message. This path (handler-body throw) was only covered indirectly before, via effect and getOrThrow throws. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCpLSmKU3MZaAyGRjmcAVc --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * Friendlier event-registration validation and simulate handler checks - Reject duplicate event names per contract at parse time, suggesting the `name` alias for overloads (replaces the never-implemented uniqueness TODO). - Reword the same-dispatch-signature error so it reads clearly (names are unique by the check above, so it never prints "X and X"). - Simulate now applies the same `where:false` drop as finishRegistration and fails loudly when an event has no handler registered, instead of silently fabricating a bare registration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lcy9XY2kAFMscJRukX7Fqi * Test the new validation and simulate handler behavior ConfigYaml: same-signature rejection with distinct names, byte-identical duplicate, same-name overload (alias suggestion), the SVM hex-casing case, and a success case where a `name` alias resolves an overload. HandlerRegister: simulate returns the handled registration, drops a `where:false` one, and returns nothing for an unhandled event. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lcy9XY2kAFMscJRukX7Fqi * Revert simulate-side changes that broke handler-less simulate The where:false drop and no-handler failure lived in the shared `getSimulateOnEventRegistrations`, which is also used by module-load-time registration probes (`MockConfig.getOnEventRegistration`) that need the bare fallback and the un-dropped registration. And failing on a handler-less event contradicts the framework's intentional pattern of simulating one (e.g. `Noop.EmptyEvent`) as a chain-advance marker. Keeps the parse-time event-name validation (items 1 and 4), which is unaffected. The simulate where:false drop needs to live in the simulate path only; revisiting separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lcy9XY2kAFMscJRukX7Fqi * Fail simulate on unhandled events; drop where:false in simulate path only Apply the no-handler failure and the where:false drop in SimulateItems (the user-facing simulate path) instead of the shared getSimulateOnEventRegistrations, so module-load registration probes (MockConfig) keep their bare-fallback behavior. Simulate now fans out only to registrations that would actually run on the chain (a real handler/contractRegister, where not excluded), and throws a clear error when none would. Give Noop.EmptyEvent a no-op handler so the multichain-ordering test still processes an event on chain 1 (its only event) without writing an entity, and point the new no-handler failure test at Gravatar.TestEventWithReservedKeyword, which is defined but genuinely handler-less. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lcy9XY2kAFMscJRukX7Fqi --------- Co-authored-by: Claude <noreply@anthropic.com>
…-registrations-4lpfgw
The merged main renamed the config helper (`MockIndexerConfig` -> `InternalTestIndexer.fromUserApi`); update the multi-registration and duplicate-event tests accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
Noop.EmptyEvent's no-op handler registered on every chain that lists the Noop contract, including chain 137 where Noop has an address — adding a fetch partition that the rollback/reorg tests (one partition per chain) don't expect. Pin the handler to chain 1, its only intended chain. Drop the MultiHandlerOrder event + its two handlers and the end-to-end dispatch-order test: adding a new event to the shared chain-1337 Gravatar contract perturbs partition snapshots in the rollback suite. Dispatch ordering by (blockNumber, logIndex, registration index) stays covered by the FetchState buffer-order test and the HandlerRegister index-order tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b90e52f40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Some(fn) => fn(r) | ||
| | None => () | ||
| switch getActiveRegistration() { | ||
| | Some(_) => () |
There was a problem hiding this comment.
When registerAllHandlers is called again in the same process with a different config or handler set, this branch keeps the previous active registration (which finishRegistration has already marked finished) and does not replay/import new registrations into a fresh store. Any newly imported handler file will then hit throwIfFinishedRegistration, and even if no new file is imported finishRegistration will reuse registrations from the prior config by chain id, so a test indexer/custom config sharing a chain id can inherit unrelated handlers. Either reset for a genuinely different config or key the persistent cache by the config/handler set rather than treating every later start as the same cycle.
Useful? React with 👍 / 👎.
2 participants
Summary
Refactors
HandlerRegisterto support multiple independent handler/contractRegister registrations per event, with automatic merging of handler and contractRegister pairs that target the same event and filter. This replaces the previous model where duplicate registrations were rejected as errors.Key Changes
Eager per-chain registration storage: Moved from a per-event-name dictionary to a per-chain array of
onEventRegistrationobjects. Registrations are resolved and appended immediately as eachindexer.onEvent/.contractRegistercall executes, rather than being stored in a temporary per-event slot and synced later.Automatic handler/contractRegister merging: When a handler and contractRegister target the same event and filter (same contract, event name, wildcard flag, and resolved
where), they merge into a single registration. A handler absorbs an earlier contractRegister-only registration, or a contractRegister attaches to an earlier handler-only registration. Multiple handlers or multiple contractRegisters remain separate registrations.Removed duplicate registration validation: Eliminated the per-event duplicate-detection logic that previously threw errors when two handlers or two contractRegisters registered for the same event. Each registration now stands independently.
Removed event ID collision validation: Deleted
eventIdValidatorand its per-chain dispatch-key collision checks. Moved duplicate-event detection to the config parsing layer (system_config.rs), which now rejects event definitions on the same contract that share a dispatch signature at parse time.Simplified registration API: Removed
loggerparameter fromsetHandlerandsetContractRegister; removedbuildOnEventRegistrationfrom the public interface (replaced bygetSimulateOnEventRegistrationsfor test/simulate use).Test-only reset function: Added
resetOnEventRegistrationsto clear the process-global store between test cycles.Implementation Details
onEventRegistrationsByChainId) now holds arrays of registrations keyed by chain ID, surviving import-cached re-registration cycles in tests.indexduringfinishRegistration, after all merging is complete.enableRawEventsis enabled; otherwise they're reported once at startup.https://claude.ai/code/session_01EBsVqEGhVkg7EyTjAdcy52
Summary by CodeRabbit