[BFT] Database migration for flow.DKGEndState
#6861
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l state. Added implementation for DKG end state migration between v1 and v2
… protocol state. Added implementation for DKG end state migration between v1 and v2" This reverts commit 4f59be0.
durkmurder
requested review from
AlexHentschel and
jordanschalm
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/efm-recovery #6861 +/- ##
========================================================
+ Coverage 41.72% 41.73% +0.01%
========================================================
Files 2033 2033
Lines 181418 181461 +43
========================================================
+ Hits 75690 75740 +50
+ Misses 99495 99487 -8
- Partials 6233 6234 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jordanschalm
reviewed
Jan 9, 2025
| // It reads already stored data by deprecated prefix and writes it to the new prefix with values converted to the new representation. | ||
| // TODO(EFM, #6794): This function is introduced to implement a backward-compatible upgrade from v1 to v2. | ||
| // Remove this once we complete the network upgrade. | ||
| func MigrateDKGEndStateFromV1() func(txn *badger.Txn) error { |
There was a problem hiding this comment.
Can we add some tests for this function? I think we should test the following:
- migrate each of the possible v1 end states
- should migrate correctly and delete v1 prefix
- migrate a database where no v1 prefixes are defined
6 tasks
jordanschalm
approved these changes
Jan 9, 2025
…kward-compatibility
Co-authored-by: Jordan Schalm <[email protected]>
AlexHentschel
approved these changes
Jan 11, 2025
|
|
||
| for _, op := range ops { | ||
| if err := op(txn); err != nil { | ||
| return err |
There was a problem hiding this comment.
Suggested change
| return err | |
| return fmt.Errorf("aborting conversion from DKG end states: %w", err) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#6786
Context
This PR implements a database migration for
flow.DKGEndState(enum that was used in protocol version v1). It is implemented by reading values under the old key(which is now deprecated), converting them to correct values and writing them back under the new key. The database migration is scheduled to happen at first launch of the consensus node. Since old keys are removed after the migration, all next runs will be no-op.In next release this logic can be removed as part of #6772