Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Malleability] ChunkDataPack #6720

Open
Tracked by #6647
UlyanaAndrukhiv opened this issue Nov 14, 2024 · 0 comments
Open
Tracked by #6647

[Malleability] ChunkDataPack #6720

UlyanaAndrukhiv opened this issue Nov 14, 2024 · 0 comments

Comments

@UlyanaAndrukhiv
Copy link
Contributor

UlyanaAndrukhiv commented Nov 14, 2024

ChunkDataPack Malleability

type ChunkDataPack struct {
ChunkID Identifier // ID of the chunk this data pack is for
StartState StateCommitment // commitment for starting state
Proof StorageProof // proof for all registers touched (read or written) during the chunk execution
Collection *Collection // collection executed in this chunk
// ExecutionDataRoot is the root data structure of an execution_data.BlockExecutionData.
// It contains the necessary information for a verification node to validate that the
// BlockExecutionData produced is valid.
ExecutionDataRoot BlockExecutionDataRoot
}

The current ChunkDataPack implementation uses the ID() method to return the ChunkID field as the unique identifier:

flow-go/model/flow/chunk.go

Lines 123 to 127 in 22daf5c

// ID returns the unique identifier for the concrete view, which is the ID of
// the chunk the view is for.
func (c *ChunkDataPack) ID() Identifier {
return c.ChunkID
}

This approach assumes that ChunkID is sufficient to uniquely identify a ChunkDataPack. While this method is straightforward, it introduces potential malleability concerns because it does not consider the integrity of the other fields within ChunkDataPack.
Specifically, fields like:

  • StartState,
  • Proof,
  • Collection,
  • ExecutionDataRoot,

are excluded from the identifier computation. This omission means that two ChunkDataPack instances with the same ChunkID but differing values for these fields would produce the same ID, which is incorrect.

Proposed Solution

Key Changes

  1. Update ID() :
    To address these concerns, the ID() method will be updated to compute the identifier based on the entire ChunkDataPack struct using the MakeID() function:
func (c *ChunkDataPack) ID() Identifier {
 body := struct {
		 ChunkID    Identifier    
 	         StartState StateCommitment 
 	         Proof      StorageProof    
 	         Collection Identifier
 	         ExecutionDataRoot BlockExecutionDataRoot 
	}{

		ChunkID: c.ChunkID,
		StartState: c.StartState,
		Proof: c.Proof,
		Collection: c.Collection.ID(),
                ExecutionDataRoot: c.ExecutionDataRoot,

	}
	return MakeID(body)
  
}

By encoding all fields, this approach guarantees that the resulting ID is malleability-resistant. Changes to any field of ChunkDataPack will produce a different Identifier.

All fields will be encoded in a way that guarantees consistency:

  • Primitive and byte array fields: fields like ChunkID, StartState, and Proof are encoded directly as raw bytes using RLP.
  • Custom structs: Collection -ID() for Collection struct needs to be updated first in [Malleability] Collection #6721. Recursive ExecutionDataRoot struct will be encoded as RLP lists of their public fields, capturing the entire structure consistently (encoding rlp rules).
  1. Remove unused function:
    During the revision process, it was identified that the function FromChunkID is unused and redundant. This function will be removed.
    Also Checksum() function will be removed.

Definition of Done

  1. The ChunkDataPack.ID() method has been updated using MakeID.
    The identifier computation has been verified to use RLP encoding or Fingerprint for all fields.
  2. The unused FromChunkID and Checksum() functions has been removed.
  3. Unit tests have been updated to validate the new behavior, ensuring identifiers change as expected when data is modified.
  4. Documentation and comments have been updated to reflect the changes and clarify the purpose of the ID() method.
@UlyanaAndrukhiv UlyanaAndrukhiv changed the title [Malleability C] ChunkDataPack [Malleability] ChunkDataPack Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant