Add isolated multichain mode support#1327
Open
DZakh wants to merge 18 commits into
Open
Conversation
Adds a root-level `multichain` option to config.yaml accepting 'unordered' (default, current behavior) or 'isolated'. In unordered mode every entity in the public config is stamped with crossChain: true; in isolated mode the field is omitted. When an entity is cross-chain, storages initialize it with an extra nullable chain_id column: Postgres adds it to the entity table and its history table (which must mirror the entity table positionally because the history backfill copies rows via SELECT e.*), and ClickHouse adds it to the entity history table and exposes it through the entity view. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
…on-se2inx # Conflicts: # packages/cli/src/config_parsing/public_config.rs # packages/envio/src/bindings/ClickHouse.res
The merge accidentally reflowed to_snake_case via default rustfmt; revert to main's formatting since this file is unrelated to the multichain feature. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
…osystems Unordered (cross-chain) entities no longer get a chain id column; in isolated mode every entity table gets a non-nullable one instead, since isolated entities are chain-scoped. The column is appended at config parse time, so it flows through the generic field pipeline to the Postgres entity and history tables and the ClickHouse history table and entity view. The column is spelled per each backend's column_name_format (chainId, or chain_id for snake_case); the public config now carries the formats in the storage section so the runtime can name columns it appends itself. The CLI rejects entity fields that collide with the chain id column in isolated mode, and the multichain option is now available for Fuel and SVM configs as well. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Makes crossChain a required boolean on the entity config. For isolated (non cross-chain) entities, storage writes now stamp the chain id onto entity copies, resolved from the checkpoint the change was made at via the batch's checkpoint arrays. Stamping happens before the Postgres/ ClickHouse split so the sink gets stamped entities too. Inserts use a write schema with the chainId field appended (the column is not part of the entity schema), and the history set schema uses it as well so rollback restore can read the chain id back from history rows. Rollback-restored changes reference a checkpoint outside the batch and already carry the chain id, so they pass through unchanged. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Flips the internal envio_addresses entity to crossChain: false and drops chain id from its entity schema, so the chain id is the isolated chain id column (stamped on write from the change's checkpoint) rather than an explicit entity field — matching how user isolated entities work. The schema uses object syntax so it isn't forced to declare chainId; the table keeps a chainId column (mapped to the chain_id db column) and the t type keeps chainId for reads, populated from the column. The custom config-address INSERT and the indexing-address read query are unaffected since the db column stays chain_id. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Marks the isolated chain id column as a primary key on the entity table (for user entities and envio_addresses), so the same entity id can exist independently per chain instead of upserting over each other. getPgPrimaryKeyFieldNames already resolves to the db column name, so the PRIMARY KEY and ON CONFLICT clauses pick up chain_id automatically. The history table keeps its (id, checkpoint) primary key — checkpoint ids are globally unique across chains, so there's no collision — with the chain id staying a plain nullable data column there (delete-update rows leave it null). Adds a test proving the same id written on two chains in separate batches yields two distinct rows. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Splits the in-memory store into shared cross-chain entity tables and per-chain tables for isolated entities, so the same entity id on different chains stays distinct without key mangling. getInMemTable now takes ~chainId and routes accordingly; the store-wide passes (size, drop, flush) fan over all chains. The flush emits one updatedEntity group per (chain, isolated entity), each carrying its chainId, which replaces the checkpoint-based chain id stamping with a direct per-group stamp and lets the write path scope deletes by chain. loadById and getWhere route to the chain's in-memory table and scope their db queries (and load-group keys) by chain. Rollback restore/removed queries return the chain id so the diff routes rows to the right per-chain table. This also chain-scopes getWhere's in-memory index for free, since each chain has its own filter indices. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
New scenario (multichain: isolated) with a Counter entity incremented on two chains under the same id. The createTestIndexer test processes events on chains 1 and 137 and asserts two independent rows (count 3 and 2), proving per-chain read scoping (get-then-set) and write isolation end to end. Makes the TestIndexer's main-thread storage mirror chain-aware to match the real store: serializableUpdatedEntity carries the group chain id, and handleWriteBatch keys rows per chain and re-stamps the chain id column (dropped by the entity schema) so loads and reads see it. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
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 (1)
📝 WalkthroughWalkthroughThis PR introduces an ChangesIsolated Multichain Mode
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Keep the internal address table cross-chain with an explicit chain_id
data column instead of treating it as an isolated entity. Its id is
already {chainId}-{address}, so it doesn't need the composite (id,
chain_id) key or the per-chain partitioning — the explicit field is
simpler and matches the original shape.
https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
…tities The entity view already selects the chain id column for isolated entities; make its LIMIT 1 BY dedup per (id, chain_id) so external readers see one current row per chain. Cross-chain entities still dedup by id alone. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
…on-se2inx # Conflicts: # packages/cli/src/config_parsing/graph_migration/mod.rs # packages/cli/src/config_parsing/system_config.rs
…shared ids Drops chainId from Persistence.updatedEntity — the chain is recovered from each change's checkpoint id (the envio_checkpoints map the batch already holds), so it never needs threading through the in-memory layer. Makes the entity-history path fully chain-aware for isolated entities so the same id can safely exist on multiple chains: - history pk is (id, chain_id, checkpoint_id); the backfill missing-history join is scoped by (id, chain_id) so the checkpoint-0 baseline no longer collides or masks across chains - delete-update history rows carry the chain id (derived from the checkpoint) instead of NULL - rollback restore/removed key by (id, chain_id) end to end, including the existence subqueries, mirroring ordered-mode rollback but respecting duplicate ids across chains - the entity-table delete is scoped by the change's chain The TestIndexer mirror derives chain from the checkpoint the same way. Adds a rollback test proving a shared id only reverts the reorged chain. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
The public-config StorageConfig gained postgres/clickhouse column name formats (the runtime needs them to spell the chain id column), but it's shared with `config view`, which leaked the format into that command's output and broke the e2e_test ConfigView snapshot. Give the view its own minimal storage struct (enabled backends only), matching prior behavior. https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/envio/src/db/EntityHistory.res (1)
86-103:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope stale-history pruning by
chain_idfor isolated entities.The prune query still computes anchors with
GROUP BY t.idand checks post-safe rows withps.id = d.id. For isolated entities, the sameidcan exist on multiple chains, so one chain’s newer checkpoint can prune another chain’s rollback anchor. Thread the optional chain-id column through pruning the same way as backfill and match/group by(id, chain_id).🤖 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/db/EntityHistory.res` around lines 86 - 103, The prune query in the WITH anchors CTE groups by only t.id, but for isolated entities that can exist on multiple chains, this causes cross-chain pruning conflicts. Modify the GROUP BY clause to include the chain_id column alongside t.id, and update the subquery condition that checks for post-safe rows (the NOT EXISTS subquery checking ps.id = d.id) to also match on chain_id, so that pruning decisions are scoped per chain rather than globally by entity id alone.
🧹 Nitpick comments (1)
scenarios/multichain_isolated_test/test/MultichainIsolated_test.res (1)
24-29: 💤 Low valuePrefer
Utils.magicoverObj.magicfor consistency with the codebase.The explicit type annotation is correct, but the context snippets and codebase patterns prefer
Utils.magicfor type casting in ReScript. Consider updating toUtils.magicfor consistency.->Array.toSorted((a, b) => Int.compare(a["count"], b["count"])) t.expect(counters).toEqual([ - ->( - Obj.magic: array<Indexer.Entities.Counter.t> => array<{ + ->( + Utils.magic: array<Indexer.Entities.Counter.t> => array<{ "id": string, "count": int, "chainId": int, }> )🤖 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/multichain_isolated_test/test/MultichainIsolated_test.res` around lines 24 - 29, Replace the `Obj.magic` call with `Utils.magic` to maintain consistency with the established codebase patterns for type casting. The explicit type annotation for the array transformation remains the same, only the magic function reference needs to be updated from the OCaml standard library function to the project's utility function.Source: Coding guidelines
🤖 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/bindings/ClickHouse.res`:
- Around line 414-419: For non-cross-chain (isolated) entities, the switch
statement for limitByFields currently falls back to using only the id field when
chainId field resolution fails (the _ pattern match). This is problematic
because it silently collapses rows across different chains into a single
deduplicated row, producing incorrect results. Instead, for isolated entities,
the code should hard-fail (throw an error or raise an exception) when the
chainId field is missing to enforce the invariant that the field must exist.
Replace the fallback case in the switch expression to throw an error when
chainId field resolution fails for non-cross-chain entities. Apply the same fix
at the other affected location mentioned in the comment (line 428).
In `@packages/envio/src/Config.res`:
- Around line 510-516: The postgresColumnNameFormat and
clickhouseColumnNameFormat fields in publicConfigStorageSchema accept arbitrary
strings via S.string, but line 977 only handles the "snake_case" value. This
allows invalid values to slip through and silently fall back to None,
potentially breaking column naming alignment. Replace the S.string definitions
for both postgresColumnNameFormat and clickhouseColumnNameFormat with a closed
enum constraint that only accepts valid column name format values (such as
"snake_case"). This ensures configuration validation fails fast for invalid
inputs rather than silently ignoring them.
In `@packages/envio/src/PgStorage.res`:
- Around line 1107-1113: For isolated rollback-created Delete changes, the
checkpointId (rollback.diffCheckpointId) does not exist in
chainIdByCheckpointId, causing groupChainId to remain None, which then falls
back to an unscoped delete in deleteByIdsOrThrow that can delete the same id
across all chains. Modify the Delete case handling in the switch statement
(around line 1110) and the corresponding second site (around 1232-1239) to carry
the per-table chain id from InMemoryStore.eachEntityTable or
Persistence.updatedEntity as a fallback for isolated entities, and fail
explicitly rather than issuing an unscoped delete when no chain id is available
for an isolated entity.
In `@packages/envio/src/TestIndexer.res`:
- Around line 123-148: The issue is that entities are being stored in entityDict
using the composite rowKey function (which combines chainId and entityId), but
the direct test-indexer lookup APIs like getEntityFromState and makeEntitySet
are still attempting to read by raw entityId alone, so stored entities will not
be found. Update the direct lookup paths in getEntityFromState and makeEntitySet
to use the same rowKey function for key construction when retrieving entities
from entityDict, ensuring they generate the same composite keys (chainId
combined with entityId) that are used when storing in the processChange
function. Alternatively, add a fast-fail check that prevents direct reads by raw
entityId when the chain is isolated (chainId is present), requiring callers to
provide chain context.
In `@scenarios/test_codegen/test/helpers/MockIndexer.res`:
- Around line 25-26: The setEntity function has a default ~chainId=0 parameter
that silently directs isolated (non-crossChain) entities to chain 0, making
their data invisible to chain `#1` handlers. Remove the default value from
~chainId parameter to make it required, or implement logic to infer chainId from
the entity's chainId field when available and throw an error if chainId is
missing for isolated entities. Apply the same fix to the other affected location
at lines 49-52 that has the identical issue with a default chainId parameter.
In `@scenarios/test_codegen/test/lib_tests/ConfigMultichain_test.res`:
- Around line 130-184: The test creates persistent database schemas and clients
via PgStorage.makeClient() and storage.initialize() but never cleans them up,
causing state leakage and open handles across test reruns. After each test
completes (or in a shared teardown), add cleanup logic to execute a DROP SCHEMA
cascade command on the pgSchema variable and close both the sql client and
storage object to ensure tests remain isolated and deterministic. This pattern
should be applied to all affected test cases that create these persistent
resources.
---
Outside diff comments:
In `@packages/envio/src/db/EntityHistory.res`:
- Around line 86-103: The prune query in the WITH anchors CTE groups by only
t.id, but for isolated entities that can exist on multiple chains, this causes
cross-chain pruning conflicts. Modify the GROUP BY clause to include the
chain_id column alongside t.id, and update the subquery condition that checks
for post-safe rows (the NOT EXISTS subquery checking ps.id = d.id) to also match
on chain_id, so that pruning decisions are scoped per chain rather than globally
by entity id alone.
---
Nitpick comments:
In `@scenarios/multichain_isolated_test/test/MultichainIsolated_test.res`:
- Around line 24-29: Replace the `Obj.magic` call with `Utils.magic` to maintain
consistency with the established codebase patterns for type casting. The
explicit type annotation for the array transformation remains the same, only the
magic function reference needs to be updated from the OCaml standard library
function to the project's utility function.
🪄 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: de692982-8472-4d00-8551-dcc79449dd69
⛔ Files ignored due to path filters (8)
packages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_generated_for_evm.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_generated_for_fuel.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_generated_for_svm.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_with_all_options.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_with_lowercase_contract_name.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_with_multiple_contracts.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__internal_config_json_code_with_no_contracts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
packages/cli/src/cli_args/init_config.rspackages/cli/src/config_parsing/field_types.rspackages/cli/src/config_parsing/human_config.rspackages/cli/src/config_parsing/public_config.rspackages/cli/src/config_parsing/system_config.rspackages/cli/src/hbs_templating/codegen_templates.rspackages/cli/src/svm_hypersync_source/mod.rspackages/cli/templates/static/shared/.claude/skills/indexer-configuration/SKILL.mdpackages/cli/test/configs/isolated-multichain.yamlpackages/envio/evm.schema.jsonpackages/envio/fuel.schema.jsonpackages/envio/src/Config.respackages/envio/src/InMemoryStore.respackages/envio/src/Internal.respackages/envio/src/LoadLayer.respackages/envio/src/Persistence.respackages/envio/src/PgStorage.respackages/envio/src/TestIndexer.respackages/envio/src/UserContext.respackages/envio/src/bindings/ClickHouse.respackages/envio/src/db/EntityHistory.respackages/envio/svm.schema.jsonscenarios/fuel_test/src/Indexer.resscenarios/multichain_isolated_test/.envio/.gitignorescenarios/multichain_isolated_test/config.yamlscenarios/multichain_isolated_test/envio-env.d.tsscenarios/multichain_isolated_test/package.jsonscenarios/multichain_isolated_test/rescript.jsonscenarios/multichain_isolated_test/schema.graphqlscenarios/multichain_isolated_test/src/Indexer.resscenarios/multichain_isolated_test/src/handlers/EventHandlers.tsscenarios/multichain_isolated_test/test/MultichainIsolated_test.resscenarios/multichain_isolated_test/tsconfig.jsonscenarios/multichain_isolated_test/vitest.config.tsscenarios/test_codegen/test/Config_test.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/ColumnNameFormat_test.resscenarios/test_codegen/test/lib_tests/ConfigMultichain_test.res
Comment on lines
+414
to
+419
| let limitByFields = switch entityConfig.crossChain | ||
| ? None | ||
| : entityConfig.table->Table.getFieldByName("chainId") { | ||
| | Some(Field(field)) => `\`${Table.idFieldName}\`, \`${field->Table.getClickHouseDbFieldName}\`` | ||
| | _ => `\`${Table.idFieldName}\`` | ||
| } |
Contributor
There was a problem hiding this comment.
Fail fast when isolated entities are missing the chain-id field instead of silently deduping by id.
On Line 418, the fallback to \id`` for non-cross-chain entities can collapse isolated rows across chains if chain-id field resolution fails, producing incorrect current-state views. For isolated mode, this should hard-fail so the invariant is enforced.
Proposed fix
- let limitByFields = switch entityConfig.crossChain
- ? None
- : entityConfig.table->Table.getFieldByName("chainId") {
- | Some(Field(field)) => `\`${Table.idFieldName}\`, \`${field->Table.getClickHouseDbFieldName}\``
- | _ => `\`${Table.idFieldName}\``
- }
+ let limitByFields = switch entityConfig.crossChain
+ ? None
+ : entityConfig.table->Table.getFieldByName("chainId") {
+ | Some(Field(field)) => `\`${Table.idFieldName}\`, \`${field->Table.getClickHouseDbFieldName}\``
+ | _ =>
+ JsError.throwWithMessage(
+ `Invariant violation: isolated entity "${entityConfig.name}" is missing chainId field required for ClickHouse LIMIT BY`,
+ )
+ }Also applies to: 428-428
🤖 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/bindings/ClickHouse.res` around lines 414 - 419, For
non-cross-chain (isolated) entities, the switch statement for limitByFields
currently falls back to using only the id field when chainId field resolution
fails (the _ pattern match). This is problematic because it silently collapses
rows across different chains into a single deduplicated row, producing incorrect
results. Instead, for isolated entities, the code should hard-fail (throw an
error or raise an exception) when the chainId field is missing to enforce the
invariant that the field must exist. Replace the fallback case in the switch
expression to throw an error when chainId field resolution fails for
non-cross-chain entities. Apply the same fix at the other affected location
mentioned in the comment (line 428).
Comment on lines
510
to
516
| let publicConfigStorageSchema = S.schema(s => | ||
| { | ||
| "postgres": s.matches(S.bool), | ||
| "postgresColumnNameFormat": s.matches(S.option(S.string)), | ||
| "clickhouse": s.matches(S.option(S.bool)), | ||
| "clickhouseColumnNameFormat": s.matches(S.option(S.string)), | ||
| } |
Contributor
There was a problem hiding this comment.
Constrain storage column-name format to a closed enum.
Line 513 and Line 515 accept arbitrary strings, but Line 977 only handles "snake_case". A typo or format drift silently falls back to None, which can misname the injected chainId DB column and break persistence/readback column alignment.
Suggested fix
+type columnNameFormat =
+ | `@as`("snake_case") SnakeCase
+ | `@as`("original") Original
+
+let columnNameFormatSchema = S.enum([SnakeCase, Original])
+
let publicConfigStorageSchema = S.schema(s =>
{
"postgres": s.matches(S.bool),
- "postgresColumnNameFormat": s.matches(S.option(S.string)),
+ "postgresColumnNameFormat": s.matches(S.option(columnNameFormatSchema)),
"clickhouse": s.matches(S.option(S.bool)),
- "clickhouseColumnNameFormat": s.matches(S.option(S.string)),
+ "clickhouseColumnNameFormat": s.matches(S.option(columnNameFormatSchema)),
}
)
-let chainIdDbName = format => format === Some("snake_case") ? Some("chain_id") : None
+let chainIdDbName = format =>
+ switch format {
+ | Some(SnakeCase) => Some("chain_id")
+ | Some(Original) | None => None
+ }🤖 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/Config.res` around lines 510 - 516, The
postgresColumnNameFormat and clickhouseColumnNameFormat fields in
publicConfigStorageSchema accept arbitrary strings via S.string, but line 977
only handles the "snake_case" value. This allows invalid values to slip through
and silently fall back to None, potentially breaking column naming alignment.
Replace the S.string definitions for both postgresColumnNameFormat and
clickhouseColumnNameFormat with a closed enum constraint that only accepts valid
column name format values (such as "snake_case"). This ensures configuration
validation fails fast for invalid inputs rather than silently ignoring them.
Comment on lines
+123
to
+148
| let rowKey = (entityId, chainId) => | ||
| switch chainId { | ||
| | Some(chainId) => `${chainId->Int.toString}-${entityId}` | ||
| | None => entityId | ||
| } | ||
| // The entity schema drops the implicit chain id column, so re-attach it for | ||
| // load filtering (Eq chainId) and so reads can see it. | ||
| let withChainId = (entity: Internal.entity, chainId): Internal.entity => | ||
| switch chainId { | ||
| | Some(chainId) => | ||
| let copy = entity->(Utils.magic: Internal.entity => dict<unknown>)->Dict.copy | ||
| copy->Dict.set("chainId", chainId->(Utils.magic: int => unknown)) | ||
| copy->(Utils.magic: dict<unknown> => Internal.entity) | ||
| | None => entity | ||
| } | ||
|
|
||
| let processChange = (change: TestIndexerProxyStorage.serializableChange) => { | ||
| switch change { | ||
| | Set({entityId, entity, checkpointId}) => | ||
| let chainId = chainIdAt(checkpointId) | ||
| // Parse entity immediately to store decoded values for proper comparisons | ||
| // (bigint/BigDecimal need actual values, not JSON strings) | ||
| let parsedEntity = entity->S.parseOrThrow(entityConfig.schema) | ||
| let parsedEntity = entity->S.parseOrThrow(entityConfig.schema)->withChainId(chainId) | ||
|
|
||
| // Update entities dict with parsed entity for load operations | ||
| entityDict->Dict.set(entityId, parsedEntity) | ||
| entityDict->Dict.set(rowKey(entityId, chainId), parsedEntity) |
Contributor
There was a problem hiding this comment.
Keep direct test-indexer lookups aligned with isolated row keys.
handleWriteBatch now stores isolated rows under rowKey(chainId, entityId), but the direct test-indexer APIs still read/write by raw entityId in getEntityFromState and makeEntitySet. After processing an isolated entity, Entity.get(id) / getOrThrow(id) will not find the stored row; add a chain-aware direct key path, or fail fast for ambiguous isolated direct reads.
🤖 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/TestIndexer.res` around lines 123 - 148, The issue is that
entities are being stored in entityDict using the composite rowKey function
(which combines chainId and entityId), but the direct test-indexer lookup APIs
like getEntityFromState and makeEntitySet are still attempting to read by raw
entityId alone, so stored entities will not be found. Update the direct lookup
paths in getEntityFromState and makeEntitySet to use the same rowKey function
for key construction when retrieving entities from entityDict, ensuring they
generate the same composite keys (chainId combined with entityId) that are used
when storing in the processChange function. Alternatively, add a fast-fail check
that prevents direct reads by raw entityId when the chain is isolated (chainId
is present), requiring callers to provide chain context.
Comment on lines
+130
to
+184
| let sql = PgStorage.makeClient() | ||
| let storage = PgStorage.makeStorageFromEnv(~config, ~sql, ~pgSchema, ~isHasuraEnabled=false) | ||
|
|
||
| let _ = await storage.initialize( | ||
| ~entities=[isolated], | ||
| ~enums=[EntityHistory.RowAction.config->Table.fromGenericEnumConfig], | ||
| ~envioInfo=%raw(`{}`), | ||
| ) | ||
|
|
||
| let batch: Batch.t = { | ||
| totalBatchSize: 2, | ||
| items: [], | ||
| progressedChainsById: Dict.make(), | ||
| isInReorgThreshold: false, | ||
| checkpointIds: [1n, 2n], | ||
| checkpointChainIds: [1, 137], | ||
| checkpointBlockNumbers: [10, 20], | ||
| checkpointBlockHashes: [Null.null, Null.null], | ||
| checkpointEventsProcessed: [1, 1], | ||
| } | ||
| let entity = id => | ||
| Dict.fromArray([("id", id->(Utils.magic: string => unknown))])->( | ||
| Utils.magic: dict<unknown> => Internal.entity | ||
| ) | ||
|
|
||
| // The write derives each change's chain from its checkpoint id (1n -> chain | ||
| // 1, 2n -> chain 137) and stamps the entity with it. | ||
| await storage.writeBatch( | ||
| ~batch, | ||
| ~rollback=None, | ||
| ~isInReorgThreshold=false, | ||
| ~config, | ||
| ~allEntities=[isolated], | ||
| ~updatedEffectsCache=[], | ||
| ~updatedEntities=[ | ||
| { | ||
| entityConfig: isolated, | ||
| changes: [ | ||
| Set({entityId: "a", entity: entity("a"), checkpointId: 1n}), | ||
| Set({entityId: "b", entity: entity("b"), checkpointId: 2n}), | ||
| ], | ||
| }, | ||
| ], | ||
| ~chainMetaData=None, | ||
| ) | ||
|
|
||
| let rows = await sql->Postgres.unsafe( | ||
| `SELECT * FROM "${pgSchema}"."IsolatedEntity" ORDER BY "id";`, | ||
| ) | ||
|
|
||
| t.expect( | ||
| rows, | ||
| ~message="Rows should carry the chain id of the group they were written in", | ||
| ).toEqual(%raw(`[{id: "a", chain_id: 1}, {id: "b", chain_id: 137}]`)) | ||
| }, |
Contributor
There was a problem hiding this comment.
Add teardown for DB schemas and clients in multichain integration tests.
These tests create persistent schemas and clients but never clean them up, which can leak state across reruns and leave open handles. Please DROP SCHEMA ... CASCADE and close storage/client in each test (or shared teardown) to keep tests isolated and deterministic.
Suggested pattern
let sql = PgStorage.makeClient()
let storage = PgStorage.makeStorageFromEnv(~config, ~sql, ~pgSchema, ~isHasuraEnabled=false)
+ try {
+ // ...test body...
+ } finally {
+ let _ = await sql->Postgres.unsafe(`DROP SCHEMA IF EXISTS "${pgSchema}" CASCADE;`)
+ await storage.close()
+ }Also applies to: 195-252, 262-323
🤖 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/lib_tests/ConfigMultichain_test.res` around lines
130 - 184, The test creates persistent database schemas and clients via
PgStorage.makeClient() and storage.initialize() but never cleans them up,
causing state leakage and open handles across test reruns. After each test
completes (or in a shared teardown), add cleanup logic to execute a DROP SCHEMA
cascade command on the pgSchema variable and close both the sql client and
storage object to ensure tests remain isolated and deterministic. This pattern
should be applied to all affected test cases that create these persistent
resources.
…on-se2inx # Conflicts: # packages/envio/src/InMemoryStore.res # packages/envio/src/LoadLayer.res # packages/envio/src/UserContext.res # scenarios/test_codegen/test/helpers/MockIndexer.res
Thread the chain id through the per-chain partition rather than re-deriving it from checkpoints at write time. Each updatedEntity group now carries its chain (None for cross-chain), so the entity-table delete during a rollback diff is always chain-scoped, and the chain_id column stamp covers rollback-restored changes whose synthetic checkpoint isn't in the batch. Scope the stale-entity-history prune by (id, chain_id) for isolated entities so one chain's anchor can't mask another chain's still-relevant history for the same id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scenarios/test_codegen/test/helpers/MockIndexer.res (1)
22-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire explicit
chainIdwhen seeding isolated entities in the mock store.
setEntitydefaults~chainId=0, andmakeseeds entities without passingchainId. For non-crossChainentities, this can silently seed chain0and make fixtures invisible to chain-scoped handlers/assertions.Suggested fix
- let setEntity = (indexerState, ~entityConfig: Internal.entityConfig, ~chainId=0, entity) => { + let setEntity = (indexerState, ~entityConfig: Internal.entityConfig, ~chainId=?, entity) => { + let chainId = switch chainId { + | Some(id) => id + | None => + if entityConfig.crossChain { + 0 + } else { + JsError.throwWithMessage( + `MockIndexer.InMemoryStore.setEntity requires ~chainId for isolated entity "${entityConfig.name}"`, + ) + } + } let inMemTable = indexerState->InMemoryStore.getInMemTable(~entityConfig, ~chainId)Also applies to: 48-49
🤖 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 22 - 24, Remove the default value from the `~chainId` parameter in the `setEntity` function signature to make it required instead of defaulting to 0, then update all callers of `setEntity` (including the `make` function around lines 48-49) to explicitly pass the `~chainId` parameter rather than relying on the default. This ensures entities are properly seeded to the correct chain and remain visible to chain-scoped handlers and assertions for non-crossChain entities.
🤖 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.
Duplicate comments:
In `@scenarios/test_codegen/test/helpers/MockIndexer.res`:
- Around line 22-24: Remove the default value from the `~chainId` parameter in
the `setEntity` function signature to make it required instead of defaulting to
0, then update all callers of `setEntity` (including the `make` function around
lines 48-49) to explicitly pass the `~chainId` parameter rather than relying on
the default. This ensures entities are properly seeded to the correct chain and
remain visible to chain-scoped handlers and assertions for non-crossChain
entities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 547485c2-9d54-431a-a61a-d9f6ab947254
📒 Files selected for processing (16)
packages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/Config.respackages/envio/src/EntityTables.respackages/envio/src/InMemoryStore.respackages/envio/src/IndexerState.respackages/envio/src/LoadLayer.respackages/envio/src/Persistence.respackages/envio/src/PgStorage.respackages/envio/src/PruneStaleHistory.respackages/envio/src/TestIndexerProxyStorage.respackages/envio/src/UserContext.respackages/envio/src/Writing.respackages/envio/src/db/EntityHistory.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/ConfigMultichain_test.res
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/envio/src/UserContext.res
- packages/envio/src/LoadLayer.res
- packages/envio/src/Config.res
- scenarios/test_codegen/test/lib_tests/ConfigMultichain_test.res
- packages/envio/src/PgStorage.res
Defaulting ~chainId to 0 would silently seed the wrong per-chain partition for an isolated entity and hide the fixture from chain-scoped reads. Make it required for isolated entities (cross-chain entities still ignore it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
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
Implements isolated multichain mode, where each chain's entities are kept separate with composite
(id, chain_id)primary keys. This complements the existing unordered mode where entities are shared across chains.Key Changes
Config & Validation
multichain: isolated | unorderedconfig option (defaults to unordered for backward compatibility)chain_idcolumnchain_idfor snake_case,chainIdfor original)In-Memory Storage
InMemoryStoreto partition isolated entities by chain while keeping cross-chain entities sharedgetChainStateto lazily create per-chain entity tablesgetInMemTableto route lookups to the correct partition based on entity type and chainDatabase Layer
PgStoragewithgetWriteSchemato appendchainIdfield for isolated entities on writestampIsolatedChainIdsto stamp chain ID onto Set changes before persistenceEntity Loading & Scoping
LoadLayerto scope isolated entity loads by chain ID in addition to entity IDUserContextto provide chain ID context to handlersTest Coverage
ConfigMultichain_test.resvalidating chain ID column naming, insertion, and write stamping across backendsMultichainIsolated_test.resdemonstrating per-chain entity isolation with composite keysmultichain_isolated_testscenario project with Counter contract exampleSchema & Codegen
multichainfieldcrossChain: falsefor isolated mode,truefor unorderedEnvioAddressesentity to excludechainIdfrom schema (stamped on write, not stored)crossChainfield in internal configImplementation Details
(id, chain_id)in all backendshttps://claude.ai/code/session_012fMLFoy8PHFm9uK2amN9LE
Summary by CodeRabbit
multichainconfiguration option with two modes:unordered(default, shared entities across chains) andisolated(per-chain entities).multichainand its chain-id behavior in the skill docs.