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

Remove duplicated top-level EncodableSnapshot fields #5884

Merged
merged 15 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions consensus/integration/epoch_test.go
jordanschalm marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ func withNextEpoch(

// convert to encodable representation for simple modification
encodableSnapshot := snapshot.Encodable()
rootResult, rootSeal, err := snapshot.SealedResult()
require.NoError(t, err)

rootProtocolState := encodableSnapshot.SealingSegment.LatestProtocolStateEntry()
epochProtocolState := rootProtocolState.EpochEntry
Expand Down Expand Up @@ -236,7 +238,7 @@ func withNextEpoch(
ActiveIdentities: flow.DynamicIdentityEntryListFromIdentities(nextEpochIdentities),
}
// Re-construct epoch protocol state with modified events (constructs ActiveIdentity fields)
epochProtocolState, err := flow.NewRichProtocolStateEntry(
epochProtocolState, err = flow.NewRichProtocolStateEntry(
epochProtocolState.ProtocolStateEntry,
epochProtocolState.PreviousEpochSetup, epochProtocolState.PreviousEpochCommit,
currEpochSetup, currEpochCommit,
Expand All @@ -258,16 +260,18 @@ func withNextEpoch(
}

// Since we modified the root protocol state, we need to update the root block's ProtocolStateID field.
// rootBlock is a pointer, so mutations apply to Snapshot
rootBlock := encodableSnapshot.SealingSegment.Blocks[0]
rootBlockPayload := rootBlock.Payload
rootBlockPayload.ProtocolStateID = rootKVStore.ID()
rootBlock.SetPayload(*rootBlockPayload)
// Since we changed the root block, we need to update the QC, root result, and root seal.
encodableSnapshot.LatestResult.BlockID = rootBlock.ID()
encodableSnapshot.LatestSeal.ResultID = encodableSnapshot.LatestResult.ID()
encodableSnapshot.LatestSeal.BlockID = rootBlock.ID()
// rootResult and rootSeal are pointers, so mutations apply to Snapshot
rootResult.BlockID = rootBlock.ID()
rootSeal.ResultID = rootResult.ID()
rootSeal.BlockID = rootBlock.ID()
encodableSnapshot.SealingSegment.LatestSeals = map[flow.Identifier]flow.Identifier{
rootBlock.ID(): encodableSnapshot.LatestSeal.ID(),
rootBlock.ID(): rootSeal.ID(),
}
encodableSnapshot.QuorumCertificate = createQC(rootBlock)

Expand Down
4 changes: 2 additions & 2 deletions state/protocol/badger/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestUnknownReferenceBlock(t *testing.T) {

util.RunWithFullProtocolState(t, rootSnapshot, func(db *badger.DB, state *bprotocol.ParticipantState) {
// build some finalized non-root blocks (heights 101-110)
head := unittest.BlockWithParentFixture(rootSnapshot.Encodable().Head)
head := unittest.BlockWithParentFixture(rootSnapshot.Encodable().Head())
head.SetPayload(unittest.PayloadFixture(unittest.WithProtocolStateID(rootProtocolStateID)))
buildFinalizedBlock(t, state, head)

Expand Down Expand Up @@ -808,7 +808,7 @@ func TestSealingSegment_FailureCases(t *testing.T) {
// Here, we want to specifically test correct handling of the edge case, where a block exists in storage
// that has _lower height_ than the node's local root block. Such blocks are typically contained in the
// bootstrapping data, such that all entities referenced in the local root block can be resolved.
// Is is possible to retrieve blocks that are lower than the local root block from storage, directly
// It is possible to retrieve blocks that are lower than the local root block from storage, directly
// via their ID. Despite these blocks existing in storage, SealingSegment construction should be
// because the known history is potentially insufficient when going below the root block.
t.Run("sealing segment from block below local state root", func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions state/protocol/badger/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestBootstrap_EpochHeightBoundaries(t *testing.T) {
t.Parallel()
// start with a regular post-spork root snapshot
rootSnapshot := unittest.RootSnapshotFixture(unittest.CompleteIdentitySet())
epoch1FirstHeight := rootSnapshot.Encodable().Head.Height
epoch1FirstHeight := rootSnapshot.Encodable().Head().Height

// For the spork root snapshot, only the first height of the root epoch should be indexed.
// [x]
Expand Down Expand Up @@ -609,7 +609,7 @@ func TestBootstrap_SealMismatch(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(unittest.CompleteIdentitySet())
// convert to encodable to easily modify snapshot
encodable := rootSnapshot.Encodable()
encodable.LatestSeal.BlockID = unittest.IdentifierFixture()
encodable.LatestSeal().BlockID = unittest.IdentifierFixture()

bootstrap(t, rootSnapshot, func(state *bprotocol.State, err error) {
assert.Error(t, err)
Expand All @@ -620,7 +620,7 @@ func TestBootstrap_SealMismatch(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(unittest.CompleteIdentitySet())
// convert to encodable to easily modify snapshot
encodable := rootSnapshot.Encodable()
encodable.LatestResult.BlockID = unittest.IdentifierFixture()
encodable.LatestSealedResult().BlockID = unittest.IdentifierFixture()

bootstrap(t, rootSnapshot, func(state *bprotocol.State, err error) {
assert.Error(t, err)
Expand All @@ -631,7 +631,7 @@ func TestBootstrap_SealMismatch(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(unittest.CompleteIdentitySet())
// convert to encodable to easily modify snapshot
encodable := rootSnapshot.Encodable()
encodable.LatestSeal.ResultID = unittest.IdentifierFixture()
encodable.LatestSeal().ResultID = unittest.IdentifierFixture()

bootstrap(t, rootSnapshot, func(state *bprotocol.State, err error) {
assert.Error(t, err)
Expand Down
8 changes: 4 additions & 4 deletions state/protocol/badger/validity.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ func IsValidRootSnapshot(snap protocol.Snapshot, verifyResultID bool) error {
if err != nil {
return fmt.Errorf("could not get sealing segment: %w", err)
}
result, seal, err := snap.SealedResult()
err = segment.Validate()
if err != nil {
return fmt.Errorf("could not latest sealed result: %w", err)
return fmt.Errorf("invalid root sealing segment: %w", err)
}

err = segment.Validate()
result, seal, err := snap.SealedResult()
if err != nil {
return fmt.Errorf("invalid root sealing segment: %w", err)
return fmt.Errorf("could not latest sealed result: %w", err)
}

highest := segment.Highest() // reference block of the snapshot
Expand Down
8 changes: 4 additions & 4 deletions state/protocol/badger/validity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ func TestEntityExpirySnapshotValidation(t *testing.T) {
})
t.Run("not-enough-history", func(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(participants)
rootSnapshot.Encodable().Head.Height += 10 // advance height to be not spork root snapshot
rootSnapshot.Encodable().Head().Height += 10 // advance height to be not spork root snapshot
err := ValidRootSnapshotContainsEntityExpiryRange(rootSnapshot)
require.Error(t, err)
})
t.Run("enough-history-spork-just-started", func(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(participants)
// advance height to be not spork root snapshot, but still lower than transaction expiry
rootSnapshot.Encodable().Head.Height += flow.DefaultTransactionExpiry / 2
rootSnapshot.Encodable().Head().Height += flow.DefaultTransactionExpiry / 2
// add blocks to sealing segment
rootSnapshot.Encodable().SealingSegment.ExtraBlocks = unittest.BlockFixtures(int(flow.DefaultTransactionExpiry / 2))
err := ValidRootSnapshotContainsEntityExpiryRange(rootSnapshot)
Expand All @@ -40,7 +40,7 @@ func TestEntityExpirySnapshotValidation(t *testing.T) {
t.Run("enough-history-long-spork", func(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(participants)
// advance height to be not spork root snapshot
rootSnapshot.Encodable().Head.Height += flow.DefaultTransactionExpiry * 2
rootSnapshot.Encodable().Head().Height += flow.DefaultTransactionExpiry * 2
// add blocks to sealing segment
rootSnapshot.Encodable().SealingSegment.ExtraBlocks = unittest.BlockFixtures(int(flow.DefaultTransactionExpiry) - 1)
err := ValidRootSnapshotContainsEntityExpiryRange(rootSnapshot)
Expand All @@ -49,7 +49,7 @@ func TestEntityExpirySnapshotValidation(t *testing.T) {
t.Run("more-history-than-needed", func(t *testing.T) {
rootSnapshot := unittest.RootSnapshotFixture(participants)
// advance height to be not spork root snapshot
rootSnapshot.Encodable().Head.Height += flow.DefaultTransactionExpiry * 2
rootSnapshot.Encodable().Head().Height += flow.DefaultTransactionExpiry * 2
// add blocks to sealing segment
rootSnapshot.Encodable().SealingSegment.ExtraBlocks = unittest.BlockFixtures(flow.DefaultTransactionExpiry * 2)
err := ValidRootSnapshotContainsEntityExpiryRange(rootSnapshot)
Expand Down
12 changes: 0 additions & 12 deletions state/protocol/inmem/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@ func FromSnapshot(from protocol.Snapshot) (*Snapshot, error) {
)

// convert top-level fields
snap.Head, err = from.Head()
if err != nil {
return nil, fmt.Errorf("could not get head: %w", err)
}
snap.LatestResult, snap.LatestSeal, err = from.SealedResult()
if err != nil {
return nil, fmt.Errorf("could not get seal: %w", err)
}

snap.SealingSegment, err = from.SealingSegment()
if err != nil {
return nil, fmt.Errorf("could not get sealing segment: %w", err)
Expand Down Expand Up @@ -218,9 +209,6 @@ func SnapshotFromBootstrapStateWithParams(
}

snap := SnapshotFromEncodable(EncodableSnapshot{
Head: root.Header,
LatestSeal: seal,
LatestResult: result,
SealingSegment: &flow.SealingSegment{
Blocks: []*flow.Block{root},
ExecutionResults: flow.ExecutionResultList{result},
Expand Down
67 changes: 64 additions & 3 deletions state/protocol/inmem/encodable.go
jordanschalm marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,76 @@ import (

// EncodableSnapshot is the encoding format for protocol.Snapshot
type EncodableSnapshot struct {
Head *flow.Header
LatestSeal *flow.Seal // TODO replace with same info from sealing segment
LatestResult *flow.ExecutionResult // TODO replace with same info from sealing segment
SealingSegment *flow.SealingSegment
QuorumCertificate *flow.QuorumCertificate
Params EncodableParams
SealedVersionBeacon *flow.SealedVersionBeacon
}

// Head returns the latest finalized header of the Snapshot, which is the block
// in the sealing segment with the greatest Height.
// The EncodableSnapshot receiver must be correctly formed.
func (snap EncodableSnapshot) Head() *flow.Header {
return snap.SealingSegment.Highest().Header
}

// LatestSeal returns the latest seal of the Snapshot. This is the seal
// for the block with the greatest height, of all seals in the Snapshot.
// The EncodableSnapshot receiver must be correctly formed.
func (snap EncodableSnapshot) LatestSeal() *flow.Seal {
head := snap.Head()
latestSealID := snap.SealingSegment.LatestSeals[head.ID()]

// CASE 1: The spork root block is the latest sealed block.
jordanschalm marked this conversation as resolved.
Show resolved Hide resolved
// By protocol definition, FirstSeal seals the spork root block.
if snap.SealingSegment.FirstSeal != nil && snap.SealingSegment.FirstSeal.ID() == latestSealID {
return snap.SealingSegment.FirstSeal
}

// CASE 2: For any other snapshot, the latest seal must be in a block payload.
jordanschalm marked this conversation as resolved.
Show resolved Hide resolved
// Since seals are included in increasing height order, the latest seal must be in the
// first block (by height descending) which contains any seals.
for i := len(snap.SealingSegment.Blocks) - 1; i >= 0; i-- {
block := snap.SealingSegment.Blocks[i]
for _, seal := range block.Payload.Seals {
if seal.ID() == latestSealID {
return seal
}
}
if len(block.Payload.Seals) > 0 {
// We encountered a block with some seals, but not the latest seal.
// This can only occur in a structurally invalid SealingSegment.
panic("LatestSeal: sanity check failed: no latest seal")
}
}
// Correctly formatted sealing segments must contain latest seal.
panic("LatestSeal: unreachable for correctly formatted sealing segments")
}

// LatestSealedResult returns the latest sealed result of the Snapshot. This is the result which is sealed by LatestSeal.
// The EncodableSnapshot receiver must be correctly formed.
func (snap EncodableSnapshot) LatestSealedResult() *flow.ExecutionResult {
latestSeal := snap.LatestSeal()

// For both spork root and mid-spork snapshots, the latest sealing result must
// either appear in a block payload or in the ExecutionResults field.
for i := len(snap.SealingSegment.Blocks) - 1; i >= 0; i-- {
block := snap.SealingSegment.Blocks[i]
for _, result := range block.Payload.Results {
if latestSeal.ResultID == result.ID() {
return result
}
}
}
for _, result := range snap.SealingSegment.ExecutionResults {
if latestSeal.ResultID == result.ID() {
return result
}
}
// Correctly formatted sealing segments must contain latest result.
panic("LatestSealedResult: unreachable for correctly formatted sealing segments")
}

// EncodableDKG is the encoding format for protocol.DKG
type EncodableDKG struct {
GroupKey encodable.RandomBeaconPubKey
Expand Down
2 changes: 1 addition & 1 deletion state/protocol/inmem/encodable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ func TestEncodeDecode(t *testing.T) {
require.NoError(t, err)

// check that the computed and stored result IDs are consistent
decodedResult, decodedSeal := decodedSnapshot.LatestResult, decodedSnapshot.LatestSeal
decodedResult, decodedSeal := decodedSnapshot.LatestSealedResult(), decodedSnapshot.LatestSeal()
assert.Equal(t, decodedResult.ID(), decodedSeal.ResultID)
}
6 changes: 3 additions & 3 deletions state/protocol/inmem/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Snapshot struct {
var _ protocol.Snapshot = (*Snapshot)(nil)

func (s Snapshot) Head() (*flow.Header, error) {
return s.enc.Head, nil
return s.enc.Head(), nil
}

func (s Snapshot) QuorumCertificate() (*flow.QuorumCertificate, error) {
Expand Down Expand Up @@ -50,11 +50,11 @@ func (s Snapshot) Identity(nodeID flow.Identifier) (*flow.Identity, error) {
}

func (s Snapshot) Commit() (flow.StateCommitment, error) {
return s.enc.LatestSeal.FinalState, nil
return s.enc.LatestSeal().FinalState, nil
}

func (s Snapshot) SealedResult() (*flow.ExecutionResult, *flow.Seal, error) {
return s.enc.LatestResult, s.enc.LatestSeal, nil
return s.enc.LatestSealedResult(), s.enc.LatestSeal(), nil
}

func (s Snapshot) SealingSegment() (*flow.SealingSegment, error) {
Expand Down
2 changes: 1 addition & 1 deletion state/protocol/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestIsSporkRootSnapshot(t *testing.T) {

t.Run("other snapshot", func(t *testing.T) {
snapshot := unittest.RootSnapshotFixture(unittest.IdentityListFixture(10, unittest.WithAllRoles()))
snapshot.Encodable().Head.Height += 1 // modify head height to break equivalence with spork root block height
snapshot.Encodable().Head().Height += 1 // modify head height to break equivalence with spork root block height
isSporkRoot, err := protocol.IsSporkRootSnapshot(snapshot)
require.NoError(t, err)
assert.False(t, isSporkRoot)
Expand Down
19 changes: 0 additions & 19 deletions utils/unittest/staker.go
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained only unused types

This file was deleted.

Loading