Merge pull request #6871 from onflow/tim/5491-chunk-start-end-consist…

…ency-check

Validate consistency of chunk Start and End states

engine/consensus/approvals/testutil.go

@@ -44,7 +44,7 @@ func (s *BaseApprovalsTestSuite) SetupTest() {
 	verifiers := make(flow.IdentifierList, 0)
 	s.AuthorizedVerifiers = make(map[flow.Identifier]*flow.Identity)
 	assignmentBuilder := chunks.NewAssignmentBuilder()
-	s.Chunks = unittest.ChunkListFixture(50, s.Block.ID())
+	s.Chunks = unittest.ChunkListFixture(50, s.Block.ID(), unittest.StateCommitmentFixture())
 	// mock public key to mock signature verifications
 	s.PublicKey = &module.PublicKey{}
 

engine/verification/utils/unittest/fixture.go

@@ -503,7 +503,7 @@ func ExecutionResultForkFixture(t *testing.T) (*flow.ExecutionResult, *flow.Exec
 	resultB := &flow.ExecutionResult{
 		PreviousResultID: resultA.PreviousResultID,
 		BlockID:          resultA.BlockID,
-		Chunks:           append(flow.ChunkList{resultA.Chunks[0]}, unittest.ChunkListFixture(1, resultA.BlockID)...),
+		Chunks:           append(flow.ChunkList{resultA.Chunks[0]}, unittest.ChunkListFixture(1, resultA.BlockID, resultA.Chunks[0].EndState)...),
 		ServiceEvents:    nil,
 	}
 

insecure/wintermute/attackOrchestrator.go

@@ -144,14 +144,14 @@ func (o *Orchestrator) corruptExecutionResult(receipt *flow.ExecutionReceipt) *f
 		BlockID:          receipt.ExecutionResult.BlockID,
 		// replace all chunks with new ones to simulate chunk corruption
 		Chunks: flow.ChunkList{
-			unittest.ChunkFixture(receipt.ExecutionResult.BlockID, 0, unittest.WithChunkStartState(receiptStartState)),
+			unittest.ChunkFixture(receipt.ExecutionResult.BlockID, 0, receiptStartState),
 		},
 		ServiceEvents:   receipt.ExecutionResult.ServiceEvents,
 		ExecutionDataID: receipt.ExecutionResult.ExecutionDataID,
 	}
 
 	if chunksNum > 1 {
-		result.Chunks = append(result.Chunks, unittest.ChunkListFixture(uint(chunksNum-1), receipt.ExecutionResult.BlockID)...)
+		result.Chunks = append(result.Chunks, unittest.ChunkListFixture(uint(chunksNum-1), receipt.ExecutionResult.BlockID, result.Chunks[0].EndState)...)
 	}
 
 	return result

model/flow/chunk_test.go

@@ -62,7 +62,7 @@ func TestDistinctChunkIDs_FullChunks(t *testing.T) {
 	require.NotEqual(t, blockIdA, blockIdB)
 
 	// generates a chunk associated with blockA
-	chunkA := unittest.ChunkFixture(blockIdA, 42)
+	chunkA := unittest.ChunkFixture(blockIdA, 42, unittest.StateCommitmentFixture())
 
 	// generates a deep copy of chunkA in chunkB
 	chunkB := *chunkA
@@ -80,7 +80,7 @@ func TestDistinctChunkIDs_FullChunks(t *testing.T) {
 
 // TestChunkList_Indices evaluates the Indices method of ChunkList on lists of different sizes.
 func TestChunkList_Indices(t *testing.T) {
-	cl := unittest.ChunkListFixture(5, unittest.IdentifierFixture())
+	cl := unittest.ChunkListFixture(5, unittest.IdentifierFixture(), unittest.StateCommitmentFixture())
 	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{}

module/validation/receipt_validator.go

@@ -96,6 +96,14 @@ func (v *receiptValidator) verifyChunksFormat(result *flow.ExecutionResult) erro
 	if result.Chunks.Len() != requiredChunks {
 		return engine.NewInvalidInputErrorf("invalid number of chunks, expected %d got %d", requiredChunks, result.Chunks.Len())
 	}
+
+	// We have at least one chunk, check chunk state consistency
+	chunks := result.Chunks.Items()
+	for i := range len(chunks) - 1 {
+		if chunks[i].EndState != chunks[i+1].StartState {
+			return engine.NewInvalidInputErrorf("chunk state mismatch at index %v, EndState %v but next StartState %v", i, chunks[i].EndState, chunks[i+1].StartState)
+		}
+	}
 	return nil
 }
 

module/validation/receipt_validator_test.go

@@ -224,6 +224,26 @@ func (s *ReceiptValidationSuite) TestReceiptForBlockWith0Collections() {
 	})
 }
 
+// TestReceiptInconsistentChunkList tests that we reject receipts when the Start and End states
+// within the chunk list are inconsistent (e.g. chunk[0].EndState != chunk[1].StartState).
+func (s *ReceiptValidationSuite) TestReceiptInconsistentChunkList() {
+	s.publicKey.On("Verify", mock.Anything, mock.Anything, mock.Anything).Return(true, nil).Maybe()
+	valSubgrph := s.ValidSubgraphFixture()
+	chunks := valSubgrph.Result.Chunks
+	require.GreaterOrEqual(s.T(), chunks.Len(), 1)
+	// swap last chunk's start and end states
+	lastChunk := chunks[len(chunks)-1]
+	lastChunk.StartState, lastChunk.EndState = lastChunk.EndState, lastChunk.StartState
+
+	receipt := unittest.ExecutionReceiptFixture(unittest.WithExecutorID(s.ExeID),
+		unittest.WithResult(valSubgrph.Result))
+	s.AddSubgraphFixtureToMempools(valSubgrph)
+
+	err := s.receiptValidator.Validate(receipt)
+	s.Require().Error(err, "should reject with invalid chunks")
+	s.Assert().True(engine.IsInvalidInputError(err))
+}
+
 // TestReceiptTooManyChunks tests that we reject receipt with more chunks than expected
 func (s *ReceiptValidationSuite) TestReceiptTooManyChunks() {
 	valSubgrph := s.ValidSubgraphFixture()

utils/unittest/fixtures.go

@@ -893,15 +893,14 @@ func WithBlock(block *flow.Block) func(*flow.ExecutionResult) {
 	return func(result *flow.ExecutionResult) {
 		startState := result.Chunks[0].StartState // retain previous start state in case it was user-defined
 		result.BlockID = blockID
-		result.Chunks = ChunkListFixture(uint(chunks), blockID)
-		result.Chunks[0].StartState = startState // set start state to value before update
+		result.Chunks = ChunkListFixture(uint(chunks), blockID, startState)
 		result.PreviousResultID = previousResultID
 	}
 }
 
 func WithChunks(n uint) func(*flow.ExecutionResult) {
 	return func(result *flow.ExecutionResult) {
-		result.Chunks = ChunkListFixture(n, result.BlockID)
+		result.Chunks = ChunkListFixture(n, result.BlockID, StateCommitmentFixture())
 	}
 }
 
@@ -966,7 +965,7 @@ func ExecutionResultFixture(opts ...func(*flow.ExecutionResult)) *flow.Execution
 	result := &flow.ExecutionResult{
 		PreviousResultID: IdentifierFixture(),
 		BlockID:          executedBlockID,
-		Chunks:           ChunkListFixture(2, executedBlockID),
+		Chunks:           ChunkListFixture(2, executedBlockID, StateCommitmentFixture()),
 		ExecutionDataID:  IdentifierFixture(),
 	}
 
@@ -1322,12 +1321,13 @@ func WithChunkStartState(startState flow.StateCommitment) func(chunk *flow.Chunk
 func ChunkFixture(
 	blockID flow.Identifier,
 	collectionIndex uint,
+	startState flow.StateCommitment,
 	opts ...func(*flow.Chunk),
 ) *flow.Chunk {
 	chunk := &flow.Chunk{
 		ChunkBody: flow.ChunkBody{
 			CollectionIndex:      collectionIndex,
-			StartState:           StateCommitmentFixture(),
+			StartState:           startState,
 			EventCollection:      IdentifierFixture(),
 			TotalComputationUsed: 4200,
 			NumberOfTransactions: 42,
@@ -1344,12 +1344,13 @@ func ChunkFixture(
 	return chunk
 }
 
-func ChunkListFixture(n uint, blockID flow.Identifier) flow.ChunkList {
+func ChunkListFixture(n uint, blockID flow.Identifier, startState flow.StateCommitment) flow.ChunkList {
 	chunks := make([]*flow.Chunk, 0, n)
 	for i := uint64(0); i < uint64(n); i++ {
-		chunk := ChunkFixture(blockID, uint(i))
+		chunk := ChunkFixture(blockID, uint(i), startState)
 		chunk.Index = i
 		chunks = append(chunks, chunk)
+		startState = chunk.EndState
 	}
 	return chunks
 }