[Storage] Add a sanity check to the events BatchStore for correct order

[peterargue]

Currently there is no check within the storage/store/events.go implementation to ensure that events passed to BatchStore are sorted by transaction index/event index and form a continuous sequence. Add this sanity check and return an error if they do not match

flow-go/storage/store/events.go

Lines 58 to 66 in 1d2ab50

	combinedEvents := make([]flow.Event, sliceSize)
	eventIndex := 0
	for _, txEvents := range blockEvents {
	for _, event := range txEvents {
	combinedEvents[eventIndex] = event
	eventIndex++
	}
	}

the logic would be something like

	combinedEvents := make([]flow.Event, sliceSize)
	globalIndex := 0
	eventIndex := uint32(0)
	txIndex := uint32(0)

	for _, txEvents := range blockEvents {
		for _, event := range txEvents {
			switch event.TransactionIndex {
			case txIndex:
				if event.EventIndex != eventIndex {
					return fmt.Errorf("event index mismatch in transaction %d: expected %d, got %d", txIndex, eventIndex, event.EventIndex)
				}
				eventIndex++
			case txIndex + 1:
				txIndex = event.TransactionIndex
				eventIndex = 0
			default:
				return fmt.Errorf("transaction index mismatch: expected %d, got %d", txIndex, event.TransactionIndex)
			}

			combinedEvents[globalIndex] = event
			globalIndex++
		}
	}

Along with this change, we will need to check the data from the current and past sporks to see if this issue exists to avoid causing an execution halt. We should be able to iterate execution data for each block and inspect the events since execution data is produced in the same order as execution.

[prateushsharma]

Hi, I’d like to work on this.

Before implementing the sanity check in BatchStore, I want to clarify one detail about the expected invariant:

The proposed logic enforces that TransactionIndex must increase either by 0 (same tx) or by +1 (next tx), and that EventIndex is strictly sequential within a transaction.

However, in cases where some transactions in a block emit no events, would we still expect the next observed TransactionIndex to increment strictly by +1 relative to the last event-bearing transaction? Or can it legitimately skip (e.g. jump by +2 or more if intermediate transactions had no events)?

I’ll also run a scan over historical execution data to confirm that existing spork data satisfies the invariant before enforcing a hard error, to avoid accidental execution halts.

Let me know if there are any edge cases I should consider.

[peterargue]

Much appreciated 🙏.

However, in cases where some transactions in a block emit no events, would we still expect the next observed TransactionIndex to increment strictly by +1 relative to the last event-bearing transaction? Or can it legitimately skip (e.g. jump by +2 or more if intermediate transactions had no events)?

yes, if a tx emits no events, the tx index would skip. off the top of my head, that's the only corner case.