Skip to content

Fix ordered multichain regression introduced in 2.30.0 - #771

Merged
DZakh merged 16 commits into
mainfrom
dz/ordered-multichain-regression-fix
Oct 3, 2025
Merged

Fix ordered multichain regression introduced in 2.30.0#771
DZakh merged 16 commits into
mainfrom
dz/ordered-multichain-regression-fix

Conversation

@DZakh

@DZakh DZakh commented Oct 2, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes

    • Improved event ordering consistency in ordered multi-chain indexing.
    • Enhanced rollback reliability across multi-chain scenarios.
    • Standardized per-iteration fetch size for more deterministic processing.
  • Tests

    • Added regression test validating ordered multi-chain event sequencing.
    • Added comprehensive rollback tests for ordered and unordered multi-chain indexers (one scenario temporarily skipped) to strengthen coverage.

@coderabbitai

coderabbitai Bot commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adjusts ordered batch preparation to request one ready item per chain iteration. Adds a new E2E test for ordered multichain event ordering. Introduces comprehensive rollback tests for unordered (skipped) and ordered multichain indexers, verifying histories and rollback behavior across chains.

Changes

Cohort / File(s) Summary of Changes
Batch fetch granularity (ordered)
codegenerator/cli/npm/envio/src/Batch.res
In prepareOrderedBatch, changes FetchState.getReadyItemsCount targetSize to 1 (from remaining target). Control flow otherwise unchanged.
E2E ordering test
scenarios/test_codegen/test/E2E_test.res
Adds regression test “Correct event ordering for ordered multichain indexer” covering two chains, controlled logs, batch write, and ordered entity history assertions.
Rollback tests (multichain)
scenarios/test_codegen/test/rollback/Rollback_test.res
Adds two rollback scenarios: unordered multichain (skipped) and ordered multichain. Simulates reorgs, rollbacks, re-requests, and validates histories and current state post-rollback.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant I as Ordered Multichain Indexer
  participant FS as FetchState
  participant C1 as Chain A
  participant C2 as Chain B
  participant DB as Storage

  rect rgb(240,248,255)
  note over I: Prepare ordered batch
  loop per-iteration
    I->>FS: getReadyItemsCount(fromItem, targetSize=1)
    alt Item ready on Chain A
      I->>C1: Fetch next ready item
      C1-->>I: Item A_i
    else Item ready on Chain B
      I->>C2: Fetch next ready item
      C2-->>I: Item B_j
    end
    I->>DB: Append item to batch (ordered)
  end
  end

  note over I,DB: Commit batch preserving cross-chain order
  I->>DB: Write batch and histories
  DB-->>I: Ack
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • JonoPrest
  • JasoonS

Poem

Thump-thump, I queue by one, not ten—
Two chains hop in a tidy pen.
If blocks rewind, I roll right back,
Ears up, I’m tracing every track.
Carrots count the ordered way—
History neat, come what may. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly states that this pull request fixes an ordered multichain regression, which matches the code change altering fetch granularity and the new regression tests. It mentions the version where the regression was introduced, giving useful context. It is concise and specific, allowing teammates to understand the primary change at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dz/ordered-multichain-regression-fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd3fad7 and 313b539.

⛔ Files ignored due to path filters (1)
  • scenarios/test_codegen/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • codegenerator/cli/npm/envio/src/Batch.res (1 hunks)
  • scenarios/test_codegen/test/E2E_test.res (1 hunks)
  • scenarios/test_codegen/test/rollback/Rollback_test.res (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{res,resi}

📄 CodeRabbit inference engine (.cursor/rules/rescript.mdc)

**/*.{res,resi}: Never use [| item |] to create an array. Use [ item ] instead.
Must always use = for setting value to a field. Use := only for ref values created using ref function.
ReScript has record types which require a type definition before hand. You can access record fields by dot like foo.myField.
It's also possible to define an inline object, it'll have quoted fields in this case.
Use records when working with structured data, and objects to conveniently pass payload data between functions.
Never use %raw to access object fields if you know the type.

Files:

  • codegenerator/cli/npm/envio/src/Batch.res
  • scenarios/test_codegen/test/E2E_test.res
  • scenarios/test_codegen/test/rollback/Rollback_test.res
**/*.res

📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)

Prefer reading ReScript .res modules directly; compiled .js artifacts can be ignored

Files:

  • codegenerator/cli/npm/envio/src/Batch.res
  • scenarios/test_codegen/test/E2E_test.res
  • scenarios/test_codegen/test/rollback/Rollback_test.res
🧠 Learnings (1)
📚 Learning: 2025-05-27T17:07:12.878Z
Learnt from: JonoPrest
PR: enviodev/hyperindex#555
File: scenarios/test_codegen/test/lib_tests/EntityHistory_test.res:631-659
Timestamp: 2025-05-27T17:07:12.878Z
Learning: In isolated tests for EntityHistory rollbacks with controlled mock data setup, the ordering of getRollbackDiff results is deterministic and doesn't require sorting before comparison with deepStrictEqual.

Applied to files:

  • scenarios/test_codegen/test/rollback/Rollback_test.res
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build_and_test
🔇 Additional comments (3)
codegenerator/cli/npm/envio/src/Batch.res (1)

97-97: Per-chain single-item fetch makes sense

Locking targetSize to 1 enforces the intended “one ready item per iteration” sequencing and aligns with the new multichain ordering tests. Nicely scoped change.

scenarios/test_codegen/test/E2E_test.res (1)

86-214: Great coverage for the ordered multichain regression

The new scenario nails the cross-chain ordering expectations and guards the regression permanently. Thanks for the thorough history assertions.

scenarios/test_codegen/test/rollback/Rollback_test.res (1)

1376-2154: Rollback scenarios look comprehensive

Really appreciate how these ordered/unordered multichain rollback flows probe fetch-state rewinds, history chains, and entity state. This should keep the regression from resurfacing.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DZakh
DZakh requested review from JasoonS and JonoPrest October 2, 2025 12:37

@moose-code moose-code left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@DZakh
DZakh merged commit 85577d3 into main Oct 3, 2025
8 of 9 checks passed
@DZakh
DZakh deleted the dz/ordered-multichain-regression-fix branch October 3, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants