EFM Integration Test Part 2 #6424
Conversation
kc1116
requested review from
jordanschalm and
AlexHentschel
and removed request for
zhangchiqing
There was a problem hiding this comment.
The test itself looks good, but I think we need to move it or adjust some configs to make sure it will run when we want 👇.
The integration tests are run in CI jobs grouped by package. The new epoch recovery test is in a new package integration/tests/epochs/recover_epoch, but there is no corresponding CI configuration, so I don't think the new test will actually run in CI, or when we run eg. make epochs-tests.
The easiest fix is to just add the new test to an existing package (cohort1 or cohort2).
If it needs to be in a new package, then:
durkmurder
changed the base branch from
master
to
yurii/6495-convert-epoch-recover
…/onflow/flow-go into khalil/6164-efm-integration-test-part2
…/onflow/flow-go into khalil/6164-efm-integration-test-part2
2. Updated logic for converting Cadence structures. 3. Updated consensus logic to inject DKG private key from previous epoch in case of epoch recovery. 4. Added logging for errors in FVM Event Emitter. 5. !Temporary! disabled a check in storage/badger/dkg_state.go
durkmurder
requested review from
ramtinms,
janezpodhostnik,
peterargue and
durkmurder
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/efm-recovery #6424 +/- ##
========================================================
- Coverage 41.72% 41.71% -0.01%
========================================================
Files 2019 2030 +11
Lines 179401 180462 +1061
========================================================
+ Hits 74849 75279 +430
- Misses 98395 98989 +594
- Partials 6157 6194 +37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
… of epoch recovery
… implementation of dedicated DKG storage for epoch recovery
EFM Recovery: Fixes from Benchnet Testing
Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>
integration/tests/epochs/cohort2/epoch_recover_from_efm_test.go
Outdated
Show resolved
Hide resolved
integration/tests/epochs/cohort2/epoch_recover_from_efm_test.go
Outdated
Show resolved
Hide resolved
| // - node is in epoch committed phase | ||
| // - exception is thrown when trying to check if there is a safe beacon key for the next epoch | ||
| // This is an unexpected error and should be propagated to the caller. | ||
| func (s *BeaconKeyRecoverySuite) TestNewBeaconKeyRecovery_NextEpochRetrieveMyBeaconPrivateKeyException() { |
There was a problem hiding this comment.
I would recommend to include another test like this one, with the only difference that
flow-go/module/dkg/recovery.go
Line 112 in eb968f8
storage.ErrNotFound. In this case, we would expect the code to proceed, right?
There was a problem hiding this comment.
I think the case what you are referring to is covered by this test case: https://github.com/onflow/flow-go/pull/6424/files#diff-ee78b25905ed7b895f6e39017067adbdfa32a628d62fba62e04073ebb7833e40R321
Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>
Co-authored-by: Alexander Hentschel <[email protected]>
| // This occurs only for epochs which are entered through the EFM Recovery process ([flow.EpochRecover] service event). | ||
| DKGEndStateRecovered | ||
| // RandomBeaconKeyRecovered - this node has recovered its beacon key from a previous epoch. | ||
| // This occurs only for epochs which are entered through the EFM Recovery process (`flow.EpochRecover` service event). |
There was a problem hiding this comment.
Minor point on the formatting:
- Wrapping code references in
[]formats it as a link to the type in Godoc - Wrapping code references in
``doesn't have any formatting support (Markdown-style code spans are unsupported).
There was a problem hiding this comment.
Good to know, thanks. I will make a PR to feature/efm-recovery to fix any of those.
Co-authored-by: Jordan Schalm <[email protected]>
This PR updates the current EFM integration test and ensures that the network can successfully transition into recovery epoch after processing recover epoch governance transaction.
ref: #6164