Skip to content

Commit

Permalink
revert sealing segment length changes from #5673 (#6086)
Browse files Browse the repository at this point in the history
  • Loading branch information
jordanschalm authored Jun 12, 2024
1 parent 5acf995 commit a5c7ade
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 19 deletions.
2 changes: 1 addition & 1 deletion model/flow/sealing_segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type SealingSegment struct {
// (see sealing_segment.md for details):
// (ii) All blocks that are sealed by `head`. This is relevant if `head` contains _multiple_ seals.
// (iii) The sealing segment holds the history of all non-expired collection guarantees, i.e.
// limitHeight := max(blockSealedAtHead.Height - flow.ExtraBlocksInRootSealingSegment, SporkRootBlockHeight)
// limitHeight := max(blockSealedAtHead.Height - flow.DefaultTransactionExpiry, SporkRootBlockHeight)
// where blockSealedAtHead is the block sealed by `head` block.
// (Potentially longer history is permitted)
ExtraBlocks []*Block
Expand Down
10 changes: 5 additions & 5 deletions model/flow/sealing_segment.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ ExtraBlocks []*Block
of the sealed block. The seals don't contain the block's height information, hence we need to resolve the block.

**Extended history to check for duplicated collection guarantees in blocks** is required by nodes that _validate_ block
payloads (e.g. consensus nodes). Also Access Nodes require these blocks. Collections expire after `flow.ExtraBlocksInRootSealingSegment` blocks.
Hence, we desire a history of `flow.ExtraBlocksInRootSealingSegment` blocks. However, there is the edge case of a recent spork (or genesis),
where the history is simply less that `flow.ExtraBlocksInRootSealingSegment`.
payloads (e.g. consensus nodes). Also Access Nodes require these blocks. Collections expire after `flow.DefaultTransactionExpiry` blocks.
Hence, we desire a history of `flow.DefaultTransactionExpiry` blocks. However, there is the edge case of a recent spork (or genesis),
where the history is simply less that `flow.DefaultTransactionExpiry`.

### Formal definition

Expand All @@ -98,7 +98,7 @@ The descriptions from the previous section can be formalized as follows
* (ii) All blocks that are sealed by `head`. This is relevant if `head` contains _multiple_ seals.
* (iii) The sealing segment should contain the history back to (including):
```
limitHeight := max(blockSealedAtHead.Height - flow.ExtraBlocksInRootSealingSegment, SporkRootBlockHeight)
limitHeight := max(blockSealedAtHead.Height - flow.DefaultTransactionExpiry, SporkRootBlockHeight)
```
where blockSealedAtHead is the block sealed by `head` block.
Note that all three conditions have to be satisfied by a sealing segment. Therefore, it must contain the longest history
Expand All @@ -110,7 +110,7 @@ additional blocks for (ii) and optionally (iii) are contained in as `SealingSegm

Condition (i) and (ii) are necessary for the sealing segment for _any node_. In contrast, (iii) is
necessary to bootstrap nodes that _validate_ block payloads (e.g. consensus nodes), to verify that
collection guarantees are not duplicated (collections expire after `flow.ExtraBlocksInRootSealingSegment` blocks).
collection guarantees are not duplicated (collections expire after `flow.DefaultTransactionExpiry` blocks).

## Special case: Root Sealing Segment

Expand Down
8 changes: 4 additions & 4 deletions state/protocol/badger/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (s *Snapshot) SealingSegment() (*flow.SealingSegment, error) {
// This is relevant if `head` does not contain any seals.
// (ii) All blocks that are sealed by `head`. This is relevant if head` contains _multiple_ seals.
// (iii) The sealing segment should contain the history back to (including):
// limitHeight := max(blockSealedAtHead.Height - flow.ExtraBlocksInRootSealingSegment, SporkRootBlockHeight)
// limitHeight := max(blockSealedAtHead.Height - flow.DefaultTransactionExpiry, SporkRootBlockHeight)
// Per convention, we include the blocks for (i) in the `SealingSegment.Blocks`, while the
// additional blocks for (ii) and optionally (iii) are contained in as `SealingSegment.ExtraBlocks`.
head, err := s.state.blocks.ByID(s.blockID)
Expand Down Expand Up @@ -299,10 +299,10 @@ func (s *Snapshot) SealingSegment() (*flow.SealingSegment, error) {
}

// STEP (iii): extended history to allow checking for duplicated collections, i.e.
// limitHeight = max(blockSealedAtHead.Height - flow.ExtraBlocksInRootSealingSegment, SporkRootBlockHeight)
// limitHeight = max(blockSealedAtHead.Height - flow.DefaultTransactionExpiry, SporkRootBlockHeight)
limitHeight := s.state.sporkRootBlockHeight
if blockSealedAtHead.Height > s.state.sporkRootBlockHeight+flow.ExtraBlocksInRootSealingSegment {
limitHeight = blockSealedAtHead.Height - flow.ExtraBlocksInRootSealingSegment
if blockSealedAtHead.Height > s.state.sporkRootBlockHeight+flow.DefaultTransactionExpiry {
limitHeight = blockSealedAtHead.Height - flow.DefaultTransactionExpiry
}

// As we have to satisfy (ii) _and_ (iii), we have to take the longest history, i.e. the lowest height.
Expand Down
18 changes: 9 additions & 9 deletions state/protocol/badger/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,17 +589,17 @@ func TestSealingSegment(t *testing.T) {
})
})

// Root <- B1 <- B2 <- ... <- B700(Seal_B689)
// Expected sealing segment: [B689, B690], Extra blocks: [B98, B99, ..., B688]
// where ExtraBlocksInRootSealingSegment = 590
t.Run("test extra blocks contain exactly ExtraBlocksInRootSealingSegment number of blocks below the sealed block", func(t *testing.T) {
// Root <- B1 <- B2 <- ... <- B700(Seal_B699)
// Expected sealing segment: [B699, B700], Extra blocks: [B98, B99, ..., B698]
// where DefaultTransactionExpiry = 600
t.Run("test extra blocks contain exactly DefaultTransactionExpiry number of blocks below the sealed block", func(t *testing.T) {
util.RunWithFollowerProtocolState(t, rootSnapshot, func(db *badger.DB, state *bprotocol.FollowerState) {
root := unittest.BlockWithParentFixture(head)
buildFinalizedBlock(t, state, root)

blocks := make([]*flow.Block, 0, flow.ExtraBlocksInRootSealingSegment+3)
blocks := make([]*flow.Block, 0, flow.DefaultTransactionExpiry+3)
parent := root
for i := 0; i < flow.ExtraBlocksInRootSealingSegment+1; i++ {
for i := 0; i < flow.DefaultTransactionExpiry+1; i++ {
next := unittest.BlockWithParentFixture(parent.Header)
next.Header.View = next.Header.Height + 1 // set view so we are still in the same epoch
buildFinalizedBlock(t, state, next)
Expand Down Expand Up @@ -634,9 +634,9 @@ func TestSealingSegment(t *testing.T) {
assert.Equal(t, lastBlock.Header, segment.Finalized().Header)
assert.Equal(t, lastSealedBlock.Header, segment.Sealed().Header)

// there are ExtraBlocksInRootSealingSegment number of blocks in total
unittest.AssertEqualBlocksLenAndOrder(t, blocks[:flow.ExtraBlocksInRootSealingSegment], segment.ExtraBlocks)
assert.Len(t, segment.ExtraBlocks, flow.ExtraBlocksInRootSealingSegment)
// there are DefaultTransactionExpiry number of blocks in total
unittest.AssertEqualBlocksLenAndOrder(t, blocks[:flow.DefaultTransactionExpiry], segment.ExtraBlocks)
assert.Len(t, segment.ExtraBlocks, flow.DefaultTransactionExpiry)
assertSealingSegmentBlocksQueryableAfterBootstrap(t, snapshot)

})
Expand Down

0 comments on commit a5c7ade

Please sign in to comment.