Make startBlock and endBlock optional in test indexer config - #1073
Merged
Conversation
Users no longer need to specify startBlock/endBlock for every process() call. When omitted, startBlock defaults to the chain config value, and endBlock defaults to startBlock (for simulate) or the chain config endBlock. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Throw user-friendly errors for non-numeric chain IDs and chain IDs not present in config.yaml, instead of silently defaulting to 0. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…tion loops endBlock no longer falls back to config.endBlock when simulate is absent — it must be explicitly provided. Also consolidates the two separate iteration loops (resolution + validation) into a single pass. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Merge resolution and validation into a single parseBlockRange function that resolves optional startBlock/endBlock, validates the range, and writes resolved values back for the worker. startBlock now defaults to progressBlock+1 when prior progress exists. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…ests - parseBlockRange no longer mutates the raw object; instead build a new resolvedProcessConfig with resolved values for the worker - Replace forEach over chainKeys with direct destructuring since there is always exactly one chain - Remove Utils.magic from happy-path tests, use proper generated types - Assert on entity block numbers instead of just change count https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Replace manual Utils.magic casts with a proper schema for the raw chain config (startBlock, endBlock, simulate). Schema validation catches type errors (e.g. string instead of int for startBlock) with clear messages. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…ogress
- workerData is now a typed record {chainId, startBlock, endBlock, simulate}
instead of raw JSON. Worker builds initialState and processConfig internally.
- endBlock defaults to max block number across simulate items (using
config.ecosystem.blockNumberName for EVM/Fuel support) instead of startBlock.
- progressBlockByChain is updated from WriteBatch checkpoint data during
processing, with onExit as fallback for empty runs.
- Main.start errors in worker are caught and logged.
https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…Batch progress - Add evmBlockConstructor, evmTransactionConstructor, fuelBlockConstructor, fuelTransactionConstructor types with all-optional fields in Envio.res - Update evmSimulateEventItem/fuelSimulateEventItem to use typed constructors instead of Js.Json.t for block and transaction fields - Add block/transaction to OnEvent constructor and makeSimulateItem in codegen - Restore initialState in workerData (worker receives it from main thread) - Remove onExit progressBlock fallback — only WriteBatch updates progress - Keep Main.start call unchanged (no Promise.catch wrapper) - Tests use Indexer.makeSimulateItem with typed block constructors https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
- Rename evmTransactionFields → evmTransactionConstructor in Internal.res, add missing accessList and authorizationList fields - Add evmBlockConstructor with all 27 block fields matching evmBlockField enum - Remove duplicate constructor types from Envio.res, reference Internal types - Codegen references Internal.evmBlockConstructor/evmTransactionConstructor - Add compile-time exhaustiveness checks in Config_test.res that verify constructor types have a field for every enum variant https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
… → evmTransactionInput Rename constructor types to Input (better describes all-optional types used as input for creating blocks/transactions). Types stay in Internal.res. Fuel types renamed similarly in Envio.res. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…em cases - Restore original field grouping and section comments in evmTransactionInput (signature fields, EIP-1559, EIP-4844, receipt fields, L2 fields) - Restore removed comments above evmBlockField enum and evmNullableBlockFields - Replace catch-all `| _` with explicit `| Evm` and `| Svm` (error) cases in SimulateItems.res block and transaction parsing https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…ndBlock from template tests - Add id?: string to fuelBlockInput matching the fuelSimulateBlockSchema - Remove startBlock/endBlock from template tests and contract import generators where simulate is present (now inferred automatically) https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant TestIndexer as TestIndexer
participant Worker as Worker
participant SimulateItems as SimulateItems
participant Parser as Parser
participant Store as Store
Client->>TestIndexer: call indexer.process(processConfig JSON)
TestIndexer->>TestIndexer: S.parseOrThrow(processConfigSchema)
TestIndexer->>SimulateItems: extract raw simulate items
TestIndexer->>Worker: spawn worker with {chainId, startBlock, endBlock, simulate, initialState}
Worker->>SimulateItems: patchConfig / iterate simulate items
SimulateItems->>Parser: parse block/transaction per ecosystem (Fuel / Evm)
Parser->>Worker: return parsed event items with typed block/tx
Worker->>Store: write entities & update progressBlockByChain
Worker->>TestIndexer: return final progress/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
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/Internal.res (1)
216-255:⚠️ Potential issue | 🟠 MajorChange
l1FeeScalarfromfloattostringin bothInternal.resandSimulateItems.res.OP Stack receipts return
l1FeeScalaras a decimal string (e.g., "0.684"), and it must be preserved asstringrather than converted to float, which loses precision and the original decimal representation. Update the type inevmTransactionInput(line ~239 inInternal.res) and the corresponding schema inSimulateItems.resto useS.nullable(S.string)instead ofS.null(S.float).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/envio/src/Internal.res` around lines 216 - 255, The evmTransactionInput type currently declares l1FeeScalar as float; change it to string in Internal.res (update the l1FeeScalar field in the evmTransactionInput record to type string) and update the corresponding schema in SimulateItems.res to use S.nullable(S.string) instead of S.null(S.float) so the OP Stack decimal string (e.g., "0.684") is preserved; locate usages of evmTransactionInput and any serialization/deserialization logic to ensure they expect a string for l1FeeScalar.
🧹 Nitpick comments (1)
packages/cli/src/hbs_templating/codegen_templates.rs (1)
1731-1741: Generate annotatedUtils.magiccasts once, not bare casts per field.These additions keep extending the template’s bare
->Utils.magicfield-access pattern. Please emit one explicitly-typed cast before readingevent/params/block/transactionso the generated.resstays within the repo’s cast rule.As per coding guidelines: "When using
Utils.magicfor type casting in ReScript, always add explicit type annotations:value->(Utils.magic: inputType => outputType)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/hbs_templating/codegen_templates.rs` around lines 1731 - 1741, The template repeatedly performs bare casts like (constructor->Utils.magic)[...] when building makeSimulateItem; instead emit a single explicitly-typed cast of the constructor (using simulateItemConstructor and the target simulate_item_type) into a local (e.g., let typed = constructor->(Utils.magic: ...)) and then read typed["event"], typed["params"], typed["block"], typed["transaction"] when constructing the returned {simulate_item_type}; update the makeSimulateItem body to use that single annotated cast so the generated .res follows the repo cast rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/hbs_templating/codegen_templates.rs`:
- Around line 1702-1719: The match on cfg.get_ecosystem() currently falls
through to the EVM branch for SVM, which causes runtime failures; add an
explicit Ecosystem::Svm arm in the same match that sets params_optional,
simulate_item_type, block_constructor_type, and transaction_constructor_type to
the SVM-specific symbols (e.g., use the SVM equivalents instead of
"Envio.evmSimulateEventItem"/"Internal.evmBlockInput"/"Internal.evmTransactionInput")
so makeSimulateItem generation uses the correct SVM types; update the match
handling near the tuple assignment (params_optional, simulate_item_type,
block_constructor_type, transaction_constructor_type) to include Ecosystem::Svm
with the proper SVM identifiers.
In `@packages/envio/src/sources/RpcSource.res`:
- Around line 660-663: The l1FeeScalar field is incorrectly using
Rpc.decimalFloatSchema->toFieldSchema; change its schema to parse as a nullable
string (e.g. S.nullable(S.string)->toFieldSchema) and ensure the field is
typed/treated downstream as a string (not a number) so OP Stack receipts like
"0.69" are preserved; update the schema for the l1FeeScalar entry and any
related type annotations to string/nullable string accordingly.
In `@packages/envio/src/TestIndexer.res`:
- Around line 385-418: After resolving startBlock and endBlock in
parseBlockRange (the block that currently returns {startBlock, endBlock}), add a
guard that checks if endBlock < startBlock and, if so, raise a Js.Exn.raiseError
with a descriptive message including chainIdStr and the offending values; place
this check after the endBlock resolution and before the existing
config/start/progress validations so inverted ranges (e.g., startBlock: 100,
endBlock: 50) are rejected early.
In `@scenarios/test_codegen/test/OptionalBlockParams_test.res`:
- Around line 23-35: The test currently assumes a hard-coded out-of-range
simulated event block (1) instead of validating that the runtime defaulting
resolves startBlock/endBlock correctly; update the test in
OptionalBlockParams_test.res to (1) ensure the
Indexer.createTestIndexer()/indexer.process call exercises the runtime
defaulting logic (the code path handling chains -> \"1337\" config), (2) assert
that when only startBlock is provided the processor sets endBlock = startBlock
(so expected events use blockNumber 5 for the first case), and (3) change the
expected entity assertions for the first and second process runs to directly
check the resolved blockNumber values (referencing the same indexer.process
invocation and the \"SimulateTestEvent\" entity collection) rather than relying
on the prior hard-coded 1/101 behavior; apply the same fixes to the other
failing cases around lines 53-80.
---
Outside diff comments:
In `@packages/envio/src/Internal.res`:
- Around line 216-255: The evmTransactionInput type currently declares
l1FeeScalar as float; change it to string in Internal.res (update the
l1FeeScalar field in the evmTransactionInput record to type string) and update
the corresponding schema in SimulateItems.res to use S.nullable(S.string)
instead of S.null(S.float) so the OP Stack decimal string (e.g., "0.684") is
preserved; locate usages of evmTransactionInput and any
serialization/deserialization logic to ensure they expect a string for
l1FeeScalar.
---
Nitpick comments:
In `@packages/cli/src/hbs_templating/codegen_templates.rs`:
- Around line 1731-1741: The template repeatedly performs bare casts like
(constructor->Utils.magic)[...] when building makeSimulateItem; instead emit a
single explicitly-typed cast of the constructor (using simulateItemConstructor
and the target simulate_item_type) into a local (e.g., let typed =
constructor->(Utils.magic: ...)) and then read typed["event"], typed["params"],
typed["block"], typed["transaction"] when constructing the returned
{simulate_item_type}; update the makeSimulateItem body to use that single
annotated cast so the generated .res follows the repo cast rule.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 806a27d1-4e72-4582-91c9-04d94bae45cd
⛔ Files ignored due to path filters (8)
packages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__indexer_code_generates_correct_types_and_values.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__codegen_templates__test__indexer_code_multiple_chains.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__contract_import_templates__test__rescript_test_file_for_evm.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__contract_import_templates__test__rescript_test_file_for_fuel.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__contract_import_templates__test__typescript_test_file_for_evm.snapis excluded by!**/*.snappackages/cli/src/hbs_templating/snapshots/envio__hbs_templating__contract_import_templates__test__typescript_test_file_for_fuel.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yamlscenarios/test_codegen/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/cli/src/hbs_templating/codegen_templates.rspackages/cli/src/hbs_templating/contract_import_templates.rspackages/cli/templates/static/erc20_template/typescript/src/indexer.test.tspackages/cli/templates/static/greeter_template/typescript/src/indexer.test.tspackages/cli/templates/static/greeteronfuel_template/typescript/src/indexer.test.tspackages/envio/src/Envio.respackages/envio/src/Internal.respackages/envio/src/SimulateItems.respackages/envio/src/TestIndexer.respackages/envio/src/sources/RpcSource.resscenarios/fuel_test/test/test.tsscenarios/test_codegen/test/Config_test.resscenarios/test_codegen/test/EventHandler.test.tsscenarios/test_codegen/test/OptionalBlockParams_test.res
💤 Files with no reviewable changes (5)
- packages/cli/templates/static/greeteronfuel_template/typescript/src/indexer.test.ts
- packages/cli/src/hbs_templating/contract_import_templates.rs
- packages/cli/templates/static/greeter_template/typescript/src/indexer.test.ts
- packages/cli/templates/static/erc20_template/typescript/src/indexer.test.ts
- scenarios/fuel_test/test/test.ts
Comment on lines
+1702
to
+1719
| let ( | ||
| params_optional, | ||
| simulate_item_type, | ||
| block_constructor_type, | ||
| transaction_constructor_type, | ||
| ) = match cfg.get_ecosystem() { | ||
| Ecosystem::Fuel => ( | ||
| "", | ||
| "Envio.fuelSimulateEventItem", | ||
| "Envio.fuelBlockInput", | ||
| "Envio.fuelTransactionInput", | ||
| ), | ||
| _ => ( | ||
| "?", | ||
| "Envio.evmSimulateEventItem", | ||
| "Internal.evmBlockInput", | ||
| "Internal.evmTransactionInput", | ||
| ), |
Contributor
There was a problem hiding this comment.
Don't route SVM through the EVM simulate types.
This _ arm also covers Ecosystem::Svm, but packages/envio/src/SimulateItems.res now throws for SVM simulate block/transaction parsing. The generated makeSimulateItem API will compile for SVM projects and then fail immediately at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/hbs_templating/codegen_templates.rs` around lines 1702 -
1719, The match on cfg.get_ecosystem() currently falls through to the EVM branch
for SVM, which causes runtime failures; add an explicit Ecosystem::Svm arm in
the same match that sets params_optional, simulate_item_type,
block_constructor_type, and transaction_constructor_type to the SVM-specific
symbols (e.g., use the SVM equivalents instead of
"Envio.evmSimulateEventItem"/"Internal.evmBlockInput"/"Internal.evmTransactionInput")
so makeSimulateItem generation uses the correct SVM types; update the match
handling near the tuple assignment (params_optional, simulate_item_type,
block_constructor_type, transaction_constructor_type) to include Ecosystem::Svm
with the proper SVM identifiers.
Comment on lines
+660
to
+663
| location: L1FeeScalar, | ||
| jsonKey: "l1FeeScalar", | ||
| schema: Rpc.decimalFloatSchema->toFieldSchema, | ||
| source: ReceiptOnly, |
Contributor
There was a problem hiding this comment.
l1FeeScalar should be parsed as string, not decimal float.
For OP Stack receipts this field is returned as a decimal string (e.g. "0.69"). Keeping it as Rpc.decimalFloatSchema risks type drift against the internal transaction field type and downstream serialization expectations.
Suggested fix
{
location: L1FeeScalar,
jsonKey: "l1FeeScalar",
- schema: Rpc.decimalFloatSchema->toFieldSchema,
+ schema: S.string->toFieldSchema,
source: ReceiptOnly,
},Based on learnings: In Optimism/OP Stack transaction receipts, l1FeeScalar is returned as a decimal string and should be parsed as S.nullable(S.string) and typed as string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/envio/src/sources/RpcSource.res` around lines 660 - 663, The
l1FeeScalar field is incorrectly using Rpc.decimalFloatSchema->toFieldSchema;
change its schema to parse as a nullable string (e.g.
S.nullable(S.string)->toFieldSchema) and ensure the field is typed/treated
downstream as a string (not a number) so OP Stack receipts like "0.69" are
preserved; update the schema for the l1FeeScalar entry and any related type
annotations to string/nullable string accordingly.
Comment on lines
+385
to
+418
| let endBlock = switch rawChainConfig.endBlock { | ||
| | Some(eb) => eb | ||
| | None if rawChainConfig.simulate->Option.isSome => | ||
| getSimulateEndBlock(~simulateItems=rawChainConfig.simulate->Option.getExn, ~config, ~startBlock) | ||
| | None => | ||
| Js.Exn.raiseError(`endBlock is required for chain ${chainIdStr} when simulate is not provided`) | ||
| } | ||
|
|
||
| if startBlock < configChain.startBlock { | ||
| Js.Exn.raiseError( | ||
| `Invalid block range for chain ${chainId}: startBlock (${processChainConfig.startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++ | ||
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++ | ||
| `Either use startBlock >= ${configChain.startBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | ||
| ) | ||
| } | ||
|
|
||
| // Check endBlock <= config.endBlock (if defined) | ||
| switch configChain.endBlock { | ||
| | Some(configEndBlock) if processChainConfig.endBlock > configEndBlock => | ||
| | Some(configEndBlock) if endBlock > configEndBlock => | ||
| Js.Exn.raiseError( | ||
| `Invalid block range for chain ${chainId}: endBlock (${processChainConfig.endBlock->Int.toString}) exceeds config.endBlock (${configEndBlock->Int.toString}). ` ++ | ||
| `Invalid block range for chain ${chainIdStr}: endBlock (${endBlock->Int.toString}) exceeds config.endBlock (${configEndBlock->Int.toString}). ` ++ | ||
| `Either use endBlock <= ${configEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | ||
| ) | ||
| | _ => () | ||
| } | ||
|
|
||
| // Check startBlock > progressBlock | ||
| switch progressBlock { | ||
| | Some(prevEndBlock) if processChainConfig.startBlock <= prevEndBlock => | ||
| | Some(prevEndBlock) if startBlock <= prevEndBlock => | ||
| Js.Exn.raiseError( | ||
| `Invalid block range for chain ${chainId}: startBlock (${processChainConfig.startBlock->Int.toString}) must be greater than previously processed endBlock (${prevEndBlock->Int.toString}). ` ++ | ||
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) must be greater than previously processed endBlock (${prevEndBlock->Int.toString}). ` ++ | ||
| `Either use startBlock > ${prevEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | ||
| ) | ||
| | _ => () | ||
| } | ||
|
|
||
| {startBlock, endBlock} |
Contributor
There was a problem hiding this comment.
Reject inverted block ranges in parseBlockRange.
startBlock and endBlock are resolved independently here, but there is no guard for endBlock < startBlock. A call like process({startBlock: 100, endBlock: 50}) currently passes validation and builds a nonsensical worker range.
🐛 Proposed fix
let endBlock = switch rawChainConfig.endBlock {
| Some(eb) => eb
| None if rawChainConfig.simulate->Option.isSome =>
getSimulateEndBlock(~simulateItems=rawChainConfig.simulate->Option.getExn, ~config, ~startBlock)
| None =>
Js.Exn.raiseError(`endBlock is required for chain ${chainIdStr} when simulate is not provided`)
}
+ if endBlock < startBlock {
+ Js.Exn.raiseError(
+ `Invalid block range for chain ${chainIdStr}: endBlock (${endBlock->Int.toString}) must be greater than or equal to startBlock (${startBlock->Int.toString}).`,
+ )
+ }
+
if startBlock < configChain.startBlock {
Js.Exn.raiseError(
`Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++
`Either use startBlock >= ${configChain.startBlock->Int.toString} or create a new test indexer with createTestIndexer().`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let endBlock = switch rawChainConfig.endBlock { | |
| | Some(eb) => eb | |
| | None if rawChainConfig.simulate->Option.isSome => | |
| getSimulateEndBlock(~simulateItems=rawChainConfig.simulate->Option.getExn, ~config, ~startBlock) | |
| | None => | |
| Js.Exn.raiseError(`endBlock is required for chain ${chainIdStr} when simulate is not provided`) | |
| } | |
| if startBlock < configChain.startBlock { | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainId}: startBlock (${processChainConfig.startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++ | |
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++ | |
| `Either use startBlock >= ${configChain.startBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| } | |
| // Check endBlock <= config.endBlock (if defined) | |
| switch configChain.endBlock { | |
| | Some(configEndBlock) if processChainConfig.endBlock > configEndBlock => | |
| | Some(configEndBlock) if endBlock > configEndBlock => | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainId}: endBlock (${processChainConfig.endBlock->Int.toString}) exceeds config.endBlock (${configEndBlock->Int.toString}). ` ++ | |
| `Invalid block range for chain ${chainIdStr}: endBlock (${endBlock->Int.toString}) exceeds config.endBlock (${configEndBlock->Int.toString}). ` ++ | |
| `Either use endBlock <= ${configEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| | _ => () | |
| } | |
| // Check startBlock > progressBlock | |
| switch progressBlock { | |
| | Some(prevEndBlock) if processChainConfig.startBlock <= prevEndBlock => | |
| | Some(prevEndBlock) if startBlock <= prevEndBlock => | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainId}: startBlock (${processChainConfig.startBlock->Int.toString}) must be greater than previously processed endBlock (${prevEndBlock->Int.toString}). ` ++ | |
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) must be greater than previously processed endBlock (${prevEndBlock->Int.toString}). ` ++ | |
| `Either use startBlock > ${prevEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| | _ => () | |
| } | |
| {startBlock, endBlock} | |
| let endBlock = switch rawChainConfig.endBlock { | |
| | Some(eb) => eb | |
| | None if rawChainConfig.simulate->Option.isSome => | |
| getSimulateEndBlock(~simulateItems=rawChainConfig.simulate->Option.getExn, ~config, ~startBlock) | |
| | None => | |
| Js.Exn.raiseError(`endBlock is required for chain ${chainIdStr} when simulate is not provided`) | |
| } | |
| if endBlock < startBlock { | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainIdStr}: endBlock (${endBlock->Int.toString}) must be greater than or equal to startBlock (${startBlock->Int.toString}).`, | |
| ) | |
| } | |
| if startBlock < configChain.startBlock { | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) is less than config.startBlock (${configChain.startBlock->Int.toString}). ` ++ | |
| `Either use startBlock >= ${configChain.startBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| } | |
| switch configChain.endBlock { | |
| | Some(configEndBlock) if endBlock > configEndBlock => | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainIdStr}: endBlock (${endBlock->Int.toString}) exceeds config.endBlock (${configEndBlock->Int.toString}). ` ++ | |
| `Either use endBlock <= ${configEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| | _ => () | |
| } | |
| switch progressBlock { | |
| | Some(prevEndBlock) if startBlock <= prevEndBlock => | |
| Js.Exn.raiseError( | |
| `Invalid block range for chain ${chainIdStr}: startBlock (${startBlock->Int.toString}) must be greater than previously processed endBlock (${prevEndBlock->Int.toString}). ` ++ | |
| `Either use startBlock > ${prevEndBlock->Int.toString} or create a new test indexer with createTestIndexer().`, | |
| ) | |
| | _ => () | |
| } | |
| {startBlock, endBlock} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/envio/src/TestIndexer.res` around lines 385 - 418, After resolving
startBlock and endBlock in parseBlockRange (the block that currently returns
{startBlock, endBlock}), add a guard that checks if endBlock < startBlock and,
if so, raise a Js.Exn.raiseError with a descriptive message including chainIdStr
and the offending values; place this check after the endBlock resolution and
before the existing config/start/progress validations so inverted ranges (e.g.,
startBlock: 100, endBlock: 50) are rejected early.
Comment on lines
+23
to
+35
| Async.it("Optional block params: defaults endBlock to startBlock when only startBlock is provided", async t => { | ||
| let indexer = Indexer.createTestIndexer() | ||
|
|
||
| let _ = await indexer.process({ | ||
| chains: { | ||
| \"1337": {startBlock: 5, simulate: [simulateItem]}, | ||
| }, | ||
| }) | ||
|
|
||
| let entities = await (indexer.\"SimulateTestEvent").getAll() | ||
| // Event block defaults to config startBlock (1) in SimulateItems.parse, | ||
| // but the process range is startBlock=5, endBlock=5 | ||
| t.expect(entities).toEqual([{id: "1_0", blockNumber: 1, logIndex: 0, timestamp: 0}]) |
Contributor
There was a problem hiding this comment.
These tests still don't validate the resolved startBlock behavior.
The startBlock: 5 case expects 1_0, which hard-codes the current out-of-range simulate default instead of the intended 5-5 range. Then the second-process case forces block.number = 101, so it would still pass even if the omitted startBlock defaulted to 2 instead of 101. Please fix the runtime defaulting first, then assert the defaulted block numbers directly.
Also applies to: 53-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scenarios/test_codegen/test/OptionalBlockParams_test.res` around lines 23 -
35, The test currently assumes a hard-coded out-of-range simulated event block
(1) instead of validating that the runtime defaulting resolves
startBlock/endBlock correctly; update the test in OptionalBlockParams_test.res
to (1) ensure the Indexer.createTestIndexer()/indexer.process call exercises the
runtime defaulting logic (the code path handling chains -> \"1337\" config), (2)
assert that when only startBlock is provided the processor sets endBlock =
startBlock (so expected events use blockNumber 5 for the first case), and (3)
change the expected entity assertions for the first and second process runs to
directly check the resolved blockNumber values (referencing the same
indexer.process invocation and the \"SimulateTestEvent\" entity collection)
rather than relying on the prior hard-coded 1/101 behavior; apply the same fixes
to the other failing cases around lines 53-80.
…ssertion
- Replace unit test of getSimulateEndBlock with a proper E2E test in
fuel_test that uses createTestIndexer with block: {height: 50}
- Assert exact schema error message instead of startsWith prefix check
https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scenarios/fuel_test/test/test.ts`:
- Around line 115-131: The test is passing a chains object to indexer.process
that omits startBlock and endBlock, but the generated type
FuelTestIndexerChainConfig currently marks startBlock and endBlock as required;
update the type so these fields are optional (make startBlock? and endBlock? in
FuelTestIndexerChainConfig in packages/envio/index.d.ts) so tests that rely on
defaulting (e.g., using simulate block height as implicit endBlock) compile;
locate the type declaration for FuelTestIndexerChainConfig and change the
startBlock and endBlock properties to optional while keeping existing semantics.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a528a65-6dd2-497c-aa56-f02d11125cb3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
scenarios/fuel_test/test/test.tsscenarios/test_codegen/test/OptionalBlockParams_test.res
✅ Files skipped from review due to trivial changes (1)
- scenarios/test_codegen/test/OptionalBlockParams_test.res
Verify result.changes contains the expected block number and chain ID, which directly proves endBlock was inferred from block height. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
The ReScript types were already optional but the manually maintained index.d.ts still had them as required, causing TS compilation errors in CI when tests omit these fields with simulate items. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
…→ fuelSimulateItem Shorter names. Also inline EvmSimulateEventItem/FuelSimulateEventItem into EvmSimulateItem/FuelSimulateItem in index.d.ts, removing the intermediate types. https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
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
This PR makes
startBlockandendBlockoptional parameters in the test indexer's process configuration, with intelligent defaults based on context. This simplifies the test indexer API by reducing boilerplate when sensible defaults can be inferred.Key Changes
Made block parameters optional in chain config types: Updated
evmChainConfigandfuelChainConfigtypes to have optionalstartBlockandendBlockfields (marked with?)Implemented smart defaults for block ranges:
startBlockdefaults to the config'sstartBlockon first call, orprogressBlock + 1on subsequent callsendBlockdefaults to the maximum block number found in simulate items, or raises an error if neitherendBlocknorsimulateis providedAdded schema validation: Created
rawChainConfigSchemaandprocessConfigSchemausing the S library to validate the raw process config before parsingRefactored block range parsing: Replaced
validateBlockRangewithparseBlockRangethat both resolves optional values and validates the resulting rangeAdded helper function: Implemented
getSimulateEndBlockto extract the maximum block number from simulate items, supporting both EVM (numberfield) and Fuel (heightfield) ecosystemsUpdated worker data structure: Changed
workerDatatype to include resolvedchainId,startBlock,endBlock, andsimulatefields instead of raw JSON, enabling proper type safetyAdded progress tracking: Updated
handleWriteBatchto track processed block numbers inprogressBlockByChainfor use in subsequent process callsAdded comprehensive tests: Created
OptionalBlockParams_test.reswith 11 test cases covering defaults, explicit values, error cases, and edge casesUpdated type definitions: Added
evmBlockInputandevmTransactionInputtypes toInternal.res, and correspondingfuelBlockInputandfuelTransactionInputtypes toEnvio.resfor type-safe block/transaction construction in simulate itemsUpdated code generation: Modified templates to generate proper types for block and transaction constructors in simulate items
Updated test templates: Removed explicit
startBlock/endBlockfrom template tests to demonstrate the new optional behaviorNotable Implementation Details
parseBlockRangefunction validates that the resolved block range doesn't overlap with previously processed blocks (tracked viaprogressBlockByChain)blockandtransactionfields with proper type constructors instead of raw JSONstartBlock/endBlockvalues still work as beforeendBlockis required (whensimulateis not provided)https://claude.ai/code/session_01Wr3tePX4A9jLAUzgfV2HgP
Summary by CodeRabbit
New Features
Tests