Merge pull request #6744 from onflow/jord/6622-chunk-service-events

Chunk Data Model supports per-chunk service event mapping

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()
 

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"
@@ -49,6 +50,18 @@ func (er *BlockExecutionResult) AllEvents() flow.EventsList {
 	return res
 }
 
+// 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)
+}
+
 func (er *BlockExecutionResult) AllServiceEvents() flow.EventsList {
 	res := make(flow.EventsList, 0)
 	for _, ce := range er.collectionExecutionResults {
@@ -199,6 +212,7 @@ func (ar *BlockAttestationResult) ChunkAt(index int) *flow.Chunk {
 		attestRes.startStateCommit,
 		len(execRes.TransactionResults()),
 		attestRes.eventCommit,
+		ar.ServiceEventCountForChunk(index),
 		attestRes.endStateCommit,
 		execRes.executionSnapshot.TotalComputationUsed(),
 	)

engine/execution/block_result_test.go

@@ -0,0 +1,81 @@
+package execution
+
+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 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 // 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++ {
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(0), count)
+		}
+	})
+	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 have zero service event count
+		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(0), count)
+		}
+		// 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
+		// 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 have 1 service event
+		for chunkIndex := 0; chunkIndex < nChunks-1; chunkIndex++ {
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(1), count)
+		}
+		// 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)
+		serviceEventAllocation := slices.Fill(1, nChunks)
+
+		blockResult := makeBlockExecutionResultFixture(serviceEventAllocation)
+		// all chunks should have service event count of 1
+		for chunkIndex := 0; chunkIndex < nChunks; chunkIndex++ {
+			count := blockResult.ServiceEventCountForChunk(chunkIndex)
+			assert.Equal(t, uint16(1), count)
+		}
+	})
+}

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)
+}

model/flow/chunk.go

@@ -2,9 +2,11 @@ package flow
 
 import (
 	"fmt"
+	"io"
 	"log"
 
 	"github.com/ipfs/go-cid"
+	"github.com/onflow/go-ethereum/rlp"
 	"github.com/vmihailenco/msgpack/v4"
 )
 
@@ -20,19 +22,111 @@ 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
+	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
+	// ServiceEventCount defines how many service events were emitted in this chunk.
+	// 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.
+	// Then the service event indices for C are given by:
+	//    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]
+	//
+	// BACKWARD COMPATIBILITY:
+	// (1) If ServiceEventCount is nil, this indicates that this chunk was created by an older software version
+	//     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.
+	// 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
 
 	// 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
 }
 
+// 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.
+// 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 {
+		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,
+		})
+	}
+	if err != nil {
+		return fmt.Errorf("failed to rlp encode ChunkBody: %w", err)
+	}
+	return nil
+}
+
 type Chunk struct {
 	ChunkBody
 
@@ -41,12 +135,34 @@ 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,
 	startState StateCommitment,
 	numberOfTransactions int,
 	eventCollection Identifier,
+	serviceEventCount uint16,
 	endState StateCommitment,
 	totalComputationUsed uint64,
 ) *Chunk {
@@ -57,6 +173,7 @@ func NewChunk(
 			StartState:           startState,
 			NumberOfTransactions: uint64(numberOfTransactions),
 			EventCollection:      eventCollection,
+			ServiceEventCount:    &serviceEventCount,
 			TotalComputationUsed: totalComputationUsed,
 		},
 		Index:    uint64(collectionIndex),