Skip to content

Commit

Permalink
Merge pull request #3677 from onflow/fxamacker/use-domain-enum-as-acc…
Browse files Browse the repository at this point in the history 
…ount-storage-map-key

Replace domain string with enum for AccountStorageMap key
  • Loading branch information
@fxamacker
fxamacker authored Nov 13, 2024
2 parents cc95223 + f68c5eb commit 0abfa9a
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 47 deletions.
15 changes: 15 additions & 0 deletions common/storagedomain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ import (
"github.com/onflow/cadence/errors"
)

// StorageDomain is used to store domain values on chain.
//
// !!! *WARNING* !!!
//
// Only add new StorageDomain by:
// - appending to the end.
//
// Only remove StorageDomain by:
// - replacing existing StorageDomain with a placeholder `_`.
//
// DO *NOT* REPLACE EXISTING STORAGEDOMAIN!
// DO *NOT* REMOVE EXISTING STORAGEDOMAIN!
// DO *NOT* INSERT NEW STORAGEDOMAIN IN BETWEEN!
type StorageDomain uint8

const (
Expand Down Expand Up @@ -54,6 +67,8 @@ const (
// StorageDomainAccountCapability is the storage domain which
// records active account capability controller IDs
StorageDomainAccountCapability

// Append new StorageDomain here (if needed).
)

var AllStorageDomains = []StorageDomain{
Expand Down
26 changes: 13 additions & 13 deletions interpreter/account_storagemap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewAccountStorageMapWithRootID(

// DomainExists returns true if the given domain exists in the account storage map.
func (s *AccountStorageMap) DomainExists(domain common.StorageDomain) bool {
key := StringStorageMapKey(domain.Identifier())
key := Uint64StorageMapKey(domain)

exists, err := s.orderedMap.Has(
key.AtreeValueCompare,
Expand All @@ -99,7 +99,7 @@ func (s *AccountStorageMap) GetDomain(
domain common.StorageDomain,
createIfNotExists bool,
) *DomainStorageMap {
key := StringStorageMapKey(domain.Identifier())
key := Uint64StorageMapKey(domain)

storedValue, err := s.orderedMap.Get(
key.AtreeValueCompare,
Expand Down Expand Up @@ -135,7 +135,7 @@ func (s *AccountStorageMap) NewDomain(

domainStorageMap := NewDomainStorageMap(gauge, s.orderedMap.Storage, s.orderedMap.Address())

key := StringStorageMapKey(domain.Identifier())
key := Uint64StorageMapKey(domain)

existingStorable, err := s.orderedMap.Set(
key.AtreeValueCompare,
Expand Down Expand Up @@ -181,7 +181,7 @@ func (s *AccountStorageMap) setDomain(
) (existed bool) {
interpreter.recordStorageMutation()

key := StringStorageMapKey(domain.Identifier())
key := Uint64StorageMapKey(domain)

existingValueStorable, err := s.orderedMap.Set(
key.AtreeValueCompare,
Expand Down Expand Up @@ -218,7 +218,7 @@ func (s *AccountStorageMap) setDomain(
func (s *AccountStorageMap) removeDomain(interpreter *Interpreter, domain common.StorageDomain) (existed bool) {
interpreter.recordStorageMutation()

key := StringStorageMapKey(domain.Identifier())
key := Uint64StorageMapKey(domain)

existingKeyStorable, existingValueStorable, err := s.orderedMap.Remove(
key.AtreeValueCompare,
Expand All @@ -236,7 +236,7 @@ func (s *AccountStorageMap) removeDomain(interpreter *Interpreter, domain common

// Key

// NOTE: Key is just an atree.Value (StringAtreeValue), not an interpreter.Value,
// NOTE: Key is just an atree.Value (Uint64AtreeValue), not an interpreter.Value,
// so do not need (can) convert and not need to deep remove
interpreter.RemoveReferencedSlab(existingKeyStorable)

Expand Down Expand Up @@ -284,7 +284,7 @@ func (s *AccountStorageMap) Domains() map[common.StorageDomain]struct{} {
break
}

domain := convertKeyToDomain(k)
domain := convertAccountStorageMapKeyToStorageDomain(k)
domains[domain] = struct{}{}
}

Expand Down Expand Up @@ -326,21 +326,21 @@ func (i *AccountStorageMapIterator) Next() (common.StorageDomain, *DomainStorage
return common.StorageDomainUnknown, nil
}

key := convertKeyToDomain(k)
key := convertAccountStorageMapKeyToStorageDomain(k)

value := NewDomainStorageMapWithAtreeValue(v)

return key, value
}

func convertKeyToDomain(v atree.Value) common.StorageDomain {
key, ok := v.(StringAtreeValue)
func convertAccountStorageMapKeyToStorageDomain(v atree.Value) common.StorageDomain {
key, ok := v.(Uint64AtreeValue)
if !ok {
panic(errors.NewUnexpectedError("domain key type %T isn't expected", key))
}
domain, found := common.StorageDomainFromIdentifier(string(key))
if !found {
panic(errors.NewUnexpectedError("domain key %s isn't expected", key))
domain, err := common.StorageDomainFromUint64(uint64(key))
if err != nil {
panic(errors.NewUnexpectedError("domain key %d isn't expected: %w", key, err))
}
return domain
}
104 changes: 74 additions & 30 deletions interpreter/account_storagemap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package interpreter_test

import (
"math/rand"
goruntime "runtime"
"slices"
"strconv"
"strings"
Expand Down Expand Up @@ -188,6 +189,8 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {
require.NotNil(t, accountStorageMap)
require.Equal(t, uint64(0), accountStorageMap.Count())

accountStorageMapRootSlabID := accountStorageMap.SlabID()

for _, domain := range common.AllStorageDomains {
const createIfNotExists = true
domainStoragemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
Expand All @@ -199,7 +202,12 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})

t.Run("non-empty", func(t *testing.T) {
Expand All @@ -225,6 +233,8 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {
const count = 10
accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)

accountStorageMapRootSlabID := accountStorageMap.SlabID()

for _, domain := range common.AllStorageDomains {
const createIfNotExists = true
domainStoragemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
Expand All @@ -238,7 +248,12 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})
}

Expand Down Expand Up @@ -271,6 +286,8 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {
require.NotNil(t, accountStorageMap)
require.Equal(t, uint64(0), accountStorageMap.Count())

accountStorageMapRootSlabID := accountStorageMap.SlabID()

const count = 10
for _, domain := range common.AllStorageDomains {

Expand All @@ -285,7 +302,12 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})

t.Run("non-empty", func(t *testing.T) {
Expand All @@ -311,6 +333,8 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {
const count = 10
accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)

accountStorageMapRootSlabID := accountStorageMap.SlabID()

for _, domain := range common.AllStorageDomains {

domainStorageMap := interpreter.NewDomainStorageMap(nil, storage, atree.Address(address))
Expand All @@ -324,7 +348,12 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})
}

Expand Down Expand Up @@ -355,14 +384,21 @@ func TestAccountStorageMapRemoveDomain(t *testing.T) {
require.NotNil(t, accountStorageMap)
require.Equal(t, uint64(0), accountStorageMap.Count())

accountStorageMapRootSlabID := accountStorageMap.SlabID()

for _, domain := range common.AllStorageDomains {
existed := accountStorageMap.WriteDomain(inter, domain, nil)
require.False(t, existed)
}

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})

t.Run("non-empty", func(t *testing.T) {
Expand All @@ -388,6 +424,8 @@ func TestAccountStorageMapRemoveDomain(t *testing.T) {
const count = 10
accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)

accountStorageMapRootSlabID := accountStorageMap.SlabID()

for _, domain := range common.AllStorageDomains {

existed := accountStorageMap.WriteDomain(inter, domain, nil)
Expand All @@ -398,7 +436,12 @@ func TestAccountStorageMapRemoveDomain(t *testing.T) {

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMapRootSlabID})

err := storage.PersistentSlabStorage.FastCommit(goruntime.NumCPU())
require.NoError(t, err)

checkAccountStorageMapDataWithRawData(t, ledger.StoredValues, ledger.StorageIndices, accountStorageMapRootSlabID, accountValues)
})
}

Expand Down Expand Up @@ -575,18 +618,7 @@ func TestAccountStorageMapLoadFromRootSlabID(t *testing.T) {

accountStorageMapRootSlabID, accountValues, storedValues, storageIndices := init()

ledger := NewTestLedgerWithData(nil, nil, storedValues, storageIndices)
storage := runtime.NewStorage(ledger, nil)

accountStorageMap := interpreter.NewAccountStorageMapWithRootID(storage, accountStorageMapRootSlabID)
require.Equal(t, uint64(0), accountStorageMap.Count())
require.Equal(t, accountStorageMapRootSlabID, accountStorageMap.SlabID())

inter := NewTestInterpreterWithStorage(t, storage)

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
checkAccountStorageMapDataWithRawData(t, storedValues, storageIndices, accountStorageMapRootSlabID, accountValues)
})

t.Run("non-empty", func(t *testing.T) {
Expand Down Expand Up @@ -619,18 +651,7 @@ func TestAccountStorageMapLoadFromRootSlabID(t *testing.T) {

accountStorageMapRootSlabID, accountValues, storedValues, storageIndices := init()

ledger := NewTestLedgerWithData(nil, nil, storedValues, storageIndices)
storage := runtime.NewStorage(ledger, nil)

accountStorageMap := interpreter.NewAccountStorageMapWithRootID(storage, accountStorageMapRootSlabID)
require.Equal(t, uint64(len(existingDomains)), accountStorageMap.Count())
require.Equal(t, accountStorageMapRootSlabID, accountStorageMap.SlabID())

inter := NewTestInterpreterWithStorage(t, storage)

checkAccountStorageMapData(t, inter, accountStorageMap, accountValues)

CheckAtreeStorageHealth(t, storage, []atree.SlabID{accountStorageMap.SlabID()})
checkAccountStorageMapDataWithRawData(t, storedValues, storageIndices, accountStorageMapRootSlabID, accountValues)
})
}

Expand Down Expand Up @@ -697,6 +718,29 @@ func writeRandomValuesToDomainStorageMap(
return domainValues
}

// checkAccountStorageMapDataWithRawData checks loaded account storage map against expected account values.
func checkAccountStorageMapDataWithRawData(
tb testing.TB,
storedValues map[string][]byte,
storageIndices map[string]uint64,
rootSlabID atree.SlabID,
expectedAccountValues accountStorageMapValues,
) {
// Create new storage from raw data
ledger := NewTestLedgerWithData(nil, nil, storedValues, storageIndices)
storage := runtime.NewStorage(ledger, nil)

inter := NewTestInterpreterWithStorage(tb, storage)

loadedAccountStorageMap := interpreter.NewAccountStorageMapWithRootID(storage, rootSlabID)
require.Equal(tb, uint64(len(expectedAccountValues)), loadedAccountStorageMap.Count())
require.Equal(tb, rootSlabID, loadedAccountStorageMap.SlabID())

checkAccountStorageMapData(tb, inter, loadedAccountStorageMap, expectedAccountValues)

CheckAtreeStorageHealth(tb, storage, []atree.SlabID{rootSlabID})
}

// checkAccountStorageMapData iterates account storage map and compares values with given expectedAccountValues.
func checkAccountStorageMapData(
tb testing.TB,
Expand Down
6 changes: 3 additions & 3 deletions runtime/runtime_memory_metering_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ func TestRuntimeMeterEncoding(t *testing.T) {
)

require.NoError(t, err)
assert.Equal(t, 114, int(meter.getMemory(common.MemoryKindBytes)))
assert.Equal(t, 107, int(meter.getMemory(common.MemoryKindBytes)))
})

t.Run("string in loop", func(t *testing.T) {
Expand Down Expand Up @@ -1122,7 +1122,7 @@ func TestRuntimeMeterEncoding(t *testing.T) {
)

require.NoError(t, err)
assert.Equal(t, 61501, int(meter.getMemory(common.MemoryKindBytes)))
assert.Equal(t, 61494, int(meter.getMemory(common.MemoryKindBytes)))
})

t.Run("composite", func(t *testing.T) {
Expand Down Expand Up @@ -1173,6 +1173,6 @@ func TestRuntimeMeterEncoding(t *testing.T) {
)

require.NoError(t, err)
assert.Equal(t, 58369, int(meter.getMemory(common.MemoryKindBytes)))
assert.Equal(t, 58362, int(meter.getMemory(common.MemoryKindBytes)))
})
}
2 changes: 1 addition & 1 deletion runtime/runtime_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7579,7 +7579,7 @@ func TestRuntimeComputationMetring(t *testing.T) {
`,
ok: true,
hits: 3,
intensity: 115,
intensity: 108,
},
}

Expand Down

0 comments on commit 0abfa9a

Please sign in to comment.