Add `Array.ID()` and `OrderedMap.ID()`

fxamacker commented 1 year ago

Closes #320 Updates #296 #292 https://github.com/onflow/flow-go/issues/1744

Description

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.

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 identify Array and OrderedMap. For now, this is implemented to be 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.

NEXT PR AS FOLLOWUP

A separate PR will be opened to do a lot of renamings such as:

StorageID to SlabID (thanks @turbolent!)
ID to ValueID
StorageIndex to SlabIndex
and etc.

So we'll have something like:

func (*Array) ValueID() ValueID {}
func (*Array) SlabID() SlabID {}

[x] Targeted PR against main branch
[x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work
[x] Code follows the standards mentioned here
[x] Updated relevant documentation
[x] Re-reviewed Files changed in the Github PR explorer
[x] Added appropriate labels

codecov-commenter commented 1 year ago

Codecov Report

Merging #321 (116c959) into main (a996413) will increase coverage by 1.38%. The diff coverage is 100.00%.

@@            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

Impacted Files	Coverage Δ
storage.go	`78.85% <ø> (+4.33%)`	:arrow_up:
array.go	`69.92% <100.00%> (+0.12%)`	:arrow_up:
map.go	`66.88% <100.00%> (+0.17%)`	:arrow_up:

... and 2 files with indirect coverage changes

fxamacker commented 1 year ago

Merging this and will open separate PR for suggested renaming (and other similar related renamings) to keep changes in this PR simple.

onflow / atree