Add Array.ID() and OrderedMap.ID()
#321
Conversation
Currently Array.StorageID() and OrderedMap.StorageID() are used as identifier in client because storage IDs are guaranteed to unique. However, storage ID should be only used to retrieve slabs (registers) from storage. In the future, when Atree register inlining is implemented, some resources may not be stored in separate slabs, so they will not have storage IDs anymore. This commit adds ID() to uniquly identify Array and OrderedMap. For now, this is implemented as identical to StorageID in raw bytes ([16]bytes). In the future, this can be changed to be decoupled from storage ID completely. Clients should use ID instead of StorageID to identify atree composite values.
Codecov Report
@@ Coverage Diff @@
## main #321 +/- ##
==========================================
+ Coverage 64.55% 65.93% +1.38%
==========================================
Files 14 14
Lines 8019 8385 +366
==========================================
+ Hits 5177 5529 +352
+ Misses 2164 2120 -44
- Partials 678 736 +58
|
| @@ -31,6 +31,8 @@ import ( | |||
|
|
|||
| const LedgerBaseStorageSlabPrefix = "$" | |||
|
|
|||
| type ID [16]byte | |||
There was a problem hiding this comment.
Given there are now multiple IDs in the package, it might make sense to make this name a bit more descriptive to reduce the likelihood of confusing it with another ID.
Maybe StorableID?
In addition, maybe we can also rename StorageID to SlabID to make it clearer that it just identifies a slab, and cannot always identify something that is stored.
There was a problem hiding this comment.
@turbolent Good point! 👍. I'll open separate PR for renaming to keep changes in this PR simple.
In addition, maybe we can also rename StorageID to SlabID to make it clearer that it just identifies a slab, and cannot always identify something that is stored.
I will rename both function and type name from StorageID to SlabID.
While at it, I will also rename related function and variable names such as StorageIndex to SlabIndex.
Given there are now multiple IDs in the package, it might make sense to make this name a bit more descriptive to reduce the likelihood of confusing it with another ID.
Maybe StorableID?
Actually, ID is only for Array and OrderedMap, which are Value (not Storable).
So maybe I can rename ID to ValueID. For example,
func (*Array) ValueID() ValueID {}
func (*Array) SlabID() SlabID {}Thoughts?
|
Merging this and will open separate PR for suggested renaming (and other similar related renamings) to keep changes in this PR simple. |
Closes #320
Updates #296 #292 onflow/flow-go#1744
Description
Currently
Array.StorageID()andOrderedMap.StorageID()are used as identifier in client because storage IDs are guaranteed to unique. However, storage ID should be only used to retrieve slabs (registers) from storage.Also, when Atree register inlining is implemented in the future, some resources may not be stored in separate slabs, so they will not have storage IDs anymore.
This PR adds
ID()to uniquly identifyArrayandOrderedMap. For now, this is implemented to be identical toStorageID()in raw bytes ([16]bytes). In the future, this can be changed to be decoupled from storage ID completely.Clients should use
ID()instead ofStorageID()to identify Atree composite values.NEXT PR AS FOLLOWUP
A separate PR will be opened to do a lot of renamings such as:
StorageIDtoSlabID(thanks @turbolent!)IDtoValueIDStorageIndextoSlabIndexSo we'll have something like:
mainbranchFiles changedin the Github PR explorer