onflow / flow-go

A fast, secure, and developer-friendly blockchain built to support the next generation of games, apps, and the digital assets that power them.

GNU Affero General Public License v3.0

531 stars 170 forks source link

[EFM] Update Consensus Committee EFM processing #5730

Open kc1116 opened 2 months ago

kc1116 commented 2 months ago

Context

The Consensus Committee maintains a cache for epochs by counter. Currently, the Consensus Committee performs explicit handling of EFM by inserting a special fallback epoch (see code).

Changes:

Remove EpochFallbackTriggered event handler
Add EpochExtended event handler
- Compute leader selection for views within extension.
- Persist new leader selection.
- If partial data exists for next epoch, remove it

⚠️ Required overall Behavior:

Do not return leader selection for a view until the epoch that view is within is committed.
Do not enter EFM while in phase EpochCommitted. Instead
- we store in the protocol state that we have left the happy path for Epoch transitions
- we still enter and progress through the already-committed epoch
- EFM extensions are added after the committed epoch, when we near the end of the committed Epoch
see https://github.com/onflow/flow-go/issues/5631 for more details
Otherwise we may give different answers for who the leader is for a given view, at different times!

Further Reading

### Depends on
- [ ] https://github.com/onflow/flow-go/issues/5717
- [ ] https://github.com/onflow/flow-go/issues/5724
- [ ] https://github.com/onflow/flow-go/issues/5726

AlexHentschel commented 2 months ago

ideally, the Consensus Committee would behave consistently:

it receives events for committed view ranges and always proceeds through those view ranges.
I would explicitly discourage including logic when or when not to enter in the Consensus Committee, because then we have logic in different places (committee and Epoch State Machine in the Protocol state).

In my opinion, the Epoch State Machine in the Protocol state should emit events accordingly, such that the consensus committee only need to follow the view ranges from the Epoch State Machine.

suggestion: I think we should re-frame this issue toRestructure Notifications emitted by the Epoch State Machine (Protocol state) to unambiguously specify committed view ranges for leader selection (Consensus Committee)

AlexHentschel commented 2 months ago

depends on https://github.com/onflow/flow-go/issues/5723

AlexHentschel commented 1 month ago

List of places in the code that need updating

committee.Consensus on l block finalization: https://github.com/onflow/flow-go/blob/58304e9c4c546ea26f7137dde5889778feae180b/consensus/hotstuff/committees/consensus_committee.go#L182-L189

Caution: This list is likely not exhaustive! Be on the lookout for additional spots in the codebase.