storage: add event ordering sanity check in Events.BatchStore

[prateushsharma]

Before writing events to storage, validate that the flattened event slice is correctly ordered and internally consistent.

Invariants enforced:

• TransactionIndex must be monotonically non-decreasing
• TransactionIndex is allowed to skip (transactions with no events are absent from the slice)
• Within a transaction, EventIndex must be contiguous starting at 0
• The first event of each new transaction must have EventIndex == 0

Adds validateEventOrder([]flow.Event) error which runs in O(n) time with no allocations. Called inside BatchStore before the cache is updated or the batch is committed.

Also fixes existing test fixtures in TestEventStoreRetrieve and TestEventStoreAndRemove where EventIndex was incorrectly set to match TransactionIndex instead of being zero-based within each transaction.

Closes #8454

Summary by CodeRabbit

• Bug Fixes
  • Added runtime validation to enforce correct event ordering during storage operations. Events are now validated before being committed, ensuring proper sequence progression and preventing invalid data storage.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces runtime validation in the event BatchStore to ensure events are correctly ordered by transaction and event indices. The validation checks that the first event has EventIndex 0, event indices increment by 1 within a transaction, and transaction indices are non-decreasing with each new transaction starting at EventIndex 0.

Changes

Cohort / File(s)	Summary
Event Order Validation storage/store/events.go	Adds validateEventOrder function to enforce event ordering invariants (EventIndex starting at 0, incrementing by 1, TransactionIndex non-decreasing). Validation runs in O(n) time and returns wrapped errors with offending block ID on failure.
Test Coverage storage/store/events_test.go	Updates existing tests to align with EventIndex 0 requirement for first event in transactions. Adds comprehensive TestValidateEventOrder suite covering valid sequences (empty, single transaction, skipped transaction indices) and invalid scenarios (incorrect first event, non-contiguous indices, decreasing transaction indices).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hop, hop! Events now align,
Each index starting at zero so fine,
Transactions flow in order true,
No gaps allowed—validation's due! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding event ordering validation to BatchStore in the storage events module.
Linked Issues check	✅ Passed	The PR fully implements the requirements from issue #8454: adds validateEventOrder function that checks TransactionIndex is non-decreasing and EventIndex is contiguous within transactions, runs in O(n) with no allocations, and is called before storage commit.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: validation logic in BatchStore, test adjustments to fix EventIndex to be zero-based within transactions, and comprehensive test coverage for the new validation function.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[prateushsharma]

One thing I want to flag proactively — the issue mentions we should scan historical execution data across past sporks to verify this invariant wasn't already violated before enforcing it as a hard error.
I haven't included that in this PR since it's a separate concern (a one-off verification tool, not storage logic), but I'm happy to build it as a follow-up if needed. Let me know how you'd like to proceed on that front.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

storage/store/events_test.go (1)

425-487: Tighten invalid-case assertions to verify error provenance.

require.Error is broad here; use require.ErrorContains(..., "invalid event ordering") so failures are specifically tied to the new ordering guard.

Proposed assertion pattern

- require.Error(t, err)
+ require.ErrorContains(t, err, "invalid event ordering")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@storage/store/events_test.go` around lines 425 - 487, The tests in
events_test.go use generic require.Error assertions which don't verify the error
comes from the ordering guard; update the four invalid-case subtests (the ones
invoking s.BatchStore) to assert the error message contains the ordering
sentinel (e.g., use require.ErrorContains(err, "invalid event ordering") or
similar) instead of require.Error so the failure indicates the new ordering
validation in BatchStore; locate the assertions around the BatchStore calls in
the subtests titled "invalid: first event has EventIndex != 0", "invalid:
non-contiguous EventIndex within same transaction", "invalid: new transaction
starts with EventIndex != 0", and "invalid: decreasing TransactionIndex" and
replace require.Error checks with require.ErrorContains referencing the ordering
error text.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@storage/store/events.go`:
- Line 77: Replace the regular error construction with an irrecoverable
exception for invariant violations: in validateEventOrder(), change each
fmt.Errorf(...) call (the four validation-failure returns inside
validateEventOrder) to use irrecoverable.NewExceptionf(...) so these checks
raise irrecoverable exceptions, and likewise replace the wrapper return in
BatchStore() that currently uses fmt.Errorf(...) with
irrecoverable.NewExceptionf(...); keep the same formatted message and wrapped
error argument but use irrecoverable.NewExceptionf to signal an exceptional,
non-recoverable validation failure.

---

Nitpick comments:
In `@storage/store/events_test.go`:
- Around line 425-487: The tests in events_test.go use generic require.Error
assertions which don't verify the error comes from the ordering guard; update
the four invalid-case subtests (the ones invoking s.BatchStore) to assert the
error message contains the ordering sentinel (e.g., use
require.ErrorContains(err, "invalid event ordering") or similar) instead of
require.Error so the failure indicates the new ordering validation in
BatchStore; locate the assertions around the BatchStore calls in the subtests
titled "invalid: first event has EventIndex != 0", "invalid: non-contiguous
EventIndex within same transaction", "invalid: new transaction starts with
EventIndex != 0", and "invalid: decreasing TransactionIndex" and replace
require.Error checks with require.ErrorContains referencing the ordering error
text.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63586af and a9198fa.

📒 Files selected for processing (2)

• storage/store/events.go
• storage/store/events_test.go