minor struct goDocs

Author: AlexHentschel

engine/execution/state/state.go

@@ -413,6 +413,12 @@ func (s *state) saveExecutionResults(
 		return fmt.Errorf("can not retrieve chunk data packs: %w", err)
 	}
 
+	// Within the following `Store` call, the chunk data packs themselves are going to be persisted into their
+	// dedicated database. However, we have not yet persisted that this execution node is committing to the
+	// result represented by the chunk data packs. Populating the index from chunk ID to chunk data pack ID
+	// in the protocol database (signifying the node's slashable commitment to the respective result) is
+	// done by the functor returned by `Store`. The functor's is invoked as part of the atomic batch update
+	// of the protocol database below.
 	storeFunc, err := s.chunkDataPacks.Store(chunks)
 	if err != nil {
 		return fmt.Errorf("can not store chunk data packs for block ID: %v: %w", blockID, err)
@@ -427,9 +433,11 @@ func (s *state) saveExecutionResults(
 		// This design guarantees consistency in two scenarios:
 		//
 		// Case 1: If the batch update is interrupted, the mapping has not yet been saved.
-		// Later, if we attempt to store another execution result that references a
-		// different chunk data pack but the same chunk ID, there is no conflict,
-		// because no previous mapping exists.
+		// Later, if we attempt to store another execution result that references a different
+		// chunk data pack but the same chunk ID, there is no conflict, because no previous mapping
+		// exists. By convention, a node should only share information once it has persisted its
+		// commitment in the database. Therefore, if the database write was interrupted, none of the
+		// information is stored and no binding commitment to a different result could have been made.
 		//
 		// Case 2: If the batch update succeeds, the mapping is saved. Later, if we
 		// attempt to store another execution result that references a different
@@ -484,7 +492,6 @@ func (s *state) saveExecutionResults(
 			return nil
 		})
 	})
-
 }
 
 func (s *state) UpdateLastExecutedBlock(ctx context.Context, executedID flow.Identifier) error {

model/flow/chunk.go

@@ -129,6 +129,11 @@ func (ch *Chunk) ID() Identifier {
 	return MakeID(ch)
 }
 
+// ChunkDataPackHeader is a reduced representation of ChunkDataPack. In a nutshell, we substitute
+// the larger [ChunkDataPack.Proof] and [ChunkDataPack.Collection] with their collision-resistant hashes.
+// Note, ChunkDataPackHeader.ID() is the same as ChunkDataPack.ID().
+//
+//structwrite:immutable - mutations allowed only within the constructor
 type ChunkDataPackHeader struct {
 	ChunkID    Identifier      // ID of the chunk this data pack is for
 	StartState StateCommitment // commitment for starting state
@@ -141,6 +146,8 @@ type ChunkDataPackHeader struct {
 	ExecutionDataRoot BlockExecutionDataRoot
 }
 
+// NewChunkDataPackHeader instantiates an "immutable"  ChunkDataPackHeader.
+// The `CollectionID` field is set to [flow.ZeroID] for system chunks.
 func NewChunkDataPackHeader(ChunkID Identifier, StartState StateCommitment, ProofID Identifier, CollectionID Identifier, ExecutionDataRoot BlockExecutionDataRoot) *ChunkDataPackHeader {
 	return &ChunkDataPackHeader{
 		ChunkID:           ChunkID,
@@ -197,8 +204,9 @@ type ChunkDataPack struct {
 // a trusted ChunkDataPack using NewChunkDataPack constructor.
 type UntrustedChunkDataPack ChunkDataPack
 
-// NewChunkDataPack returns an initialized chunk data pack.
-// Construction ChunkDataPack allowed only within the constructor.
+// FromUntrustedChunkDataPack converts a chunk data pack from an untrusted source
+// into its canonical representation. Here, basic structural validation is performed.
+// Construction of ChunkDataPacks is ONLY allowed via THIS CONSTRUCTOR.
 //
 // All errors indicate a valid ChunkDataPack cannot be constructed from the input.
 func FromUntrustedChunkDataPack(untrusted UntrustedChunkDataPack) (*ChunkDataPack, error) {
@@ -231,6 +239,8 @@ func FromUntrustedChunkDataPack(untrusted UntrustedChunkDataPack) (*ChunkDataPac
 	), nil
 }
 
+// NewChunkDataPack instantiates an "immutable"  ChunkDataPack.
+// The `collection` field is set to `nil` for system chunks.
 func NewChunkDataPack(chunkID Identifier, startState StateCommitment, proof StorageProof, collection *Collection, executionDataRoot BlockExecutionDataRoot) *ChunkDataPack {
 	return &ChunkDataPack{
 		ChunkID:           chunkID,

storage/chunk_data_packs_stored.go

@@ -29,9 +29,9 @@ type StoredChunkDataPacks interface {
 	BatchRemove(chunkDataPackIDs []flow.Identifier, rw ReaderBatchWriter) error
 }
 
-// StoredChunkDataPack is an in-storage representation of chunk data pack.
-// Its prime difference is instead of an actual collection, it keeps a collection ID hence relying on maintaining
-// the collection on a secondary storage.
+// StoredChunkDataPack is an in-storage representation of chunk data pack. Its prime difference is instead of an
+// actual collection, it keeps a collection ID hence relying on maintaining the collection on a secondary storage.
+// Note, StoredChunkDataPack.ID() is the same as ChunkDataPack.ID()
 //
 //structwrite:immutable - mutations allowed only within the constructor
 type StoredChunkDataPack struct {
@@ -42,6 +42,8 @@ type StoredChunkDataPack struct {
 	ExecutionDataRoot flow.BlockExecutionDataRoot
 }
 
+// NewStoredChunkDataPack instantiates an "immutable"  [StoredChunkDataPack].
+// The `collectionID` field is set to [flow.ZeroID] for system chunks.
 func NewStoredChunkDataPack(
 	chunkID flow.Identifier,
 	startState flow.StateCommitment,
@@ -63,6 +65,8 @@ func (s *StoredChunkDataPack) IsSystemChunk() bool {
 	return s.CollectionID == flow.ZeroID
 }
 
+// ToStoredChunkDataPack converts the given Chunk Data Pack to its reduced representation.
+// (Collections are stored separately and don't need to be included again here).
 func ToStoredChunkDataPack(c *flow.ChunkDataPack) *StoredChunkDataPack {
 	collectionID := flow.ZeroID
 	if c.Collection != nil {
@@ -78,8 +82,7 @@ func ToStoredChunkDataPack(c *flow.ChunkDataPack) *StoredChunkDataPack {
 }
 
 // ToStoredChunkDataPacks converts the given Chunk Data Packs to their reduced representation.
-// This is useful for reducing storage consumption, by avoiding repeated storage of the full collections
-// (stored individually anyway).
+// (Collections are stored separately and don't need to be included again here).
 func ToStoredChunkDataPacks(cs []*flow.ChunkDataPack) []*StoredChunkDataPack {
 	scs := make([]*StoredChunkDataPack, 0, len(cs))
 	for _, c := range cs {

storage/store/chunk_data_packs.go

@@ -271,7 +271,7 @@ func (ch *ChunkDataPacks) ByChunkID(chunkID flow.Identifier) (*flow.ChunkDataPac
 		return nil, fmt.Errorf("cannot retrieve stored chunk data pack %x for chunk %x: %w", chunkDataPackID, chunkID, err)
 	}
 
-	var collection *flow.Collection
+	var collection *flow.Collection // nil by default, which only represents system chunk
 	if schdp.CollectionID != flow.ZeroID {
 		collection, err = ch.collections.ByID(schdp.CollectionID)
 		if err != nil {

storage/store/chunk_data_packs_stored.go

@@ -10,6 +10,9 @@ import (
 	"github.com/onflow/flow-go/storage/operation"
 )
 
+// StoredChunkDataPacks represents persistent storage for chunk data packs.
+// It works with the reduced representation `StoredChunkDataPack` for chunk data packs,
+// where instead of the full collection data, only the collection's hash (ID) is contained.
 type StoredChunkDataPacks struct {
 	db        storage.DB
 	byIDCache *Cache[flow.Identifier, *storage.StoredChunkDataPack]

storage/store/chunk_data_packs_test.go

@@ -41,7 +41,7 @@ func TestChunkDataPacks_Store(t *testing.T) {
 				require.Equal(t, chunkDataPack, stored, "mismatched chunk data pack at index %d", i)
 			}
 
-			// Store again is idemopotent
+			// Store again is idempotent
 			storeFunc, err = chunkDataPackStore.Store(chunkDataPacks)
 			if err != nil {
 				return err