Extract raw event and ecosystem-specific logic into pluggable interfaces - #1335
Merged
Conversation
Items now carry an opaque `Internal.eventPayload` instead of the user-facing `event`. The ecosystem materialises what each consumer needs from that payload via three new `Ecosystem.t` methods: - `toEvent` — the event handed to handlers / contract registration - `toEventLogger` — the per-item child logger (EVM/Fuel: contract/event/ address; SVM: program/instruction/programId), built off a logger injected into the ecosystem constructor - `toRawEvent` — the raw_events row (EVM/Fuel; throws on SVM) This removes consumers' dependence on the event shape (`toGenericEvent` is gone) and gives sources a seam to later return a compact/lazy representation of heavy fields like `transaction.input`. The raw_events row type moves to `Internal.rawEvent` (aliased by `InternalTable.RawEvents.t`) so the ecosystem can reference it without pulling in `InternalTable`'s dependency on `Config`. Raw-event assembly moves to the new `RawEvent` module. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0124SBjKuB5vfwcoJyt5k3v4
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRefactors internal event representation from direct ChangesPayload-based event pipeline and ecosystem-aware logging
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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: 4
🧹 Nitpick comments (1)
packages/envio/src/RawEvent.res (1)
40-51: 💤 Low valueWorkaround for null params serializes to string
"null"instead of JSON null.The special case converts
nullparams to the string literal"null", which will be stored differently than actual JSONnullin the database. The comment acknowledges this is a workaround. If consumers expect actual JSON null, they may need adjustment.🤖 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/RawEvent.res` around lines 40 - 51, The code currently converts null params to the string literal "null" instead of preserving them as actual JSON null, creating inconsistency in database storage. To fix this, modify the eventConfig.paramsRawEventSchema to properly support nullable params, then remove the special case condition checking if params equals null and the workaround that converts it to the string "null". This will allow actual null values to pass through naturally from the reverseConvertOrThrow call, ensuring consumers receive consistent JSON null values rather than string literals.
🤖 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/PgStorage.res`:
- Line 846: The code at line 846 calls config.ecosystem.toRawEvent() without
verifying that the ecosystem type is not SVM, even though SVM's toRawEvent
implementation throws an error for raw events. The config.enableRawEvents guard
at line 843 only controls whether to process raw events, not whether the
ecosystem supports them. Fix this by either adding validation during config
parsing to reject enableRawEvents: true when ecosystem is SVM, or adding an
explicit ecosystem type check before calling config.ecosystem.toRawEvent() at
line 846 to ensure SVM ecosystems are excluded. Choose whichever approach aligns
better with your codebase design patterns.
In `@scenarios/test_codegen/test/lib_tests/FetchState_onBlock_test.res`:
- Line 62: The payload field assignment uses Utils.magic without explicit type
annotations, which violates the coding guidelines for type casting in ReScript.
Modify the payload field on line 62 to add an explicit type annotation using the
pipe operator syntax. The Utils.magic call should be annotated with both the
input type (the string "Mock event in fetchstate test") and the output type it
should be cast to, following the pattern: value->(Utils.magic: inputType =>
outputType). Ensure the type annotation clearly specifies what type the mock
string is being converted to.
In `@scenarios/test_codegen/test/lib_tests/FetchState_test.res`:
- Line 71: The payload field uses an untyped Utils.magic call on line 71 in
FetchState_test.res, which violates the coding guideline requiring explicit type
annotations. Change the call from Utils.magic("Mock event in fetchstate test")
to use the explicit cast syntax pattern value->(Utils.magic: inputType =>
outputType), where inputType should be string (the type of the input string
literal) and outputType should be the expected type of the payload field based
on the context of the mock event structure.
In `@scenarios/test_codegen/test/lib_tests/PgStorage_test.res`:
- Line 1065: The Utils.magic cast on the line with pattern `_ =>
Internal.eventPayload` uses a wildcard for the input type instead of an explicit
type annotation. Replace the wildcard `_` with the actual explicit input type
that is being cast to Internal.eventPayload, following the pattern
`(Utils.magic: inputType => Internal.eventPayload)` to comply with ReScript
coding guidelines requiring explicit type annotations for Utils.magic casts.
---
Nitpick comments:
In `@packages/envio/src/RawEvent.res`:
- Around line 40-51: The code currently converts null params to the string
literal "null" instead of preserving them as actual JSON null, creating
inconsistency in database storage. To fix this, modify the
eventConfig.paramsRawEventSchema to properly support nullable params, then
remove the special case condition checking if params equals null and the
workaround that converts it to the string "null". This will allow actual null
values to pass through naturally from the reverseConvertOrThrow call, ensuring
consumers receive consistent JSON null values rather than string literals.
🪄 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: 5418d4c6-8615-4cad-9a72-ce57626177ea
⛔ Files ignored due to path filters (4)
scenarios/test_codegen/test/__snapshots__/Logging.both-prettyconsole.snapis excluded by!**/*.snapscenarios/test_codegen/test/__snapshots__/Logging.console-pretty.snapis excluded by!**/*.snapscenarios/test_codegen/test/__snapshots__/Logging.console-raw.snapis excluded by!**/*.snapscenarios/test_codegen/test/__snapshots__/Logging.ecs-console.snapis excluded by!**/*.snap
📒 Files selected for processing (31)
packages/envio/src/Config.respackages/envio/src/ContractRegisterContext.respackages/envio/src/Ecosystem.respackages/envio/src/EventProcessing.respackages/envio/src/FetchState.respackages/envio/src/Internal.respackages/envio/src/Logging.respackages/envio/src/PgStorage.respackages/envio/src/RawEvent.respackages/envio/src/SimulateItems.respackages/envio/src/db/InternalTable.respackages/envio/src/sources/Evm.respackages/envio/src/sources/Fuel.respackages/envio/src/sources/HyperFuelSource.respackages/envio/src/sources/HyperSyncSource.respackages/envio/src/sources/RpcSource.respackages/envio/src/sources/Svm.respackages/envio/src/sources/SvmHyperSyncSource.resscenarios/test_codegen/test/ClientAddressFilter_test.resscenarios/test_codegen/test/EventBlockFilter_test.resscenarios/test_codegen/test/IndexerState_test.resscenarios/test_codegen/test/OnBlockSchema_test.resscenarios/test_codegen/test/SvmHyperSyncSource_test.resscenarios/test_codegen/test/__mocks__/MockEvents.resscenarios/test_codegen/test/fixtures/LogTesting.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/FetchState_onBlock_test.resscenarios/test_codegen/test/lib_tests/FetchState_test.resscenarios/test_codegen/test/lib_tests/PgStorage_test.resscenarios/test_codegen/test/lib_tests/SameSignatureEventDecode_test.resscenarios/test_codegen/test/rollback/ChainMocking.res
…stem-threaded loggers Addresses PR review feedback on the item/event decoupling: - Memoise the materialised event on the item (mirrors the logger cache), via `Ecosystem.getItemEvent`. - `clientAddressFilter` now operates on the opaque `eventPayload` directly instead of casting it to the user-facing event. - `Internal.rawEvent` uses snake_case fields matching the DB columns, with no `@as` indirection. - Drop the global event-logger-maker ref: `getItemLogger`/`getItemUserLogger` move to `Ecosystem` and take `~ecosystem` explicitly, threaded through EventProcessing, ContractRegisterContext, UserContext, ChainFetching and LoadLayer. The base logger is a field on `Ecosystem.t`. - Remove the shared `genericEvent` from the payload path: each ecosystem converts the opaque payload to its own `evmEventPayload` / `fuelEventPayload` (SVM uses its instruction type). `RawEvent.make` takes the extracted block and transaction so it stays payload-shape-agnostic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0124SBjKuB5vfwcoJyt5k3v4
`evmEventPayload`/`fuelEventPayload` and their identity casts move out of `Internal` and into `Evm`/`Fuel` as `payload` + `fromPayload`/`toPayload`, where the ecosystem-specific shapes belong. `Internal` keeps only the opaque `eventPayload` (the item field depends on it) and the generic `payloadToEvent`. Sources and test helpers reference the ecosystem casts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0124SBjKuB5vfwcoJyt5k3v4
- Reject `rawEvents: true` for the SVM ecosystem during config parsing, so it fails fast with a clear message instead of hitting Svm.toRawEvent's throw mid-indexing. - Add explicit input/output type annotations to the `Utils.magic` casts in the FetchState and PgStorage tests, per the ReScript guideline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0124SBjKuB5vfwcoJyt5k3v4
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
This was referenced Jun 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 handling to decouple ecosystem-specific logic from storage and logging concerns. Introduces a new
Ecosystem.tinterface with three pluggable functions (toEvent,toEventLogger,toRawEvent) that each ecosystem (EVM, Fuel, SVM) implements, replacing hardcoded logic inPgStorageandLogging.Key Changes
New
RawEventmodule: ExtractedmakeRawEventandconvertFieldsToJsonfromPgStorageinto a dedicated module. Themakefunction now accepts acleanUpRawEventFieldsInPlacecallback instead of accessingconfig.ecosystemdirectly.Ecosystem interface expansion: Added three new fields to
Ecosystem.t:toEvent: Materializes the user-facing event from an item's opaque payloadtoEventLogger: Builds per-item child loggers with ecosystem-specific fields (contract/event/address for EVM/Fuel; program/instruction/programId for SVM)toRawEvent: Builds raw event rows for theraw_eventstable (throws on SVM)Ecosystem implementations: Updated
Evm,Fuel, andSvmmodules to be factory functions (make(~logger)) that return configuredEcosystem.tinstances, closing over the injected logger.Event payload abstraction: Renamed
eventfield topayloadinInternal.eventItemand introducedeventPayloadas an opaque type. AddedpayloadToEventandpayloadToGenericEventfunctions to materialize from the payload.Logging refactor: Replaced inline event logger construction in
Logging.getItemLoggerwith a callback (eventLoggerMaker) set byConfig.fromPublic. This breaks the circular dependency betweenLoggingandEcosystem.Raw event type relocation: Moved
RawEvents.ttype definition fromInternalTabletoInternalso ecosystem implementations can reference it without pulling inConfigdependencies.Snapshot updates: Updated test snapshots to reflect field name changes in logging output (
contractName→contract,eventName→event).Notable Implementation Details
eventPayloadtype uses%identityexternals to maintain runtime shape compatibility while providing type safety at the boundary.Config.fromPublic, with the logger injected at that point. ThetoEventLoggerfunction closes over this logger, enabling future removal of the global logger.RawEvent.makeis called fromEvm.toRawEventandFuel.toRawEvent;Svm.toRawEventthrows since SVM doesn't support raw events.Ecosystem.make(~logger)instead of accessing a staticecosystemvalue.https://claude.ai/code/session_0124SBjKuB5vfwcoJyt5k3v4
Summary by CodeRabbit
Release Notes
Refactor
Tests