Skip to content

[LN Decentralization E] Differentiate between consensus Headers and cluster Headers #4204

@jordanschalm

Description

@jordanschalm

Problem Definition

Cluster and consensus instances share the same storage.Headers interface for retrieving headers. This means that, given a block ID, a component may read a cluster header or a consensus header without error. This makes bugs and attacks more likely by requiring components to check that stored headers are for the correct chain.

An additional source of confusion is that storage.Headers provides methods ByHeight and BlockIDByHeight which return only consensus blocks (the storage module is hooked up only to the consensus height index), but this storage.Headers interfaces is nevertheless used in contexts where it expects to only interact with cluster blocks. These methods should use the cluster height index when Headers is bound to a cluster chain ID.

Finally, Headers includes methods related to chunk ID indexing (IndexByChunkID, etc.). These are irrelevant to cluster header storage. (They are also relevant to all node roles but Execution Nodes, even for consensus headers.) Minimally these should fail in a predictable and documented way if used in the wrong context (cluster chain). We should consider splitting them into a separate EN-only module as well.

Proposed Solution

  • Bind a chain ID to each storage.Headers instance at construction.
  • Modify storage.Headers to operate only on headers with a particular chain ID. It should return a sentinel error if it is queried with a block ID for the wrong type of header
  • Modify storage.Headers to use the appropriate height index, depending on the chain ID the instance is bound to.

Definition of Done

  • retrieve cluster header from Headers instance bound to consensus chain ID -> should fail with documented sentinel error
  • retrieve consensus header from Headers instance bound to cluster chain ID -> should fail with documented sentinel error
  • look up header by block height from Headers instance bound to consensus and cluster chain ID -> should use the appropriate height index
  • Add test case: the builder should reject a transaction which has a reference block ID targeting a cluster block
  • Re-enable test TestExtend_WithReferenceBlockFromClusterChain, should pass
  • Address this TODO:
    // TODO ensure the reference block is part of the main chain https://github.com/onflow/flow-go/issues/4204
    _ = refBlock
  • Test that empty collections must have ReferenceBlockID=ZeroID

Metadata

Metadata

Assignees

Labels

BugSomething isn't workingPreserveStale Bot repellentProtocolTeam: Issues assigned to the Protocol Pillar.good first issueGood for newcomers

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions