From aaa47cc22914855c3ec82fe61ae1da77fc83f86f Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:05:40 -0500 Subject: [PATCH 01/16] Rename TypeInfo.ID() to TypeInfo.Identifier() --- cmd/stress/typeinfo.go | 6 +++--- cmd/stress/utils.go | 2 +- typeinfo.go | 8 ++++---- utils_test.go | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/stress/typeinfo.go b/cmd/stress/typeinfo.go index 5caa03b6..ddeee106 100644 --- a/cmd/stress/typeinfo.go +++ b/cmd/stress/typeinfo.go @@ -53,7 +53,7 @@ func (i arrayTypeInfo) IsComposite() bool { return false } -func (i arrayTypeInfo) ID() string { +func (i arrayTypeInfo) Identifier() string { return fmt.Sprintf("array(%d)", i) } @@ -88,7 +88,7 @@ func (i mapTypeInfo) IsComposite() bool { return false } -func (i mapTypeInfo) ID() string { +func (i mapTypeInfo) Identifier() string { return fmt.Sprintf("map(%d)", i) } @@ -153,7 +153,7 @@ func (i compositeTypeInfo) IsComposite() bool { return true } -func (i compositeTypeInfo) ID() string { +func (i compositeTypeInfo) Identifier() string { return fmt.Sprintf("composite(%d_%d)", i.fieldStartIndex, i.fieldEndIndex) } diff --git a/cmd/stress/utils.go b/cmd/stress/utils.go index 42283c2d..e5ffba91 100644 --- a/cmd/stress/utils.go +++ b/cmd/stress/utils.go @@ -540,5 +540,5 @@ func (v mapValue) Storable(atree.SlabStorage, atree.Address, uint64) (atree.Stor } var typeInfoComparator = func(a atree.TypeInfo, b atree.TypeInfo) bool { - return a.ID() == b.ID() + return a.Identifier() == b.Identifier() } diff --git a/typeinfo.go b/typeinfo.go index cabb1469..9d5911f9 100644 --- a/typeinfo.go +++ b/typeinfo.go @@ -29,7 +29,7 @@ import ( type TypeInfo interface { Encode(*cbor.StreamEncoder) error IsComposite() bool - ID() string + Identifier() string Copy() TypeInfo } @@ -312,7 +312,7 @@ func (ied *inlinedExtraData) addArrayExtraData(data *ArrayExtraData) int { ied.arrayTypes = make(map[string]int) } - id := data.TypeInfo.ID() + id := data.TypeInfo.Identifier() index, exist := ied.arrayTypes[id] if exist { return index @@ -376,14 +376,14 @@ func makeCompactMapTypeID(t TypeInfo, names []ComparableStorable) string { const separator = "," if len(names) == 1 { - return t.ID() + separator + names[0].ID() + return t.Identifier() + separator + names[0].ID() } sorter := newFieldNameSorter(names) sort.Sort(sorter) - return t.ID() + separator + sorter.join(separator) + return t.Identifier() + separator + sorter.join(separator) } // fieldNameSorter sorts names by index (not in place sort). diff --git a/utils_test.go b/utils_test.go index 5762b3d3..0648885b 100644 --- a/utils_test.go +++ b/utils_test.go @@ -100,7 +100,7 @@ func (i testTypeInfo) IsComposite() bool { return false } -func (i testTypeInfo) ID() string { +func (i testTypeInfo) Identifier() string { return fmt.Sprintf("uint64(%d)", i) } @@ -129,7 +129,7 @@ func (i testCompositeTypeInfo) IsComposite() bool { return true } -func (i testCompositeTypeInfo) ID() string { +func (i testCompositeTypeInfo) Identifier() string { return fmt.Sprintf("composite(%d)", i) } From f56c2e7bc43ac2b0f3b8f7ed2c6f05c3d77d1c75 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:20:41 -0500 Subject: [PATCH 02/16] Change array encoding error type Some errors that should be returned as external error are being returned as fatal error. This caused a problem that was detected during Cadence integration. This commit resolves the problem by returning these fatal errors as external errors. --- array.go | 7 ++++--- encode.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/array.go b/array.go index d4e6fca5..eff16e72 100644 --- a/array.go +++ b/array.go @@ -754,7 +754,8 @@ func (a *ArrayDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo *inlinedEx // element 2: array elements err = a.encodeElements(enc, inlinedTypeInfo) if err != nil { - return NewEncodingError(err) + // err is already categorized by ArrayDataSlab.encodeElements(). + return err } err = enc.CBOR.Flush() @@ -908,8 +909,8 @@ func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo *inlinedExt for _, e := range a.elements { err = encodeStorableAsElement(enc, e, inlinedTypeInfo) if err != nil { - // Wrap err as external error (if needed) because err is returned by Storable interface. - return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode array element") + // err is already categorized by encodeStorableAsElement(). + return err } } diff --git a/encode.go b/encode.go index 5f46505c..e422778f 100644 --- a/encode.go +++ b/encode.go @@ -58,7 +58,7 @@ func encodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo *i err := 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 map value") + return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode storable as element") } } From c7ae14743ede7f301e9324b75032f57475c60767 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:35:01 -0500 Subject: [PATCH 03/16] Change map encoding error type Some errors that should be returned as external error are being returned as fatal error. This caused a problem that was detected during Cadence integration. This commit resolves the problem by returning these fatal errors as external errors. --- map.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/map.go b/map.go index 13654279..bddd420e 100644 --- a/map.go +++ b/map.go @@ -2923,7 +2923,8 @@ func encodeAsInlinedCompactMap( // element 2: compact map values in the order of cachedKeys err = encodeCompactMapValues(enc, cachedKeys, keys, values, inlinedTypeInfo) if err != nil { - return NewEncodingError(err) + // err is already categorized by encodeCompactMapValues(). + return err } err = enc.CBOR.Flush() @@ -2968,7 +2969,7 @@ func encodeCompactMapValues( err = encodeStorableAsElement(enc, values[index], inlinedTypeInfo) if err != nil { - // Don't need to wrap error as external error because err is already categorized by encodeStorable(). + // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). return err } From 945826b673152c078131ad6e6db3ff705f24a233 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:50:40 -0500 Subject: [PATCH 04/16] Add InlinedExtraData interface Cadence integration requires InlinedExtraData interface. --- array.go | 4 ++-- encode.go | 2 +- map.go | 24 ++++++++++++------------ typeinfo.go | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/array.go b/array.go index eff16e72..c0a75dd3 100644 --- a/array.go +++ b/array.go @@ -704,7 +704,7 @@ func DecodeInlinedArrayStorable( // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (a *ArrayDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (a *ArrayDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if a.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root array data slab as inlined")) @@ -886,7 +886,7 @@ func (a *ArrayDataSlab) Encode(enc *Encoder) error { return nil } -func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { // Encode CBOR array size manually for fix-sized encoding enc.Scratch[0] = 0x80 | 25 diff --git a/encode.go b/encode.go index e422778f..f9f5e029 100644 --- a/encode.go +++ b/encode.go @@ -44,7 +44,7 @@ func NewEncoder(w io.Writer, encMode cbor.EncMode) *Encoder { // encodeStorableAsElement encodes storable as Array or OrderedMap element. // Storable is encode as an inlined ArrayDataSlab or MapDataSlab if it is ArrayDataSlab or MapDataSlab. -func encodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo *inlinedExtraData) error { +func encodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo InlinedExtraData) error { switch storable := storable.(type) { diff --git a/map.go b/map.go index bddd420e..61e98df9 100644 --- a/map.go +++ b/map.go @@ -148,7 +148,7 @@ type element interface { key Value, ) (MapKey, MapValue, element, error) - Encode(*Encoder, *inlinedExtraData) error + Encode(*Encoder, InlinedExtraData) error hasPointer() bool @@ -215,7 +215,7 @@ type elements interface { Element(int) (element, error) - Encode(*Encoder, *inlinedExtraData) error + Encode(*Encoder, InlinedExtraData) error hasPointer() bool @@ -586,7 +586,7 @@ func newSingleElementFromData(cborDec *cbor.StreamDecoder, decodeStorable Storab // Encode encodes singleElement to the given encoder. // // CBOR encoded array of 2 elements (key, value). -func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { // Encode CBOR array head for 2 elements err := enc.CBOR.EncodeRawBytes([]byte{0x82}) @@ -763,7 +763,7 @@ func newInlineCollisionGroupFromData(cborDec *cbor.StreamDecoder, decodeStorable // Encode encodes inlineCollisionGroup to the given encoder. // // CBOR tag (number: CBORTagInlineCollisionGroup, content: elements) -func (e *inlineCollisionGroup) Encode(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (e *inlineCollisionGroup) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { err := enc.CBOR.EncodeRawBytes([]byte{ // tag number CBORTagInlineCollisionGroup @@ -953,7 +953,7 @@ func newExternalCollisionGroupFromData(cborDec *cbor.StreamDecoder, decodeStorab // Encode encodes externalCollisionGroup to the given encoder. // // CBOR tag (number: CBORTagExternalCollisionGroup, content: slab ID) -func (e *externalCollisionGroup) Encode(enc *Encoder, _ *inlinedExtraData) error { +func (e *externalCollisionGroup) Encode(enc *Encoder, _ InlinedExtraData) error { err := enc.CBOR.EncodeRawBytes([]byte{ // tag number CBORTagExternalCollisionGroup 0xd8, CBORTagExternalCollisionGroup, @@ -1259,7 +1259,7 @@ func newHkeyElementsWithElement(level uint, hkey Digest, elem element) *hkeyElem // 1: hkeys (byte string) // 2: elements (array) // ] -func (e *hkeyElements) Encode(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (e *hkeyElements) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if e.level > maxDigestLevel { return NewFatalError(fmt.Errorf("hash level %d exceeds max digest level %d", e.level, maxDigestLevel)) @@ -1921,7 +1921,7 @@ func newSingleElementsWithElement(level uint, elem *singleElement) *singleElemen // 1: hkeys (0 length byte string) // 2: elements (array) // ] -func (e *singleElements) Encode(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (e *singleElements) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if e.level > maxDigestLevel { return NewFatalError(fmt.Errorf("digest level %d exceeds max digest level %d", e.level, maxDigestLevel)) @@ -2777,7 +2777,7 @@ func (m *MapDataSlab) Encode(enc *Encoder) error { return nil } -func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes *inlinedExtraData) error { +func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes InlinedExtraData) error { err := m.elements.Encode(enc, inlinedTypes) if err != nil { // Don't need to wrap error as external error because err is already categorized by elements.Encode(). @@ -2799,7 +2799,7 @@ func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes *inlinedExtraDat // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (m *MapDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (m *MapDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if m.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root map data slab as inlined")) @@ -2817,7 +2817,7 @@ func (m *MapDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo *inlinedExtr return m.encodeAsInlinedMap(enc, inlinedTypeInfo) } -func (m *MapDataSlab) encodeAsInlinedMap(enc *Encoder, inlinedTypeInfo *inlinedExtraData) error { +func (m *MapDataSlab) encodeAsInlinedMap(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { extraDataIndex := inlinedTypeInfo.addMapExtraData(m.extraData) @@ -2877,7 +2877,7 @@ func encodeAsInlinedCompactMap( hkeys []Digest, keys []ComparableStorable, values []Storable, - inlinedTypeInfo *inlinedExtraData, + inlinedTypeInfo InlinedExtraData, ) error { extraDataIndex, cachedKeys := inlinedTypeInfo.addCompactMapExtraData(extraData, hkeys, keys) @@ -2941,7 +2941,7 @@ func encodeCompactMapValues( cachedKeys []ComparableStorable, keys []ComparableStorable, values []Storable, - inlinedTypeInfo *inlinedExtraData, + inlinedTypeInfo InlinedExtraData, ) error { var err error diff --git a/typeinfo.go b/typeinfo.go index 9d5911f9..4082a3b5 100644 --- a/typeinfo.go +++ b/typeinfo.go @@ -199,12 +199,26 @@ type compactMapTypeInfo struct { keys []ComparableStorable } +type InlinedExtraData interface { + Encode(*Encoder) error + + addArrayExtraData(data *ArrayExtraData) int + addMapExtraData(data *MapExtraData) int + addCompactMapExtraData( + data *MapExtraData, + digests []Digest, + keys []ComparableStorable, + ) (int, []ComparableStorable) +} + type inlinedExtraData struct { extraData []ExtraData compactMapTypes map[string]compactMapTypeInfo arrayTypes map[string]int } +var _ InlinedExtraData = &inlinedExtraData{} + func newInlinedExtraData() *inlinedExtraData { return &inlinedExtraData{} } From 2e86400276c723a7786474516c0732dffc3691e3 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:13:55 -0500 Subject: [PATCH 05/16] Add ContainerStorable interface ContainerStorable interface supports encoding container storable (storable containing other storables) as element. This is needed for integration with Cadence. --- array.go | 5 +++-- encode.go | 11 ++++++----- map.go | 6 +++--- storable.go | 7 +++++++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/array.go b/array.go index c0a75dd3..1fc8f0fc 100644 --- a/array.go +++ b/array.go @@ -132,6 +132,7 @@ func (a *ArrayDataSlab) StoredValue(storage SlabStorage) (Value, error) { } var _ ArraySlab = &ArrayDataSlab{} +var _ ContainerStorable = &ArrayDataSlab{} // ArrayMetaDataSlab is internal node, implementing ArraySlab. type ArrayMetaDataSlab struct { @@ -697,14 +698,14 @@ func DecodeInlinedArrayStorable( }, nil } -// encodeAsInlined encodes inlined array data slab. Encoding is +// EncodeAsElement encodes inlined array data slab. Encoding is // version 1 with CBOR tag having tag number CBORTagInlinedArray, // and tag contant as 3-element array: // // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (a *ArrayDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (a *ArrayDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if a.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root array data slab as inlined")) diff --git a/encode.go b/encode.go index f9f5e029..0d01e758 100644 --- a/encode.go +++ b/encode.go @@ -48,11 +48,12 @@ func encodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo In switch storable := storable.(type) { - case *ArrayDataSlab: - return storable.encodeAsInlined(enc, inlinedTypeInfo) - - case *MapDataSlab: - return storable.encodeAsInlined(enc, inlinedTypeInfo) + case ContainerStorable: + err := storable.EncodeAsElement(enc, inlinedTypeInfo) + if err != nil { + // Wrap err as external error (if needed) because err is returned by ContainerStorable interface. + return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode container storable as element") + } default: err := storable.Encode(enc) diff --git a/map.go b/map.go index 61e98df9..b732815b 100644 --- a/map.go +++ b/map.go @@ -300,7 +300,7 @@ type MapDataSlab struct { } var _ MapSlab = &MapDataSlab{} -var _ Storable = &MapDataSlab{} +var _ ContainerStorable = &MapDataSlab{} // MapMetaDataSlab is internal node, implementing MapSlab. type MapMetaDataSlab struct { @@ -2792,14 +2792,14 @@ func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes InlinedExtraData return nil } -// encodeAsInlined encodes inlined map data slab. Encoding is +// EncodeAsElement encodes inlined map data slab. Encoding is // version 1 with CBOR tag having tag number CBORTagInlinedMap, // and tag contant as 3-element array: // // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (m *MapDataSlab) encodeAsInlined(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (m *MapDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { if m.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root map data slab as inlined")) diff --git a/storable.go b/storable.go index 02888130..7e252748 100644 --- a/storable.go +++ b/storable.go @@ -54,6 +54,13 @@ type ComparableStorable interface { Copy() Storable } +// ContainerStorable is an interface that supports Storable containing other storables. +type ContainerStorable interface { + Storable + + EncodeAsElement(*Encoder, InlinedExtraData) error +} + type containerStorable interface { Storable hasPointer() bool From 2d5e4a493acd2c8386cc756db2a101b771a36944 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:27:54 -0500 Subject: [PATCH 06/16] Rename Encode to EncodeSlab --- array_debug.go | 4 ++-- map_debug.go | 4 ++-- storable.go | 5 ++--- storage.go | 6 +++--- storage_test.go | 4 ++-- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/array_debug.go b/array_debug.go index 18b88556..ce4188a2 100644 --- a/array_debug.go +++ b/array_debug.go @@ -550,7 +550,7 @@ func (v *serializationVerifier) verifyArraySlab(slab ArraySlab) error { id := slab.SlabID() // Encode slab - data, err := Encode(slab, v.cborEncMode) + data, err := EncodeSlab(slab, v.cborEncMode) if err != nil { // Don't need to wrap error as external error because err is already categorized by Encode(). return err @@ -564,7 +564,7 @@ func (v *serializationVerifier) verifyArraySlab(slab ArraySlab) error { } // Re-encode decoded slab - dataFromDecodedSlab, err := Encode(decodedSlab, v.cborEncMode) + dataFromDecodedSlab, err := EncodeSlab(decodedSlab, v.cborEncMode) if err != nil { // Don't need to wrap error as external error because err is already categorized by Encode(). return err diff --git a/map_debug.go b/map_debug.go index 81edfce5..36fc950d 100644 --- a/map_debug.go +++ b/map_debug.go @@ -958,7 +958,7 @@ func (v *serializationVerifier) verifyMapSlab(slab MapSlab) error { id := slab.SlabID() // Encode slab - data, err := Encode(slab, v.cborEncMode) + data, err := EncodeSlab(slab, v.cborEncMode) if err != nil { // Don't need to wrap error as external error because err is already categorized by Encode(). return err @@ -972,7 +972,7 @@ func (v *serializationVerifier) verifyMapSlab(slab MapSlab) error { } // Re-encode decoded slab - dataFromDecodedSlab, err := Encode(decodedSlab, v.cborEncMode) + dataFromDecodedSlab, err := EncodeSlab(decodedSlab, v.cborEncMode) if err != nil { // Don't need to wrap error as external error because err is already categorized by Encode(). return err diff --git a/storable.go b/storable.go index 7e252748..dc46cfc0 100644 --- a/storable.go +++ b/storable.go @@ -164,12 +164,11 @@ func (v SlabIDStorable) String() string { return fmt.Sprintf("SlabIDStorable(%d)", v) } -// Encode is a wrapper for Storable.Encode() -func Encode(storable Storable, encMode cbor.EncMode) ([]byte, error) { +func EncodeSlab(slab Slab, encMode cbor.EncMode) ([]byte, error) { var buf bytes.Buffer enc := NewEncoder(&buf, encMode) - err := storable.Encode(enc) + err := slab.Encode(enc) if err != nil { // Wrap err as external error (if needed) because err is returned by Storable interface. return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode storable") diff --git a/storage.go b/storage.go index 42ca7490..0b324381 100644 --- a/storage.go +++ b/storage.go @@ -390,7 +390,7 @@ func (s *BasicSlabStorage) SlabIDs() []SlabID { func (s *BasicSlabStorage) Encode() (map[SlabID][]byte, error) { m := make(map[SlabID][]byte) for id, slab := range s.Slabs { - b, err := Encode(slab, s.cborEncMode) + b, err := EncodeSlab(slab, s.cborEncMode) if err != nil { // err is already categorized by Encode(). return nil, err @@ -800,7 +800,7 @@ func (s *PersistentSlabStorage) Commit() error { } // serialize - data, err := Encode(slab, s.cborEncMode) + data, err := EncodeSlab(slab, s.cborEncMode) if err != nil { // err is categorized already by Encode() return err @@ -879,7 +879,7 @@ func (s *PersistentSlabStorage) FastCommit(numWorkers int) error { continue } // serialize - data, err := Encode(slab, s.cborEncMode) + data, err := EncodeSlab(slab, s.cborEncMode) results <- &encodedSlabs{ slabID: id, data: data, diff --git a/storage_test.go b/storage_test.go index 2cd2a929..2b90bb11 100644 --- a/storage_test.go +++ b/storage_test.go @@ -749,7 +749,7 @@ func TestPersistentStorage(t *testing.T) { require.NoError(t, err) // capture data for accuracy testing - simpleMap[slabID], err = Encode(slab, encMode) + simpleMap[slabID], err = EncodeSlab(slab, encMode) require.NoError(t, err) } } @@ -1000,7 +1000,7 @@ func TestPersistentStorageSlabIterator(t *testing.T) { break } - encodedSlab, err := Encode(slab, storage.cborEncMode) + encodedSlab, err := EncodeSlab(slab, storage.cborEncMode) require.NoError(t, err) require.Equal(t, encodedSlab, data[id]) From 485b80014c1a8619c128fe889c817caacaceca66 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:54:16 -0500 Subject: [PATCH 07/16] Encode inlined map as map key This is needed for integration with Cadence because map key can be Cadence enums. --- map.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/map.go b/map.go index b732815b..8e1b31c0 100644 --- a/map.go +++ b/map.go @@ -595,17 +595,17 @@ func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) e } // Encode key - err = e.key.Encode(enc) + err = encodeStorableAsElement(enc, e.key, inlinedTypeInfo) if err != nil { - // Wrap err as external error (if needed) because err is returned by Storable interface. - return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode map key") + // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). + return err } // Encode value err = encodeStorableAsElement(enc, e.value, inlinedTypeInfo) if err != nil { - // Wrap err as external error (if needed) because err is returned by Storable interface. - return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode map value") + // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). + return err } err = enc.CBOR.Flush() From 5eb10f14f2f68172ae5f9944bff1b1079add4fb9 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:03:17 -0500 Subject: [PATCH 08/16] Add test verification for compact map serialization --- array_debug.go | 25 ++++++++---- map_debug.go | 108 +++++++++++++++++++++++++++++++------------------ 2 files changed, 86 insertions(+), 47 deletions(-) diff --git a/array_debug.go b/array_debug.go index ce4188a2..4436ee4d 100644 --- a/array_debug.go +++ b/array_debug.go @@ -272,12 +272,8 @@ func (v *arrayVerifier) verifySlab( // Verify that inlined slab is not in storage if slab.Inlined() { - _, exist, err := v.storage.Retrieve(id) - if err != nil { - // Wrap err as external error (if needed) because err is returned by Storage interface. - return 0, nil, nil, wrapErrorAsExternalErrorIfNeeded(err) - } - if exist { + slab := v.storage.RetrieveIfLoaded(id) + if slab != nil { return 0, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s is in storage", id)) } } @@ -345,8 +341,16 @@ func (v *arrayVerifier) verifyDataSlab( } // Verify that only root data slab can be inlined - if level > 0 && dataSlab.Inlined() { - return 0, nil, nil, NewFatalError(fmt.Errorf("non-root slab %s is inlined", id)) + if dataSlab.Inlined() { + if level > 0 { + return 0, nil, nil, NewFatalError(fmt.Errorf("non-root slab %s is inlined", id)) + } + if dataSlab.extraData == nil { + return 0, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s doesn't have extra data", id)) + } + if dataSlab.next != SlabIDUndefined { + return 0, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s has next slab ID", id)) + } } // Verify that aggregated element size + slab prefix is the same as header.size @@ -524,6 +528,11 @@ func VerifyArraySerialization( decodeTypeInfo TypeInfoDecoder, compare StorableComparator, ) error { + // Skip verification of inlined array serialization. + if a.Inlined() { + return nil + } + v := &serializationVerifier{ storage: a.Storage, cborDecMode: cborDecMode, diff --git a/map_debug.go b/map_debug.go index 36fc950d..afb69fb4 100644 --- a/map_debug.go +++ b/map_debug.go @@ -386,12 +386,8 @@ func (v *mapVerifier) verifySlab( // Verify that inlined slab is not in storage if slab.Inlined() { - _, exist, err := v.storage.Retrieve(id) - if err != nil { - // Wrap err as external error (if needed) because err is returned by Storage interface. - return 0, nil, nil, nil, wrapErrorAsExternalErrorIfNeeded(err) - } - if exist { + slab := v.storage.RetrieveIfLoaded(id) + if slab != nil { return 0, nil, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s is in storage", id)) } } @@ -470,8 +466,16 @@ func (v *mapVerifier) verifyDataSlab( } // Verify that only root slab can be inlined - if level > 0 && dataSlab.Inlined() { - return 0, nil, nil, nil, NewFatalError(fmt.Errorf("non-root slab %s is inlined", id)) + if dataSlab.Inlined() { + if level > 0 { + return 0, nil, nil, nil, NewFatalError(fmt.Errorf("non-root slab %s is inlined", id)) + } + if dataSlab.extraData == nil { + return 0, nil, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s doesn't have extra data", id)) + } + if dataSlab.next != SlabIDUndefined { + return 0, nil, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s has next slab ID", id)) + } } // Verify that aggregated element size + slab prefix is the same as header.size @@ -846,12 +850,6 @@ func (v *mapVerifier) verifySingleElement( return 0, 0, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("element %s key can't be converted to value", e)) } - switch e.key.(type) { - case *ArrayDataSlab, *MapDataSlab: - // Verify key can't be inlined array or map - return 0, 0, NewFatalError(fmt.Errorf("element %s key shouldn't be inlined array or map", e)) - } - err = verifyValue(kv, v.address, nil, v.tic, v.hip, v.inlineEnabled, slabIDs) if err != nil { // Don't need to wrap error as external error because err is already categorized by verifyValue(). @@ -942,6 +940,11 @@ func VerifyMapSerialization( decodeTypeInfo TypeInfoDecoder, compare StorableComparator, ) error { + // Skip verification of inlined map serialization. + if m.Inlined() { + return nil + } + v := &serializationVerifier{ storage: m.Storage, cborDecMode: cborDecMode, @@ -1065,8 +1068,10 @@ func (v *serializationVerifier) verifyMapSlab(slab MapSlab) error { func (v *serializationVerifier) mapDataSlabEqual(expected, actual *MapDataSlab) error { + _, _, _, actualDecodedFromCompactMap := expected.canBeEncodedAsCompactMap() + // Compare extra data - err := mapExtraDataEqual(expected.extraData, actual.extraData) + err := mapExtraDataEqual(expected.extraData, actual.extraData, actualDecodedFromCompactMap) if err != nil { // Don't need to wrap error as external error because err is already categorized by mapExtraDataEqual(). return err @@ -1093,12 +1098,21 @@ func (v *serializationVerifier) mapDataSlabEqual(expected, actual *MapDataSlab) } // Compare header - if !reflect.DeepEqual(expected.header, actual.header) { - return NewFatalError(fmt.Errorf("header %+v is wrong, want %+v", actual.header, expected.header)) + if actualDecodedFromCompactMap { + if expected.header.slabID != actual.header.slabID { + return NewFatalError(fmt.Errorf("header.slabID %s is wrong, want %s", actual.header.slabID, expected.header.slabID)) + } + if expected.header.size != actual.header.size { + return NewFatalError(fmt.Errorf("header.size %d is wrong, want %d", actual.header.size, expected.header.size)) + } + } else { + if !reflect.DeepEqual(expected.header, actual.header) { + return NewFatalError(fmt.Errorf("header %+v is wrong, want %+v", actual.header, expected.header)) + } } // Compare elements - err = v.mapElementsEqual(expected.elements, actual.elements) + err = v.mapElementsEqual(expected.elements, actual.elements, actualDecodedFromCompactMap) if err != nil { // Don't need to wrap error as external error because err is already categorized by mapElementsEqual(). return err @@ -1107,7 +1121,7 @@ func (v *serializationVerifier) mapDataSlabEqual(expected, actual *MapDataSlab) return nil } -func (v *serializationVerifier) mapElementsEqual(expected, actual elements) error { +func (v *serializationVerifier) mapElementsEqual(expected, actual elements, actualDecodedFromCompactMap bool) error { switch expectedElems := expected.(type) { case *hkeyElements: @@ -1115,7 +1129,7 @@ func (v *serializationVerifier) mapElementsEqual(expected, actual elements) erro if !ok { return NewFatalError(fmt.Errorf("elements type %T is wrong, want %T", actual, expected)) } - return v.mapHkeyElementsEqual(expectedElems, actualElems) + return v.mapHkeyElementsEqual(expectedElems, actualElems, actualDecodedFromCompactMap) case *singleElements: actualElems, ok := actual.(*singleElements) @@ -1129,7 +1143,7 @@ func (v *serializationVerifier) mapElementsEqual(expected, actual elements) erro return nil } -func (v *serializationVerifier) mapHkeyElementsEqual(expected, actual *hkeyElements) error { +func (v *serializationVerifier) mapHkeyElementsEqual(expected, actual *hkeyElements, actualDecodedFromCompactMap bool) error { if expected.level != actual.level { return NewFatalError(fmt.Errorf("hkeyElements level %d is wrong, want %d", actual.level, expected.level)) @@ -1139,12 +1153,12 @@ func (v *serializationVerifier) mapHkeyElementsEqual(expected, actual *hkeyEleme return NewFatalError(fmt.Errorf("hkeyElements size %d is wrong, want %d", actual.size, expected.size)) } - if len(expected.hkeys) == 0 { - if len(actual.hkeys) != 0 { - return NewFatalError(fmt.Errorf("hkeyElements hkeys %v is wrong, want %v", actual.hkeys, expected.hkeys)) - } - } else { - if !reflect.DeepEqual(expected.hkeys, actual.hkeys) { + if len(expected.hkeys) != len(actual.hkeys) { + return NewFatalError(fmt.Errorf("hkeyElements hkeys len %d is wrong, want %d", len(actual.hkeys), len(expected.hkeys))) + } + + if !actualDecodedFromCompactMap { + if len(expected.hkeys) > 0 && !reflect.DeepEqual(expected.hkeys, actual.hkeys) { return NewFatalError(fmt.Errorf("hkeyElements hkeys %v is wrong, want %v", actual.hkeys, expected.hkeys)) } } @@ -1153,14 +1167,30 @@ func (v *serializationVerifier) mapHkeyElementsEqual(expected, actual *hkeyEleme return NewFatalError(fmt.Errorf("hkeyElements elems len %d is wrong, want %d", len(actual.elems), len(expected.elems))) } - for i := 0; i < len(expected.elems); i++ { - expectedEle := expected.elems[i] - actualEle := actual.elems[i] + if actualDecodedFromCompactMap { + for _, expectedEle := range expected.elems { + found := false + for _, actualEle := range actual.elems { + err := v.mapElementEqual(expectedEle, actualEle, actualDecodedFromCompactMap) + if err == nil { + found = true + break + } + } + if !found { + return NewFatalError(fmt.Errorf("hkeyElements elem %v is not found", expectedEle)) + } + } + } else { + for i := 0; i < len(expected.elems); i++ { + expectedEle := expected.elems[i] + actualEle := actual.elems[i] - err := v.mapElementEqual(expectedEle, actualEle) - if err != nil { - // Don't need to wrap error as external error because err is already categorized by mapElementEqual(). - return err + err := v.mapElementEqual(expectedEle, actualEle, actualDecodedFromCompactMap) + if err != nil { + // Don't need to wrap error as external error because err is already categorized by mapElementEqual(). + return err + } } } @@ -1195,7 +1225,7 @@ func (v *serializationVerifier) mapSingleElementsEqual(expected, actual *singleE return nil } -func (v *serializationVerifier) mapElementEqual(expected, actual element) error { +func (v *serializationVerifier) mapElementEqual(expected, actual element, actualDecodedFromCompactMap bool) error { switch expectedElem := expected.(type) { case *singleElement: @@ -1210,7 +1240,7 @@ func (v *serializationVerifier) mapElementEqual(expected, actual element) error if !ok { return NewFatalError(fmt.Errorf("elements type %T is wrong, want %T", actual, expected)) } - return v.mapElementsEqual(expectedElem.elements, actualElem.elements) + return v.mapElementsEqual(expectedElem.elements, actualElem.elements, actualDecodedFromCompactMap) case *externalCollisionGroup: actualElem, ok := actual.(*externalCollisionGroup) @@ -1322,7 +1352,7 @@ func (v *serializationVerifier) mapSingleElementEqual(expected, actual *singleEl func (v *serializationVerifier) mapMetaDataSlabEqual(expected, actual *MapMetaDataSlab) error { // Compare extra data - err := mapExtraDataEqual(expected.extraData, actual.extraData) + err := mapExtraDataEqual(expected.extraData, actual.extraData, false) if err != nil { // Don't need to wrap error as external error because err is already categorized by mapExtraDataEqual(). return err @@ -1341,7 +1371,7 @@ func (v *serializationVerifier) mapMetaDataSlabEqual(expected, actual *MapMetaDa return nil } -func mapExtraDataEqual(expected, actual *MapExtraData) error { +func mapExtraDataEqual(expected, actual *MapExtraData, actualDecodedFromCompactMap bool) error { if (expected == nil) && (actual == nil) { return nil @@ -1359,7 +1389,7 @@ func mapExtraDataEqual(expected, actual *MapExtraData) error { return NewFatalError(fmt.Errorf("map extra data count %d is wrong, want %d", actual.Count, expected.Count)) } - if !expected.TypeInfo.IsComposite() { + if !actualDecodedFromCompactMap { if expected.Seed != actual.Seed { return NewFatalError(fmt.Errorf("map extra data seed %d is wrong, want %d", actual.Seed, expected.Seed)) } From 5be1f94f6fbec756bbcfa348560b90bdd93a8cf5 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:12:35 -0500 Subject: [PATCH 09/16] Add more tests for decoded array/map --- array_test.go | 24 ++++++++++++++++++++++++ map_test.go | 25 +++++++++++++++++++++++++ utils_test.go | 9 +++++++++ 3 files changed, 58 insertions(+) diff --git a/array_test.go b/array_test.go index 588f475e..1a30d718 100644 --- a/array_test.go +++ b/array_test.go @@ -142,6 +142,30 @@ func _testArray( require.Equal(t, 1, len(rootIDs)) require.Equal(t, array.SlabID(), rootIDs[0]) + // Encode all non-nil slab + encodedSlabs := make(map[SlabID][]byte) + for id, slab := range storage.deltas { + if slab != nil { + b, err := EncodeSlab(slab, storage.cborEncMode) + require.NoError(t, err) + encodedSlabs[id] = b + } + } + + // Test decoded array from new storage to force slab decoding + decodedArray, err := NewArrayWithRootID( + newTestPersistentStorageWithBaseStorageAndDeltas(t, storage.baseStorage, encodedSlabs), + array.SlabID()) + require.NoError(t, err) + + // Verify decoded array elements + for i, expected := range expectedValues { + actual, err := decodedArray.Get(uint64(i)) + require.NoError(t, err) + + valueEqual(t, expected, actual) + } + if !hasNestedArrayMapElement { // Need to call Commit before calling storage.Count() for PersistentSlabStorage. err = storage.Commit() diff --git a/map_test.go b/map_test.go index 30d25ccb..dccfd4bc 100644 --- a/map_test.go +++ b/map_test.go @@ -215,6 +215,31 @@ func _testMap( require.Equal(t, 1, len(rootIDs)) require.Equal(t, m.SlabID(), rootIDs[0]) + // Encode all non-nil slab + encodedSlabs := make(map[SlabID][]byte) + for id, slab := range storage.deltas { + if slab != nil { + b, err := EncodeSlab(slab, storage.cborEncMode) + require.NoError(t, err) + encodedSlabs[id] = b + } + } + + // Test decoded map from new storage to force slab decoding + decodedMap, err := NewMapWithRootID( + newTestPersistentStorageWithBaseStorageAndDeltas(t, storage.baseStorage, encodedSlabs), + m.SlabID(), + m.digesterBuilder) + require.NoError(t, err) + + // Verify decoded map elements + for k, expected := range expectedKeyValues { + actual, err := decodedMap.Get(compare, hashInputProvider, k) + require.NoError(t, err) + + valueEqual(t, expected, actual) + } + if !hasNestedArrayMapElement { // Need to call Commit before calling storage.Count() for PersistentSlabStorage. err = storage.Commit() diff --git a/utils_test.go b/utils_test.go index 0648885b..84aba2c6 100644 --- a/utils_test.go +++ b/utils_test.go @@ -200,6 +200,15 @@ func newTestPersistentStorageWithBaseStorage(t testing.TB, baseStorage BaseStora ) } +func newTestPersistentStorageWithBaseStorageAndDeltas(t testing.TB, baseStorage BaseStorage, data map[SlabID][]byte) *PersistentSlabStorage { + storage := newTestPersistentStorageWithBaseStorage(t, baseStorage) + for id, b := range data { + err := storage.baseStorage.Store(id, b) + require.NoError(t, err) + } + return storage +} + func newTestBasicStorage(t testing.TB) *BasicSlabStorage { encMode, err := cbor.EncOptions{}.EncMode() require.NoError(t, err) From 800a44a4c769ebbd5b2110ea5cc4dfedb681b5a7 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:29:42 -0500 Subject: [PATCH 10/16] Export EncodeStorableAsElement() --- array.go | 2 +- encode.go | 4 ++-- map.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/array.go b/array.go index 1fc8f0fc..4211128c 100644 --- a/array.go +++ b/array.go @@ -908,7 +908,7 @@ func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo InlinedExtr // Encode data slab content (array of elements) for _, e := range a.elements { - err = encodeStorableAsElement(enc, e, inlinedTypeInfo) + err = EncodeStorableAsElement(enc, e, inlinedTypeInfo) if err != nil { // err is already categorized by encodeStorableAsElement(). return err diff --git a/encode.go b/encode.go index 0d01e758..1d876d50 100644 --- a/encode.go +++ b/encode.go @@ -42,9 +42,9 @@ func NewEncoder(w io.Writer, encMode cbor.EncMode) *Encoder { } } -// encodeStorableAsElement encodes storable as Array or OrderedMap element. +// EncodeStorableAsElement encodes storable as Array or OrderedMap element. // Storable is encode as an inlined ArrayDataSlab or MapDataSlab if it is ArrayDataSlab or MapDataSlab. -func encodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo InlinedExtraData) error { +func EncodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo InlinedExtraData) error { switch storable := storable.(type) { diff --git a/map.go b/map.go index 8e1b31c0..f053da44 100644 --- a/map.go +++ b/map.go @@ -595,14 +595,14 @@ func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) e } // Encode key - err = encodeStorableAsElement(enc, e.key, inlinedTypeInfo) + err = EncodeStorableAsElement(enc, e.key, inlinedTypeInfo) if err != nil { // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). return err } // Encode value - err = encodeStorableAsElement(enc, e.value, inlinedTypeInfo) + err = EncodeStorableAsElement(enc, e.value, inlinedTypeInfo) if err != nil { // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). return err @@ -2967,7 +2967,7 @@ func encodeCompactMapValues( found = true keyIndexes[i], keyIndexes[j] = keyIndexes[j], keyIndexes[i] - err = encodeStorableAsElement(enc, values[index], inlinedTypeInfo) + err = EncodeStorableAsElement(enc, values[index], inlinedTypeInfo) if err != nil { // Don't need to wrap error as external error because err is already categorized by encodeStorableAsElement(). return err From eecb91a42d071e44f670d91b750261a4391bef12 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:58:24 -0500 Subject: [PATCH 11/16] Add ContainerStorable.HasPointer() --- array.go | 4 ++-- map.go | 4 ++-- storable.go | 19 +++++++++---------- storable_test.go | 19 +++++++++++++++---- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/array.go b/array.go index 4211128c..0170c7d9 100644 --- a/array.go +++ b/array.go @@ -819,7 +819,7 @@ func (a *ArrayDataSlab) Encode(enc *Encoder) error { return NewEncodingError(err) } - if a.hasPointer() { + if a.HasPointer() { h.setHasPointers() } @@ -1002,7 +1002,7 @@ func (a *ArrayDataSlab) Uninline(storage SlabStorage) error { return nil } -func (a *ArrayDataSlab) hasPointer() bool { +func (a *ArrayDataSlab) HasPointer() bool { for _, e := range a.elements { if hasPointer(e) { return true diff --git a/map.go b/map.go index f053da44..3fb56e88 100644 --- a/map.go +++ b/map.go @@ -2705,7 +2705,7 @@ func (m *MapDataSlab) Encode(enc *Encoder) error { return NewEncodingError(err) } - if m.hasPointer() { + if m.HasPointer() { h.setHasPointers() } @@ -3031,7 +3031,7 @@ func (m *MapDataSlab) canBeEncodedAsCompactMap() ([]Digest, []ComparableStorable return elements.hkeys, keys, values, true } -func (m *MapDataSlab) hasPointer() bool { +func (m *MapDataSlab) HasPointer() bool { return m.elements.hasPointer() } diff --git a/storable.go b/storable.go index dc46cfc0..c1c83620 100644 --- a/storable.go +++ b/storable.go @@ -59,16 +59,12 @@ type ContainerStorable interface { Storable EncodeAsElement(*Encoder, InlinedExtraData) error -} - -type containerStorable interface { - Storable - hasPointer() bool + HasPointer() bool } func hasPointer(storable Storable) bool { - if cs, ok := storable.(containerStorable); ok { - return cs.hasPointer() + if cs, ok := storable.(ContainerStorable); ok { + return cs.HasPointer() } return false } @@ -95,10 +91,9 @@ const ( type SlabIDStorable SlabID -var _ Storable = SlabIDStorable{} -var _ containerStorable = SlabIDStorable{} +var _ ContainerStorable = SlabIDStorable{} -func (v SlabIDStorable) hasPointer() bool { +func (v SlabIDStorable) HasPointer() bool { return true } @@ -155,6 +150,10 @@ func (v SlabIDStorable) Encode(enc *Encoder) error { return nil } +func (v SlabIDStorable) EncodeAsElement(enc *Encoder, _ InlinedExtraData) error { + return v.Encode(enc) +} + func (v SlabIDStorable) ByteSize() uint32 { // tag number (2 bytes) + byte string header (1 byte) + slab id (16 bytes) return 2 + 1 + slabIDSize diff --git a/storable_test.go b/storable_test.go index 12b732f2..539dd9e3 100644 --- a/storable_test.go +++ b/storable_test.go @@ -698,11 +698,11 @@ type SomeStorable struct { Storable Storable } -var _ Storable = SomeStorable{} +var _ ContainerStorable = SomeStorable{} -func (v SomeStorable) hasPointer() bool { - if ms, ok := v.Storable.(containerStorable); ok { - return ms.hasPointer() +func (v SomeStorable) HasPointer() bool { + if ms, ok := v.Storable.(ContainerStorable); ok { + return ms.HasPointer() } return false } @@ -723,6 +723,17 @@ func (v SomeStorable) Encode(enc *Encoder) error { return v.Storable.Encode(enc) } +func (v SomeStorable) EncodeAsElement(enc *Encoder, inlinedExtraData InlinedExtraData) error { + err := enc.CBOR.EncodeRawBytes([]byte{ + // tag number + 0xd8, cborTagSomeValue, + }) + if err != nil { + return err + } + return EncodeStorableAsElement(enc, v.Storable, inlinedExtraData) +} + func (v SomeStorable) ChildStorables() []Storable { return []Storable{v.Storable} } From eb9a50354126252aa2b4aa247a705a5e000328ff Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Wed, 18 Oct 2023 20:00:36 -0500 Subject: [PATCH 12/16] Fix lint warning --- map_debug.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/map_debug.go b/map_debug.go index afb69fb4..df1c2002 100644 --- a/map_debug.go +++ b/map_debug.go @@ -1105,10 +1105,8 @@ func (v *serializationVerifier) mapDataSlabEqual(expected, actual *MapDataSlab) if expected.header.size != actual.header.size { return NewFatalError(fmt.Errorf("header.size %d is wrong, want %d", actual.header.size, expected.header.size)) } - } else { - if !reflect.DeepEqual(expected.header, actual.header) { - return NewFatalError(fmt.Errorf("header %+v is wrong, want %+v", actual.header, expected.header)) - } + } else if !reflect.DeepEqual(expected.header, actual.header) { + return NewFatalError(fmt.Errorf("header %+v is wrong, want %+v", actual.header, expected.header)) } // Compare elements From c8f01db5b13b814e05b302d401cd80b448ad94e5 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Fri, 20 Oct 2023 09:59:03 -0500 Subject: [PATCH 13/16] Test inlined array/map not stored in storage --- array_debug.go | 8 ++++++-- map_debug.go | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/array_debug.go b/array_debug.go index 4436ee4d..eb0d2fe7 100644 --- a/array_debug.go +++ b/array_debug.go @@ -272,8 +272,12 @@ func (v *arrayVerifier) verifySlab( // Verify that inlined slab is not in storage if slab.Inlined() { - slab := v.storage.RetrieveIfLoaded(id) - if slab != nil { + _, exist, err := v.storage.Retrieve(id) + if err != nil { + // Wrap err as external error (if needed) because err is returned by Storage interface. + return 0, nil, nil, wrapErrorAsExternalErrorIfNeeded(err) + } + if exist { return 0, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s is in storage", id)) } } diff --git a/map_debug.go b/map_debug.go index df1c2002..dbfd6834 100644 --- a/map_debug.go +++ b/map_debug.go @@ -386,8 +386,12 @@ func (v *mapVerifier) verifySlab( // Verify that inlined slab is not in storage if slab.Inlined() { - slab := v.storage.RetrieveIfLoaded(id) - if slab != nil { + _, exist, err := v.storage.Retrieve(id) + if err != nil { + // Wrap err as external error (if needed) because err is returned by Storage interface. + return 0, nil, nil, nil, wrapErrorAsExternalErrorIfNeeded(err) + } + if exist { return 0, nil, nil, nil, NewFatalError(fmt.Errorf("inlined slab %s is in storage", id)) } } From 25c4d5e0072db937470b2ae98e1d873f46ab064c Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Fri, 20 Oct 2023 10:12:10 -0500 Subject: [PATCH 14/16] Add more comments --- storable.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/storable.go b/storable.go index c1c83620..b152e40c 100644 --- a/storable.go +++ b/storable.go @@ -58,7 +58,13 @@ type ComparableStorable interface { type ContainerStorable interface { Storable + // EncodeAsElement encodes ContainerStorable and its child storables as an element + // of parent array/map. Since child storable can be inlined array or map, + // encoding inlined array or map requires extra parameter InlinedExtraData. EncodeAsElement(*Encoder, InlinedExtraData) error + + // HasPointer returns true if any of its child storables is SlabIDStorable + // (references to another slab). This function is used during encoding. HasPointer() bool } From 3688e8969973c35b03f5bb0a2607028fb12e791a Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Fri, 20 Oct 2023 10:32:45 -0500 Subject: [PATCH 15/16] Export InlinedExtraData struct to replace interface --- array.go | 4 ++-- encode.go | 2 +- map.go | 24 ++++++++++++------------ storable.go | 4 ++-- storable_test.go | 2 +- typeinfo.go | 30 ++++++++---------------------- 6 files changed, 26 insertions(+), 40 deletions(-) diff --git a/array.go b/array.go index 0170c7d9..62b2aedd 100644 --- a/array.go +++ b/array.go @@ -705,7 +705,7 @@ func DecodeInlinedArrayStorable( // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (a *ArrayDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (a *ArrayDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { if a.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root array data slab as inlined")) @@ -887,7 +887,7 @@ func (a *ArrayDataSlab) Encode(enc *Encoder) error { return nil } -func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (a *ArrayDataSlab) encodeElements(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { // Encode CBOR array size manually for fix-sized encoding enc.Scratch[0] = 0x80 | 25 diff --git a/encode.go b/encode.go index 1d876d50..7d8eb3c4 100644 --- a/encode.go +++ b/encode.go @@ -44,7 +44,7 @@ func NewEncoder(w io.Writer, encMode cbor.EncMode) *Encoder { // EncodeStorableAsElement encodes storable as Array or OrderedMap element. // Storable is encode as an inlined ArrayDataSlab or MapDataSlab if it is ArrayDataSlab or MapDataSlab. -func EncodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo InlinedExtraData) error { +func EncodeStorableAsElement(enc *Encoder, storable Storable, inlinedTypeInfo *InlinedExtraData) error { switch storable := storable.(type) { diff --git a/map.go b/map.go index 3fb56e88..a4b52e81 100644 --- a/map.go +++ b/map.go @@ -148,7 +148,7 @@ type element interface { key Value, ) (MapKey, MapValue, element, error) - Encode(*Encoder, InlinedExtraData) error + Encode(*Encoder, *InlinedExtraData) error hasPointer() bool @@ -215,7 +215,7 @@ type elements interface { Element(int) (element, error) - Encode(*Encoder, InlinedExtraData) error + Encode(*Encoder, *InlinedExtraData) error hasPointer() bool @@ -586,7 +586,7 @@ func newSingleElementFromData(cborDec *cbor.StreamDecoder, decodeStorable Storab // Encode encodes singleElement to the given encoder. // // CBOR encoded array of 2 elements (key, value). -func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (e *singleElement) Encode(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { // Encode CBOR array head for 2 elements err := enc.CBOR.EncodeRawBytes([]byte{0x82}) @@ -763,7 +763,7 @@ func newInlineCollisionGroupFromData(cborDec *cbor.StreamDecoder, decodeStorable // Encode encodes inlineCollisionGroup to the given encoder. // // CBOR tag (number: CBORTagInlineCollisionGroup, content: elements) -func (e *inlineCollisionGroup) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (e *inlineCollisionGroup) Encode(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { err := enc.CBOR.EncodeRawBytes([]byte{ // tag number CBORTagInlineCollisionGroup @@ -953,7 +953,7 @@ func newExternalCollisionGroupFromData(cborDec *cbor.StreamDecoder, decodeStorab // Encode encodes externalCollisionGroup to the given encoder. // // CBOR tag (number: CBORTagExternalCollisionGroup, content: slab ID) -func (e *externalCollisionGroup) Encode(enc *Encoder, _ InlinedExtraData) error { +func (e *externalCollisionGroup) Encode(enc *Encoder, _ *InlinedExtraData) error { err := enc.CBOR.EncodeRawBytes([]byte{ // tag number CBORTagExternalCollisionGroup 0xd8, CBORTagExternalCollisionGroup, @@ -1259,7 +1259,7 @@ func newHkeyElementsWithElement(level uint, hkey Digest, elem element) *hkeyElem // 1: hkeys (byte string) // 2: elements (array) // ] -func (e *hkeyElements) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (e *hkeyElements) Encode(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { if e.level > maxDigestLevel { return NewFatalError(fmt.Errorf("hash level %d exceeds max digest level %d", e.level, maxDigestLevel)) @@ -1921,7 +1921,7 @@ func newSingleElementsWithElement(level uint, elem *singleElement) *singleElemen // 1: hkeys (0 length byte string) // 2: elements (array) // ] -func (e *singleElements) Encode(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (e *singleElements) Encode(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { if e.level > maxDigestLevel { return NewFatalError(fmt.Errorf("digest level %d exceeds max digest level %d", e.level, maxDigestLevel)) @@ -2777,7 +2777,7 @@ func (m *MapDataSlab) Encode(enc *Encoder) error { return nil } -func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes InlinedExtraData) error { +func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes *InlinedExtraData) error { err := m.elements.Encode(enc, inlinedTypes) if err != nil { // Don't need to wrap error as external error because err is already categorized by elements.Encode(). @@ -2799,7 +2799,7 @@ func (m *MapDataSlab) encodeElements(enc *Encoder, inlinedTypes InlinedExtraData // +------------------+----------------+----------+ // | extra data index | value ID index | elements | // +------------------+----------------+----------+ -func (m *MapDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (m *MapDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { if m.extraData == nil { return NewEncodingError( fmt.Errorf("failed to encode non-root map data slab as inlined")) @@ -2817,7 +2817,7 @@ func (m *MapDataSlab) EncodeAsElement(enc *Encoder, inlinedTypeInfo InlinedExtra return m.encodeAsInlinedMap(enc, inlinedTypeInfo) } -func (m *MapDataSlab) encodeAsInlinedMap(enc *Encoder, inlinedTypeInfo InlinedExtraData) error { +func (m *MapDataSlab) encodeAsInlinedMap(enc *Encoder, inlinedTypeInfo *InlinedExtraData) error { extraDataIndex := inlinedTypeInfo.addMapExtraData(m.extraData) @@ -2877,7 +2877,7 @@ func encodeAsInlinedCompactMap( hkeys []Digest, keys []ComparableStorable, values []Storable, - inlinedTypeInfo InlinedExtraData, + inlinedTypeInfo *InlinedExtraData, ) error { extraDataIndex, cachedKeys := inlinedTypeInfo.addCompactMapExtraData(extraData, hkeys, keys) @@ -2941,7 +2941,7 @@ func encodeCompactMapValues( cachedKeys []ComparableStorable, keys []ComparableStorable, values []Storable, - inlinedTypeInfo InlinedExtraData, + inlinedTypeInfo *InlinedExtraData, ) error { var err error diff --git a/storable.go b/storable.go index b152e40c..34f83648 100644 --- a/storable.go +++ b/storable.go @@ -61,7 +61,7 @@ type ContainerStorable interface { // EncodeAsElement encodes ContainerStorable and its child storables as an element // of parent array/map. Since child storable can be inlined array or map, // encoding inlined array or map requires extra parameter InlinedExtraData. - EncodeAsElement(*Encoder, InlinedExtraData) error + EncodeAsElement(*Encoder, *InlinedExtraData) error // HasPointer returns true if any of its child storables is SlabIDStorable // (references to another slab). This function is used during encoding. @@ -156,7 +156,7 @@ func (v SlabIDStorable) Encode(enc *Encoder) error { return nil } -func (v SlabIDStorable) EncodeAsElement(enc *Encoder, _ InlinedExtraData) error { +func (v SlabIDStorable) EncodeAsElement(enc *Encoder, _ *InlinedExtraData) error { return v.Encode(enc) } diff --git a/storable_test.go b/storable_test.go index 539dd9e3..1e747120 100644 --- a/storable_test.go +++ b/storable_test.go @@ -723,7 +723,7 @@ func (v SomeStorable) Encode(enc *Encoder) error { return v.Storable.Encode(enc) } -func (v SomeStorable) EncodeAsElement(enc *Encoder, inlinedExtraData InlinedExtraData) error { +func (v SomeStorable) EncodeAsElement(enc *Encoder, inlinedExtraData *InlinedExtraData) error { err := enc.CBOR.EncodeRawBytes([]byte{ // tag number 0xd8, cborTagSomeValue, diff --git a/typeinfo.go b/typeinfo.go index 4082a3b5..b472ee6d 100644 --- a/typeinfo.go +++ b/typeinfo.go @@ -199,32 +199,18 @@ type compactMapTypeInfo struct { keys []ComparableStorable } -type InlinedExtraData interface { - Encode(*Encoder) error - - addArrayExtraData(data *ArrayExtraData) int - addMapExtraData(data *MapExtraData) int - addCompactMapExtraData( - data *MapExtraData, - digests []Digest, - keys []ComparableStorable, - ) (int, []ComparableStorable) -} - -type inlinedExtraData struct { +type InlinedExtraData struct { extraData []ExtraData compactMapTypes map[string]compactMapTypeInfo arrayTypes map[string]int } -var _ InlinedExtraData = &inlinedExtraData{} - -func newInlinedExtraData() *inlinedExtraData { - return &inlinedExtraData{} +func newInlinedExtraData() *InlinedExtraData { + return &InlinedExtraData{} } // Encode encodes inlined extra data as CBOR array. -func (ied *inlinedExtraData) Encode(enc *Encoder) error { +func (ied *InlinedExtraData) Encode(enc *Encoder) error { err := enc.CBOR.EncodeArrayHead(uint64(len(ied.extraData))) if err != nil { return NewEncodingError(err) @@ -321,7 +307,7 @@ func newInlinedExtraDataFromData( // addArrayExtraData returns index of deduplicated array extra data. // Array extra data is deduplicated by array type info ID because array // extra data only contains type info. -func (ied *inlinedExtraData) addArrayExtraData(data *ArrayExtraData) int { +func (ied *InlinedExtraData) addArrayExtraData(data *ArrayExtraData) int { if ied.arrayTypes == nil { ied.arrayTypes = make(map[string]int) } @@ -340,7 +326,7 @@ func (ied *inlinedExtraData) addArrayExtraData(data *ArrayExtraData) int { // addMapExtraData returns index of map extra data. // Map extra data is not deduplicated because it also contains count and seed. -func (ied *inlinedExtraData) addMapExtraData(data *MapExtraData) int { +func (ied *InlinedExtraData) addMapExtraData(data *MapExtraData) int { index := len(ied.extraData) ied.extraData = append(ied.extraData, data) return index @@ -348,7 +334,7 @@ func (ied *inlinedExtraData) addMapExtraData(data *MapExtraData) int { // addCompactMapExtraData returns index of deduplicated compact map extra data. // Compact map extra data is deduplicated by TypeInfo.ID() with sorted field names. -func (ied *inlinedExtraData) addCompactMapExtraData( +func (ied *InlinedExtraData) addCompactMapExtraData( data *MapExtraData, digests []Digest, keys []ComparableStorable, @@ -381,7 +367,7 @@ func (ied *inlinedExtraData) addCompactMapExtraData( return index, keys } -func (ied *inlinedExtraData) empty() bool { +func (ied *InlinedExtraData) empty() bool { return len(ied.extraData) == 0 } From 87e6b6b3299678b710c3b26de57d7e00406026c4 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Fri, 20 Oct 2023 10:47:22 -0500 Subject: [PATCH 16/16] Optimize compact map field names concatenation --- typeinfo.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/typeinfo.go b/typeinfo.go index b472ee6d..86c9fe67 100644 --- a/typeinfo.go +++ b/typeinfo.go @@ -22,6 +22,7 @@ import ( "encoding/binary" "fmt" "sort" + "strings" "github.com/fxamacker/cbor/v2" ) @@ -418,9 +419,12 @@ func (fn *fieldNameSorter) Swap(i, j int) { } func (fn *fieldNameSorter) join(sep string) string { - var s string - for _, i := range fn.index { - s += sep + fn.names[i].ID() + var sb strings.Builder + for i, index := range fn.index { + if i > 0 { + sb.WriteString(sep) + } + sb.WriteString(fn.names[index].ID()) } - return s + return sb.String() }