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

Feature: EFM Recovery, Protocol HCU (core-contracts) #465

Open
wants to merge 174 commits into
base: master
Choose a base branch
from

Conversation

jordanschalm
Copy link
Member

@jordanschalm jordanschalm commented Oct 28, 2024

This PR merges the EFM Recovery feature branch to master.

Changes

  • Adds an index mapping to the DKG result data model
  • Adds the EpochRecover service event, which facilitates spork-less recovery from Epoch Fallback Mode
  • Adds the ProtocolStateVersionUpgrade service event, which facilitates upgrades to the Protocol State

It includes the following PRs, which have been separately reviewed and approved:

kc1116 and others added 21 commits July 28, 2024 21:50
…nflow/flow-core-contracts into khalil/5639-epoch-recovery-transaction
…ction

EpochRecover service event and transaction
…very-transaction

Revert "EpochRecover service event and transaction"
* define ResultSubmission struct

* annotate required changes

* setup test framework

* add preliminary new fields to EpochCommit

* add submission tracker sketch

* ResultSubmission tests

* ResultSubmission docs

* addSubmission method

* add tx todos

* add init test, validation on ResultSubmission init

* test comments

* basic SubmissionTracker tests

* update other contract methods to use SubmissionTracker

* add dummy values for EpochCommit event

* add docs

* add exceedsThreshold test

* update FlowEpoch to compile

* update transactions

* improve representation for "empty submissions"

* add test for empty submission

* test empty submission exceeds threshold

* fix go test: send submission

* wip

* remove '

* incredibly, the tests are passing

* note where tests still need updating

* consistent terminology: empty submission

* resolve some todos

- make funs view
- improve docs
- remove commented-out code

* fix empty submission dkg tests

* fix epoch tests

* rename publish admin script

was called "publish participant" but was actually publishing admin

* store as much as possible in EpochMetadata

* build static files

* make more functions view

* fix test

* rename isValidNilSubmission

since it is specifically enforcing the named Invariant (1), and checking
both empty and non-empty submissions.

* update error messages

* go generate

* Apply suggestions from code review

Co-authored-by: Tarak Ben Youssef <[email protected]>
Co-authored-by: Joshua Hannan <[email protected]>

* 2nd pass over error messages

* fix empty whiteboard message test

* improve post whiteboard message test

* rm dupe test

* address remaining todos in tests

* add preconditions for dkg enabled

* fix message, generate

* add documentation for test helper functions

---------

Co-authored-by: Tarak Ben Youssef <[email protected]>
Co-authored-by: Joshua Hannan <[email protected]>
* efm recovery transaction

* use provided recovery epoch counter as the source of truth

* add proper line indentation

* update assets

* define ResultSubmission struct

* annotate required changes

* setup test framework

* add preliminary new fields to EpochCommit

* add submission tracker sketch

* Apply suggestions from code review

Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Jordan Schalm <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jordan Schalm <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>

* adjust recover transaction logic

* move recoverEpochPreChecks call to emitEpochRecoverEvent

* fix broken sentence

* add comment describing unsafeAllowOverwrite

* use previous syntax for randomSource generation

* replace usage of numViewsInStakingAuction and numViewsInDKGPhase

- replace numViewsInStakingAuction with stakingEndView - startView
- don't accept numViewsInDKGPhase as a parameter read it from configurable epoch metadata

* add godoc for convertClusterQcsCdc

* add additional test cases

* ResultSubmission tests

* ResultSubmission docs

* addSubmission method

* add tx todos

* add init test, validation on ResultSubmission init

* test comments

* basic SubmissionTracker tests

* update other contract methods to use SubmissionTracker

* add dummy values for EpochCommit event

* add docs

* add exceedsThreshold test

* update FlowEpoch to compile

* update transactions

* improve representation for "empty submissions"

* add test for empty submission

* test empty submission exceeds threshold

* fix go test: send submission

* wip

* remove '

* incredibly, the tests are passing

* note where tests still need updating

* consistent terminology: empty submission

* resolve some todos

- make funs view
- improve docs
- remove commented-out code

* fix empty submission dkg tests

* fix epoch tests

* rename publish admin script

was called "publish participant" but was actually publishing admin

* store as much as possible in EpochMetadata

* build static files

* make more functions view

* fix test

* add dkgIdMapping

* rename isValidNilSubmission

since it is specifically enforcing the named Invariant (1), and checking
both empty and non-empty submissions.

* Updated epoch recovery tx to accept dkg group key as well as emitting correct EpochRecover service event

* update error messages

* go generate

* Apply suggestions from code review

Co-authored-by: Tarak Ben Youssef <[email protected]>
Co-authored-by: Joshua Hannan <[email protected]>

* 2nd pass over error messages

* fix empty whiteboard message test

* improve post whiteboard message test

* rm dupe test

* address remaining todos in tests

* fix whitespace

* Apply suggestions from code review

* Update contracts/epochs/FlowDKG.cdc

* random source generatation doc/postconditions

* recovery function docs

* recoverCurrentEpoch precondition

* recovery methods internal documentation

* add todos

* ClusterQCVoteData docs

* recover_epoch tx comments

* remove duplicated LN vote tag

* add todos

* make generate

* tweak godocs

* fixing and annotating tests

* all existing epoch recovery tests passing

* refactor happy path validation check test

* rm duplicate tests

* refactor overwrite validation tests

* clean up verifyEpochTotalRewardsPaid

* fix extra / in comments

* refactor success efm recovery tests

* rename and document rewards test case

* Apply suggestions from code review

Co-authored-by: Joshua Hannan <[email protected]>

* use stopEpochComponents

remove dupe code fragment

* comments

* validate epoch counter and phase in tests

* recoverCurrentEpoch: always start in stakign phase

* make generate

* totalRewards todos

* pass over remaining todos

* make generate

* update test

---------

Co-authored-by: Jordan Schalm <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Yurii Oleksyshyn <[email protected]>
Co-authored-by: Tarak Ben Youssef <[email protected]>
Co-authored-by: Joshua Hannan <[email protected]>
* add service event definition and doc

* move service event to version beacon

* add heartbeat/admin process for proto state version

accepting broken assets.go, will regenerate after cherry-pick is
complete

* add smoketest for setting protostate version

broken assets.go

* Apply suggestions from code review

* generate

broken assets.go

* update contract for cadence1

* update admin tx for cadence 1

* generate

* emit service event in governance transaction

Previously service events could only be emitted in the system chunk.
With onflow/flow-go#5828, we can directly emit
them in governance transactions.

* re-generate assets

* fix field reflection

* add context for where validation occurs and multiple emissions

* make generate
@jordanschalm jordanschalm marked this pull request as ready for review October 29, 2024 15:54
@joshuahannan
Copy link
Member

joshuahannan commented Oct 29, 2024

Haven't reviewed the files yet, but it seems kind of weird that there are 174 commits that aren't related to the changes, but probably not an issue. Probably because I merged from master a couple times instead of doing git pull. 😬

Also, what is the plan for actually introducing this upgrade to testnet and mainnet? Is it going to happen before the end of the year or sometime early next year?

@jordanschalm
Copy link
Member Author

it seems kind of weird that there are 174 commits, but probably not an issue.

Yeah - the PRs were all squash-merged. My guess is the individual commits also show up because the main feature branch was synced with master in between those merges. Not sure though.

what is the plan for actually introducing this upgrade to testnet and mainnet?

It's dependent on Benchnet testing and coordinating deployment with a "Protocol HCU". I'd like to be able to deploy this to Testnet and Mainnet this year, assuming the dependent tasks go smoothly.

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

Looks good to me! Lets wait to merge until we have the upgrade plan figured out and scheduled

@jordanschalm jordanschalm changed the title Feature: EFM Recovery Feature: EFM Recovery, VersionUpgrade event (core-contracts) Nov 29, 2024
@jordanschalm jordanschalm changed the title Feature: EFM Recovery, VersionUpgrade event (core-contracts) Feature: EFM Recovery, Protocol HCU (core-contracts) Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants