From 0c5a9e05ac186ced2f0361e87fe52b03ac993459 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Thu, 6 Jul 2023 15:57:52 -0500 Subject: [PATCH 1/2] Refactor creating new StorableSlab Currently, in order to create StorableSlab, user needs to: - generate SlabID from storage - create new StorableSlab - store created StorableSlab in storage This commit unexports StorableSlab fields and adds NewStorableSlab(). With this change, user can simply call NewStorableSlab(). --- array.go | 1 - array_debug.go | 2 +- array_test.go | 2 +- cmd/stress/storable.go | 25 ++------------- encode.go | 6 ++-- map.go | 1 - map_debug.go | 2 +- map_test.go | 4 +-- slab.go | 3 ++ storable_slab.go | 69 +++++++++++++++++++++++++++++++----------- storable_test.go | 4 +-- 11 files changed, 67 insertions(+), 52 deletions(-) diff --git a/array.go b/array.go index ffcad590..8fb02002 100644 --- a/array.go +++ b/array.go @@ -112,7 +112,6 @@ func (a *ArrayMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { type ArraySlab interface { Slab - fmt.Stringer Get(storage SlabStorage, index uint64) (Storable, error) Set(storage SlabStorage, address Address, index uint64, value Value) (Storable, error) diff --git a/array_debug.go b/array_debug.go index 72fd20bf..18012557 100644 --- a/array_debug.go +++ b/array_debug.go @@ -167,7 +167,7 @@ func DumpArraySlabs(a *Array) ([]string, error) { if !found { return nil, NewSlabNotFoundErrorf(id, "slab not found during array slab dump") } - dumps = append(dumps, fmt.Sprintf("overflow: %s", slab)) + dumps = append(dumps, slab.String()) } return dumps, nil diff --git a/array_test.go b/array_test.go index 42396ccd..4977a0d4 100644 --- a/array_test.go +++ b/array_test.go @@ -2563,7 +2563,7 @@ func TestArraySlabDump(t *testing.T) { want := []string{ "level 1, ArrayDataSlab id:0x102030405060708.1 size:24 count:1 elements: [SlabIDStorable({[1 2 3 4 5 6 7 8] [0 0 0 0 0 0 0 2]})]", - "overflow: &{0x102030405060708.2 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}", + "StorableSlab id:0x102030405060708.2 storable:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", } dumps, err := DumpArraySlabs(array) diff --git a/cmd/stress/storable.go b/cmd/stress/storable.go index 80b97196..b3fba90a 100644 --- a/cmd/stress/storable.go +++ b/cmd/stress/storable.go @@ -348,30 +348,11 @@ func (v StringValue) StoredValue(_ atree.SlabStorage) (atree.Value, error) { } func (v StringValue) Storable(storage atree.SlabStorage, address atree.Address, maxInlineSize uint64) (atree.Storable, error) { - if uint64(v.ByteSize()) > maxInlineSize { - - // Create StorableSlab - id, err := storage.GenerateSlabID(address) - if err != nil { - return nil, err - } - - slab := &atree.StorableSlab{ - ID: id, - Storable: v, - } - - // Store StorableSlab in storage - err = storage.Store(id, slab) - if err != nil { - return nil, err - } - - // Return slab id as storable - return atree.SlabIDStorable(id), nil + if uint64(v.ByteSize()) <= maxInlineSize { + return v, nil } - return v, nil + return atree.NewStorableSlab(storage, address, v) } func (v StringValue) Encode(enc *atree.Encoder) error { diff --git a/encode.go b/encode.go index 6ad88108..fb84d49a 100644 --- a/encode.go +++ b/encode.go @@ -100,9 +100,9 @@ func DecodeSlab( // Wrap err as external error (if needed) because err is returned by StorableDecoder callback. return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to decode slab storable") } - return StorableSlab{ - ID: id, - Storable: storable, + return &StorableSlab{ + slabID: id, + storable: storable, }, nil default: diff --git a/map.go b/map.go index 2007ceba..fe8065d6 100644 --- a/map.go +++ b/map.go @@ -263,7 +263,6 @@ var _ MapSlab = &MapMetaDataSlab{} type MapSlab interface { Slab - fmt.Stringer Get(storage SlabStorage, digester Digester, level uint, hkey Digest, comparator ValueComparator, key Value) (MapValue, error) Set(storage SlabStorage, b DigesterBuilder, digester Digester, level uint, hkey Digest, comparator ValueComparator, hip HashInputProvider, key Value, value Value) (existingValue MapValue, err error) diff --git a/map_debug.go b/map_debug.go index 5b11a0f2..0b7e3054 100644 --- a/map_debug.go +++ b/map_debug.go @@ -241,7 +241,7 @@ func DumpMapSlabs(m *OrderedMap) ([]string, error) { if !found { return nil, NewSlabNotFoundErrorf(id, "slab not found during map slab dump") } - dumps = append(dumps, fmt.Sprintf("overflow: %s", slab)) + dumps = append(dumps, slab.String()) } return dumps, nil diff --git a/map_test.go b/map_test.go index 1c5c7fea..8bcae77d 100644 --- a/map_test.go +++ b/map_test.go @@ -3909,7 +3909,7 @@ func TestMapSlabDump(t *testing.T) { want := []string{ "level 1, MapDataSlab id:0x102030405060708.1 size:102 firstkey:0 elements: [0:SlabIDStorable({[1 2 3 4 5 6 7 8] [0 0 0 0 0 0 0 2]}):bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb]", - "overflow: &{0x102030405060708.2 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}", + "StorableSlab id:0x102030405060708.2 storable:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", } dumps, err := DumpMapSlabs(m) require.NoError(t, err) @@ -3936,7 +3936,7 @@ func TestMapSlabDump(t *testing.T) { want := []string{ "level 1, MapDataSlab id:0x102030405060708.1 size:100 firstkey:0 elements: [0:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:SlabIDStorable({[1 2 3 4 5 6 7 8] [0 0 0 0 0 0 0 2]})]", - "overflow: &{0x102030405060708.2 bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb}", + "StorableSlab id:0x102030405060708.2 storable:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", } dumps, err := DumpMapSlabs(m) require.NoError(t, err) diff --git a/slab.go b/slab.go index 86ea763b..fdfc17ad 100644 --- a/slab.go +++ b/slab.go @@ -18,8 +18,11 @@ package atree +import "fmt" + type Slab interface { Storable + fmt.Stringer SlabID() SlabID Split(SlabStorage) (Slab, Slab, error) diff --git a/storable_slab.go b/storable_slab.go index 1c076cc9..3a24618a 100644 --- a/storable_slab.go +++ b/storable_slab.go @@ -18,23 +18,56 @@ package atree +import "fmt" + // StorableSlab allows storing storables (CBOR encoded data) directly in a slab. // Eventually we will only have a dictionary at the account storage root, // so this won't be needed, but during the refactor we have the need to store // other non-dictionary values (e.g. strings, integers, etc.) directly in accounts // (i.e. directly in slabs aka registers) type StorableSlab struct { - ID SlabID - Storable Storable + slabID SlabID + storable Storable } -var _ Slab = StorableSlab{} +var _ Slab = &StorableSlab{} + +func NewStorableSlab(storage SlabStorage, address Address, storable Storable) (Storable, error) { + id, err := storage.GenerateSlabID(address) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return nil, wrapErrorfAsExternalErrorIfNeeded( + err, + fmt.Sprintf( + "failed to generate slab ID for address 0x%x", + address, + ), + ) + } + + slab := &StorableSlab{ + slabID: id, + storable: storable, + } + + err = storage.Store(id, slab) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", id)) + } + + return SlabIDStorable(id), nil +} + +func (s *StorableSlab) String() string { + return fmt.Sprintf("StorableSlab id:%s storable:%s", s.slabID, s.storable) +} -func (s StorableSlab) ChildStorables() []Storable { - return []Storable{s.Storable} +func (s *StorableSlab) ChildStorables() []Storable { + return []Storable{s.storable} } -func (s StorableSlab) Encode(enc *Encoder) error { +func (s *StorableSlab) Encode(enc *Encoder) error { // Encode version enc.Scratch[0] = 0 @@ -42,7 +75,7 @@ func (s StorableSlab) Encode(enc *Encoder) error { flag := maskStorable flag = setNoSizeLimit(flag) - if _, ok := s.Storable.(SlabIDStorable); ok { + if _, ok := s.storable.(SlabIDStorable); ok { flag = setHasPointers(flag) } @@ -53,7 +86,7 @@ func (s StorableSlab) Encode(enc *Encoder) error { return NewEncodingError(err) } - err = s.Storable.Encode(enc) + err = s.storable.Encode(enc) if err != nil { // Wrap err as external error (if needed) because err is returned by Storable interface. return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode storable") @@ -62,16 +95,16 @@ func (s StorableSlab) Encode(enc *Encoder) error { return nil } -func (s StorableSlab) ByteSize() uint32 { - return versionAndFlagSize + s.Storable.ByteSize() +func (s *StorableSlab) ByteSize() uint32 { + return versionAndFlagSize + s.storable.ByteSize() } -func (s StorableSlab) SlabID() SlabID { - return s.ID +func (s *StorableSlab) SlabID() SlabID { + return s.slabID } -func (s StorableSlab) StoredValue(storage SlabStorage) (Value, error) { - value, err := s.Storable.StoredValue(storage) +func (s *StorableSlab) StoredValue(storage SlabStorage) (Value, error) { + value, err := s.storable.StoredValue(storage) if err != nil { // Wrap err as external error (if needed) because err is returned by Storable interface. return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get storable's stored value") @@ -79,18 +112,18 @@ func (s StorableSlab) StoredValue(storage SlabStorage) (Value, error) { return value, nil } -func (StorableSlab) Split(_ SlabStorage) (Slab, Slab, error) { +func (*StorableSlab) Split(_ SlabStorage) (Slab, Slab, error) { return nil, nil, NewNotApplicableError("StorableSlab", "Slab", "Split") } -func (StorableSlab) Merge(_ Slab) error { +func (*StorableSlab) Merge(_ Slab) error { return NewNotApplicableError("StorableSlab", "Slab", "Merge") } -func (StorableSlab) LendToRight(_ Slab) error { +func (*StorableSlab) LendToRight(_ Slab) error { return NewNotApplicableError("StorableSlab", "Slab", "LendToRight") } -func (StorableSlab) BorrowFromRight(_ Slab) error { +func (*StorableSlab) BorrowFromRight(_ Slab) error { return NewNotApplicableError("StorableSlab", "Slab", "BorrowFromRight") } diff --git a/storable_test.go b/storable_test.go index 96b736c4..be725054 100644 --- a/storable_test.go +++ b/storable_test.go @@ -355,8 +355,8 @@ func (v StringValue) Storable(storage SlabStorage, address Address, maxInlineSiz } slab := &StorableSlab{ - ID: id, - Storable: v, + slabID: id, + storable: v, } // Store StorableSlab in storage From 3fa18fe8fa8572a27316f88e4ec6f2307e7b319c Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Thu, 6 Jul 2023 16:25:41 -0500 Subject: [PATCH 2/2] Rename ID to ValueID ValueID identifies atree Array and OrderedMap while SlabID identifies slab in storage. SlabID should only be used to retrieve, store, and remove slab in storage. --- array.go | 4 ++-- array_test.go | 2 +- map.go | 4 ++-- map_test.go | 2 +- storage.go | 6 +++++- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/array.go b/array.go index 8fb02002..ddf3e95b 100644 --- a/array.go +++ b/array.go @@ -2372,10 +2372,10 @@ func (a *Array) SlabID() SlabID { return a.root.SlabID() } -func (a *Array) ID() ID { +func (a *Array) ValueID() ValueID { sid := a.SlabID() - var id ID + var id ValueID copy(id[:], sid.address[:]) copy(id[8:], sid.index[:]) diff --git a/array_test.go b/array_test.go index 4977a0d4..66b06d08 100644 --- a/array_test.go +++ b/array_test.go @@ -2599,7 +2599,7 @@ func TestArrayID(t *testing.T) { require.NoError(t, err) sid := array.SlabID() - id := array.ID() + id := array.ValueID() require.Equal(t, sid.address[:], id[:8]) require.Equal(t, sid.index[:], id[8:]) diff --git a/map.go b/map.go index fe8065d6..26d72b59 100644 --- a/map.go +++ b/map.go @@ -3861,10 +3861,10 @@ func (m *OrderedMap) SlabID() SlabID { return m.root.SlabID() } -func (m *OrderedMap) ID() ID { +func (m *OrderedMap) ValueID() ValueID { sid := m.SlabID() - var id ID + var id ValueID copy(id[:], sid.address[:]) copy(id[8:], sid.index[:]) diff --git a/map_test.go b/map_test.go index 8bcae77d..d31f1172 100644 --- a/map_test.go +++ b/map_test.go @@ -4227,7 +4227,7 @@ func TestMapID(t *testing.T) { require.NoError(t, err) sid := m.SlabID() - id := m.ID() + id := m.ValueID() require.Equal(t, sid.address[:], id[:8]) require.Equal(t, sid.index[:], id[8:]) diff --git a/storage.go b/storage.go index c0646d5c..d36d0237 100644 --- a/storage.go +++ b/storage.go @@ -31,12 +31,16 @@ import ( const LedgerBaseStorageSlabPrefix = "$" -type ID [16]byte +// ValueID identifies Array and OrderedMap. +type ValueID [16]byte type ( Address [8]byte SlabIndex [8]byte + // SlabID identifies slab in storage. + // SlabID should only be used to retrieve, + // store, and remove slab in storage. SlabID struct { address Address index SlabIndex