From e59dc76e4d2793ba43113b356dc0ef972f88569f Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 08:56:51 -0800
Subject: [PATCH 01/31] add chunk model changes and encoding tests

---
 model/flow/chunk.go        |  39 ++++++++-
 model/flow/chunk_test.go   | 157 ++++++++++++++++++++++++++++++++++---
 utils/unittest/encoding.go |  16 ++++
 3 files changed, 199 insertions(+), 13 deletions(-)
 create mode 100644 utils/unittest/encoding.go

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 83eabde4b1e..a2e17f15311 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -6,6 +6,8 @@ import (
 
 	"github.com/ipfs/go-cid"
 	"github.com/vmihailenco/msgpack/v4"
+
+	"github.com/onflow/flow-go/model/encoding/rlp"
 )
 
 var EmptyEventCollectionID Identifier
@@ -20,19 +22,54 @@ func init() {
 	}
 }
 
+// TODO doc
+type chunkBodyV0 struct {
+	CollectionIndex      uint
+	StartState           StateCommitment
+	EventCollection      Identifier
+	BlockID              Identifier
+	TotalComputationUsed uint64
+	NumberOfTransactions uint64
+}
+
 type ChunkBody struct {
 	CollectionIndex uint
 
 	// execution info
 	StartState      StateCommitment // start state when starting executing this chunk
 	EventCollection Identifier      // Events generated by executing results
-	BlockID         Identifier      // Block id of the execution result this chunk belongs to
+	// ServiceEventIndices is a list of indices of service events which were emitted.
+	// If ServiceEventIndices is nil, this indicates that this chunk was created by an older software version
+	// which did support specifying a mapping between chunks and service events.
+	// TODO doc
+	ServiceEventIndices []uint32
+	BlockID             Identifier // Block id of the execution result this chunk belongs to
 
 	// Computation consumption info
 	TotalComputationUsed uint64 // total amount of computation used by running all txs in this chunk
 	NumberOfTransactions uint64 // number of transactions inside the collection
 }
 
+// Fingerprint returns the unique binary representation for the receiver ChunkBody,
+// used to compute the ID (hash).
+// The fingerprint is backward-compatible with the prior data model for ChunkBody: chunkBodyV0.
+//   - All new ChunkBody instances must have non-nil ServiceEventIndices
+//   - A nil ServiceEventIndices field indicates a v0 version of ChunkBody
+//   - when computing the ID of such a ChunkBody, the ServiceEventIndices field is omitted from the fingerprint
+func (ch ChunkBody) Fingerprint() []byte {
+	if ch.ServiceEventIndices == nil {
+		return rlp.NewMarshaler().MustMarshal(chunkBodyV0{
+			CollectionIndex:      ch.CollectionIndex,
+			StartState:           ch.StartState,
+			EventCollection:      ch.EventCollection,
+			BlockID:              ch.BlockID,
+			TotalComputationUsed: ch.TotalComputationUsed,
+			NumberOfTransactions: ch.NumberOfTransactions,
+		})
+	}
+	return rlp.NewMarshaler().MustMarshal(ch)
+}
+
 type Chunk struct {
 	ChunkBody
 
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 9da330dcaaa..614e4a8f4e5 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -1,12 +1,13 @@
-package flow_test
+package flow
 
 import (
+	"encoding/json"
 	"testing"
 
+	"github.com/fxamacker/cbor/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/rand"
 	"github.com/onflow/flow-go/utils/unittest"
 )
@@ -15,7 +16,7 @@ import (
 // out of range indices
 func TestChunkList_ByIndex(t *testing.T) {
 	// creates a chunk list with the size of 10
-	var chunkList flow.ChunkList = make([]*flow.Chunk, 10)
+	var chunkList ChunkList = make([]*Chunk, 10)
 
 	// an out of index chunk by index
 	_, ok := chunkList.ByIndex(11)
@@ -36,14 +37,14 @@ func TestDistinctChunkIDs_EmptyChunks(t *testing.T) {
 	require.NotEqual(t, blockIdA, blockIdB)
 
 	// generates a chunk associated with each block id
-	chunkA := &flow.Chunk{
-		ChunkBody: flow.ChunkBody{
+	chunkA := &Chunk{
+		ChunkBody: ChunkBody{
 			BlockID: blockIdA,
 		},
 	}
 
-	chunkB := &flow.Chunk{
-		ChunkBody: flow.ChunkBody{
+	chunkB := &Chunk{
+		ChunkBody: ChunkBody{
 			BlockID: blockIdB,
 		},
 	}
@@ -83,7 +84,7 @@ func TestChunkList_Indices(t *testing.T) {
 	cl := unittest.ChunkListFixture(5, unittest.IdentifierFixture())
 	t.Run("empty chunk subset indices", func(t *testing.T) {
 		// subset of chunk list that is empty should return an empty list
-		subset := flow.ChunkList{}
+		subset := ChunkList{}
 		indices := subset.Indices()
 		require.Len(t, indices, 0)
 	})
@@ -100,7 +101,7 @@ func TestChunkList_Indices(t *testing.T) {
 	t.Run("multiple chunk subset indices", func(t *testing.T) {
 		// subset that only contains even chunk indices, should return
 		// a uint64 slice that only contains even chunk indices
-		subset := flow.ChunkList{cl[0], cl[2], cl[4]}
+		subset := ChunkList{cl[0], cl[2], cl[4]}
 		indices := subset.Indices()
 		require.Len(t, indices, 3)
 		require.Contains(t, indices, uint64(0), uint64(2), uint64(4))
@@ -111,7 +112,7 @@ func TestChunkIndexIsSet(t *testing.T) {
 
 	i, err := rand.Uint()
 	require.NoError(t, err)
-	chunk := flow.NewChunk(
+	chunk := NewChunk(
 		unittest.IdentifierFixture(),
 		int(i),
 		unittest.StateCommitmentFixture(),
@@ -129,7 +130,7 @@ func TestChunkNumberOfTxsIsSet(t *testing.T) {
 
 	i, err := rand.Uint32()
 	require.NoError(t, err)
-	chunk := flow.NewChunk(
+	chunk := NewChunk(
 		unittest.IdentifierFixture(),
 		3,
 		unittest.StateCommitmentFixture(),
@@ -146,7 +147,7 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 
 	i, err := rand.Uint64()
 	require.NoError(t, err)
-	chunk := flow.NewChunk(
+	chunk := NewChunk(
 		unittest.IdentifierFixture(),
 		3,
 		unittest.StateCommitmentFixture(),
@@ -158,3 +159,135 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 
 	assert.Equal(t, i, chunk.TotalComputationUsed)
 }
+
+// TODO doc
+func TestChunkEncodeDecode(t *testing.T) {
+	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 0)
+
+	t.Run("encode/decode preserves nil ServiceEventIndices", func(t *testing.T) {
+		chunk.ServiceEventIndices = nil
+		t.Run("json", func(t *testing.T) {
+			bz, err := json.Marshal(chunk)
+			require.NoError(t, err)
+			unmarshaled := new(Chunk)
+			err = json.Unmarshal(bz, unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunk, unmarshaled)
+			assert.Nil(t, unmarshaled.ServiceEventIndices)
+		})
+		t.Run("cbor", func(t *testing.T) {
+			bz, err := cbor.Marshal(chunk)
+			require.NoError(t, err)
+			unmarshaled := new(Chunk)
+			err = cbor.Unmarshal(bz, unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunk, unmarshaled)
+			assert.Nil(t, unmarshaled.ServiceEventIndices)
+		})
+	})
+	t.Run("encode/decode preserves empty but non-nil ServiceEventIndices", func(t *testing.T) {
+		chunk.ServiceEventIndices = []uint32{}
+		t.Run("json", func(t *testing.T) {
+			bz, err := json.Marshal(chunk)
+			require.NoError(t, err)
+			unmarshaled := new(Chunk)
+			err = json.Unmarshal(bz, unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunk, unmarshaled)
+			assert.NotNil(t, unmarshaled.ServiceEventIndices)
+		})
+		t.Run("cbor", func(t *testing.T) {
+			bz, err := cbor.Marshal(chunk)
+			require.NoError(t, err)
+			unmarshaled := new(Chunk)
+			err = cbor.Unmarshal(bz, unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunk, unmarshaled)
+			assert.NotNil(t, unmarshaled.ServiceEventIndices)
+		})
+	})
+}
+
+// TODO doc
+func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
+	chunkFixture := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
+	chunkFixture.ServiceEventIndices = []uint32{1} // non-nil extra field
+
+	t.Run("writing v0 and reading v1 should yield nil for new field", func(t *testing.T) {
+		var chunkv0 chunkBodyV0
+		unittest.CopyStructure(t, chunkFixture.ChunkBody, chunkv0)
+
+		t.Run("json", func(t *testing.T) {
+			bz, err := json.Marshal(chunkv0)
+			require.NoError(t, err)
+
+			var unmarshaled ChunkBody
+			err = json.Unmarshal(bz, &unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
+			assert.Equal(t, chunkv0.BlockID, unmarshaled.BlockID)
+			assert.Nil(t, unmarshaled.ServiceEventIndices)
+		})
+
+		t.Run("cbor", func(t *testing.T) {
+			bz, err := cbor.Marshal(chunkv0)
+			require.NoError(t, err)
+
+			var unmarshaled ChunkBody
+			err = cbor.Unmarshal(bz, &unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
+			assert.Equal(t, chunkv0.BlockID, unmarshaled.BlockID)
+			assert.Nil(t, unmarshaled.ServiceEventIndices)
+		})
+	})
+	t.Run("writing v1 and reading v0 does not error", func(t *testing.T) {
+		chunkv1 := chunkFixture.ChunkBody
+		chunkv1.ServiceEventIndices = []uint32{0} // ensure non-nil ServiceEventIndices field
+
+		t.Run("json", func(t *testing.T) {
+			bz, err := json.Marshal(chunkv1)
+			require.NoError(t, err)
+
+			var unmarshaled chunkBodyV0
+			err = json.Unmarshal(bz, &unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunkv1.EventCollection, unmarshaled.EventCollection)
+			assert.Equal(t, chunkv1.BlockID, unmarshaled.BlockID)
+		})
+		t.Run("cbor", func(t *testing.T) {
+			bz, err := cbor.Marshal(chunkv1)
+			require.NoError(t, err)
+
+			var unmarshaled chunkBodyV0
+			err = cbor.Unmarshal(bz, &unmarshaled)
+			require.NoError(t, err)
+			assert.Equal(t, chunkv1.EventCollection, unmarshaled.EventCollection)
+			assert.Equal(t, chunkv1.BlockID, unmarshaled.BlockID)
+		})
+	})
+}
+
+// TODO doc
+func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
+	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
+	chunkBody := chunk.ChunkBody
+	var chunkv0 chunkBodyV0
+	unittest.CopyStructure(t, chunkBody, &chunkv0)
+
+	// A nil ServiceEventIndices fields indicates a prior model version.
+	// The ID calculation for the old and new model version should be the same.
+	t.Run("nil ServiceEventIndices fields", func(t *testing.T) {
+		chunkBody.ServiceEventIndices = nil
+		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
+		assert.Equal(t, MakeID(chunkv0), MakeID(chunkBody))
+	})
+	// A non-nil ServiceEventIndices fields indicates an up-to-date model version.
+	// The ID calculation for the old and new model version should be different,
+	// because the new model should include the ServiceEventIndices field value.
+	t.Run("non-nil ServiceEventIndices fields", func(t *testing.T) {
+		chunkBody.ServiceEventIndices = []uint32{}
+		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
+		assert.NotEqual(t, MakeID(chunkv0), MakeID(chunkBody))
+	})
+}
diff --git a/utils/unittest/encoding.go b/utils/unittest/encoding.go
new file mode 100644
index 00000000000..b2fb5efb9d9
--- /dev/null
+++ b/utils/unittest/encoding.go
@@ -0,0 +1,16 @@
+package unittest
+
+import (
+	"testing"
+
+	"github.com/fxamacker/cbor/v2"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO doc
+func CopyStructure(t *testing.T, src, dst any) {
+	bz, err := cbor.Marshal(src)
+	require.NoError(t, err)
+	err = cbor.Unmarshal(bz, dst)
+	require.NoError(t, err)
+}

From 869feff6a64a51f4c08655ddf4d32cd23b2f3504 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 09:21:04 -0800
Subject: [PATCH 02/31] add servcie event indices to chunk

---
 engine/execution/block_result.go | 18 ++++++++++++++++++
 model/flow/chunk.go              |  5 +++++
 model/flow/chunk_test.go         |  3 +++
 3 files changed, 26 insertions(+)

diff --git a/engine/execution/block_result.go b/engine/execution/block_result.go
index 1cfbb9bc0d4..72f65a22629 100644
--- a/engine/execution/block_result.go
+++ b/engine/execution/block_result.go
@@ -49,6 +49,23 @@ func (er *BlockExecutionResult) AllEvents() flow.EventsList {
 	return res
 }
 
+func (er *BlockExecutionResult) ServiceEventIndicesForChunk(chunkIndex int) []uint32 {
+	nServiceEventsForChunk := len(er.collectionExecutionResults[chunkIndex].serviceEvents)
+	if nServiceEventsForChunk == 0 {
+		return []uint32{}
+	}
+
+	firstIndex := 0
+	for i := 0; i < chunkIndex; i++ {
+		firstIndex += len(er.collectionExecutionResults[i].serviceEvents)
+	}
+	indices := make([]uint32, 0, nServiceEventsForChunk)
+	for i := firstIndex; i < len(er.collectionExecutionResults[chunkIndex].serviceEvents); i++ {
+		indices = append(indices, uint32(i))
+	}
+	return indices
+}
+
 func (er *BlockExecutionResult) AllServiceEvents() flow.EventsList {
 	res := make(flow.EventsList, 0)
 	for _, ce := range er.collectionExecutionResults {
@@ -199,6 +216,7 @@ func (ar *BlockAttestationResult) ChunkAt(index int) *flow.Chunk {
 		attestRes.startStateCommit,
 		len(execRes.TransactionResults()),
 		attestRes.eventCommit,
+		ar.ServiceEventIndicesForChunk(index),
 		attestRes.endStateCommit,
 		execRes.executionSnapshot.TotalComputationUsed(),
 	)
diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index a2e17f15311..e2ddb1c136a 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -84,9 +84,13 @@ func NewChunk(
 	startState StateCommitment,
 	numberOfTransactions int,
 	eventCollection Identifier,
+	serviceEventIndices []uint32,
 	endState StateCommitment,
 	totalComputationUsed uint64,
 ) *Chunk {
+	if serviceEventIndices == nil {
+		serviceEventIndices = []uint32{}
+	}
 	return &Chunk{
 		ChunkBody: ChunkBody{
 			BlockID:              blockID,
@@ -94,6 +98,7 @@ func NewChunk(
 			StartState:           startState,
 			NumberOfTransactions: uint64(numberOfTransactions),
 			EventCollection:      eventCollection,
+			ServiceEventIndices:  serviceEventIndices,
 			TotalComputationUsed: totalComputationUsed,
 		},
 		Index:    uint64(collectionIndex),
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 614e4a8f4e5..8b331ab614d 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -118,6 +118,7 @@ func TestChunkIndexIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		21,
 		unittest.IdentifierFixture(),
+		[]uint32{},
 		unittest.StateCommitmentFixture(),
 		17995,
 	)
@@ -136,6 +137,7 @@ func TestChunkNumberOfTxsIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		int(i),
 		unittest.IdentifierFixture(),
+		[]uint32{},
 		unittest.StateCommitmentFixture(),
 		17995,
 	)
@@ -153,6 +155,7 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		21,
 		unittest.IdentifierFixture(),
+		[]uint32{},
 		unittest.StateCommitmentFixture(),
 		i,
 	)

From 6e4c8de50110a80d1f1b217584c41368ac614b5f Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 09:30:12 -0800
Subject: [PATCH 03/31] revert test package change

---
 model/flow/chunk.go        |  6 ++---
 model/flow/chunk_test.go   | 49 +++++++++++++++++++-------------------
 utils/unittest/fixtures.go |  2 ++
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index e2ddb1c136a..990c9dfdb52 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -23,7 +23,7 @@ func init() {
 }
 
 // TODO doc
-type chunkBodyV0 struct {
+type ChunkBodyV0 struct {
 	CollectionIndex      uint
 	StartState           StateCommitment
 	EventCollection      Identifier
@@ -52,13 +52,13 @@ type ChunkBody struct {
 
 // Fingerprint returns the unique binary representation for the receiver ChunkBody,
 // used to compute the ID (hash).
-// The fingerprint is backward-compatible with the prior data model for ChunkBody: chunkBodyV0.
+// The fingerprint is backward-compatible with the prior data model for ChunkBody: ChunkBodyV0.
 //   - All new ChunkBody instances must have non-nil ServiceEventIndices
 //   - A nil ServiceEventIndices field indicates a v0 version of ChunkBody
 //   - when computing the ID of such a ChunkBody, the ServiceEventIndices field is omitted from the fingerprint
 func (ch ChunkBody) Fingerprint() []byte {
 	if ch.ServiceEventIndices == nil {
-		return rlp.NewMarshaler().MustMarshal(chunkBodyV0{
+		return rlp.NewMarshaler().MustMarshal(ChunkBodyV0{
 			CollectionIndex:      ch.CollectionIndex,
 			StartState:           ch.StartState,
 			EventCollection:      ch.EventCollection,
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 8b331ab614d..4e185cd22bd 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -1,4 +1,4 @@
-package flow
+package flow_test
 
 import (
 	"encoding/json"
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/rand"
 	"github.com/onflow/flow-go/utils/unittest"
 )
@@ -16,7 +17,7 @@ import (
 // out of range indices
 func TestChunkList_ByIndex(t *testing.T) {
 	// creates a chunk list with the size of 10
-	var chunkList ChunkList = make([]*Chunk, 10)
+	var chunkList flow.ChunkList = make([]*flow.Chunk, 10)
 
 	// an out of index chunk by index
 	_, ok := chunkList.ByIndex(11)
@@ -37,14 +38,14 @@ func TestDistinctChunkIDs_EmptyChunks(t *testing.T) {
 	require.NotEqual(t, blockIdA, blockIdB)
 
 	// generates a chunk associated with each block id
-	chunkA := &Chunk{
-		ChunkBody: ChunkBody{
+	chunkA := &flow.Chunk{
+		ChunkBody: flow.ChunkBody{
 			BlockID: blockIdA,
 		},
 	}
 
-	chunkB := &Chunk{
-		ChunkBody: ChunkBody{
+	chunkB := &flow.Chunk{
+		ChunkBody: flow.ChunkBody{
 			BlockID: blockIdB,
 		},
 	}
@@ -84,7 +85,7 @@ func TestChunkList_Indices(t *testing.T) {
 	cl := unittest.ChunkListFixture(5, unittest.IdentifierFixture())
 	t.Run("empty chunk subset indices", func(t *testing.T) {
 		// subset of chunk list that is empty should return an empty list
-		subset := ChunkList{}
+		subset := flow.ChunkList{}
 		indices := subset.Indices()
 		require.Len(t, indices, 0)
 	})
@@ -101,7 +102,7 @@ func TestChunkList_Indices(t *testing.T) {
 	t.Run("multiple chunk subset indices", func(t *testing.T) {
 		// subset that only contains even chunk indices, should return
 		// a uint64 slice that only contains even chunk indices
-		subset := ChunkList{cl[0], cl[2], cl[4]}
+		subset := flow.ChunkList{cl[0], cl[2], cl[4]}
 		indices := subset.Indices()
 		require.Len(t, indices, 3)
 		require.Contains(t, indices, uint64(0), uint64(2), uint64(4))
@@ -112,7 +113,7 @@ func TestChunkIndexIsSet(t *testing.T) {
 
 	i, err := rand.Uint()
 	require.NoError(t, err)
-	chunk := NewChunk(
+	chunk := flow.NewChunk(
 		unittest.IdentifierFixture(),
 		int(i),
 		unittest.StateCommitmentFixture(),
@@ -131,7 +132,7 @@ func TestChunkNumberOfTxsIsSet(t *testing.T) {
 
 	i, err := rand.Uint32()
 	require.NoError(t, err)
-	chunk := NewChunk(
+	chunk := flow.NewChunk(
 		unittest.IdentifierFixture(),
 		3,
 		unittest.StateCommitmentFixture(),
@@ -149,7 +150,7 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 
 	i, err := rand.Uint64()
 	require.NoError(t, err)
-	chunk := NewChunk(
+	chunk := flow.NewChunk(
 		unittest.IdentifierFixture(),
 		3,
 		unittest.StateCommitmentFixture(),
@@ -172,7 +173,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunk)
 			require.NoError(t, err)
-			unmarshaled := new(Chunk)
+			unmarshaled := new(flow.Chunk)
 			err = json.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
@@ -181,7 +182,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 		t.Run("cbor", func(t *testing.T) {
 			bz, err := cbor.Marshal(chunk)
 			require.NoError(t, err)
-			unmarshaled := new(Chunk)
+			unmarshaled := new(flow.Chunk)
 			err = cbor.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
@@ -193,7 +194,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunk)
 			require.NoError(t, err)
-			unmarshaled := new(Chunk)
+			unmarshaled := new(flow.Chunk)
 			err = json.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
@@ -202,7 +203,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 		t.Run("cbor", func(t *testing.T) {
 			bz, err := cbor.Marshal(chunk)
 			require.NoError(t, err)
-			unmarshaled := new(Chunk)
+			unmarshaled := new(flow.Chunk)
 			err = cbor.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
@@ -217,14 +218,14 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	chunkFixture.ServiceEventIndices = []uint32{1} // non-nil extra field
 
 	t.Run("writing v0 and reading v1 should yield nil for new field", func(t *testing.T) {
-		var chunkv0 chunkBodyV0
-		unittest.CopyStructure(t, chunkFixture.ChunkBody, chunkv0)
+		var chunkv0 flow.ChunkBodyV0
+		unittest.CopyStructure(t, chunkFixture.ChunkBody, &chunkv0)
 
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunkv0)
 			require.NoError(t, err)
 
-			var unmarshaled ChunkBody
+			var unmarshaled flow.ChunkBody
 			err = json.Unmarshal(bz, &unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
@@ -236,7 +237,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			bz, err := cbor.Marshal(chunkv0)
 			require.NoError(t, err)
 
-			var unmarshaled ChunkBody
+			var unmarshaled flow.ChunkBody
 			err = cbor.Unmarshal(bz, &unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
@@ -252,7 +253,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			bz, err := json.Marshal(chunkv1)
 			require.NoError(t, err)
 
-			var unmarshaled chunkBodyV0
+			var unmarshaled flow.ChunkBodyV0
 			err = json.Unmarshal(bz, &unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunkv1.EventCollection, unmarshaled.EventCollection)
@@ -262,7 +263,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			bz, err := cbor.Marshal(chunkv1)
 			require.NoError(t, err)
 
-			var unmarshaled chunkBodyV0
+			var unmarshaled flow.ChunkBodyV0
 			err = cbor.Unmarshal(bz, &unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunkv1.EventCollection, unmarshaled.EventCollection)
@@ -275,7 +276,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
 	chunkBody := chunk.ChunkBody
-	var chunkv0 chunkBodyV0
+	var chunkv0 flow.ChunkBodyV0
 	unittest.CopyStructure(t, chunkBody, &chunkv0)
 
 	// A nil ServiceEventIndices fields indicates a prior model version.
@@ -283,7 +284,7 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	t.Run("nil ServiceEventIndices fields", func(t *testing.T) {
 		chunkBody.ServiceEventIndices = nil
 		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
-		assert.Equal(t, MakeID(chunkv0), MakeID(chunkBody))
+		assert.Equal(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
 	// A non-nil ServiceEventIndices fields indicates an up-to-date model version.
 	// The ID calculation for the old and new model version should be different,
@@ -291,6 +292,6 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	t.Run("non-nil ServiceEventIndices fields", func(t *testing.T) {
 		chunkBody.ServiceEventIndices = []uint32{}
 		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
-		assert.NotEqual(t, MakeID(chunkv0), MakeID(chunkBody))
+		assert.NotEqual(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
 }
diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go
index 166330b1466..c12bca8fc34 100644
--- a/utils/unittest/fixtures.go
+++ b/utils/unittest/fixtures.go
@@ -19,6 +19,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/onflow/flow-go-sdk"
+
 	hotstuff "github.com/onflow/flow-go/consensus/hotstuff/model"
 	"github.com/onflow/flow-go/engine"
 	"github.com/onflow/flow-go/engine/access/rest/util"
@@ -1328,6 +1329,7 @@ func ChunkFixture(
 			CollectionIndex:      collectionIndex,
 			StartState:           StateCommitmentFixture(),
 			EventCollection:      IdentifierFixture(),
+			ServiceEventIndices:  make([]uint32, 0),
 			TotalComputationUsed: 4200,
 			NumberOfTransactions: 42,
 			BlockID:              blockID,

From d3830684a10106949e542135e97384fe99f1d1f4 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 10:07:27 -0800
Subject: [PATCH 04/31] add test file

---
 engine/execution/block_result_test.go | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 engine/execution/block_result_test.go

diff --git a/engine/execution/block_result_test.go b/engine/execution/block_result_test.go
new file mode 100644
index 00000000000..b0c83466bab
--- /dev/null
+++ b/engine/execution/block_result_test.go
@@ -0,0 +1,3 @@
+package execution
+
+// TODO: add tests for service event indices logic

From 97515e8629dea266c066ebe6c54f37b8ce451580 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 13:36:41 -0800
Subject: [PATCH 05/31] rm indices from fixture

---
 utils/unittest/fixtures.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go
index c12bca8fc34..97b845ee76f 100644
--- a/utils/unittest/fixtures.go
+++ b/utils/unittest/fixtures.go
@@ -1329,7 +1329,6 @@ func ChunkFixture(
 			CollectionIndex:      collectionIndex,
 			StartState:           StateCommitmentFixture(),
 			EventCollection:      IdentifierFixture(),
-			ServiceEventIndices:  make([]uint32, 0),
 			TotalComputationUsed: 4200,
 			NumberOfTransactions: 42,
 			BlockID:              blockID,

From ec7c74e08d89b6ccd7010f67f9a593d4e0726a7c Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 13:48:38 -0800
Subject: [PATCH 06/31] add ServiceEventsByChunk method on result

---
 model/flow/execution_result.go | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/model/flow/execution_result.go b/model/flow/execution_result.go
index a24af0be53c..8a3d4c10d91 100644
--- a/model/flow/execution_result.go
+++ b/model/flow/execution_result.go
@@ -43,7 +43,7 @@ func (er ExecutionResult) Checksum() Identifier {
 	return MakeID(er)
 }
 
-// ValidateChunksLength checks whether the number of chuncks is zero.
+// ValidateChunksLength checks whether the number of chunks is zero.
 //
 // It returns false if the number of chunks is zero (invalid).
 // By protocol definition, each ExecutionReceipt must contain at least one
@@ -76,6 +76,33 @@ func (er ExecutionResult) InitialStateCommit() (StateCommitment, error) {
 	return er.Chunks[0].StartState, nil
 }
 
+// SystemChunk is a system-generated chunk added to every block.
+// It is always the final chunk in an execution result.
+func (er ExecutionResult) SystemChunk() *Chunk {
+	return er.Chunks[len(er.Chunks)-1]
+}
+
+// ServiceEventsByChunk returns the list of service events emitted during the given chunk.
+func (er ExecutionResult) ServiceEventsByChunk(chunkIndex uint64) ServiceEventList {
+	indices := er.Chunks[chunkIndex].ServiceEventIndices
+	// CASE 1: Service event indices are specified (non-nil)
+	if indices != nil {
+		serviceEventsForChunk := make(ServiceEventList, 0, len(indices))
+		for _, eventIndex := range indices {
+			serviceEventsForChunk = append(serviceEventsForChunk, er.ServiceEvents[eventIndex])
+		}
+		return serviceEventsForChunk
+	}
+	// CASE 2: Service event indices are omitted (nil)
+	// This indicates the chunk was generated in an older data model version.
+	// In this case, any service events associated with the result are assumed
+	// to have been emitted within the system chunk (last chunk)
+	if chunkIndex == er.SystemChunk().Index {
+		return er.ServiceEvents
+	}
+	return nil
+}
+
 func (er ExecutionResult) MarshalJSON() ([]byte, error) {
 	type Alias ExecutionResult
 	return json.Marshal(struct {

From e90625190c0eb21295fcec91a16a2eaa3d6ff324 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 14:25:30 -0800
Subject: [PATCH 07/31] fix chunk verifier test

---
 model/flow/execution_result.go      |  1 +
 module/chunks/chunkVerifier.go      | 15 +++++++--------
 module/chunks/chunkVerifier_test.go |  4 ++++
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/model/flow/execution_result.go b/model/flow/execution_result.go
index 8a3d4c10d91..bacb639aa71 100644
--- a/model/flow/execution_result.go
+++ b/model/flow/execution_result.go
@@ -83,6 +83,7 @@ func (er ExecutionResult) SystemChunk() *Chunk {
 }
 
 // ServiceEventsByChunk returns the list of service events emitted during the given chunk.
+// TODO test
 func (er ExecutionResult) ServiceEventsByChunk(chunkIndex uint64) ServiceEventList {
 	indices := er.Chunks[chunkIndex].ServiceEventIndices
 	// CASE 1: Service event indices are specified (non-nil)
diff --git a/module/chunks/chunkVerifier.go b/module/chunks/chunkVerifier.go
index e71bc8b63e8..f110aa8b2b4 100644
--- a/module/chunks/chunkVerifier.go
+++ b/module/chunks/chunkVerifier.go
@@ -291,14 +291,13 @@ func (fcv *ChunkVerifier) verifyTransactionsInContext(
 		return nil, chmodels.NewCFInvalidEventsCollection(chunk.EventCollection, eventsHash, chIndex, execResID, events)
 	}
 
-	if systemChunk {
-		equal, err := result.ServiceEvents.EqualTo(serviceEvents)
-		if err != nil {
-			return nil, fmt.Errorf("error while comparing service events: %w", err)
-		}
-		if !equal {
-			return nil, chmodels.CFInvalidServiceSystemEventsEmitted(result.ServiceEvents, serviceEvents, chIndex, execResID)
-		}
+	serviceEventsInChunk := result.ServiceEventsByChunk(chunk.Index)
+	equal, err := serviceEventsInChunk.EqualTo(serviceEvents)
+	if err != nil {
+		return nil, fmt.Errorf("error while comparing service events: %w", err)
+	}
+	if !equal {
+		return nil, chmodels.CFInvalidServiceSystemEventsEmitted(serviceEventsInChunk, serviceEvents, chIndex, execResID)
 	}
 
 	// Applying chunk updates to the partial trie.	This returns the expected
diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 4ea50cb3fed..6d57ed1c1df 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -245,6 +245,7 @@ func (s *ChunkVerifierTestSuite) TestEventsMismatch() {
 
 // TestServiceEventsMismatch tests verification behavior in case
 // of emitted service events not matching chunks'
+// TODO: fix
 func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
 	meta := s.GetTestSetup(s.T(), "doesn't matter", true)
 	vch := meta.RefreshChunkData(s.T())
@@ -257,6 +258,7 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
 	s.snapshots[string(serviceTxBody.Script)] = &snapshot.ExecutionSnapshot{}
 	s.outputs[string(serviceTxBody.Script)] = fvm.ProcedureOutput{
 		ComputationUsed:        computationUsed,
+		ServiceEvents:          unittest.EventsFixture(1),
 		ConvertedServiceEvents: flow.ServiceEventList{*epochCommitServiceEvent},
 		Events:                 meta.ChunkEvents,
 	}
@@ -267,6 +269,8 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
 	assert.IsType(s.T(), &chunksmodels.CFInvalidServiceEventsEmitted{}, err)
 }
 
+// TODO: add test case for non-system chunk service event
+
 // TestServiceEventsAreChecked ensures that service events are in fact checked
 func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked() {
 	meta := s.GetTestSetup(s.T(), "doesn't matter", true)

From b9849c6e10f49d882cc9f3676b2d6f2c0d7d748c Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 15:57:23 -0800
Subject: [PATCH 08/31] add indices getter tests

---
 engine/execution/block_result.go      |  2 +-
 engine/execution/block_result_test.go | 85 ++++++++++++++++++++++++++-
 utils/slices/slices.go                | 24 ++++++--
 3 files changed, 104 insertions(+), 7 deletions(-)

diff --git a/engine/execution/block_result.go b/engine/execution/block_result.go
index 72f65a22629..3e0ffd0311c 100644
--- a/engine/execution/block_result.go
+++ b/engine/execution/block_result.go
@@ -60,7 +60,7 @@ func (er *BlockExecutionResult) ServiceEventIndicesForChunk(chunkIndex int) []ui
 		firstIndex += len(er.collectionExecutionResults[i].serviceEvents)
 	}
 	indices := make([]uint32, 0, nServiceEventsForChunk)
-	for i := firstIndex; i < len(er.collectionExecutionResults[chunkIndex].serviceEvents); i++ {
+	for i := firstIndex; i < firstIndex+nServiceEventsForChunk; i++ {
 		indices = append(indices, uint32(i))
 	}
 	return indices
diff --git a/engine/execution/block_result_test.go b/engine/execution/block_result_test.go
index b0c83466bab..f2858d79874 100644
--- a/engine/execution/block_result_test.go
+++ b/engine/execution/block_result_test.go
@@ -1,3 +1,86 @@
 package execution
 
-// TODO: add tests for service event indices logic
+import (
+	"math/rand"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/onflow/flow-go/utils/slices"
+	"github.com/onflow/flow-go/utils/unittest"
+)
+
+// makeBlockExecutionResultFixture makes a BlockExecutionResult fixture
+// with the specified allocation of service events to chunks.
+func makeBlockExecutionResultFixture(serviceEventsPerChunk []int) *BlockExecutionResult {
+	fixture := new(BlockExecutionResult)
+	for _, nServiceEvents := range serviceEventsPerChunk {
+		fixture.collectionExecutionResults = append(fixture.collectionExecutionResults,
+			CollectionExecutionResult{
+				serviceEvents:          unittest.EventsFixture(nServiceEvents),
+				convertedServiceEvents: unittest.ServiceEventsFixture(nServiceEvents),
+			})
+	}
+	return fixture
+}
+
+// Tests that ServiceEventIndicesForChunk method works as expected under various circumstances:
+func TestBlockExecutionResult_ServiceEventIndicesForChunk(t *testing.T) {
+	t.Run("no service events", func(t *testing.T) {
+		nChunks := rand.Intn(10) + 1
+		blockResult := makeBlockExecutionResultFixture(make([]int, nChunks))
+		// all chunks should yield an empty list of service event indices
+		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
+			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
+			assert.Equal(t, make([]uint32, 0), indices)
+		}
+	})
+	t.Run("service events only in system chunk", func(t *testing.T) {
+		nChunks := rand.Intn(10) + 2 // at least 2 chunks
+		// add between 1 and 10 service events, all in the system chunk
+		serviceEventAllocation := make([]int, nChunks)
+		nServiceEvents := rand.Intn(10) + 1
+		serviceEventAllocation[nChunks-1] = nServiceEvents
+
+		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
+		// all non-system chunks should yield an empty list of service event indices
+		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
+			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
+			assert.Equal(t, make([]uint32, 0), indices)
+		}
+		// the system chunk should contain indices for all events
+		expected := slices.MakeRange[uint32](0, uint32(nServiceEvents))
+		assert.Equal(t, expected, blockResult.ServiceEventIndicesForChunk(nChunks-1))
+	})
+	t.Run("service events only outside system chunk", func(t *testing.T) {
+		nChunks := rand.Intn(10) + 2 // at least 2 chunks
+		// add 1 service event to all non-system chunks
+		serviceEventAllocation := slices.Fill(1, nChunks)
+		serviceEventAllocation[nChunks-1] = 0
+
+		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
+		// all non-system chunks should yield a length-1 list of service event indices
+		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
+			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
+			// 1 service event per chunk => service event indices match chunk indices
+			expected := slices.Fill(uint32(chunkIndex), 1)
+			assert.Equal(t, expected, indices)
+		}
+		// the system chunk should contain indices for all events
+		assert.Equal(t, make([]uint32, 0), blockResult.ServiceEventIndicesForChunk(nChunks-1))
+	})
+	t.Run("service events in both system chunk and other chunks", func(t *testing.T) {
+		nChunks := rand.Intn(10) + 2 // at least 2 chunks
+		// add 1 service event to all chunks (including system chunk
+		serviceEventAllocation := slices.Fill(1, nChunks)
+
+		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
+		// all  chunks should yield a length-1 list of service event indices
+		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
+			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
+			// 1 service event per chunk => service event indices match chunk indices
+			expected := slices.Fill(uint32(chunkIndex), 1)
+			assert.Equal(t, expected, indices)
+		}
+	})
+}
diff --git a/utils/slices/slices.go b/utils/slices/slices.go
index d2333f2d5aa..a8ac7982467 100644
--- a/utils/slices/slices.go
+++ b/utils/slices/slices.go
@@ -1,6 +1,10 @@
 package slices
 
-import "sort"
+import (
+	"sort"
+
+	"golang.org/x/exp/constraints"
+)
 
 // Concat concatenates multiple []byte into one []byte with efficient one-time allocation.
 func Concat(slices [][]byte) []byte {
@@ -28,15 +32,25 @@ func EnsureByteSliceSize(b []byte, length int) []byte {
 	return stateBytes
 }
 
-// MakeRange returns a slice of int from [min, max]
-func MakeRange(min, max int) []int {
-	a := make([]int, max-min+1)
+// MakeRange returns a slice of numbers [min, max).
+// The range includes min and excludes max.
+func MakeRange[T constraints.Integer](min, max T) []T {
+	a := make([]T, max-min)
 	for i := range a {
-		a[i] = min + i
+		a[i] = min + T(i)
 	}
 	return a
 }
 
+// Fill constructs a slice of type T with length n. The slice is then filled with input "val".
+func Fill[T any](val T, n int) []T {
+	arr := make([]T, n)
+	for i := 0; i < n; i++ {
+		arr[i] = val
+	}
+	return arr
+}
+
 // AreStringSlicesEqual returns true if the two string slices are equal.
 func AreStringSlicesEqual(a, b []string) bool {
 	if len(a) != len(b) {

From 8961e5112213cd5fbd77629f323c2e913b1f3ed5 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 16:34:53 -0800
Subject: [PATCH 09/31] add ServiceEventsByChunk tests

---
 model/flow/execution_result.go      |   1 -
 model/flow/execution_result_test.go | 100 ++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/model/flow/execution_result.go b/model/flow/execution_result.go
index bacb639aa71..8a3d4c10d91 100644
--- a/model/flow/execution_result.go
+++ b/model/flow/execution_result.go
@@ -83,7 +83,6 @@ func (er ExecutionResult) SystemChunk() *Chunk {
 }
 
 // ServiceEventsByChunk returns the list of service events emitted during the given chunk.
-// TODO test
 func (er ExecutionResult) ServiceEventsByChunk(chunkIndex uint64) ServiceEventList {
 	indices := er.Chunks[chunkIndex].ServiceEventIndices
 	// CASE 1: Service event indices are specified (non-nil)
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index b697488b86d..f873da1e596 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -1,11 +1,13 @@
 package flow_test
 
 import (
+	"math/rand"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
 	"github.com/onflow/flow-go/model/flow"
+	"github.com/onflow/flow-go/utils/slices"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
@@ -41,3 +43,101 @@ func TestExecutionResultGroupBy(t *testing.T) {
 	unknown := groups.GetGroup(unittest.IdentifierFixture())
 	assert.Equal(t, 0, unknown.Size())
 }
+
+// Tests that [ExecutionResult.ServiceEventsByChunk] method works in a variety of circumstances.
+// It also tests the method against an ExecutionResult instance backed by both the
+// current and old data model version (with and with ServiceEventIndices field)
+func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
+	t.Run("no service events", func(t *testing.T) {
+		t.Run("nil ServiceEventIndices field (old model)", func(t *testing.T) {
+			result := unittest.ExecutionResultFixture()
+			for _, chunk := range result.Chunks {
+				chunk.ServiceEventIndices = nil
+			}
+			// should return empty list for all chunks
+			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
+				serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+				assert.Len(t, serviceEvents, 0)
+			}
+		})
+		t.Run("populated ServiceEventIndices field", func(t *testing.T) {
+			result := unittest.ExecutionResultFixture()
+			for _, chunk := range result.Chunks {
+				chunk.ServiceEventIndices = make([]uint32, 0)
+			}
+			// should return empty list for all chunks
+			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
+				serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+				assert.Len(t, serviceEvents, 0)
+			}
+		})
+	})
+
+	t.Run("service events only in system chunk", func(t *testing.T) {
+		t.Run("nil ServiceEventIndices field (old model)", func(t *testing.T) {
+			result := unittest.ExecutionResultFixture()
+			for _, chunk := range result.Chunks {
+				chunk.ServiceEventIndices = nil
+			}
+
+			// should return empty list for all chunks
+			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
+				serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+				assert.Len(t, serviceEvents, 0)
+			}
+		})
+		t.Run("populated ServiceEventIndices field", func(t *testing.T) {
+			nServiceEvents := rand.Intn(10) + 1
+			result := unittest.ExecutionResultFixture(unittest.WithServiceEvents(nServiceEvents))
+			for _, chunk := range result.Chunks[:result.Chunks.Len()-1] {
+				chunk.ServiceEventIndices = make([]uint32, 0)
+			}
+			result.SystemChunk().ServiceEventIndices = slices.MakeRange(0, uint32(nServiceEvents))
+
+			// should return empty list for all non-system chunks
+			for chunkIndex := 0; chunkIndex < result.Chunks.Len()-1; chunkIndex++ {
+				serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+				assert.Len(t, serviceEvents, 0)
+			}
+			// should return list of service events for system chunk
+			assert.Equal(t, result.ServiceEvents, result.ServiceEventsByChunk(result.SystemChunk().Index))
+		})
+	})
+
+	// NOTE: service events in non-system chunks in unsupported by the old data model
+	t.Run("service only in non-system chunks", func(t *testing.T) {
+		result := unittest.ExecutionResultFixture()
+		unittest.WithServiceEvents(result.Chunks.Len() - 1)(result) // one service event per non-system chunk
+
+		for chunkIndex, chunk := range result.Chunks {
+			// 1 service event per chunk => service event indices match chunk indices
+			chunk.ServiceEventIndices = []uint32{uint32(chunkIndex)}
+		}
+		result.SystemChunk().ServiceEventIndices = make([]uint32, 0) // none in system chunk
+
+		// should return one service event per non-system chunk
+		for chunkIndex := 0; chunkIndex < result.Chunks.Len()-1; chunkIndex++ {
+			serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+			assert.Equal(t, result.ServiceEvents[chunkIndex:chunkIndex+1], serviceEvents)
+		}
+		// should return empty list for system chunk
+		assert.Len(t, result.ServiceEventsByChunk(result.SystemChunk().Index), 0)
+	})
+
+	// NOTE: service events in non-system chunks in unsupported by the old data model
+	t.Run("service events in all chunks", func(t *testing.T) {
+		result := unittest.ExecutionResultFixture()
+		unittest.WithServiceEvents(result.Chunks.Len())(result) // one service event per chunk
+
+		for chunkIndex, chunk := range result.Chunks {
+			// 1 service event per chunk => service event indices match chunk indices
+			chunk.ServiceEventIndices = []uint32{uint32(chunkIndex)}
+		}
+
+		// should return one service event per chunk
+		for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
+			serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
+			assert.Equal(t, result.ServiceEvents[chunkIndex:chunkIndex+1], serviceEvents)
+		}
+	})
+}

From 5579f2af1b0b481e622f70e9bfaffb9ec4ef13f6 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 19:11:39 -0800
Subject: [PATCH 10/31] chunk verifier tests

---
 module/chunks/chunkVerifier_test.go | 115 ++++++++++++++++++++--------
 1 file changed, 84 insertions(+), 31 deletions(-)

diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 6d57ed1c1df..143029c8afa 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -28,9 +28,11 @@ import (
 	"github.com/onflow/flow-go/module/executiondatasync/execution_data"
 	"github.com/onflow/flow-go/module/executiondatasync/provider"
 	"github.com/onflow/flow-go/module/metrics"
+	"github.com/onflow/flow-go/utils/slices"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
+// eventsList is the set of events emitted by each transaction, by default
 var eventsList = flow.EventsList{
 	{
 		Type:             "event.someType",
@@ -58,7 +60,8 @@ var testChain = flow.Emulator
 var epochSetupEvent, _ = unittest.EpochSetupFixtureByChainID(testChain)
 var epochCommitEvent, _ = unittest.EpochCommitFixtureByChainID(testChain)
 
-var systemEventsList = []flow.Event{
+// serviceEventsList is the list of service events emitted by default.
+var serviceEventsList = []flow.Event{
 	epochSetupEvent,
 }
 
@@ -72,6 +75,10 @@ type ChunkVerifierTestSuite struct {
 	verifier *chunks.ChunkVerifier
 	ledger   *completeLedger.Ledger
 
+	// Below, snapshots and outputs map transaction scripts to execution artifacts
+	// Test cases can inject a script when constructing a chunk, then associate
+	// it with the desired execution artifacts by adding entries to these maps.
+	// If no entry exists, then the default snapshot/output is used.
 	snapshots map[string]*snapshot.ExecutionSnapshot
 	outputs   map[string]fvm.ProcedureOutput
 }
@@ -139,7 +146,7 @@ func TestChunkVerifier(t *testing.T) {
 
 // TestHappyPath tests verification of the baseline verifiable chunk
 func (s *ChunkVerifierTestSuite) TestHappyPath() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	spockSecret, err := s.verifier.Verify(vch)
@@ -151,7 +158,7 @@ func (s *ChunkVerifierTestSuite) TestHappyPath() {
 func (s *ChunkVerifierTestSuite) TestMissingRegisterTouchForUpdate() {
 	unittest.SkipUnless(s.T(), unittest.TEST_DEPRECATED, "Check new partial ledger for missing keys")
 
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// remove the second register touch
@@ -166,7 +173,7 @@ func (s *ChunkVerifierTestSuite) TestMissingRegisterTouchForUpdate() {
 func (s *ChunkVerifierTestSuite) TestMissingRegisterTouchForRead() {
 	unittest.SkipUnless(s.T(), unittest.TEST_DEPRECATED, "Check new partial ledger for missing keys")
 
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// remove the second register touch
@@ -181,7 +188,7 @@ func (s *ChunkVerifierTestSuite) TestMissingRegisterTouchForRead() {
 // the state commitment computed after updating the partial trie
 // doesn't match the one provided by the chunks
 func (s *ChunkVerifierTestSuite) TestWrongEndState() {
-	meta := s.GetTestSetup(s.T(), "wrongEndState", false)
+	meta := s.GetTestSetup(s.T(), "wrongEndState", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// modify calculated end state, which is different from the one provided by the vch
@@ -201,7 +208,7 @@ func (s *ChunkVerifierTestSuite) TestWrongEndState() {
 // of failed transaction. if a transaction fails, it should
 // still change the state commitment.
 func (s *ChunkVerifierTestSuite) TestFailedTx() {
-	meta := s.GetTestSetup(s.T(), "failedTx", false)
+	meta := s.GetTestSetup(s.T(), "failedTx", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// modify the FVM output to include a failing tx. the input already has a failing tx, but we need to
@@ -223,7 +230,7 @@ func (s *ChunkVerifierTestSuite) TestFailedTx() {
 // TestEventsMismatch tests verification behavior in case
 // of emitted events not matching chunks
 func (s *ChunkVerifierTestSuite) TestEventsMismatch() {
-	meta := s.GetTestSetup(s.T(), "eventsMismatch", false)
+	meta := s.GetTestSetup(s.T(), "eventsMismatch", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// add an additional event to the list of events produced by FVM
@@ -245,9 +252,8 @@ func (s *ChunkVerifierTestSuite) TestEventsMismatch() {
 
 // TestServiceEventsMismatch tests verification behavior in case
 // of emitted service events not matching chunks'
-// TODO: fix
 func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
-	meta := s.GetTestSetup(s.T(), "doesn't matter", true)
+	meta := s.GetTestSetup(s.T(), "doesn't matter", true, true)
 	vch := meta.RefreshChunkData(s.T())
 
 	// modify the list of service events produced by FVM
@@ -269,11 +275,9 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
 	assert.IsType(s.T(), &chunksmodels.CFInvalidServiceEventsEmitted{}, err)
 }
 
-// TODO: add test case for non-system chunk service event
-
 // TestServiceEventsAreChecked ensures that service events are in fact checked
 func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked() {
-	meta := s.GetTestSetup(s.T(), "doesn't matter", true)
+	meta := s.GetTestSetup(s.T(), "doesn't matter", true, true)
 	vch := meta.RefreshChunkData(s.T())
 
 	// setup the verifier output to include the correct data for the service events
@@ -286,9 +290,54 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked() {
 	assert.NoError(s.T(), err)
 }
 
+// Tests the case where a service event is emitted outside the system chunk
+// and the event computed by the VN does not match the Result.
+// NOTE: this test case relies on the ordering of transactions in generateCollection.
+func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch_NonSystemChunk() {
+	script := "service event mismatch in non-system chunk"
+	meta := s.GetTestSetup(s.T(), script, false, true)
+	vch := meta.RefreshChunkData(s.T())
+
+	// modify the list of service events produced by FVM
+	// EpochSetup event is expected, but we emit EpochCommit here resulting in a chunk fault
+	epochCommitServiceEvent, err := convert.ServiceEvent(testChain, epochCommitEvent)
+	require.NoError(s.T(), err)
+
+	s.snapshots[script] = &snapshot.ExecutionSnapshot{}
+	s.outputs[script] = fvm.ProcedureOutput{
+		ComputationUsed:        computationUsed,
+		ConvertedServiceEvents: flow.ServiceEventList{*epochCommitServiceEvent},
+		Events:                 meta.ChunkEvents[:3],
+	}
+
+	_, err = s.verifier.Verify(vch)
+
+	assert.Error(s.T(), err)
+	assert.True(s.T(), chunksmodels.IsChunkFaultError(err))
+	assert.IsType(s.T(), &chunksmodels.CFInvalidServiceEventsEmitted{}, err)
+}
+
+// Tests that service events are checked, when they appear outside the system chunk.
+// NOTE: this test case relies on the ordering of transactions in generateCollection.
+func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked_NonSystemChunk() {
+	script := "service event in non-system chunk"
+	meta := s.GetTestSetup(s.T(), script, false, true)
+	vch := meta.RefreshChunkData(s.T())
+
+	// setup the verifier output to include the correct data for the service events
+	output := generateDefaultOutput()
+	output.ConvertedServiceEvents = meta.ServiceEvents
+	output.Events = meta.ChunkEvents[:3] // 2 default events + 1 service event
+	s.outputs[script] = output
+
+	spockSecret, err := s.verifier.Verify(vch)
+	assert.NoError(s.T(), err)
+	assert.NotNil(s.T(), spockSecret)
+}
+
 // TestSystemChunkWithCollectionFails ensures verification fails for system chunks with collections
 func (s *ChunkVerifierTestSuite) TestSystemChunkWithCollectionFails() {
-	meta := s.GetTestSetup(s.T(), "doesn't matter", true)
+	meta := s.GetTestSetup(s.T(), "doesn't matter", true, true)
 
 	// add a collection to the system chunk
 	col := unittest.CollectionFixture(1)
@@ -305,7 +354,7 @@ func (s *ChunkVerifierTestSuite) TestSystemChunkWithCollectionFails() {
 // TestEmptyCollection tests verification behaviour if a
 // collection doesn't have any transaction.
 func (s *ChunkVerifierTestSuite) TestEmptyCollection() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 
 	// reset test to use an empty collection
 	collection := unittest.CollectionFixture(0)
@@ -327,7 +376,7 @@ func (s *ChunkVerifierTestSuite) TestEmptyCollection() {
 }
 
 func (s *ChunkVerifierTestSuite) TestExecutionDataBlockMismatch() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 
 	// modify Block in the ExecutionDataRoot
 	meta.ExecDataBlockID = unittest.IdentifierFixture()
@@ -341,7 +390,7 @@ func (s *ChunkVerifierTestSuite) TestExecutionDataBlockMismatch() {
 }
 
 func (s *ChunkVerifierTestSuite) TestExecutionDataChunkIdsLengthDiffers() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// add an additional ChunkExecutionDataID into the ExecutionDataRoot passed into Verify
@@ -354,7 +403,7 @@ func (s *ChunkVerifierTestSuite) TestExecutionDataChunkIdsLengthDiffers() {
 }
 
 func (s *ChunkVerifierTestSuite) TestExecutionDataChunkIdMismatch() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// modify one of the ChunkExecutionDataIDs passed into Verify
@@ -367,7 +416,7 @@ func (s *ChunkVerifierTestSuite) TestExecutionDataChunkIdMismatch() {
 }
 
 func (s *ChunkVerifierTestSuite) TestExecutionDataIdMismatch() {
-	meta := s.GetTestSetup(s.T(), "", false)
+	meta := s.GetTestSetup(s.T(), "", false, false)
 	vch := meta.RefreshChunkData(s.T())
 
 	// modify ExecutionDataID passed into Verify
@@ -460,13 +509,13 @@ func generateExecutionData(t *testing.T, blockID flow.Identifier, ced *execution
 	return executionDataID, executionDataRoot
 }
 
-func generateEvents(t *testing.T, isSystemChunk bool, collection *flow.Collection) (flow.EventsList, []flow.ServiceEvent) {
+func generateEvents(t *testing.T, includeServiceEvent bool, collection *flow.Collection) (flow.EventsList, []flow.ServiceEvent) {
 	var chunkEvents flow.EventsList
 	serviceEvents := make([]flow.ServiceEvent, 0)
 
 	// service events are also included as regular events
-	if isSystemChunk {
-		for _, e := range systemEventsList {
+	if includeServiceEvent {
+		for _, e := range serviceEventsList {
 			e := e
 			event, err := convert.ServiceEvent(testChain, e)
 			require.NoError(t, err)
@@ -504,6 +553,12 @@ func generateTransactionResults(t *testing.T, collection *flow.Collection) []flo
 	return txResults
 }
 
+// generateCollection generates a collection fixture that is predictable based on inputs.
+// Test cases in this file rely on the predictable pattern of collections generated here.
+// If this is a system chunk, we return a collection containing only the service transaction.
+// Otherwise, we return a collection with 5 transactions. Only the first of these 5 uses the input script.
+// The transaction script is the lookup key for determining the result of transaction execution,
+// so test cases can inject a desired transaction output associated with the input script.
 func generateCollection(t *testing.T, isSystemChunk bool, script string) *flow.Collection {
 	if isSystemChunk {
 		// the system chunk's data pack does not include the collection, but the execution data does.
@@ -514,11 +569,12 @@ func generateCollection(t *testing.T, isSystemChunk bool, script string) *flow.C
 	}
 
 	collectionSize := 5
-	magicTxIndex := 3
+	// add the user-specified transaction first
+	userSpecifiedTxIndex := 0
 
 	coll := unittest.CollectionFixture(collectionSize)
 	if script != "" {
-		coll.Transactions[magicTxIndex] = &flow.TransactionBody{Script: []byte(script)}
+		coll.Transactions[userSpecifiedTxIndex] = &flow.TransactionBody{Script: []byte(script)}
 	}
 
 	return &coll
@@ -544,7 +600,7 @@ func generateDefaultOutput() fvm.ProcedureOutput {
 	}
 }
 
-func (s *ChunkVerifierTestSuite) GetTestSetup(t *testing.T, script string, system bool) *testMetadata {
+func (s *ChunkVerifierTestSuite) GetTestSetup(t *testing.T, script string, system bool, includeServiceEvents bool) *testMetadata {
 	collection := generateCollection(t, system, script)
 	block := blockFixture(collection)
 
@@ -558,14 +614,9 @@ func (s *ChunkVerifierTestSuite) GetTestSetup(t *testing.T, script string, syste
 	}
 
 	// events
-	chunkEvents, serviceEvents := generateEvents(t, system, collection)
+	chunkEvents, serviceEvents := generateEvents(t, includeServiceEvents, collection)
 	// make sure this includes events even for the service tx
 	require.NotEmpty(t, chunkEvents)
-	if system {
-		require.Len(t, serviceEvents, 1)
-	} else {
-		require.Empty(t, serviceEvents)
-	}
 
 	// registerTouch and State setup
 	startState, proof, update := generateStateUpdates(t, s.ledger)
@@ -642,7 +693,9 @@ func (m *testMetadata) RefreshChunkData(t *testing.T) *verification.VerifiableCh
 			CollectionIndex: 0,
 			StartState:      flow.StateCommitment(m.StartState),
 			BlockID:         m.Header.ID(),
-			EventCollection: eventsMerkleRootHash,
+			// in these test cases, all defined service events correspond to the current chunk
+			ServiceEventIndices: slices.MakeRange(0, uint32(len(m.ServiceEvents))),
+			EventCollection:     eventsMerkleRootHash,
 		},
 		Index: 0,
 	}

From f1125b375ab4d9da1549c8ab4eb7c755370abc9b Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Wed, 20 Nov 2024 19:19:19 -0800
Subject: [PATCH 11/31] improve docs

---
 engine/execution/block_result.go |  2 ++
 model/flow/chunk.go              | 17 ++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/engine/execution/block_result.go b/engine/execution/block_result.go
index 3e0ffd0311c..1c7bcda98b6 100644
--- a/engine/execution/block_result.go
+++ b/engine/execution/block_result.go
@@ -49,6 +49,8 @@ func (er *BlockExecutionResult) AllEvents() flow.EventsList {
 	return res
 }
 
+// ServiceEventIndicesForChunk returns the list of service event indices associated with the given chunk.
+// Outputs are index ranges with no gaps, and index into the flow.ExecutionResult.ServiceEvents field.
 func (er *BlockExecutionResult) ServiceEventIndicesForChunk(chunkIndex int) []uint32 {
 	nServiceEventsForChunk := len(er.collectionExecutionResults[chunkIndex].serviceEvents)
 	if nServiceEventsForChunk == 0 {
diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 990c9dfdb52..7d4ee9dd981 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -22,7 +22,8 @@ func init() {
 	}
 }
 
-// TODO doc
+// ChunkBodyV0 is the prior version of ChunkBody, used for backward compatibility and tests.
+// Compared to ChunkBody, ChunkBodyV0 does not have the ServiceEventIndices field.
 type ChunkBodyV0 struct {
 	CollectionIndex      uint
 	StartState           StateCommitment
@@ -38,10 +39,16 @@ type ChunkBody struct {
 	// execution info
 	StartState      StateCommitment // start state when starting executing this chunk
 	EventCollection Identifier      // Events generated by executing results
-	// ServiceEventIndices is a list of indices of service events which were emitted.
-	// If ServiceEventIndices is nil, this indicates that this chunk was created by an older software version
-	// which did support specifying a mapping between chunks and service events.
-	// TODO doc
+	// ServiceEventIndices is a list of indices defining which service events were emitted in this chunk.
+	// These indices index into the ExecutionResult.ServiceEvents field.
+	//
+	// BACKWARD COMPATIBILITY:
+	// (1) If ServiceEventIndices is nil, this indicates that this chunk was created by an older software version
+	//     which did support specifying a mapping between chunks and service events.
+	//     In this case, all service events are assumed to have been emitted in the system chunk (last chunk).
+	// (2) Otherwise, ServiceEventIndices must be non-nil. A chunk in which no service events were emitted
+	//     is denoted with an empty list: []uint32{}.
+	// Within an ExecutionResult, all chunks must use either representation (1) or (2), not both.
 	ServiceEventIndices []uint32
 	BlockID             Identifier // Block id of the execution result this chunk belongs to
 

From 718a5eee0957aa513bac134f095e1426b5a2cb35 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Thu, 21 Nov 2024 09:25:21 -0800
Subject: [PATCH 12/31] improve docs

---
 model/flow/chunk_test.go   | 13 ++++++++-----
 utils/unittest/encoding.go |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 4e185cd22bd..820114bb093 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -164,7 +164,10 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 	assert.Equal(t, i, chunk.TotalComputationUsed)
 }
 
-// TODO doc
+// TestChunkEncodeDecode test encoding and decoding properties.
+// In particular, we want to demonstrate that nil-ness of the ServiceEventIndices field
+// is preserved by the encoding schemes we use, because this difference is meaningful and
+// important for backward compatibility (see [ChunkBody.ServiceEventIndices] for details).
 func TestChunkEncodeDecode(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 0)
 
@@ -212,7 +215,8 @@ func TestChunkEncodeDecode(t *testing.T) {
 	})
 }
 
-// TODO doc
+// TestChunk_ModelVersions_EncodeDecode tests that encoding and decoding between
+// supported versions works as expected.
 func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	chunkFixture := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
 	chunkFixture.ServiceEventIndices = []uint32{1} // non-nil extra field
@@ -272,7 +276,8 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	})
 }
 
-// TODO doc
+// TestChunk_ModelVersions_IDConsistentAcrossVersions ensures that the ID function
+// is backward compatible with old data model versions.
 func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
 	chunkBody := chunk.ChunkBody
@@ -283,7 +288,6 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	// The ID calculation for the old and new model version should be the same.
 	t.Run("nil ServiceEventIndices fields", func(t *testing.T) {
 		chunkBody.ServiceEventIndices = nil
-		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
 		assert.Equal(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
 	// A non-nil ServiceEventIndices fields indicates an up-to-date model version.
@@ -291,7 +295,6 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	// because the new model should include the ServiceEventIndices field value.
 	t.Run("non-nil ServiceEventIndices fields", func(t *testing.T) {
 		chunkBody.ServiceEventIndices = []uint32{}
-		assert.Equal(t, chunkBody.BlockID, chunkv0.BlockID)
 		assert.NotEqual(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
 }
diff --git a/utils/unittest/encoding.go b/utils/unittest/encoding.go
index b2fb5efb9d9..bd1533b58fc 100644
--- a/utils/unittest/encoding.go
+++ b/utils/unittest/encoding.go
@@ -7,7 +7,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TODO doc
+// CopyStructure copies public fields of compatible types from src to dst.
 func CopyStructure(t *testing.T, src, dst any) {
 	bz, err := cbor.Marshal(src)
 	require.NoError(t, err)

From 44f3c1a33db2de05ce3f4bbb91ad03aff18d7bd8 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Thu, 28 Nov 2024 08:55:47 -0800
Subject: [PATCH 13/31] adjust field and documentation

---
 model/flow/chunk.go | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 7d4ee9dd981..97046189335 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -39,18 +39,26 @@ type ChunkBody struct {
 	// execution info
 	StartState      StateCommitment // start state when starting executing this chunk
 	EventCollection Identifier      // Events generated by executing results
-	// ServiceEventIndices is a list of indices defining which service events were emitted in this chunk.
-	// These indices index into the ExecutionResult.ServiceEvents field.
+	// ServiceEventCount defines how many service events were emitted in this chunk.
+	// By reading this field in prior chunks in the same ExecutionResult, we can
+	// compute exactly what service events were emitted in this chunk.
+	//
+	// Let C be this chunk, Cr be the set of chunks in the ExecutionResult containing C.
+	// Then the service event indices for C are given by:
+	//    StartIndex = ∑Ci.ServiceEventCount : Ci ∈ Cr, Ci.Index < C.Index
+	//    EndIndex = StartIndex + C.ServiceEventCount
+	// The service events for C are given by:
+	//    ExecutionResult.ServiceEvents[StartIndex:EndIndex]
 	//
 	// BACKWARD COMPATIBILITY:
-	// (1) If ServiceEventIndices is nil, this indicates that this chunk was created by an older software version
+	// (1) If ServiceEventCount is nil, this indicates that this chunk was created by an older software version
 	//     which did support specifying a mapping between chunks and service events.
 	//     In this case, all service events are assumed to have been emitted in the system chunk (last chunk).
-	// (2) Otherwise, ServiceEventIndices must be non-nil. A chunk in which no service events were emitted
-	//     is denoted with an empty list: []uint32{}.
+	//     This was the implicit behaviour prior to the introduction of this field.
+	// (2) Otherwise, ServiceEventCount must be non-nil.
 	// Within an ExecutionResult, all chunks must use either representation (1) or (2), not both.
-	ServiceEventIndices []uint32
-	BlockID             Identifier // Block id of the execution result this chunk belongs to
+	ServiceEventCount *uint16
+	BlockID           Identifier // Block id of the execution result this chunk belongs to
 
 	// Computation consumption info
 	TotalComputationUsed uint64 // total amount of computation used by running all txs in this chunk
@@ -64,7 +72,7 @@ type ChunkBody struct {
 //   - A nil ServiceEventIndices field indicates a v0 version of ChunkBody
 //   - when computing the ID of such a ChunkBody, the ServiceEventIndices field is omitted from the fingerprint
 func (ch ChunkBody) Fingerprint() []byte {
-	if ch.ServiceEventIndices == nil {
+	if ch.ServiceEventCount == nil {
 		return rlp.NewMarshaler().MustMarshal(ChunkBodyV0{
 			CollectionIndex:      ch.CollectionIndex,
 			StartState:           ch.StartState,
@@ -91,13 +99,10 @@ func NewChunk(
 	startState StateCommitment,
 	numberOfTransactions int,
 	eventCollection Identifier,
-	serviceEventIndices []uint32,
+	serviceEventCount uint16,
 	endState StateCommitment,
 	totalComputationUsed uint64,
 ) *Chunk {
-	if serviceEventIndices == nil {
-		serviceEventIndices = []uint32{}
-	}
 	return &Chunk{
 		ChunkBody: ChunkBody{
 			BlockID:              blockID,
@@ -105,7 +110,7 @@ func NewChunk(
 			StartState:           startState,
 			NumberOfTransactions: uint64(numberOfTransactions),
 			EventCollection:      eventCollection,
-			ServiceEventIndices:  serviceEventIndices,
+			ServiceEventCount:    &serviceEventCount,
 			TotalComputationUsed: totalComputationUsed,
 		},
 		Index:    uint64(collectionIndex),

From 13aba217899a12f2f6b60266686973b42d851816 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Thu, 28 Nov 2024 09:08:29 -0800
Subject: [PATCH 14/31] update tests

---
 model/flow/chunk.go        | 14 +++++------
 model/flow/chunk_test.go   | 48 +++++++++++++++++++-------------------
 utils/unittest/encoding.go |  6 +++++
 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 97046189335..2287c8f4650 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -22,8 +22,8 @@ func init() {
 	}
 }
 
-// ChunkBodyV0 is the prior version of ChunkBody, used for backward compatibility and tests.
-// Compared to ChunkBody, ChunkBodyV0 does not have the ServiceEventIndices field.
+// ChunkBodyV0 is the prior version of ChunkBody, used for computing backward-compatible IDs and tests.
+// Compared to ChunkBody, ChunkBodyV0 does not have the ServiceEventCount field.
 type ChunkBodyV0 struct {
 	CollectionIndex      uint
 	StartState           StateCommitment
@@ -43,9 +43,9 @@ type ChunkBody struct {
 	// By reading this field in prior chunks in the same ExecutionResult, we can
 	// compute exactly what service events were emitted in this chunk.
 	//
-	// Let C be this chunk, Cr be the set of chunks in the ExecutionResult containing C.
+	// Let C be this chunk, K be the set of chunks in the ExecutionResult containing C.
 	// Then the service event indices for C are given by:
-	//    StartIndex = ∑Ci.ServiceEventCount : Ci ∈ Cr, Ci.Index < C.Index
+	//    StartIndex = ∑Ci.ServiceEventCount : Ci ∈ K, Ci.Index < C.Index
 	//    EndIndex = StartIndex + C.ServiceEventCount
 	// The service events for C are given by:
 	//    ExecutionResult.ServiceEvents[StartIndex:EndIndex]
@@ -68,9 +68,9 @@ type ChunkBody struct {
 // Fingerprint returns the unique binary representation for the receiver ChunkBody,
 // used to compute the ID (hash).
 // The fingerprint is backward-compatible with the prior data model for ChunkBody: ChunkBodyV0.
-//   - All new ChunkBody instances must have non-nil ServiceEventIndices
-//   - A nil ServiceEventIndices field indicates a v0 version of ChunkBody
-//   - when computing the ID of such a ChunkBody, the ServiceEventIndices field is omitted from the fingerprint
+//   - All new ChunkBody instances must have non-nil ServiceEventCount field
+//   - A nil ServiceEventCount field indicates a v0 version of ChunkBody
+//   - when computing the ID of such a ChunkBody, the ServiceEventCount field is omitted from the fingerprint
 func (ch ChunkBody) Fingerprint() []byte {
 	if ch.ServiceEventCount == nil {
 		return rlp.NewMarshaler().MustMarshal(ChunkBodyV0{
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 820114bb093..0f8cc8c30e6 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -119,7 +119,7 @@ func TestChunkIndexIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		21,
 		unittest.IdentifierFixture(),
-		[]uint32{},
+		0,
 		unittest.StateCommitmentFixture(),
 		17995,
 	)
@@ -138,7 +138,7 @@ func TestChunkNumberOfTxsIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		int(i),
 		unittest.IdentifierFixture(),
-		[]uint32{},
+		0,
 		unittest.StateCommitmentFixture(),
 		17995,
 	)
@@ -156,7 +156,7 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 		unittest.StateCommitmentFixture(),
 		21,
 		unittest.IdentifierFixture(),
-		[]uint32{},
+		0,
 		unittest.StateCommitmentFixture(),
 		i,
 	)
@@ -165,14 +165,14 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 }
 
 // TestChunkEncodeDecode test encoding and decoding properties.
-// In particular, we want to demonstrate that nil-ness of the ServiceEventIndices field
+// In particular, we want to demonstrate that nil-ness of the ServiceEventCount field
 // is preserved by the encoding schemes we use, because this difference is meaningful and
-// important for backward compatibility (see [ChunkBody.ServiceEventIndices] for details).
+// important for backward compatibility (see [ChunkBody.ServiceEventCount] for details).
 func TestChunkEncodeDecode(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 0)
 
-	t.Run("encode/decode preserves nil ServiceEventIndices", func(t *testing.T) {
-		chunk.ServiceEventIndices = nil
+	t.Run("encode/decode preserves nil ServiceEventCount", func(t *testing.T) {
+		chunk.ServiceEventCount = nil
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunk)
 			require.NoError(t, err)
@@ -180,7 +180,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 			err = json.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
-			assert.Nil(t, unmarshaled.ServiceEventIndices)
+			assert.Nil(t, unmarshaled.ServiceEventCount)
 		})
 		t.Run("cbor", func(t *testing.T) {
 			bz, err := cbor.Marshal(chunk)
@@ -189,11 +189,11 @@ func TestChunkEncodeDecode(t *testing.T) {
 			err = cbor.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
-			assert.Nil(t, unmarshaled.ServiceEventIndices)
+			assert.Nil(t, unmarshaled.ServiceEventCount)
 		})
 	})
-	t.Run("encode/decode preserves empty but non-nil ServiceEventIndices", func(t *testing.T) {
-		chunk.ServiceEventIndices = []uint32{}
+	t.Run("encode/decode preserves empty but non-nil ServiceEventCount", func(t *testing.T) {
+		chunk.ServiceEventCount = unittest.PtrTo[uint16](0)
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunk)
 			require.NoError(t, err)
@@ -201,7 +201,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 			err = json.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
-			assert.NotNil(t, unmarshaled.ServiceEventIndices)
+			assert.NotNil(t, unmarshaled.ServiceEventCount)
 		})
 		t.Run("cbor", func(t *testing.T) {
 			bz, err := cbor.Marshal(chunk)
@@ -210,7 +210,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 			err = cbor.Unmarshal(bz, unmarshaled)
 			require.NoError(t, err)
 			assert.Equal(t, chunk, unmarshaled)
-			assert.NotNil(t, unmarshaled.ServiceEventIndices)
+			assert.NotNil(t, unmarshaled.ServiceEventCount)
 		})
 	})
 }
@@ -219,7 +219,7 @@ func TestChunkEncodeDecode(t *testing.T) {
 // supported versions works as expected.
 func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	chunkFixture := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
-	chunkFixture.ServiceEventIndices = []uint32{1} // non-nil extra field
+	chunkFixture.ServiceEventCount = unittest.PtrTo[uint16](0) // non-nil extra field
 
 	t.Run("writing v0 and reading v1 should yield nil for new field", func(t *testing.T) {
 		var chunkv0 flow.ChunkBodyV0
@@ -234,7 +234,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
 			assert.Equal(t, chunkv0.BlockID, unmarshaled.BlockID)
-			assert.Nil(t, unmarshaled.ServiceEventIndices)
+			assert.Nil(t, unmarshaled.ServiceEventCount)
 		})
 
 		t.Run("cbor", func(t *testing.T) {
@@ -246,12 +246,12 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, chunkv0.EventCollection, unmarshaled.EventCollection)
 			assert.Equal(t, chunkv0.BlockID, unmarshaled.BlockID)
-			assert.Nil(t, unmarshaled.ServiceEventIndices)
+			assert.Nil(t, unmarshaled.ServiceEventCount)
 		})
 	})
 	t.Run("writing v1 and reading v0 does not error", func(t *testing.T) {
 		chunkv1 := chunkFixture.ChunkBody
-		chunkv1.ServiceEventIndices = []uint32{0} // ensure non-nil ServiceEventIndices field
+		chunkv1.ServiceEventCount = unittest.PtrTo[uint16](0) // ensure non-nil ServiceEventCount field
 
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunkv1)
@@ -284,17 +284,17 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	var chunkv0 flow.ChunkBodyV0
 	unittest.CopyStructure(t, chunkBody, &chunkv0)
 
-	// A nil ServiceEventIndices fields indicates a prior model version.
+	// A nil ServiceEventCount fields indicates a prior model version.
 	// The ID calculation for the old and new model version should be the same.
-	t.Run("nil ServiceEventIndices fields", func(t *testing.T) {
-		chunkBody.ServiceEventIndices = nil
+	t.Run("nil ServiceEventCount fields", func(t *testing.T) {
+		chunkBody.ServiceEventCount = nil
 		assert.Equal(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
-	// A non-nil ServiceEventIndices fields indicates an up-to-date model version.
+	// A non-nil ServiceEventCount fields indicates an up-to-date model version.
 	// The ID calculation for the old and new model version should be different,
-	// because the new model should include the ServiceEventIndices field value.
-	t.Run("non-nil ServiceEventIndices fields", func(t *testing.T) {
-		chunkBody.ServiceEventIndices = []uint32{}
+	// because the new model should include the ServiceEventCount field value.
+	t.Run("non-nil ServiceEventCount fields", func(t *testing.T) {
+		chunkBody.ServiceEventCount = unittest.PtrTo[uint16](0)
 		assert.NotEqual(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
 	})
 }
diff --git a/utils/unittest/encoding.go b/utils/unittest/encoding.go
index bd1533b58fc..c2fd26b5ddb 100644
--- a/utils/unittest/encoding.go
+++ b/utils/unittest/encoding.go
@@ -14,3 +14,9 @@ func CopyStructure(t *testing.T, src, dst any) {
 	err = cbor.Unmarshal(bz, dst)
 	require.NoError(t, err)
 }
+
+// PtrTo returns a pointer to the input. Useful for concisely constructing
+// a reference-typed argument to a function or similar.
+func PtrTo[T any](target T) *T {
+	return &target
+}

From defcdea0c1cb90293080d119cf03ce5daf2febf2 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Fri, 29 Nov 2024 13:44:15 -0800
Subject: [PATCH 15/31] wip

---
 model/flow/execution_result.go      | 22 ++++++++++-------
 model/flow/execution_result_test.go | 37 +++++++++++++++--------------
 2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/model/flow/execution_result.go b/model/flow/execution_result.go
index 8a3d4c10d91..c0669533bab 100644
--- a/model/flow/execution_result.go
+++ b/model/flow/execution_result.go
@@ -84,18 +84,22 @@ func (er ExecutionResult) SystemChunk() *Chunk {
 
 // ServiceEventsByChunk returns the list of service events emitted during the given chunk.
 func (er ExecutionResult) ServiceEventsByChunk(chunkIndex uint64) ServiceEventList {
-	indices := er.Chunks[chunkIndex].ServiceEventIndices
-	// CASE 1: Service event indices are specified (non-nil)
-	if indices != nil {
-		serviceEventsForChunk := make(ServiceEventList, 0, len(indices))
-		for _, eventIndex := range indices {
-			serviceEventsForChunk = append(serviceEventsForChunk, er.ServiceEvents[eventIndex])
+	serviceEventCount := er.Chunks[chunkIndex].ServiceEventCount
+	// CASE 1: Service event count is specified (non-nil)
+	if serviceEventCount != nil {
+		if *serviceEventCount == 0 {
+			return nil
 		}
-		return serviceEventsForChunk
+
+		startIndex := 0
+		for i := uint64(0); i < chunkIndex; i++ {
+			startIndex += int(*er.Chunks[i].ServiceEventCount)
+		}
+		return er.ServiceEvents[startIndex : startIndex+int(*serviceEventCount)]
 	}
-	// CASE 2: Service event indices are omitted (nil)
+	// CASE 2: Service event count omitted (nil)
 	// This indicates the chunk was generated in an older data model version.
-	// In this case, any service events associated with the result are assumed
+	// In this case, all service events associated with the result are assumed
 	// to have been emitted within the system chunk (last chunk)
 	if chunkIndex == er.SystemChunk().Index {
 		return er.ServiceEvents
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index f873da1e596..5cd12cf1b48 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -7,7 +7,6 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/onflow/flow-go/model/flow"
-	"github.com/onflow/flow-go/utils/slices"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
@@ -44,15 +43,17 @@ func TestExecutionResultGroupBy(t *testing.T) {
 	assert.Equal(t, 0, unknown.Size())
 }
 
+func TestExecutionResult_FingerprintBackwardCompatibility(t *testing.T) {}
+
 // Tests that [ExecutionResult.ServiceEventsByChunk] method works in a variety of circumstances.
 // It also tests the method against an ExecutionResult instance backed by both the
-// current and old data model version (with and with ServiceEventIndices field)
+// current and old data model version (with and with ServiceEventCount field)
 func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 	t.Run("no service events", func(t *testing.T) {
-		t.Run("nil ServiceEventIndices field (old model)", func(t *testing.T) {
+		t.Run("nil ServiceEventCount field (old model)", func(t *testing.T) {
 			result := unittest.ExecutionResultFixture()
 			for _, chunk := range result.Chunks {
-				chunk.ServiceEventIndices = nil
+				chunk.ServiceEventCount = nil
 			}
 			// should return empty list for all chunks
 			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
@@ -60,10 +61,10 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 				assert.Len(t, serviceEvents, 0)
 			}
 		})
-		t.Run("populated ServiceEventIndices field", func(t *testing.T) {
+		t.Run("populated ServiceEventCount field", func(t *testing.T) {
 			result := unittest.ExecutionResultFixture()
 			for _, chunk := range result.Chunks {
-				chunk.ServiceEventIndices = make([]uint32, 0)
+				chunk.ServiceEventCount = unittest.PtrTo[uint16](0)
 			}
 			// should return empty list for all chunks
 			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
@@ -74,10 +75,10 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 	})
 
 	t.Run("service events only in system chunk", func(t *testing.T) {
-		t.Run("nil ServiceEventIndices field (old model)", func(t *testing.T) {
+		t.Run("nil ServiceEventCount field (old model)", func(t *testing.T) {
 			result := unittest.ExecutionResultFixture()
 			for _, chunk := range result.Chunks {
-				chunk.ServiceEventIndices = nil
+				chunk.ServiceEventCount = nil
 			}
 
 			// should return empty list for all chunks
@@ -86,13 +87,13 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 				assert.Len(t, serviceEvents, 0)
 			}
 		})
-		t.Run("populated ServiceEventIndices field", func(t *testing.T) {
+		t.Run("populated ServiceEventCount field", func(t *testing.T) {
 			nServiceEvents := rand.Intn(10) + 1
 			result := unittest.ExecutionResultFixture(unittest.WithServiceEvents(nServiceEvents))
 			for _, chunk := range result.Chunks[:result.Chunks.Len()-1] {
-				chunk.ServiceEventIndices = make([]uint32, 0)
+				chunk.ServiceEventCount = unittest.PtrTo[uint16](0)
 			}
-			result.SystemChunk().ServiceEventIndices = slices.MakeRange(0, uint32(nServiceEvents))
+			result.SystemChunk().ServiceEventCount = unittest.PtrTo(uint16(nServiceEvents))
 
 			// should return empty list for all non-system chunks
 			for chunkIndex := 0; chunkIndex < result.Chunks.Len()-1; chunkIndex++ {
@@ -109,11 +110,11 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 		result := unittest.ExecutionResultFixture()
 		unittest.WithServiceEvents(result.Chunks.Len() - 1)(result) // one service event per non-system chunk
 
-		for chunkIndex, chunk := range result.Chunks {
-			// 1 service event per chunk => service event indices match chunk indices
-			chunk.ServiceEventIndices = []uint32{uint32(chunkIndex)}
+		for _, chunk := range result.Chunks {
+			// 1 service event per chunk
+			chunk.ServiceEventCount = unittest.PtrTo(uint16(1))
 		}
-		result.SystemChunk().ServiceEventIndices = make([]uint32, 0) // none in system chunk
+		result.SystemChunk().ServiceEventCount = unittest.PtrTo(uint16(0))
 
 		// should return one service event per non-system chunk
 		for chunkIndex := 0; chunkIndex < result.Chunks.Len()-1; chunkIndex++ {
@@ -129,9 +130,9 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 		result := unittest.ExecutionResultFixture()
 		unittest.WithServiceEvents(result.Chunks.Len())(result) // one service event per chunk
 
-		for chunkIndex, chunk := range result.Chunks {
-			// 1 service event per chunk => service event indices match chunk indices
-			chunk.ServiceEventIndices = []uint32{uint32(chunkIndex)}
+		for _, chunk := range result.Chunks {
+			// 1 service event per chunk
+			chunk.ServiceEventCount = unittest.PtrTo(uint16(1))
 		}
 
 		// should return one service event per chunk

From 405626e642a64f9fa9f8121b6e975580f183ee55 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 2 Dec 2024 09:31:14 -0800
Subject: [PATCH 16/31] update BlockResult tests

---
 engine/execution/block_result.go      | 23 ++++-----------
 engine/execution/block_result_test.go | 41 ++++++++++++---------------
 model/flow/execution_result_test.go   |  1 +
 3 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/engine/execution/block_result.go b/engine/execution/block_result.go
index 1c7bcda98b6..1e62b22d038 100644
--- a/engine/execution/block_result.go
+++ b/engine/execution/block_result.go
@@ -49,23 +49,10 @@ func (er *BlockExecutionResult) AllEvents() flow.EventsList {
 	return res
 }
 
-// ServiceEventIndicesForChunk returns the list of service event indices associated with the given chunk.
-// Outputs are index ranges with no gaps, and index into the flow.ExecutionResult.ServiceEvents field.
-func (er *BlockExecutionResult) ServiceEventIndicesForChunk(chunkIndex int) []uint32 {
-	nServiceEventsForChunk := len(er.collectionExecutionResults[chunkIndex].serviceEvents)
-	if nServiceEventsForChunk == 0 {
-		return []uint32{}
-	}
-
-	firstIndex := 0
-	for i := 0; i < chunkIndex; i++ {
-		firstIndex += len(er.collectionExecutionResults[i].serviceEvents)
-	}
-	indices := make([]uint32, 0, nServiceEventsForChunk)
-	for i := firstIndex; i < firstIndex+nServiceEventsForChunk; i++ {
-		indices = append(indices, uint32(i))
-	}
-	return indices
+// ServiceEventCountForChunk returns the number of service events emitted in the given chunk.
+func (er *BlockExecutionResult) ServiceEventCountForChunk(chunkIndex int) uint16 {
+	serviceEventCount := len(er.collectionExecutionResults[chunkIndex].serviceEvents)
+	return uint16(serviceEventCount)
 }
 
 func (er *BlockExecutionResult) AllServiceEvents() flow.EventsList {
@@ -218,7 +205,7 @@ func (ar *BlockAttestationResult) ChunkAt(index int) *flow.Chunk {
 		attestRes.startStateCommit,
 		len(execRes.TransactionResults()),
 		attestRes.eventCommit,
-		ar.ServiceEventIndicesForChunk(index),
+		ar.ServiceEventCountForChunk(index),
 		attestRes.endStateCommit,
 		execRes.executionSnapshot.TotalComputationUsed(),
 	)
diff --git a/engine/execution/block_result_test.go b/engine/execution/block_result_test.go
index f2858d79874..2c1e0bd0b98 100644
--- a/engine/execution/block_result_test.go
+++ b/engine/execution/block_result_test.go
@@ -24,15 +24,15 @@ func makeBlockExecutionResultFixture(serviceEventsPerChunk []int) *BlockExecutio
 	return fixture
 }
 
-// Tests that ServiceEventIndicesForChunk method works as expected under various circumstances:
+// Tests that ServiceEventCountForChunk method works as expected under various circumstances:
 func TestBlockExecutionResult_ServiceEventIndicesForChunk(t *testing.T) {
 	t.Run("no service events", func(t *testing.T) {
 		nChunks := rand.Intn(10) + 1
 		blockResult := makeBlockExecutionResultFixture(make([]int, nChunks))
-		// all chunks should yield an empty list of service event indices
+		// all chunks should have 0 service event count
 		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
-			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
-			assert.Equal(t, make([]uint32, 0), indices)
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(0), count)
 		}
 	})
 	t.Run("service events only in system chunk", func(t *testing.T) {
@@ -43,14 +43,13 @@ func TestBlockExecutionResult_ServiceEventIndicesForChunk(t *testing.T) {
 		serviceEventAllocation[nChunks-1] = nServiceEvents
 
 		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
-		// all non-system chunks should yield an empty list of service event indices
+		// all non-system chunks should have zero service event count
 		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
-			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
-			assert.Equal(t, make([]uint32, 0), indices)
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(0), count)
 		}
-		// the system chunk should contain indices for all events
-		expected := slices.MakeRange[uint32](0, uint32(nServiceEvents))
-		assert.Equal(t, expected, blockResult.ServiceEventIndicesForChunk(nChunks-1))
+		// the system chunk should contain all service events
+		assert.Equal(t, uint16(nServiceEvents), blockResult.ServiceEventCountForChunk(nChunks-1))
 	})
 	t.Run("service events only outside system chunk", func(t *testing.T) {
 		nChunks := rand.Intn(10) + 2 // at least 2 chunks
@@ -59,28 +58,24 @@ func TestBlockExecutionResult_ServiceEventIndicesForChunk(t *testing.T) {
 		serviceEventAllocation[nChunks-1] = 0
 
 		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
-		// all non-system chunks should yield a length-1 list of service event indices
+		// all non-system chunks should have 1 service event
 		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
-			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
-			// 1 service event per chunk => service event indices match chunk indices
-			expected := slices.Fill(uint32(chunkIndex), 1)
-			assert.Equal(t, expected, indices)
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(1), count)
 		}
-		// the system chunk should contain indices for all events
-		assert.Equal(t, make([]uint32, 0), blockResult.ServiceEventIndicesForChunk(nChunks-1))
+		// the system chunk service event count should include all service events
+		assert.Equal(t, uint16(0), blockResult.ServiceEventCountForChunk(nChunks-1))
 	})
 	t.Run("service events in both system chunk and other chunks", func(t *testing.T) {
 		nChunks := rand.Intn(10) + 2 // at least 2 chunks
-		// add 1 service event to all chunks (including system chunk
+		// add 1 service event to all chunks (including system chunk)
 		serviceEventAllocation := slices.Fill(1, nChunks)
 
 		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
-		// all  chunks should yield a length-1 list of service event indices
+		// all chunks should have service event count of 1
 		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
-			indices := blockResult.ServiceEventIndicesForChunk(chunkIndex)
-			// 1 service event per chunk => service event indices match chunk indices
-			expected := slices.Fill(uint32(chunkIndex), 1)
-			assert.Equal(t, expected, indices)
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(1), count)
 		}
 	})
 }
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index 5cd12cf1b48..00697349475 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -43,6 +43,7 @@ func TestExecutionResultGroupBy(t *testing.T) {
 	assert.Equal(t, 0, unknown.Size())
 }
 
+// TODO
 func TestExecutionResult_FingerprintBackwardCompatibility(t *testing.T) {}
 
 // Tests that [ExecutionResult.ServiceEventsByChunk] method works in a variety of circumstances.

From 447017bbd2eb2cd81dd04490f0b3a211ee549e9d Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 2 Dec 2024 10:50:28 -0800
Subject: [PATCH 17/31] fix test name

---
 engine/execution/block_result_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/engine/execution/block_result_test.go b/engine/execution/block_result_test.go
index 2c1e0bd0b98..e250415334a 100644
--- a/engine/execution/block_result_test.go
+++ b/engine/execution/block_result_test.go
@@ -25,7 +25,7 @@ func makeBlockExecutionResultFixture(serviceEventsPerChunk []int) *BlockExecutio
 }
 
 // Tests that ServiceEventCountForChunk method works as expected under various circumstances:
-func TestBlockExecutionResult_ServiceEventIndicesForChunk(t *testing.T) {
+func TestBlockExecutionResult_ServiceEventCountForChunk(t *testing.T) {
 	t.Run("no service events", func(t *testing.T) {
 		nChunks := rand.Intn(10) + 1
 		blockResult := makeBlockExecutionResultFixture(make([]int, nChunks))

From e465bbcabca241349906baa054d4e2e6cf44452b Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 2 Dec 2024 10:53:18 -0800
Subject: [PATCH 18/31] fix chunk verifier test

---
 module/chunks/chunkVerifier_test.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 143029c8afa..515b840d638 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -28,7 +28,6 @@ import (
 	"github.com/onflow/flow-go/module/executiondatasync/execution_data"
 	"github.com/onflow/flow-go/module/executiondatasync/provider"
 	"github.com/onflow/flow-go/module/metrics"
-	"github.com/onflow/flow-go/utils/slices"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
@@ -694,8 +693,8 @@ func (m *testMetadata) RefreshChunkData(t *testing.T) *verification.VerifiableCh
 			StartState:      flow.StateCommitment(m.StartState),
 			BlockID:         m.Header.ID(),
 			// in these test cases, all defined service events correspond to the current chunk
-			ServiceEventIndices: slices.MakeRange(0, uint32(len(m.ServiceEvents))),
-			EventCollection:     eventsMerkleRootHash,
+			ServiceEventCount: unittest.PtrTo(uint16(len(m.ServiceEvents))),
+			EventCollection:   eventsMerkleRootHash,
 		},
 		Index: 0,
 	}

From 140b324a21f6518d1bfd7a395addc783fe99ec65 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 2 Dec 2024 16:20:58 -0800
Subject: [PATCH 19/31] backward compatible RLP encoding + tests

---
 model/flow/chunk.go                 | 59 +++++++++++++++++++++++++----
 model/flow/chunk_test.go            | 13 +++++--
 model/flow/execution_result_test.go | 38 ++++++++++++++++++-
 3 files changed, 97 insertions(+), 13 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 2287c8f4650..ff17e99ff03 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -2,12 +2,12 @@ package flow
 
 import (
 	"fmt"
+	"io"
 	"log"
 
 	"github.com/ipfs/go-cid"
+	"github.com/onflow/go-ethereum/rlp"
 	"github.com/vmihailenco/msgpack/v4"
-
-	"github.com/onflow/flow-go/model/encoding/rlp"
 )
 
 var EmptyEventCollectionID Identifier
@@ -65,24 +65,67 @@ type ChunkBody struct {
 	NumberOfTransactions uint64 // number of transactions inside the collection
 }
 
-// Fingerprint returns the unique binary representation for the receiver ChunkBody,
-// used to compute the ID (hash).
-// The fingerprint is backward-compatible with the prior data model for ChunkBody: ChunkBodyV0.
+// EncodeRLP defines custom encoding logic for the Chunk type.
+// This method exists only so that the embedded ChunkBody's EncodeRLP method is
+// not interpreted as the RLP encoding for the entire Chunk.
+func (ch Chunk) EncodeRLP(w io.Writer) error {
+	return rlp.Encode(w, struct {
+		ChunkBody ChunkBody
+		Index     uint64
+		EndState  StateCommitment
+	}{
+		ChunkBody: ch.ChunkBody,
+		Index:     ch.Index,
+		EndState:  ch.EndState,
+	})
+}
+
+// EncodeRLP defines custom encoding logic for the ChunkBody type.
+// The encoding is defined for backward compatibility with prior data model version (ChunkBodyV0):
 //   - All new ChunkBody instances must have non-nil ServiceEventCount field
 //   - A nil ServiceEventCount field indicates a v0 version of ChunkBody
 //   - when computing the ID of such a ChunkBody, the ServiceEventCount field is omitted from the fingerprint
-func (ch ChunkBody) Fingerprint() []byte {
+func (ch ChunkBody) EncodeRLP(w io.Writer) error {
+	var err error
 	if ch.ServiceEventCount == nil {
-		return rlp.NewMarshaler().MustMarshal(ChunkBodyV0{
+		err = rlp.Encode(w, struct {
+			CollectionIndex      uint
+			StartState           StateCommitment
+			EventCollection      Identifier
+			BlockID              Identifier
+			TotalComputationUsed uint64
+			NumberOfTransactions uint64
+		}{
+			CollectionIndex:      ch.CollectionIndex,
+			StartState:           ch.StartState,
+			EventCollection:      ch.EventCollection,
+			BlockID:              ch.BlockID,
+			TotalComputationUsed: ch.TotalComputationUsed,
+			NumberOfTransactions: ch.NumberOfTransactions,
+		})
+	} else {
+		err = rlp.Encode(w, struct {
+			CollectionIndex      uint
+			StartState           StateCommitment
+			EventCollection      Identifier
+			ServiceEventCount    *uint16
+			BlockID              Identifier
+			TotalComputationUsed uint64
+			NumberOfTransactions uint64
+		}{
 			CollectionIndex:      ch.CollectionIndex,
 			StartState:           ch.StartState,
 			EventCollection:      ch.EventCollection,
+			ServiceEventCount:    ch.ServiceEventCount,
 			BlockID:              ch.BlockID,
 			TotalComputationUsed: ch.TotalComputationUsed,
 			NumberOfTransactions: ch.NumberOfTransactions,
 		})
 	}
-	return rlp.NewMarshaler().MustMarshal(ch)
+	if err != nil {
+		return fmt.Errorf("failed to rlp encode ChunkBody: %w", err)
+	}
+	return nil
 }
 
 type Chunk struct {
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 0f8cc8c30e6..028204a53db 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/rand"
 	"github.com/onflow/flow-go/utils/unittest"
@@ -276,10 +277,15 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	})
 }
 
-// TestChunk_ModelVersions_IDConsistentAcrossVersions ensures that the ID function
-// is backward compatible with old data model versions.
-func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
+// FingerprintBackwardCompatibility ensures that the Fingerprint and ID functions
+// are backward compatible with old data model versions. Specifically, if the new
+// ServiceEventCount field is nil, then the new model should produce IDs consistent
+// with the old model.
+//
+// Backward compatibility is implemented by providing a custom EncodeRLP method.
+func TestChunk_FingerprintBackwardCompatibility(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
+	chunk.ServiceEventCount = nil
 	chunkBody := chunk.ChunkBody
 	var chunkv0 flow.ChunkBodyV0
 	unittest.CopyStructure(t, chunkBody, &chunkv0)
@@ -289,6 +295,7 @@ func TestChunk_ModelVersions_IDConsistentAcrossVersions(t *testing.T) {
 	t.Run("nil ServiceEventCount fields", func(t *testing.T) {
 		chunkBody.ServiceEventCount = nil
 		assert.Equal(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
+		assert.Equal(t, fingerprint.Fingerprint(chunkv0), fingerprint.Fingerprint(chunkBody))
 	})
 	// A non-nil ServiceEventCount fields indicates an up-to-date model version.
 	// The ID calculation for the old and new model version should be different,
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index 00697349475..a2a42f87197 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/unittest"
 )
@@ -43,8 +44,41 @@ func TestExecutionResultGroupBy(t *testing.T) {
 	assert.Equal(t, 0, unknown.Size())
 }
 
-// TODO
-func TestExecutionResult_FingerprintBackwardCompatibility(t *testing.T) {}
+// FingerprintBackwardCompatibility ensures that the Fingerprint and ID functions
+// are backward compatible with old data model versions. Specifically, if the new
+// ServiceEventCount field is nil, then the new model should produce IDs consistent
+// with the old model.
+//
+// Backward compatibility is implemented by providing a custom EncodeRLP method.
+func TestExecutionResult_FingerprintBackwardCompatibility(t *testing.T) {
+	// Define a series of types which use flow.ChunkBodyV0
+	type ChunkV0 struct {
+		flow.ChunkBodyV0
+		Index    uint64
+		EndState flow.StateCommitment
+	}
+	type ChunkListV0 []*ChunkV0
+	type ExecutionResultV0 struct {
+		PreviousResultID flow.Identifier
+		BlockID          flow.Identifier
+		Chunks           ChunkListV0
+		ServiceEvents    flow.ServiceEventList
+		ExecutionDataID  flow.Identifier
+	}
+
+	// Construct an ExecutionResult with nil ServiceEventCount fields
+	result := unittest.ExecutionResultFixture()
+	for i := range result.Chunks {
+		result.Chunks[i].ServiceEventCount = nil
+	}
+
+	// Copy all fields to the prior-version model
+	var resultv0 ExecutionResultV0
+	unittest.CopyStructure(t, result, &resultv0)
+
+	assert.Equal(t, result.ID(), flow.MakeID(resultv0))
+	assert.Equal(t, fingerprint.Fingerprint(result), fingerprint.Fingerprint(resultv0))
+}
 
 // Tests that [ExecutionResult.ServiceEventsByChunk] method works in a variety of circumstances.
 // It also tests the method against an ExecutionResult instance backed by both the

From 49bcbf6b63b3011c3027280067502b4f36c2bf89 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 2 Dec 2024 16:21:19 -0800
Subject: [PATCH 20/31] note rpc change requirements

---
 engine/common/rpc/convert/execution_results_test.go | 3 +++
 utils/unittest/fixtures.go                          | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/engine/common/rpc/convert/execution_results_test.go b/engine/common/rpc/convert/execution_results_test.go
index cce0bd175e6..9529fc7769e 100644
--- a/engine/common/rpc/convert/execution_results_test.go
+++ b/engine/common/rpc/convert/execution_results_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
+// TODO: fails with input non-nil ChunkBody.ServiceEventCount
 func TestConvertExecutionResult(t *testing.T) {
 	t.Parallel()
 
@@ -25,6 +26,7 @@ func TestConvertExecutionResult(t *testing.T) {
 	assert.Equal(t, er, converted)
 }
 
+// TODO: fails with input non-nil ChunkBody.ServiceEventCount
 func TestConvertExecutionResults(t *testing.T) {
 	t.Parallel()
 
@@ -43,6 +45,7 @@ func TestConvertExecutionResults(t *testing.T) {
 	assert.Equal(t, results, converted)
 }
 
+// TODO: fails with input non-nil ChunkBody.ServiceEventCount
 func TestConvertExecutionResultMetaList(t *testing.T) {
 	t.Parallel()
 
diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go
index 97b845ee76f..15bc9593e62 100644
--- a/utils/unittest/fixtures.go
+++ b/utils/unittest/fixtures.go
@@ -1326,9 +1326,10 @@ func ChunkFixture(
 ) *flow.Chunk {
 	chunk := &flow.Chunk{
 		ChunkBody: flow.ChunkBody{
-			CollectionIndex:      collectionIndex,
-			StartState:           StateCommitmentFixture(),
-			EventCollection:      IdentifierFixture(),
+			CollectionIndex: collectionIndex,
+			StartState:      StateCommitmentFixture(),
+			EventCollection: IdentifierFixture(),
+			//ServiceEventCount:    PtrTo[uint16](0),
 			TotalComputationUsed: 4200,
 			NumberOfTransactions: 42,
 			BlockID:              blockID,

From ee99871df74c22335f3885d976d8eddea160cd5f Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan.schalm@flowfoundation.org>
Date: Mon, 9 Dec 2024 14:29:21 -0800
Subject: [PATCH 21/31] Apply suggestions from code review

Co-authored-by: Alexander Hentschel <alex.hentschel@flowfoundation.org>
---
 model/flow/chunk.go                 |  3 ++-
 model/flow/chunk_test.go            | 15 ++++++++-------
 model/flow/execution_result_test.go |  4 ++--
 module/chunks/chunkVerifier_test.go |  4 ++--
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index ff17e99ff03..7a925f62d16 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -40,7 +40,7 @@ type ChunkBody struct {
 	StartState      StateCommitment // start state when starting executing this chunk
 	EventCollection Identifier      // Events generated by executing results
 	// ServiceEventCount defines how many service events were emitted in this chunk.
-	// By reading this field in prior chunks in the same ExecutionResult, we can
+	// By reading these fields from the prior chunks in the same ExecutionResult, we can
 	// compute exactly what service events were emitted in this chunk.
 	//
 	// Let C be this chunk, K be the set of chunks in the ExecutionResult containing C.
@@ -85,6 +85,7 @@ func (ch Chunk) EncodeRLP(w io.Writer) error {
 //   - All new ChunkBody instances must have non-nil ServiceEventCount field
 //   - A nil ServiceEventCount field indicates a v0 version of ChunkBody
 //   - when computing the ID of such a ChunkBody, the ServiceEventCount field is omitted from the fingerprint
+// No errors expected during normal operations. 
 func (ch ChunkBody) EncodeRLP(w io.Writer) error {
 	var err error
 	if ch.ServiceEventCount == nil {
diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 028204a53db..370e4269ba9 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -166,8 +166,8 @@ func TestChunkTotalComputationUsedIsSet(t *testing.T) {
 }
 
 // TestChunkEncodeDecode test encoding and decoding properties.
-// In particular, we want to demonstrate that nil-ness of the ServiceEventCount field
-// is preserved by the encoding schemes we use, because this difference is meaningful and
+// In particular, we confirm that `nil` values of the ServiceEventCount field are preserved (and
+// not conflated with 0) by the encoding schemes we use, because this difference is meaningful and
 // important for backward compatibility (see [ChunkBody.ServiceEventCount] for details).
 func TestChunkEncodeDecode(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 0)
@@ -222,7 +222,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 	chunkFixture := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
 	chunkFixture.ServiceEventCount = unittest.PtrTo[uint16](0) // non-nil extra field
 
-	t.Run("writing v0 and reading v1 should yield nil for new field", func(t *testing.T) {
+	t.Run("encoding v0 and decoding it into v1 should yield nil for ServiceEventCount", func(t *testing.T) {
 		var chunkv0 flow.ChunkBodyV0
 		unittest.CopyStructure(t, chunkFixture.ChunkBody, &chunkv0)
 
@@ -250,7 +250,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 			assert.Nil(t, unmarshaled.ServiceEventCount)
 		})
 	})
-	t.Run("writing v1 and reading v0 does not error", func(t *testing.T) {
+	t.Run("encoding v1 and decoding it into v0 should not error", func(t *testing.T) {
 		chunkv1 := chunkFixture.ChunkBody
 		chunkv1.ServiceEventCount = unittest.PtrTo[uint16](0) // ensure non-nil ServiceEventCount field
 
@@ -278,9 +278,10 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 }
 
 // FingerprintBackwardCompatibility ensures that the Fingerprint and ID functions
-// are backward compatible with old data model versions. Specifically, if the new
-// ServiceEventCount field is nil, then the new model should produce IDs consistent
-// with the old model.
+// are backward compatible with old data model versions. We emulate the
+// case where a peer running an older software version receives a `ChunkBody` that
+// was encoded in the new version. Specifically, if the new ServiceEventCount field
+// is nil, then the new model should produce IDs consistent with the old model.
 //
 // Backward compatibility is implemented by providing a custom EncodeRLP method.
 func TestChunk_FingerprintBackwardCompatibility(t *testing.T) {
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index a2a42f87197..d1536eec8bc 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -140,7 +140,7 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 		})
 	})
 
-	// NOTE: service events in non-system chunks in unsupported by the old data model
+	// NOTE: service events in non-system chunks is unsupported by the old data model
 	t.Run("service only in non-system chunks", func(t *testing.T) {
 		result := unittest.ExecutionResultFixture()
 		unittest.WithServiceEvents(result.Chunks.Len() - 1)(result) // one service event per non-system chunk
@@ -160,7 +160,7 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 		assert.Len(t, result.ServiceEventsByChunk(result.SystemChunk().Index), 0)
 	})
 
-	// NOTE: service events in non-system chunks in unsupported by the old data model
+	// NOTE: service events in non-system chunks is unsupported by the old data model
 	t.Run("service events in all chunks", func(t *testing.T) {
 		result := unittest.ExecutionResultFixture()
 		unittest.WithServiceEvents(result.Chunks.Len())(result) // one service event per chunk
diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 515b840d638..31ee7b4924e 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -251,7 +251,7 @@ func (s *ChunkVerifierTestSuite) TestEventsMismatch() {
 
 // TestServiceEventsMismatch tests verification behavior in case
 // of emitted service events not matching chunks'
-func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
+func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch_SystemChunk() {
 	meta := s.GetTestSetup(s.T(), "doesn't matter", true, true)
 	vch := meta.RefreshChunkData(s.T())
 
@@ -275,7 +275,7 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch() {
 }
 
 // TestServiceEventsAreChecked ensures that service events are in fact checked
-func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked() {
+func (s *ChunkVerifierTestSuite) TestServiceEventsAreChecked_SystemChunk() {
 	meta := s.GetTestSetup(s.T(), "doesn't matter", true, true)
 	vch := meta.RefreshChunkData(s.T())
 

From 4d40dcf7b55afc4d2798d5eb2bd698bd73a31453 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 14:30:18 -0800
Subject: [PATCH 22/31] add deprecated notes

---
 model/flow/chunk.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index ff17e99ff03..adeff31c1a6 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -24,6 +24,8 @@ func init() {
 
 // ChunkBodyV0 is the prior version of ChunkBody, used for computing backward-compatible IDs and tests.
 // Compared to ChunkBody, ChunkBodyV0 does not have the ServiceEventCount field.
+// Deprecated: to be removed in Mainnet27
+// TODO(mainnet27): Remove this data structure https://github.com/onflow/flow-go/issues/6773
 type ChunkBodyV0 struct {
 	CollectionIndex      uint
 	StartState           StateCommitment
@@ -57,6 +59,7 @@ type ChunkBody struct {
 	//     This was the implicit behaviour prior to the introduction of this field.
 	// (2) Otherwise, ServiceEventCount must be non-nil.
 	// Within an ExecutionResult, all chunks must use either representation (1) or (2), not both.
+	// TODO(mainnet27): make this field non-pointer https://github.com/onflow/flow-go/issues/6773
 	ServiceEventCount *uint16
 	BlockID           Identifier // Block id of the execution result this chunk belongs to
 

From ad346e80398aa7696a061cf6b186b12d77311035 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 14:38:56 -0800
Subject: [PATCH 23/31] RLP notes

---
 model/flow/chunk.go | 46 ++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index 37b66c27718..b03e8a21802 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -68,27 +68,22 @@ type ChunkBody struct {
 	NumberOfTransactions uint64 // number of transactions inside the collection
 }
 
-// EncodeRLP defines custom encoding logic for the Chunk type.
-// This method exists only so that the embedded ChunkBody's EncodeRLP method is
-// not interpreted as the RLP encoding for the entire Chunk.
-func (ch Chunk) EncodeRLP(w io.Writer) error {
-	return rlp.Encode(w, struct {
-		ChunkBody ChunkBody
-		Index     uint64
-		EndState  StateCommitment
-	}{
-		ChunkBody: ch.ChunkBody,
-		Index:     ch.Index,
-		EndState:  ch.EndState,
-	})
-}
+// We TEMPORARILY implement the [rlp.Encoder] interface to implement backwards-compatible ID computation.
+// TODO(mainnet27): remove EncodeRLP methods on Chunk and ChunkBody https://github.com/onflow/flow-go/issues/6773
+var _ rlp.Encoder = &ChunkBody{}
 
 // EncodeRLP defines custom encoding logic for the ChunkBody type.
+// NOTE: For correct operation when encoding a larger structure containing ChunkBody,
+// this method depends on Chunk also overriding EncodeRLP. Otherwise, since ChunkBody
+// is an embedded field, the RLP encoder will skip Chunk fields besides those in ChunkBody.
+//
 // The encoding is defined for backward compatibility with prior data model version (ChunkBodyV0):
 //   - All new ChunkBody instances must have non-nil ServiceEventCount field
 //   - A nil ServiceEventCount field indicates a v0 version of ChunkBody
 //   - when computing the ID of such a ChunkBody, the ServiceEventCount field is omitted from the fingerprint
-// No errors expected during normal operations. 
+//
+// No errors expected during normal operations.
+// TODO(mainnet27): remove this method https://github.com/onflow/flow-go/issues/6773
 func (ch ChunkBody) EncodeRLP(w io.Writer) error {
 	var err error
 	if ch.ServiceEventCount == nil {
@@ -140,6 +135,27 @@ type Chunk struct {
 	EndState StateCommitment
 }
 
+// We TEMPORARILY implement the [rlp.Encoder] interface to implement backwards-compatible ID computation.
+// TODO(mainnet27): remove EncodeRLP methods on Chunk and ChunkBody https://github.com/onflow/flow-go/issues/6773
+var _ rlp.Encoder = &Chunk{}
+
+// EncodeRLP defines custom encoding logic for the Chunk type.
+// This method exists only so that the embedded ChunkBody's EncodeRLP method is
+// not interpreted as the RLP encoding for the entire Chunk.
+// No errors expected during normal operation.
+// TODO(mainnet27): remove this method https://github.com/onflow/flow-go/issues/6773
+func (ch Chunk) EncodeRLP(w io.Writer) error {
+	return rlp.Encode(w, struct {
+		ChunkBody ChunkBody
+		Index     uint64
+		EndState  StateCommitment
+	}{
+		ChunkBody: ch.ChunkBody,
+		Index:     ch.Index,
+		EndState:  ch.EndState,
+	})
+}
+
 func NewChunk(
 	blockID Identifier,
 	collectionIndex int,

From b484203db1e4267e7ef058c8f1cc4ee84afacc4b Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 14:42:36 -0800
Subject: [PATCH 24/31] rename var

---
 model/flow/chunk_test.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index 370e4269ba9..e7bdcb7e493 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -288,21 +288,21 @@ func TestChunk_FingerprintBackwardCompatibility(t *testing.T) {
 	chunk := unittest.ChunkFixture(unittest.IdentifierFixture(), 1)
 	chunk.ServiceEventCount = nil
 	chunkBody := chunk.ChunkBody
-	var chunkv0 flow.ChunkBodyV0
-	unittest.CopyStructure(t, chunkBody, &chunkv0)
+	var chunkBodyV0 flow.ChunkBodyV0
+	unittest.CopyStructure(t, chunkBody, &chunkBodyV0)
 
 	// A nil ServiceEventCount fields indicates a prior model version.
 	// The ID calculation for the old and new model version should be the same.
 	t.Run("nil ServiceEventCount fields", func(t *testing.T) {
 		chunkBody.ServiceEventCount = nil
-		assert.Equal(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
-		assert.Equal(t, fingerprint.Fingerprint(chunkv0), fingerprint.Fingerprint(chunkBody))
+		assert.Equal(t, flow.MakeID(chunkBodyV0), flow.MakeID(chunkBody))
+		assert.Equal(t, fingerprint.Fingerprint(chunkBodyV0), fingerprint.Fingerprint(chunkBody))
 	})
 	// A non-nil ServiceEventCount fields indicates an up-to-date model version.
 	// The ID calculation for the old and new model version should be different,
 	// because the new model should include the ServiceEventCount field value.
 	t.Run("non-nil ServiceEventCount fields", func(t *testing.T) {
 		chunkBody.ServiceEventCount = unittest.PtrTo[uint16](0)
-		assert.NotEqual(t, flow.MakeID(chunkv0), flow.MakeID(chunkBody))
+		assert.NotEqual(t, flow.MakeID(chunkBodyV0), flow.MakeID(chunkBody))
 	})
 }

From a61e30ed7a5aed77038e1f40a97a86a5289d5008 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 14:42:47 -0800
Subject: [PATCH 25/31] add test case demonstrating rlp order dependence

---
 model/encoding/rlp/rlp_test.go | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 model/encoding/rlp/rlp_test.go

diff --git a/model/encoding/rlp/rlp_test.go b/model/encoding/rlp/rlp_test.go
new file mode 100644
index 00000000000..4b07e5d8a71
--- /dev/null
+++ b/model/encoding/rlp/rlp_test.go
@@ -0,0 +1,29 @@
+package rlp_test
+
+import (
+	"testing"
+
+	"github.com/onflow/go-ethereum/rlp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRLPStructFieldOrder tests the field ordering property of RLP encoding.
+// It provides evidence that RLP encoding depends on struct field ordering.
+func TestRLPStructFieldOrder(t *testing.T) {
+	a := struct {
+		A uint32 // A first
+		B uint32
+	}{A: 2, B: 3}
+
+	b := struct {
+		B uint32 // B first
+		A uint32
+	}{A: 2, B: 3}
+
+	abin, err := rlp.EncodeToBytes(a)
+	require.NoError(t, err)
+	bbin, err := rlp.EncodeToBytes(b)
+	require.NoError(t, err)
+	assert.NotEqual(t, abin, bbin)
+}

From 6fe51f6a5e3d37f7752219820547c94798c8390d Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 14:53:38 -0800
Subject: [PATCH 26/31] EncodeDecodeDifferentVersions docs

---
 model/flow/chunk_test.go            |  4 ++--
 model/flow/execution_result_test.go |  2 +-
 utils/unittest/encoding.go          | 11 +++++++++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/model/flow/chunk_test.go b/model/flow/chunk_test.go
index e7bdcb7e493..1af4fd4db2c 100644
--- a/model/flow/chunk_test.go
+++ b/model/flow/chunk_test.go
@@ -224,7 +224,7 @@ func TestChunk_ModelVersions_EncodeDecode(t *testing.T) {
 
 	t.Run("encoding v0 and decoding it into v1 should yield nil for ServiceEventCount", func(t *testing.T) {
 		var chunkv0 flow.ChunkBodyV0
-		unittest.CopyStructure(t, chunkFixture.ChunkBody, &chunkv0)
+		unittest.EncodeDecodeDifferentVersions(t, chunkFixture.ChunkBody, &chunkv0)
 
 		t.Run("json", func(t *testing.T) {
 			bz, err := json.Marshal(chunkv0)
@@ -289,7 +289,7 @@ func TestChunk_FingerprintBackwardCompatibility(t *testing.T) {
 	chunk.ServiceEventCount = nil
 	chunkBody := chunk.ChunkBody
 	var chunkBodyV0 flow.ChunkBodyV0
-	unittest.CopyStructure(t, chunkBody, &chunkBodyV0)
+	unittest.EncodeDecodeDifferentVersions(t, chunkBody, &chunkBodyV0)
 
 	// A nil ServiceEventCount fields indicates a prior model version.
 	// The ID calculation for the old and new model version should be the same.
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index d1536eec8bc..1e8eef25825 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -74,7 +74,7 @@ func TestExecutionResult_FingerprintBackwardCompatibility(t *testing.T) {
 
 	// Copy all fields to the prior-version model
 	var resultv0 ExecutionResultV0
-	unittest.CopyStructure(t, result, &resultv0)
+	unittest.EncodeDecodeDifferentVersions(t, result, &resultv0)
 
 	assert.Equal(t, result.ID(), flow.MakeID(resultv0))
 	assert.Equal(t, fingerprint.Fingerprint(result), fingerprint.Fingerprint(resultv0))
diff --git a/utils/unittest/encoding.go b/utils/unittest/encoding.go
index c2fd26b5ddb..b2ab5fd0fa3 100644
--- a/utils/unittest/encoding.go
+++ b/utils/unittest/encoding.go
@@ -7,8 +7,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// CopyStructure copies public fields of compatible types from src to dst.
-func CopyStructure(t *testing.T, src, dst any) {
+// EncodeDecodeDifferentVersions emulates the situation where a peer running software version A receives
+// a message from a sender running software version B, where the format of the message may have been upgraded between
+// the different software versions. This method works irrespective whether version A or B is the older/newer version
+// (also allowing that both versions are the same; in this degenerate edge-case the old and new format would be the same).
+//
+// This function works by encoding src using CBOR, then decoding the result into dst.
+// Compatible fields as defined by CBOR will be copied into dst; in-compatible fields
+// may be omitted.
+func EncodeDecodeDifferentVersions(t *testing.T, src, dst any) {
 	bz, err := cbor.Marshal(src)
 	require.NoError(t, err)
 	err = cbor.Unmarshal(bz, dst)

From 4a651b66ccbde55e14bc2d58fc9f63238ca0149a Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 16:40:41 -0800
Subject: [PATCH 27/31] sanity check for service event count field

---
 engine/execution/block_result.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/engine/execution/block_result.go b/engine/execution/block_result.go
index 1e62b22d038..905ea58eb2d 100644
--- a/engine/execution/block_result.go
+++ b/engine/execution/block_result.go
@@ -2,6 +2,7 @@ package execution
 
 import (
 	"fmt"
+	"math"
 
 	"github.com/onflow/flow-go/fvm/storage/snapshot"
 	"github.com/onflow/flow-go/model/flow"
@@ -52,6 +53,12 @@ func (er *BlockExecutionResult) AllEvents() flow.EventsList {
 // ServiceEventCountForChunk returns the number of service events emitted in the given chunk.
 func (er *BlockExecutionResult) ServiceEventCountForChunk(chunkIndex int) uint16 {
 	serviceEventCount := len(er.collectionExecutionResults[chunkIndex].serviceEvents)
+	if serviceEventCount > math.MaxUint16 {
+		// The current protocol demands that the ServiceEventCount does not exceed 65535.
+		// For defensive programming, we explicitly enforce this limit as 65k could be produced by a bug.
+		// Execution nodes would be first to realize that this bound is violated, and crash (fail early).
+		panic(fmt.Sprintf("service event count (%d) exceeds maximum value of 65535", serviceEventCount))
+	}
 	return uint16(serviceEventCount)
 }
 

From 4a27b73a81734af7a446945b5591a0b617de1d3a Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 16:45:55 -0800
Subject: [PATCH 28/31] fix ER test

---
 engine/execution/block_result_test.go | 2 +-
 model/flow/execution_result.go        | 1 +
 model/flow/execution_result_test.go   | 7 +++++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/engine/execution/block_result_test.go b/engine/execution/block_result_test.go
index e250415334a..a96e3576728 100644
--- a/engine/execution/block_result_test.go
+++ b/engine/execution/block_result_test.go
@@ -27,7 +27,7 @@ func makeBlockExecutionResultFixture(serviceEventsPerChunk []int) *BlockExecutio
 // Tests that ServiceEventCountForChunk method works as expected under various circumstances:
 func TestBlockExecutionResult_ServiceEventCountForChunk(t *testing.T) {
 	t.Run("no service events", func(t *testing.T) {
-		nChunks := rand.Intn(10) + 1
+		nChunks := rand.Intn(10) + 1 // always contains at least system chunk
 		blockResult := makeBlockExecutionResultFixture(make([]int, nChunks))
 		// all chunks should have 0 service event count
 		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
diff --git a/model/flow/execution_result.go b/model/flow/execution_result.go
index c0669533bab..5a4fd9b91a4 100644
--- a/model/flow/execution_result.go
+++ b/model/flow/execution_result.go
@@ -101,6 +101,7 @@ func (er ExecutionResult) ServiceEventsByChunk(chunkIndex uint64) ServiceEventLi
 	// This indicates the chunk was generated in an older data model version.
 	// In this case, all service events associated with the result are assumed
 	// to have been emitted within the system chunk (last chunk)
+	// TODO(mainnet27): remove this path https://github.com/onflow/flow-go/issues/6773
 	if chunkIndex == er.SystemChunk().Index {
 		return er.ServiceEvents
 	}
diff --git a/model/flow/execution_result_test.go b/model/flow/execution_result_test.go
index 1e8eef25825..6e5a86228de 100644
--- a/model/flow/execution_result_test.go
+++ b/model/flow/execution_result_test.go
@@ -111,16 +111,19 @@ func TestExecutionResult_ServiceEventsByChunk(t *testing.T) {
 
 	t.Run("service events only in system chunk", func(t *testing.T) {
 		t.Run("nil ServiceEventCount field (old model)", func(t *testing.T) {
-			result := unittest.ExecutionResultFixture()
+			nServiceEvents := rand.Intn(10) + 1
+			result := unittest.ExecutionResultFixture(unittest.WithServiceEvents(nServiceEvents))
 			for _, chunk := range result.Chunks {
 				chunk.ServiceEventCount = nil
 			}
 
 			// should return empty list for all chunks
-			for chunkIndex := 0; chunkIndex < result.Chunks.Len(); chunkIndex++ {
+			for chunkIndex := 0; chunkIndex < result.Chunks.Len()-1; chunkIndex++ {
 				serviceEvents := result.ServiceEventsByChunk(uint64(chunkIndex))
 				assert.Len(t, serviceEvents, 0)
 			}
+			// should return list of service events for system chunk
+			assert.Equal(t, result.ServiceEvents, result.ServiceEventsByChunk(result.SystemChunk().Index))
 		})
 		t.Run("populated ServiceEventCount field", func(t *testing.T) {
 			nServiceEvents := rand.Intn(10) + 1

From e1ee2f02d84679baf1fd346f46b99ae0ab28a755 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 18:03:01 -0800
Subject: [PATCH 29/31] add context to chunkverifier test

---
 module/chunks/chunkVerifier_test.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 31ee7b4924e..999bfa0bad6 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -303,10 +303,12 @@ func (s *ChunkVerifierTestSuite) TestServiceEventsMismatch_NonSystemChunk() {
 	require.NoError(s.T(), err)
 
 	s.snapshots[script] = &snapshot.ExecutionSnapshot{}
+	// overwrite the expected output for our custom transaction, passing
+	// in the non-matching EpochCommit event (should cause validation failure)
 	s.outputs[script] = fvm.ProcedureOutput{
 		ComputationUsed:        computationUsed,
 		ConvertedServiceEvents: flow.ServiceEventList{*epochCommitServiceEvent},
-		Events:                 meta.ChunkEvents[:3],
+		Events:                 meta.ChunkEvents[:3], // 2 default event + EpochSetup
 	}
 
 	_, err = s.verifier.Verify(vch)

From 9c1ef709dbb1ef79ba836fef1ed6c29363339f30 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan@dapperlabs.com>
Date: Mon, 9 Dec 2024 18:04:13 -0800
Subject: [PATCH 30/31] refactor generateEvents

---
 module/chunks/chunkVerifier_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/module/chunks/chunkVerifier_test.go b/module/chunks/chunkVerifier_test.go
index 999bfa0bad6..6ac2edd8366 100644
--- a/module/chunks/chunkVerifier_test.go
+++ b/module/chunks/chunkVerifier_test.go
@@ -510,7 +510,7 @@ func generateExecutionData(t *testing.T, blockID flow.Identifier, ced *execution
 	return executionDataID, executionDataRoot
 }
 
-func generateEvents(t *testing.T, includeServiceEvent bool, collection *flow.Collection) (flow.EventsList, []flow.ServiceEvent) {
+func generateEvents(t *testing.T, collection *flow.Collection, includeServiceEvent bool) (flow.EventsList, []flow.ServiceEvent) {
 	var chunkEvents flow.EventsList
 	serviceEvents := make([]flow.ServiceEvent, 0)
 
@@ -615,7 +615,7 @@ func (s *ChunkVerifierTestSuite) GetTestSetup(t *testing.T, script string, syste
 	}
 
 	// events
-	chunkEvents, serviceEvents := generateEvents(t, includeServiceEvents, collection)
+	chunkEvents, serviceEvents := generateEvents(t, collection, includeServiceEvents)
 	// make sure this includes events even for the service tx
 	require.NotEmpty(t, chunkEvents)
 

From 9a10320ec494a306a0099cf7e2183c342a02de76 Mon Sep 17 00:00:00 2001
From: Jordan Schalm <jordan.schalm@flowfoundation.org>
Date: Mon, 9 Dec 2024 18:05:40 -0800
Subject: [PATCH 31/31] Update model/flow/chunk.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
---
 model/flow/chunk.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/model/flow/chunk.go b/model/flow/chunk.go
index b03e8a21802..d7ebfe4f102 100644
--- a/model/flow/chunk.go
+++ b/model/flow/chunk.go
@@ -54,7 +54,7 @@ type ChunkBody struct {
 	//
 	// BACKWARD COMPATIBILITY:
 	// (1) If ServiceEventCount is nil, this indicates that this chunk was created by an older software version
-	//     which did support specifying a mapping between chunks and service events.
+	//     which did not support specifying a mapping between chunks and service events.
 	//     In this case, all service events are assumed to have been emitted in the system chunk (last chunk).
 	//     This was the implicit behaviour prior to the introduction of this field.
 	// (2) Otherwise, ServiceEventCount must be non-nil.