[Dynamic Protocol State] badger.Snapshot uses dynamic protocol state
#4625
Conversation
…thub.com/onflow/flow-go into yurii/6803-snapshot-updates
…thub.com/onflow/flow-go into yurii/6803-snapshot-updates
| // TODO: CAUTION SHORTCUT | ||
| // we retrieve identities based on the initial identity table from the EpochSetup | ||
| // event here -- this will need revision to support mid-epoch identity changes | ||
| // once slashing is implemented |
| @@ -64,7 +64,6 @@ const ( | |||
| codePayloadSeals = 53 // index mapping block ID to payload seals | |||
| codeCollectionBlock = 54 // index mapping collection ID to block ID | |||
| codeOwnBlockReceipt = 55 // index mapping block ID to execution receipt ID for execution nodes | |||
There was a problem hiding this comment.
| codeOwnBlockReceipt = 55 // index mapping block ID to execution receipt ID for execution nodes | |
| codeOwnBlockReceipt = 55 // index mapping block ID to execution receipt ID for execution nodes | |
| // code 56 previously used for epoch status index, now deprecated |
may be useful to leave a tombstone explaining the missing code
There was a problem hiding this comment.
What is the rational behind leaving a tombstone? Just out of curiosity.
My default would have been to decrement the values of the following prefixes each by 1:
flow-go/storage/badger/operation/prefix.go
Lines 67 to 69 in 780880a
Thereby, we close the gap. We have extended, shortened or replaced prefix conventions previously, with
prefix.go only representing the latest version (and changes being incompatible). I wonder why we would do it differently now? I don't think it is tractable for us to keep a useful documentation of past usages for certain numerical values in this file.
There was a problem hiding this comment.
decrement the values of the following prefixes each by 1
That works too -- mainly just didn't want to leave a gap with no explanation. I was hesitant to change the key prefixes, but you're right we've done that before, and don't have backward-compatibility anyway.
There was a problem hiding this comment.
I've updated it to be a continuous range
…nflow/flow-go into yurii/6803-snapshot-updates
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## feature/dynamic-protocol-state #4625 +/- ##
===================================================================
+ Coverage 52.07% 64.71% +12.63%
===================================================================
Files 740 184 -556
Lines 66757 19496 -47261
===================================================================
- Hits 34765 12617 -22148
+ Misses 29342 5913 -23429
+ Partials 2650 966 -1684
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
https://github.com/dapperlabs/flow-go/issues/6803
Context
This PR updates
Snapshotimplementation to use dynamic protocol state to query identities and epochs.badger.Snapshotno more determines identity table using epoch setup events it just relies on protocol state, same happens for epoch queries.Updated
inmem.EncodableSnapshotto carry only needed information without redundancies, removed identities and epoch phase since they can be derived from dynamic protocol state. Finally removedEpochStatusesas it's no more needed, updated related badger util to work with protocol state instead.Making those changes allowed to simplify our code base, tests became simpler.