Resolve event filters per-chain at registration time - #1396
Merged
Conversation
Replace the lazy Static/Dynamic eventFilters closures with a structured resolvedWhere resolved once per chain when the handler registers: - Internal: add topicFilter (Values / ContractAddresses marker), resolvedTopicSelection and resolvedWhere; drop eventFilters and the getEventFiltersOrThrow field on evmOnEventRegistration. - LogSelection: parseWhereOrThrow invokes the where callback exactly once per chain with the real chainId and the addresses Proxy sentinel; the sentinel becomes a ContractAddresses marker parsed into resolvedWhere. Address-typed topic values are lowercased so mixed-case input matches. Markers are expanded to the partition's addresses via materializeTopicSelections when source queries are built. - HandlerRegister: startRegistration takes the config; registrations are resolved per chain incrementally on setHandler/setContractRegister, so invalid where throws at the user's registration call site, a false where drops the chain, and duplicate registrations are compared on the resolved structure instead of the raw where reference. - registerOnBlock validates chainId presence and start/end block bounds at registration (moved out of ChainState). - onBlock registration defers its per-chain loop until the registration's config is known. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rr49Jzbmp6CSDnC75rBxkY
…ter fields - registerOnBlock now owns the full onBlock registration: where predicate normalization, per-chain evaluation, range parsing (blockRange schema and extractRange moved from Main), block bounds validation, and the zero-match warning. Main.onBlockFn only parses the raw options and delegates, passing a chains-object builder so the loop runs against the registration's config. Drops the withConfig wrapper added earlier. - Make the inner block object of onEventBlockFilterSchema strict for EVM and Fuel so unknown fields in a where block filter (typos, or block.number on Fuel) throw a user-friendly error instead of being silently ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rr49Jzbmp6CSDnC75rBxkY
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (5)
📝 WalkthroughWalkthroughThis PR replaces function-based event filter probing with resolved where data, rewires handler and block registration around per-chain pending state, updates log-selection consumers to materialize topic selections at query time, and applies small CLI formatting changes in Rust error paths. ChangesWhere resolution and registration flow
Incremental CLI formatting cleanup
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Move all cross-module-instance state into a single version-gated globalThis.__envioGlobal record owned by the new EnvioGlobal module: the HandlerRegister registration slots (eventRegistrations, activeRegistration, preRegistered), Main's indexerState and persistence refs, and RollbackCommit's callbacks. Slots are opaque in EnvioGlobal so it stays at the bottom of the dependency graph; each owner casts its slot once. Mutable fields replace the ref cells. This closes the duplicate-envio-instance hole for the indexer.chains getters and rollback-commit callbacks: previously only the registration registry was shared across instances, so a duplicated envio silently served static config values from never-assigned module refs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rr49Jzbmp6CSDnC75rBxkY
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scenarios/test_codegen/test/helpers/MockIndexer.res (1)
997-1016: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefault
topic1selection can disagree with the computeddependsOnAddressesfield.
dependsOnAddresses(the record field, Line 993, unchanged) defaults to!isWildcardwhen the caller omits~dependsOnAddresses. But the newtopic1default here checks the raw~dependsOnAddressesargument directly, so a non-wildcard registration built without explicitly passing~dependsOnAddresses=trueends up withdependsOnAddresses: trueon the record whiletopicSelections[0].topic1staysValues([])instead ofContractAddresses. Any test relying on the implicit default to exercise address-based topic filtering would silently miss it.🔧 Suggested fix
- topic1: switch dependsOnAddresses { - | Some(true) => ContractAddresses({contractName: contractName}) - | _ => Values([]) - }, + topic1: switch dependsOnAddresses->Option.getOr(!isWildcard) { + | true => ContractAddresses({contractName: contractName}) + | false => Values([]) + },🤖 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/helpers/MockIndexer.res` around lines 997 - 1016, The default topic1 selection in MockIndexer.res is using the raw dependsOnAddresses option instead of the computed record field, which can make it diverge from the registration’s effective setting. Update the resolvedWhere.topicSelections default logic to base the topic1 branch on the same derived dependsOnAddresses value used for the record (the one tied to isWildcard), so implicit non-wildcard registrations consistently select ContractAddresses. Use the existing dependsOnAddresses handling in the surrounding registration setup to keep the default topic filtering aligned.scenarios/test_codegen/test/EventFilters_test.res (1)
8-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfusingly similar names: local
getEvmEventConfigalias vs. module-levelMockConfig.getEvmEventConfig.Line 10 binds the local name
getEvmEventConfigto a partial application ofMockConfig.getEvmOnEventRegistration, while line 188 calls a distinctly-named module functionMockConfig.getEvmEventConfigdirectly. Both are legitimately different functions (registered registration vs. static config lookup), but the near-identical naming is easy to misread during future edits.Also applies to: 188-193
🤖 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 8 - 14, The local alias `getEvmEventConfig` is too similar to the module-level `MockConfig.getEvmEventConfig`, which makes the test hard to read and easy to edit incorrectly. Rename the local binding in `EventFilters_test.res` to something that clearly reflects `MockConfig.getEvmOnEventRegistration`, and update the nearby usage so the partial application and the direct `MockConfig.getEvmEventConfig` lookup are visually distinct.
🤖 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 `@packages/envio/src/HandlerRegister.res`:
- Around line 673-734: The repeated-registration fallback in HandlerRegister.res
rebuilds event registrations from the persistent dict, but onBlock handlers are
still lost when pending is None. Add the same replay/rebuild path for
onBlockRegistrations, either by storing them persistently alongside event
handlers or by reconstructing them during the fallback before updating
registrationsByChainId. Make sure the logic around pending,
registrationsByChainId->Dict.set, and the onBlockRegistrations field preserves
block handlers across restart cycles just like the event path does.
In `@packages/envio/src/LogSelection.res`:
- Around line 66-70: `materializeTopicFilter` is using raw `Address.t` values
for `ContractAddresses`, which can produce topic filters that differ from the
lowercase-normalized path used by `EventConfigBuilder.getTopicEncoder`. Update
the `materializeTopicFilter` branch in `LogSelection.res` to normalize addresses
before calling `TopicFilter.fromAddress`, or route it through the same
address-encoding logic used by `getTopicEncoder`, so `ContractAddresses`
materialize consistently with the rest of the topic filter encoding.
---
Nitpick comments:
In `@scenarios/test_codegen/test/EventFilters_test.res`:
- Around line 8-14: The local alias `getEvmEventConfig` is too similar to the
module-level `MockConfig.getEvmEventConfig`, which makes the test hard to read
and easy to edit incorrectly. Rename the local binding in
`EventFilters_test.res` to something that clearly reflects
`MockConfig.getEvmOnEventRegistration`, and update the nearby usage so the
partial application and the direct `MockConfig.getEvmEventConfig` lookup are
visually distinct.
In `@scenarios/test_codegen/test/helpers/MockIndexer.res`:
- Around line 997-1016: The default topic1 selection in MockIndexer.res is using
the raw dependsOnAddresses option instead of the computed record field, which
can make it diverge from the registration’s effective setting. Update the
resolvedWhere.topicSelections default logic to base the topic1 branch on the
same derived dependsOnAddresses value used for the record (the one tied to
isWildcard), so implicit non-wildcard registrations consistently select
ContractAddresses. Use the existing dependsOnAddresses handling in the
surrounding registration setup to keep the default topic filtering aligned.
🪄 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: e14454bb-65ac-449d-a957-af5b000673be
📒 Files selected for processing (25)
packages/envio/src/ChainState.respackages/envio/src/EventConfigBuilder.respackages/envio/src/HandlerLoader.respackages/envio/src/HandlerRegister.respackages/envio/src/HandlerRegister.resipackages/envio/src/Internal.respackages/envio/src/LogSelection.respackages/envio/src/Main.respackages/envio/src/SimulateItems.respackages/envio/src/sources/Evm.respackages/envio/src/sources/Fuel.respackages/envio/src/sources/HyperSyncSource.respackages/envio/src/sources/RpcSource.resscenarios/test_codegen/src/handlers/EventHandlers.tsscenarios/test_codegen/test/ClientAddressFilter_test.resscenarios/test_codegen/test/EventBlockFilter_test.resscenarios/test_codegen/test/EventFilters_test.resscenarios/test_codegen/test/HandlerRegisterLifecycle_test.resscenarios/test_codegen/test/HyperSyncSource_test.resscenarios/test_codegen/test/OnBlockSchema_test.resscenarios/test_codegen/test/RpcSource_test.resscenarios/test_codegen/test/SourceBlockHashes_test.resscenarios/test_codegen/test/__mocks__/MockConfig.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/SameSignatureEventDecode_test.res
💤 Files with no reviewable changes (1)
- packages/envio/src/ChainState.res
Comment on lines
+673
to
+734
| let registration = switch registration { | ||
| | Some(_) as registration => registration | ||
| | None => | ||
| // No entry in the incremental store, but the persistent dict | ||
| // may still hold a handler: handler modules are import-cached, | ||
| // so a repeated registration cycle in the same process (tests | ||
| // restarting the indexer) never re-runs the `indexer.onEvent` | ||
| // calls. Rebuild from the dict in that case. Events without a | ||
| // handler/contractRegister aren't fetched or dispatched | ||
| // (unless raw events are enabled). | ||
| if hasRegistration(~contractName, ~eventName) || config.enableRawEvents { | ||
| Some( | ||
| buildOnEventRegistration( | ||
| ~config, | ||
| ~chainId=chainConfig.id, | ||
| ~eventConfig, | ||
| ~startBlock=?contract.startBlock, | ||
| ), | ||
| ) | ||
| } else { | ||
| let eventNames = switch notRegisteredEventsByContract->Utils.Dict.dangerouslyGetNonOption( | ||
| contractName, | ||
| ) { | ||
| | Some(set) => set | ||
| | None => { | ||
| let set = Utils.Set.make() | ||
| notRegisteredEventsByContract->Dict.set(contractName, set) | ||
| set | ||
| } | ||
| } | ||
| eventNames->Utils.Set.add(eventName)->ignore | ||
| None | ||
| } | ||
| } | ||
|
|
||
| switch registration { | ||
| | Some(registration) => | ||
| // A `where` that resolved to no topic selections (`false` for | ||
| // this chain) drops the chain's registration entirely — the | ||
| // event should never be fetched here. | ||
| let isDroppedByWhere = | ||
| config.ecosystem.name === Evm && | ||
| (registration->getResolvedWhere).topicSelections->Utils.Array.isEmpty | ||
| if !isDroppedByWhere { | ||
| onEventRegistrations->Array.push(registration) | ||
| } | ||
| | None => () | ||
| } | ||
| }, | ||
| ) | ||
| }) | ||
|
|
||
| registrationsByChainId->Dict.set( | ||
| key, | ||
| { | ||
| onEventRegistrations, | ||
| onBlockRegistrations: switch pending { | ||
| | Some(pending) => pending.onBlockRegistrations | ||
| | None => [] | ||
| }, | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for repeated startRegistration/finishRegistration cycles and onBlock persistence
rg -nP -C3 '\b(startRegistration|finishRegistration)\b' packages/envio/src scenarios/test_codegen/test
# Any persistent store for onBlock registrations outside per-chain pending?
rg -nP -C3 'onBlockRegistration' packages/envio/srcRepository: enviodev/hyperindex
Length of output: 26793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the registration lifecycle and any repeated startup paths.
sed -n '1,220p' packages/envio/src/HandlerLoader.res
printf '\n---\n'
sed -n '1,220p' scenarios/test_codegen/test/helpers/MockIndexer.res
printf '\n---\n'
sed -n '1,220p' scenarios/test_codegen/test/HandlerRegisterLifecycle_test.res
printf '\n---\n'
# Look for any direct repeated calls to registerAllHandlers/startRegistration/finishRegistration.
rg -n --no-heading '\b(registerAllHandlers|startRegistration|finishRegistration)\b' packages/envio/src scenarios/test_codegen/testRepository: enviodev/hyperindex
Length of output: 18990
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '123,170p' packages/envio/src/HandlerRegister.res
printf '\n---\n'
sed -n '626,760p' packages/envio/src/HandlerRegister.res
printf '\n---\n'
sed -n '280,340p' scenarios/test_codegen/test/helpers/MockIndexer.res
printf '\n---\n'
sed -n '590,620p' packages/envio/src/Main.resRepository: enviodev/hyperindex
Length of output: 10414
onBlock handlers need the same replay path as events
On repeated registration cycles in the same process, this fallback rebuilds only event registrations from the persistent dict. onBlockRegistrations has no persistent backing, so when pending is None it drops to [] and onBlock handlers disappear. Add a rebuild path or persistent store for onBlock registrations too.
🤖 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/envio/src/HandlerRegister.res` around lines 673 - 734, The
repeated-registration fallback in HandlerRegister.res rebuilds event
registrations from the persistent dict, but onBlock handlers are still lost when
pending is None. Add the same replay/rebuild path for onBlockRegistrations,
either by storing them persistently alongside event handlers or by
reconstructing them during the fallback before updating registrationsByChainId.
Make sure the logic around pending, registrationsByChainId->Dict.set, and the
onBlockRegistrations field preserves block handlers across restart cycles just
like the event path does.
Comment on lines
+66
to
+70
| let materializeTopicFilter = (filter: Internal.topicFilter, ~addresses: array<Address.t>) => | ||
| switch filter { | ||
| | Values(values) => values | ||
| | ContractAddresses(_) => addresses->Array.map(TopicFilter.fromAddress) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect TopicFilter.fromAddress and Address normalization behavior
fd -e res -e resi 'TopicFilter' --exec sh -c 'echo "== $1 =="; cat -n "$1"' _ {}
echo "===== Address module ====="
fd -e res -e resi '^Address' --exec sh -c 'echo "== $1 =="; sed -n "1,80p" "$1"' _ {}Repository: enviodev/hyperindex
Length of output: 4856
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the call sites and surrounding logic for materializeTopicFilter
fd -e res 'LogSelection.res' --exec sh -c 'echo "== $1 =="; cat -n "$1"' _ {}
echo "===== TopicFilter / Address use sites ====="
rg -n "materializeTopicFilter|fromAddressLowercaseOrThrow|fromAddress\\b|getTopicEncoder|ContractAddresses|Values\\(" packages/envio/src -A4 -B4
echo "===== EventConfigBuilder around topic encoder ====="
fd -e res 'EventConfigBuilder.res' --exec sh -c 'echo "== $1 =="; rg -n "getTopicEncoder|lowercase|address" "$1" -A8 -B8' _ {}Repository: enviodev/hyperindex
Length of output: 44107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Trace how indexingAddresses are populated and whether they are normalized
rg -n "normalizeUserAddress|fromAddressLowercaseOrThrow|fromAddressOrThrow|IndexingAddresses|indexingAddresses" packages/envio/src -A4 -B4
echo "===== IndexingAddresses.res ====="
fd -e res 'IndexingAddresses.res' --exec sh -c 'echo "== $1 =="; cat -n "$1"' _ {}
echo "===== Config.res normalization sites ====="
fd -e res 'Config.res' --exec sh -c 'echo "== $1 =="; rg -n "normalizeUserAddress|lowercaseAddresses|fromAddressLowercaseOrThrow|fromAddressOrThrow" "$1" -A6 -B6' _ {}Repository: enviodev/hyperindex
Length of output: 50375
Normalize ContractAddresses before encoding
materializeTopicFilter passes Address.t straight into TopicFilter.fromAddress, but EventConfigBuilder.getTopicEncoder lowercases address inputs first. With checksum-cased addresses, chain.<Contract>.addresses can materialize to topics that won’t match the lowercase topic filters used elsewhere. Normalize here too, or reuse the same address encoder path.
🤖 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/envio/src/LogSelection.res` around lines 66 - 70,
`materializeTopicFilter` is using raw `Address.t` values for
`ContractAddresses`, which can produce topic filters that differ from the
lowercase-normalized path used by `EventConfigBuilder.getTopicEncoder`. Update
the `materializeTopicFilter` branch in `LogSelection.res` to normalize addresses
before calling `TopicFilter.fromAddress`, or route it through the same
address-encoding logic used by `getTopicEncoder`, so `ContractAddresses`
materialize consistently with the rest of the topic filter encoding.
Unrelated to this branch's changes: main's Clippy step passed as of our fork commit, but the CI toolchain (dtolnay/rust-toolchain@stable, unpinned) has since picked up a newer stable Rust whose clippy enables these lints more aggressively, breaking cargo-test for any PR right now. Mechanical fixes only — drop redundant & before format!/anyhow! args, and a redundant .to_string() call on an already-Display type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rr49Jzbmp6CSDnC75rBxkY
This was referenced Jul 22, 2026
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
2 participants
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.
Summary
Refactors event filter registration to resolve
wherecallbacks per-chain immediately when handlers are registered, rather than deferring resolution until chain startup. This enables duplicate registrations to be detected at the user's call site with proper error context, and simplifies the runtime architecture by eliminating deferred filter parsing.Key Changes
HandlerRegister: Restructured registration lifecycle to incrementally build per-chain registrations as handlers are registered:
pendingRegistrations→pendingChainRegistrations(per-chain dict of event registrations + onBlock handlers)syncOnEventRegistrationsresolveswherecallbacks for all configured chains immediately and validates against existing registrationsbuildOnEventRegistrationWithextracted to accept pre-resolvedwhereand other options, decoupling filter building from handler lookupstartRegistrationnow takesConfig.tinstead ofEcosystem.tto access chain definitionsbuildOnEventRegistrations(deferred per-chain resolution) — resolution now happens incrementallyLogSelection: Renamed and refactored filter parsing:
parseEventFiltersOrThrow→parseWhereOrThrow(clearer intent)parsedWherewithresolvedWhere(topic selections + startBlock) instead ofparsedEventFiltersgetEventFiltersOrThrowcallback from registration — filters are now fully resolved at registration timematerializeTopicSelectionsadded to expandContractAddressesmarkers to concrete topic values when building queriesInternal: Updated event registration types:
onEventRegistrationnow carriesresolvedWhere: resolvedWhere(pre-resolved topic selections + startBlock)topicFiltertype distinguishes staticValuesfromContractAddressesmarkersgetEventFiltersOrThrowcallback fieldEventConfigBuilder: Address values in topic filters are now lowercased before encoding to match lowercase hex topics from sources (handles mixed-case checksummed input)
RpcSource / HyperSyncSource: Simplified to work with pre-resolved filters:
getSelectionConfigno longer takeschainparameterresolvedWhereTests: Updated to reflect new registration model:
HandlerRegisterLifecycle_test.rescovers per-chain immediate resolution and duplicate detectionEventFilters_test.resnow accessesresolvedWhere.topicSelectionsdirectlyresolvedWherestructureNotable Implementation Details
wherestructure (topic selections + startBlock), not the callback reference. Two distinct callbacks that resolve to identical filters compose instead of throwing.whereconfigurations (e.g., referencing non-existent indexed parameters) now throw at the user's registration call site with proper stack context, rather than during chain startup.preRegisteredcallbacks (registered beforestartRegistration) are replayed through the samesyncOnEventRegistrationscode path, ensuring consistent validation.ContractAddressesmarker in resolved filters allows sources to defer address expansion until query time, supporting dynamic address registration.https://claude.ai/code/session_01Rr49Jzbmp6CSDnC75rBxkY
Summary by CodeRabbit
New Features
where-based event/block filtering with per-chain resolved topic selection.Bug Fixes
wherecriteria.blockfields.Breaking Changes
startRegistrationnow takes full config;registerOnBlocknow uses awherepredicate plus a chain resolver.