The SealingSegment has an important role in safely bootstrapping a node. Specifically a sealing segment is a section (one or more blocks) of the finalized chain. In essence, the sealing segment is the only trusted input the node software needs; from there on the software can cryptographically verify legitimacy of any extension.

While the current implementation of SealingSegment works, the code has grown over time and is by now relatively complex, because it covers a two distinct cases in a monolithic implementation. In addition, there is a builder to nicely encapsulate the complexity from higher level code. However, the builder adds complexity to the inner working of the SealingSegment as well, and partially duplicates verification logic. Lastly, the data that SealingSegment internally stores is partially redundant, which adds unnecessary complexity to the validation step.

Goal

1. The goal of this issue is to increaser modularization of the code. The protocol handles the root blocks (including the genesis block) slightly differently compared later blocks. (Specifically, the root block is the only block, where the sealed and finalized block of this height is the block itself.) Therefore, the sealing segment has slightly different information when it represents a root block vs a section from the later chain. Also, validity conditions are different. While the protocol differentiates both cases, the implementation mixes them.

   Suggestions

   • Generate a separate struct for holding unverified segment data, e.g. UnverifiedSealingSegmentData
   • The builder could internally create such a struct and progressively populate it with data, instead of replicating the fields:

     flow-go/model/flow/sealing_segment.go

     Lines 185 to 188 in 569ecdf

     	blocks []*Block
     	results []*ExecutionResult
     	latestSeals map[Identifier]Identifier
     	firstSeal *Seal

   • Change to meaning of SealingSegment to now represents a validated version of SealingSegment data.
   • UnverifiedSealingSegmentData has only one method to turn the unverified data into a validated SealingSegment:

     // Validate sanity-checks the SealingSegment's data, pre-generates some higher level 
     // data and returns the data wrapped into a SealingSegment instance.  
     func (segment *UnverifiedSealingSegmentData) Validate() (SealingSegment, error) 

   • during the validation step, we can generate the content SealingSegment.LatestSeals, but it does not need to be part of UnverifiedSealingSegmentData

2. I think the implementation differentiation between root block and multi-block is now narrowly tied to the number of blocks. The protocol definition is more general in subtle ways, which results in edge cases which the current implementation does not cover:

   • Consider the scenario where R is the root block, additional three blocks (A, B, C) have been created and finalized, but not sealed. While it is an edge case in practise, it is nevertheless a valid instance of a sealing segment according to protocol rules. However, the current implementation cannot handle this case.

   • The key differentiating factor for the root block is that it is the only block, where the sealed and finalized block of this height is the block itself. (we call this "self-sealing"). Requiring its view to be 0 is mere convention.

   • this logic could be improved as follows:

     • It walks from the highest lock backwards and checks each block whether it contains seals.
     • If the block contains seals, the highest seal must be for the lowest block of the segment (otherwise, the segment is violating the minimality criterion). caution, I think the implementation is slightly to weak in that it does not enforce the minimality. Specifically, I feel this particular case here is not recognized as invalid:

       flow-go/model/flow/sealing_segment.go

       Line 294 in 569ecdf

       // A <- B <- C <- D(seal_A,seal_B) ==> invalid, because latest seal is B, but lowest block is A

     • if we have passed all blocks without encountering any seals, then the sealing segment is valid if and only if the lowest block is a root block:
       • FirstSeal is for the lowest block.
       • lowest has view 0 (more specifically rootSegmentBlockView)

     Thereby, our implementation would allow for any block (even blocks very close to the root block) to generate sealing segments. Furthermore, it consolidates the handling of a single root block with the logic of the multi-block sealing segments.

3. Write up markdown file:

   • explaining the handling of the root block
   • include illustrations
   • document minimality requirement (We rely on this minimality requirement here. Otherwise, our data base is incompletely initialized).