Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sync feature/combine-domain-payloads branch with master #3676

Merged
merged 6 commits into from
Nov 12, 2024
Merged

Sync feature/combine-domain-payloads branch with master #3676

merged 6 commits into from
Nov 12, 2024
+385 −276

Conversation

fxamacker
Copy link
Member

Updates #3584

Conflicts due to replacing domain string with StorageDomain enum 
commit 7660d65de4a4188d8f0dc1316ac5fb73af4c525a
Merge: b13fdba74 33c0dede6
Author: Faye Amacker <[email protected]>
Date:   Tue Nov 12 14:08:12 2024 -0600

    Merge branch 'master' into fxamacker/sync-feature-branch-combine-domain-payloads

diff --git a/cmd/decode-state-values/main.go b/cmd/decode-state-values/main.go
remerge CONFLICT (content): Merge conflict in cmd/decode-state-values/main.go
index 1f6e59932..5c61e11ba 100644
--- a/cmd/decode-state-values/main.go
+++ b/cmd/decode-state-values/main.go
@@ -234,11 +234,7 @@ type interpreterStorage struct {
 
 var _ interpreter.Storage = &interpreterStorage{}
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-func (i interpreterStorage) GetStorageMap(_ *interpreter.Interpreter, _ common.Address, _ string, _ bool) *interpreter.DomainStorageMap {
-=======
-func (i interpreterStorage) GetStorageMap(_ common.Address, _ common.StorageDomain, _ bool) *interpreter.StorageMap {
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+func (i interpreterStorage) GetStorageMap(_ *interpreter.Interpreter, _ common.Address, _ common.StorageDomain, _ bool) *interpreter.DomainStorageMap {
 	panic("unexpected GetStorageMap call")
 }
 
diff --git a/interpreter/account_storagemap.go b/interpreter/account_storagemap.go
index 67a51c1c3..7498ae14c 100644
--- a/interpreter/account_storagemap.go
+++ b/interpreter/account_storagemap.go
@@ -75,8 +75,8 @@ func NewAccountStorageMapWithRootID(
 }
 
 // DomainExists returns true if the given domain exists in the account storage map.
-func (s *AccountStorageMap) DomainExists(domain string) bool {
-	key := StringStorageMapKey(domain)
+func (s *AccountStorageMap) DomainExists(domain common.StorageDomain) bool {
+	key := StringStorageMapKey(domain.Identifier())
 
 	exists, err := s.orderedMap.Has(
 		key.AtreeValueCompare,
@@ -96,10 +96,10 @@ func (s *AccountStorageMap) DomainExists(domain string) bool {
 func (s *AccountStorageMap) GetDomain(
 	gauge common.MemoryGauge,
 	interpreter *Interpreter,
-	domain string,
+	domain common.StorageDomain,
 	createIfNotExists bool,
 ) *DomainStorageMap {
-	key := StringStorageMapKey(domain)
+	key := StringStorageMapKey(domain.Identifier())
 
 	storedValue, err := s.orderedMap.Get(
 		key.AtreeValueCompare,
@@ -129,13 +129,13 @@ func (s *AccountStorageMap) GetDomain(
 func (s *AccountStorageMap) NewDomain(
 	gauge common.MemoryGauge,
 	interpreter *Interpreter,
-	domain string,
+	domain common.StorageDomain,
 ) *DomainStorageMap {
 	interpreter.recordStorageMutation()
 
 	domainStorageMap := NewDomainStorageMap(gauge, s.orderedMap.Storage, s.orderedMap.Address())
 
-	key := StringStorageMapKey(domain)
+	key := StringStorageMapKey(domain.Identifier())
 
 	existingStorable, err := s.orderedMap.Set(
 		key.AtreeValueCompare,
@@ -149,7 +149,8 @@ func (s *AccountStorageMap) NewDomain(
 	if existingStorable != nil {
 		panic(errors.NewUnexpectedError(
 			"account %x domain %s should not exist",
-			s.orderedMap.Address(), domain,
+			s.orderedMap.Address(),
+			domain.Identifier(),
 		))
 	}
 
@@ -162,7 +163,7 @@ func (s *AccountStorageMap) NewDomain(
 // Returns true if domain storage map previously existed at the given domain.
 func (s *AccountStorageMap) WriteDomain(
 	interpreter *Interpreter,
-	domain string,
+	domain common.StorageDomain,
 	storageMap *DomainStorageMap,
 ) (existed bool) {
 	if storageMap == nil {
@@ -175,12 +176,12 @@ func (s *AccountStorageMap) WriteDomain(
 // If the given domain already stores a domain storage map, it is overwritten.
 func (s *AccountStorageMap) setDomain(
 	interpreter *Interpreter,
-	domain string,
+	domain common.StorageDomain,
 	storageMap *DomainStorageMap,
 ) (existed bool) {
 	interpreter.recordStorageMutation()
 
-	key := StringStorageMapKey(domain)
+	key := StringStorageMapKey(domain.Identifier())
 
 	existingValueStorable, err := s.orderedMap.Set(
 		key.AtreeValueCompare,
@@ -214,10 +215,10 @@ func (s *AccountStorageMap) setDomain(
 }
 
 // removeDomain removes domain storage map with given domain in account storage map, if it exists.
-func (s *AccountStorageMap) removeDomain(interpreter *Interpreter, domain string) (existed bool) {
+func (s *AccountStorageMap) removeDomain(interpreter *Interpreter, domain common.StorageDomain) (existed bool) {
 	interpreter.recordStorageMutation()
 
-	key := StringStorageMapKey(domain)
+	key := StringStorageMapKey(domain.Identifier())
 
 	existingKeyStorable, existingValueStorable, err := s.orderedMap.Remove(
 		key.AtreeValueCompare,
@@ -268,8 +269,8 @@ func (s *AccountStorageMap) Count() uint64 {
 }
 
 // Domains returns a set of domains in account storage map
-func (s *AccountStorageMap) Domains() map[string]struct{} {
-	domains := make(map[string]struct{})
+func (s *AccountStorageMap) Domains() map[common.StorageDomain]struct{} {
+	domains := make(map[common.StorageDomain]struct{})
 
 	iterator := s.Iterator()
 
@@ -314,15 +315,15 @@ type AccountStorageMapIterator struct {
 }
 
 // Next returns the next domain and domain storage map.
-// If there is no more domain, ("", nil) is returned.
-func (i *AccountStorageMapIterator) Next() (string, *DomainStorageMap) {
+// If there is no more domain, (common.StorageDomainUnknown, nil) is returned.
+func (i *AccountStorageMapIterator) Next() (common.StorageDomain, *DomainStorageMap) {
 	k, v, err := i.mapIterator.Next()
 	if err != nil {
 		panic(errors.NewExternalError(err))
 	}
 
 	if k == nil || v == nil {
-		return "", nil
+		return common.StorageDomainUnknown, nil
 	}
 
 	key := convertKeyToDomain(k)
@@ -332,10 +333,14 @@ func (i *AccountStorageMapIterator) Next() (string, *DomainStorageMap) {
 	return key, value
 }
 
-func convertKeyToDomain(v atree.Value) string {
+func convertKeyToDomain(v atree.Value) common.StorageDomain {
 	key, ok := v.(StringAtreeValue)
 	if !ok {
 		panic(errors.NewUnexpectedError("domain key type %T isn't expected", key))
 	}
-	return string(key)
+	domain, found := common.StorageDomainFromIdentifier(string(key))
+	if !found {
+		panic(errors.NewUnexpectedError("domain key %s isn't expected", key))
+	}
+	return domain
 }
diff --git a/interpreter/account_storagemap_test.go b/interpreter/account_storagemap_test.go
index cdc6fc868..b926f2724 100644
--- a/interpreter/account_storagemap_test.go
+++ b/interpreter/account_storagemap_test.go
@@ -49,7 +49,7 @@ func TestAccountStorageMapDomainExists(t *testing.T) {
 		require.NotNil(t, accountStorageMap)
 		require.Equal(t, uint64(0), accountStorageMap.Count())
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			exist := accountStorageMap.DomainExists(domain)
 			require.False(t, exist)
 		}
@@ -75,13 +75,13 @@ func TestAccountStorageMapDomainExists(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 		const count = 10
 		accountStorageMap, _ := createAccountStorageMap(storage, inter, address, existingDomains, count, random)
 
 		// Check if domain exists
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			exist := accountStorageMap.DomainExists(domain)
 			require.Equal(t, slices.Contains(existingDomains, domain), exist)
 		}
@@ -115,7 +115,7 @@ func TestAccountStorageMapGetDomain(t *testing.T) {
 		require.NotNil(t, accountStorageMap)
 		require.Equal(t, uint64(0), accountStorageMap.Count())
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			const createIfNotExists = false
 			storagemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
 			require.Nil(t, storagemap)
@@ -142,12 +142,12 @@ func TestAccountStorageMapGetDomain(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 		const count = 10
 		accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			const createIfNotExists = false
 			domainStoragemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
 			require.Equal(t, slices.Contains(existingDomains, domain), domainStoragemap != nil)
@@ -188,7 +188,7 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {
 		require.NotNil(t, accountStorageMap)
 		require.Equal(t, uint64(0), accountStorageMap.Count())
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			const createIfNotExists = true
 			domainStoragemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
 			require.NotNil(t, domainStoragemap)
@@ -220,12 +220,12 @@ func TestAccountStorageMapCreateDomain(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 		const count = 10
 		accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			const createIfNotExists = true
 			domainStoragemap := accountStorageMap.GetDomain(nil, inter, domain, createIfNotExists)
 			require.NotNil(t, domainStoragemap)
@@ -272,7 +272,7 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {
 		require.Equal(t, uint64(0), accountStorageMap.Count())
 
 		const count = 10
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 
 			domainStorageMap := interpreter.NewDomainStorageMap(nil, storage, atree.Address(address))
 			domainValues := writeRandomValuesToDomainStorageMap(inter, domainStorageMap, count, random)
@@ -306,12 +306,12 @@ func TestAccountStorageMapSetAndUpdateDomain(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 		const count = 10
 		accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 
 			domainStorageMap := interpreter.NewDomainStorageMap(nil, storage, atree.Address(address))
 			domainValues := writeRandomValuesToDomainStorageMap(inter, domainStorageMap, count, random)
@@ -355,7 +355,7 @@ func TestAccountStorageMapRemoveDomain(t *testing.T) {
 		require.NotNil(t, accountStorageMap)
 		require.Equal(t, uint64(0), accountStorageMap.Count())
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 			existed := accountStorageMap.WriteDomain(inter, domain, nil)
 			require.False(t, existed)
 		}
@@ -383,12 +383,12 @@ func TestAccountStorageMapRemoveDomain(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 		const count = 10
 		accountStorageMap, accountValues := createAccountStorageMap(storage, inter, address, existingDomains, count, random)
 
-		for _, domain := range runtime.AccountDomains {
+		for _, domain := range common.AllStorageDomains {
 
 			existed := accountStorageMap.WriteDomain(inter, domain, nil)
 			require.Equal(t, slices.Contains(existingDomains, domain), existed)
@@ -461,9 +461,9 @@ func TestAccountStorageMapIterator(t *testing.T) {
 			atreeStorageValidationEnabled,
 		)
 
-		existingDomains := []string{
-			common.PathDomainStorage.Identifier(),
-			common.PathDomainPublic.Identifier(),
+		existingDomains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
+			common.PathDomainPublic.StorageDomain(),
 		}
 
 		const count = 10
@@ -474,7 +474,7 @@ func TestAccountStorageMapIterator(t *testing.T) {
 		domainCount := 0
 		for {
 			domain, domainStorageMap := iterator.Next()
-			if domain == "" {
+			if domain == common.StorageDomainUnknown {
 				break
 			}
 
@@ -488,7 +488,7 @@ func TestAccountStorageMapIterator(t *testing.T) {
 
 		// Test calling Next() after iterator reaches the end.
 		domain, domainStorageMap := iterator.Next()
-		require.True(t, domain == "")
+		require.Equal(t, common.StorageDomainUnknown, domain)
 		require.Nil(t, domainStorageMap)
 
 		require.Equal(t, len(existingDomains), domainCount)
@@ -530,10 +530,10 @@ func TestAccountStorageMapDomains(t *testing.T) {
 		const atreeStorageValidationEnabled = false
 		inter := NewTestInterpreterWithStorageAndAtreeValidationConfig(t, storage, atreeValueValidationEnabled, atreeStorageValidationEnabled)
 
-		existingDomains := []string{
-			common.PathDomainStorage.Identifier(),
-			common.PathDomainPublic.Identifier(),
-			common.PathDomainPrivate.Identifier(),
+		existingDomains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
+			common.PathDomainPublic.StorageDomain(),
+			common.PathDomainPrivate.StorageDomain(),
 		}
 
 		const count = 10
@@ -590,10 +590,10 @@ func TestAccountStorageMapLoadFromRootSlabID(t *testing.T) {
 	})
 
 	t.Run("non-empty", func(t *testing.T) {
-		existingDomains := []string{
-			common.PathDomainStorage.Identifier(),
-			common.PathDomainPublic.Identifier(),
-			common.PathDomainPrivate.Identifier(),
+		existingDomains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
+			common.PathDomainPublic.StorageDomain(),
+			common.PathDomainPrivate.StorageDomain(),
 		}
 
 		init := func() (atree.SlabID, accountStorageMapValues, map[string][]byte, map[string]uint64) {
@@ -636,14 +636,14 @@ func TestAccountStorageMapLoadFromRootSlabID(t *testing.T) {
 
 type (
 	domainStorageMapValues  map[interpreter.StorageMapKey]interpreter.Value
-	accountStorageMapValues map[string]domainStorageMapValues
+	accountStorageMapValues map[common.StorageDomain]domainStorageMapValues
 )
 
 func createAccountStorageMap(
 	storage atree.SlabStorage,
 	inter *interpreter.Interpreter,
 	address common.Address,
-	domains []string,
+	domains []common.StorageDomain,
 	count int,
 	random *rand.Rand,
 ) (*interpreter.AccountStorageMap, accountStorageMapValues) {
@@ -710,7 +710,7 @@ func checkAccountStorageMapData(
 	iter := accountStorageMap.Iterator()
 	for {
 		domain, domainStorageMap := iter.Next()
-		if domain == "" {
+		if domain == common.StorageDomainUnknown {
 			break
 		}
 
diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go
remerge CONFLICT (content): Merge conflict in interpreter/interpreter.go
index 2b813c9c6..7b8abb0d7 100644
--- a/interpreter/interpreter.go
+++ b/interpreter/interpreter.go
@@ -237,11 +237,7 @@ func (c TypeCodes) Merge(codes TypeCodes) {
 
 type Storage interface {
 	atree.SlabStorage
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	GetStorageMap(inter *Interpreter, address common.Address, domain string, createIfNotExists bool) *DomainStorageMap
-=======
-	GetStorageMap(address common.Address, domain common.StorageDomain, createIfNotExists bool) *StorageMap
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+	GetStorageMap(inter *Interpreter, address common.Address, domain common.StorageDomain, createIfNotExists bool) *DomainStorageMap
 	CheckHealth() error
 }
 
@@ -4073,11 +4069,7 @@ func (interpreter *Interpreter) IsSubTypeOfSemaType(staticSubType StaticType, su
 }
 
 func (interpreter *Interpreter) domainPaths(address common.Address, domain common.PathDomain) []Value {
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	storageMap := interpreter.Storage().GetStorageMap(interpreter, address, domain.Identifier(), false)
-=======
-	storageMap := interpreter.Storage().GetStorageMap(address, domain.StorageDomain(), false)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+	storageMap := interpreter.Storage().GetStorageMap(interpreter, address, domain.StorageDomain(), false)
 	if storageMap == nil {
 		return []Value{}
 	}
@@ -4172,11 +4164,7 @@ func (interpreter *Interpreter) newStorageIterationFunction(
 			parameterTypes := fnType.ParameterTypes()
 			returnType := fnType.ReturnTypeAnnotation.Type
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-			storageMap := config.Storage.GetStorageMap(interpreter, address, domain.Identifier(), false)
-=======
-			storageMap := config.Storage.GetStorageMap(address, domain.StorageDomain(), false)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+			storageMap := config.Storage.GetStorageMap(interpreter, address, domain.StorageDomain(), false)
 			if storageMap == nil {
 				// if nothing is stored, no iteration is required
 				return Void
diff --git a/interpreter/misc_test.go b/interpreter/misc_test.go
remerge CONFLICT (content): Merge conflict in interpreter/misc_test.go
index 088bce59a..f81454cbf 100644
--- a/interpreter/misc_test.go
+++ b/interpreter/misc_test.go
@@ -5349,13 +5349,8 @@ func TestInterpretReferenceFailableDowncasting(t *testing.T) {
 			true, // r is standalone.
 		)
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-		domain := storagePath.Domain.Identifier()
-		storageMap := storage.GetStorageMap(inter, storageAddress, domain, true)
-=======
 		domain := storagePath.Domain.StorageDomain()
-		storageMap := storage.GetStorageMap(storageAddress, domain, true)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+		storageMap := storage.GetStorageMap(inter, storageAddress, domain, true)
 		storageMapKey := interpreter.StringStorageMapKey(storagePath.Identifier)
 		storageMap.WriteValue(inter, storageMapKey, r)
 
diff --git a/interpreter/storage.go b/interpreter/storage.go
remerge CONFLICT (content): Merge conflict in interpreter/storage.go
index 6cc3d100c..e89417af3 100644
--- a/interpreter/storage.go
+++ b/interpreter/storage.go
@@ -147,11 +147,7 @@ func (k StorageKey) IsLess(o StorageKey) bool {
 // InMemoryStorage
 type InMemoryStorage struct {
 	*atree.BasicSlabStorage
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	StorageMaps map[StorageKey]*DomainStorageMap
-=======
-	StorageMaps map[StorageDomainKey]*StorageMap
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+	StorageMaps map[StorageDomainKey]*DomainStorageMap
 	memoryGauge common.MemoryGauge
 }
 
@@ -179,11 +175,7 @@ func NewInMemoryStorage(memoryGauge common.MemoryGauge) InMemoryStorage {
 
 	return InMemoryStorage{
 		BasicSlabStorage: slabStorage,
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-		StorageMaps:      make(map[StorageKey]*DomainStorageMap),
-=======
-		StorageMaps:      make(map[StorageDomainKey]*StorageMap),
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+		StorageMaps:      make(map[StorageDomainKey]*DomainStorageMap),
 		memoryGauge:      memoryGauge,
 	}
 }
diff --git a/interpreter/storage_test.go b/interpreter/storage_test.go
remerge CONFLICT (content): Merge conflict in interpreter/storage_test.go
index 16976f1d2..147385e05 100644
--- a/interpreter/storage_test.go
+++ b/interpreter/storage_test.go
@@ -524,11 +524,7 @@ func TestStorageOverwriteAndRemove(t *testing.T) {
 
 		const storageMapKey = StringStorageMapKey("test")
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-		storageMap := storage.GetStorageMap(inter, address, "storage", true)
-=======
-		storageMap := storage.GetStorageMap(address, common.StorageDomainPathStorage, true)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+		storageMap := storage.GetStorageMap(inter, address, common.StorageDomainPathStorage, true)
 		storageMap.WriteValue(inter, storageMapKey, array1)
 
 		// Overwriting delete any existing child slabs
diff --git a/interpreter/stringatreevalue_test.go b/interpreter/stringatreevalue_test.go
remerge CONFLICT (content): Merge conflict in interpreter/stringatreevalue_test.go
index b5dd848ed..bbeccd941 100644
--- a/interpreter/stringatreevalue_test.go
+++ b/interpreter/stringatreevalue_test.go
@@ -36,15 +36,6 @@ func TestLargeStringAtreeValueInSeparateSlab(t *testing.T) {
 
 	storage := NewInMemoryStorage(nil)
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-=======
-	storageMap := storage.GetStorageMap(
-		common.MustBytesToAddress([]byte{0x1}),
-		common.PathDomainStorage.StorageDomain(),
-		true,
-	)
-
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 	inter, err := NewInterpreter(
 		nil,
 		common.StringLocation("test"),
@@ -57,7 +48,7 @@ func TestLargeStringAtreeValueInSeparateSlab(t *testing.T) {
 	storageMap := storage.GetStorageMap(
 		inter,
 		common.MustBytesToAddress([]byte{0x1}),
-		common.PathDomainStorage.Identifier(),
+		common.PathDomainStorage.StorageDomain(),
 		true,
 	)
 
diff --git a/interpreter/value_test.go b/interpreter/value_test.go
remerge CONFLICT (content): Merge conflict in interpreter/value_test.go
index 01b4ce1c9..09f1e233d 100644
--- a/interpreter/value_test.go
+++ b/interpreter/value_test.go
@@ -3806,11 +3806,7 @@ func TestValue_ConformsToStaticType(t *testing.T) {
 		)
 		require.NoError(t, err)
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-		storageMap := storage.GetStorageMap(inter, testAddress, "storage", true)
-=======
-		storageMap := storage.GetStorageMap(testAddress, common.StorageDomainPathStorage, true)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+		storageMap := storage.GetStorageMap(inter, testAddress, common.StorageDomainPathStorage, true)
 		storageMap.WriteValue(inter, StringStorageMapKey("test"), TrueValue)
 
 		value := valueFactory(inter)
diff --git a/runtime/contract_test.go b/runtime/contract_test.go
remerge CONFLICT (content): Merge conflict in runtime/contract_test.go
index e2c955f7f..05a91e1b8 100644
--- a/runtime/contract_test.go
+++ b/runtime/contract_test.go
@@ -223,11 +223,7 @@ func TestRuntimeContract(t *testing.T) {
 
 		getContractValueExists := func() bool {
 			storageMap := NewStorage(storage, nil).
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-				GetStorageMap(inter, signerAddress, StorageDomainContract, false)
-=======
-				GetStorageMap(signerAddress, common.StorageDomainContract, false)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+				GetStorageMap(inter, signerAddress, common.StorageDomainContract, false)
 			if storageMap == nil {
 				return false
 			}
diff --git a/runtime/migrate_domain_registers.go b/runtime/migrate_domain_registers.go
index 2298e99e3..e51bf8188 100644
--- a/runtime/migrate_domain_registers.go
+++ b/runtime/migrate_domain_registers.go
@@ -33,7 +33,7 @@ type GetDomainStorageMapFunc func(
 	ledger atree.Ledger,
 	storage atree.SlabStorage,
 	address common.Address,
-	domain string,
+	domain common.StorageDomain,
 ) (*interpreter.DomainStorageMap, error)
 
 type DomainRegisterMigration struct {
@@ -152,7 +152,7 @@ func (m *DomainRegisterMigration) migrateDomains(
 
 	var accountStorageMap *interpreter.AccountStorageMap
 
-	for _, domain := range AccountDomains {
+	for _, domain := range common.AllStorageDomains {
 
 		domainStorageMap, err := m.getDomainStorageMap(m.ledger, m.storage, address, domain)
 		if err != nil {
@@ -173,7 +173,9 @@ func (m *DomainRegisterMigration) migrateDomains(
 		if existed {
 			// This shouldn't happen because we are inserting domain storage map into empty account storage map.
 			return nil, errors.NewUnexpectedError(
-				"failed to migrate domain %s for account %x: domain already exists in account storage map", domain, address,
+				"failed to migrate domain %s for account %x: domain already exists in account storage map",
+				domain.Identifier(),
+				address,
 			)
 		}
 
@@ -182,7 +184,7 @@ func (m *DomainRegisterMigration) migrateDomains(
 			// NOTE: removing non-existent domain registers is no-op.
 			err = m.ledger.SetValue(
 				address[:],
-				[]byte(domain),
+				[]byte(domain.Identifier()),
 				nil)
 		})
 		if err != nil {
diff --git a/runtime/migrate_domain_registers_test.go b/runtime/migrate_domain_registers_test.go
index 28769c9e1..2c18879db 100644
--- a/runtime/migrate_domain_registers_test.go
+++ b/runtime/migrate_domain_registers_test.go
@@ -119,14 +119,14 @@ func TestMigrateDomainRegisters(t *testing.T) {
 			{
 				address: address1,
 				domains: []domainInfo{
-					{domain: common.PathDomainStorage.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
-					{domain: common.PathDomainPrivate.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPrivate.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 			{
 				address: address2,
 				domains: []domainInfo{
-					{domain: common.PathDomainPublic.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 		}
@@ -178,14 +178,14 @@ func TestMigrateDomainRegisters(t *testing.T) {
 			{
 				address: address1,
 				domains: []domainInfo{
-					{domain: common.PathDomainStorage.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
-					{domain: common.PathDomainPrivate.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPrivate.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 			{
 				address: address2,
 				domains: []domainInfo{
-					{domain: common.PathDomainPublic.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 		}
@@ -217,13 +217,13 @@ func TestMigrateDomainRegisters(t *testing.T) {
 			{
 				address: address1,
 				domains: []domainInfo{
-					{domain: common.PathDomainStorage.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 			{
 				address: address2,
 				domains: []domainInfo{
-					{domain: common.PathDomainPublic.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 		}
@@ -250,13 +250,13 @@ func TestMigrateDomainRegisters(t *testing.T) {
 			{
 				address: address1,
 				domains: []domainInfo{
-					{domain: common.PathDomainStorage.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 			{
 				address: address2,
 				domains: []domainInfo{
-					{domain: common.PathDomainPublic.Identifier(), domainStorageMapCount: 10, maxDepth: 3},
+					{domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3},
 				},
 			},
 		}
@@ -281,7 +281,7 @@ func TestMigrateDomainRegisters(t *testing.T) {
 }
 
 type domainInfo struct {
-	domain                string
+	domain                common.StorageDomain
 	domainStorageMapCount int
 	maxDepth              int
 }
@@ -339,7 +339,7 @@ func newTestLedgerWithUnmigratedAccounts(
 
 			// Write domain register
 			domainStorageMapValueID := domainStorageMap.ValueID()
-			err := ledger.SetValue(address[:], []byte(domain), domainStorageMapValueID[8:])
+			err := ledger.SetValue(address[:], []byte(domain.Identifier()), domainStorageMapValueID[8:])
 			require.NoError(tb, err)
 
 			vid := domainStorageMap.ValueID()
diff --git a/runtime/sharedstate_test.go b/runtime/sharedstate_test.go
remerge CONFLICT (content): Merge conflict in runtime/sharedstate_test.go
index 181a714f3..287f03519 100644
--- a/runtime/sharedstate_test.go
+++ b/runtime/sharedstate_test.go
@@ -28,7 +28,6 @@ import (
 	"github.com/onflow/cadence/common"
 	"github.com/onflow/cadence/interpreter"
 	. "github.com/onflow/cadence/runtime"
-	"github.com/onflow/cadence/stdlib"
 	. "github.com/onflow/cadence/test_utils/runtime_utils"
 )
 
@@ -222,7 +221,7 @@ func TestRuntimeSharedState(t *testing.T) {
 			// Read returns no value.
 			{
 				owner: signerAddress[:],
-				key:   []byte(StorageDomainContract),
+				key:   []byte(common.StorageDomainContract.Identifier()),
 			},
 			// Read all available domain registers to check if it is a new account
 			// Read returns no value.
@@ -244,27 +243,23 @@ func TestRuntimeSharedState(t *testing.T) {
 			},
 			{
 				owner: signerAddress[:],
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-				key:   []byte(stdlib.InboxStorageDomain),
-=======
-				key:   []byte(common.StorageDomainContract.Identifier()),
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+				key:   []byte(common.StorageDomainInbox.Identifier()),
 			},
 			{
 				owner: signerAddress[:],
-				key:   []byte(stdlib.CapabilityControllerStorageDomain),
+				key:   []byte(common.StorageDomainCapabilityController.Identifier()),
 			},
 			{
 				owner: signerAddress[:],
-				key:   []byte(stdlib.CapabilityControllerTagStorageDomain),
+				key:   []byte(common.StorageDomainCapabilityControllerTag.Identifier()),
 			},
 			{
 				owner: signerAddress[:],
-				key:   []byte(stdlib.PathCapabilityStorageDomain),
+				key:   []byte(common.StorageDomainPathCapability.Identifier()),
 			},
 			{
 				owner: signerAddress[:],
-				key:   []byte(stdlib.AccountCapabilityStorageDomain),
+				key:   []byte(common.StorageDomainAccountCapability.Identifier()),
 			},
 			// Read account domain register
 			{
diff --git a/runtime/storage.go b/runtime/storage.go
remerge CONFLICT (content): Merge conflict in runtime/storage.go
index eb7440e93..253aeabe6 100644
--- a/runtime/storage.go
+++ b/runtime/storage.go
@@ -30,12 +30,9 @@ import (
 	"github.com/onflow/cadence/common/orderedmap"
 	"github.com/onflow/cadence/errors"
 	"github.com/onflow/cadence/interpreter"
-	"github.com/onflow/cadence/stdlib"
 )
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
 const (
-	StorageDomainContract = "contract"
 	AccountStorageKey = "stored"
 )
 
@@ -56,16 +53,10 @@ type Storage struct {
 
 	// cachedDomainStorageMaps is a cache of domain storage maps.
 	// Key is StorageKey{address, domain} and value is domain storage map.
-	cachedDomainStorageMaps map[interpreter.StorageKey]*interpreter.DomainStorageMap
+	cachedDomainStorageMaps map[interpreter.StorageDomainKey]*interpreter.DomainStorageMap
 
 	// contractUpdates is a cache of contract updates.
 	// Key is StorageKey{contract_address, contract_name} and value is contract composite value.
-=======
-type Storage struct {
-	*atree.PersistentSlabStorage
-	NewStorageMaps  *orderedmap.OrderedMap[interpreter.StorageDomainKey, atree.SlabIndex]
-	storageMaps     map[interpreter.StorageDomainKey]*interpreter.StorageMap
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 	contractUpdates *orderedmap.OrderedMap[interpreter.StorageKey, *interpreter.CompositeValue]
 
 	Ledger atree.Ledger
@@ -106,18 +97,11 @@ func NewStorage(ledger atree.Ledger, memoryGauge common.MemoryGauge) *Storage {
 		decodeTypeInfo,
 	)
 	return &Storage{
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
 		Ledger:                   ledger,
 		PersistentSlabStorage:    persistentSlabStorage,
 		cachedAccountStorageMaps: map[interpreter.StorageKey]*interpreter.AccountStorageMap{},
-		cachedDomainStorageMaps:  map[interpreter.StorageKey]*interpreter.DomainStorageMap{},
+		cachedDomainStorageMaps:  map[interpreter.StorageDomainKey]*interpreter.DomainStorageMap{},
 		memoryGauge:              memoryGauge,
-=======
-		Ledger:                ledger,
-		PersistentSlabStorage: persistentSlabStorage,
-		storageMaps:           map[interpreter.StorageDomainKey]*interpreter.StorageMap{},
-		memoryGauge:           memoryGauge,
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 	}
 }
 
@@ -132,10 +116,6 @@ func (s *Storage) GetStorageMap(
 ) (
 	storageMap *interpreter.DomainStorageMap,
 ) {
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-=======
-	key := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 
 	// Account can be migrated account, new account, or unmigrated account.
 	//
@@ -196,8 +176,7 @@ func (s *Storage) GetStorageMap(
 
 	// Get cached domain storage map if it exists.
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	domainStorageKey := interpreter.NewStorageKey(s.memoryGauge, address, domain)
+	domainStorageKey := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain)
 
 	if domainStorageMap := s.cachedDomainStorageMaps[domainStorageKey]; domainStorageMap != nil {
 		return domainStorageMap
@@ -222,18 +201,6 @@ func (s *Storage) GetStorageMap(
 		// Cache domain storage map
 		if domainStorageMap != nil {
 			s.cachedDomainStorageMaps[domainStorageKey] = domainStorageMap
-=======
-		var data []byte
-		var err error
-		errors.WrapPanic(func() {
-			data, err = s.Ledger.GetValue(
-				key.Address[:],
-				[]byte(key.Domain.Identifier()),
-			)
-		})
-		if err != nil {
-			panic(interpreter.WrappedExternalError(err))
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 		}
 
 		return domainStorageMap
@@ -249,19 +216,6 @@ func (s *Storage) GetStorageMap(
 	if domainStorageMap != nil {
 		// This is a unmigrated account with given domain register.
 
-		// Sanity check that domain is among expected domains.
-		if _, exist := accountDomainsSet[domain]; !exist {
-			// TODO: maybe also log unexpected domain.
-			panic(errors.NewUnexpectedError(
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-				"unexpected domain %s exists for account %s", domain, address.String(),
-=======
-				"invalid storage index for storage map with domain '%s': expected length %d, got %d",
-				domain.Identifier(), storageIndexLength, dataLength,
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
-			))
-		}
-
 		// Cache domain storage map
 		s.cachedDomainStorageMaps[domainStorageKey] = domainStorageMap
 
@@ -360,9 +314,12 @@ func getDomainStorageMapFromLegacyDomainRegister(
 	ledger atree.Ledger,
 	storage atree.SlabStorage,
 	address common.Address,
-	domain string,
+	domain common.StorageDomain,
 ) (*interpreter.DomainStorageMap, error) {
-	domainStorageSlabIndex, domainRegisterExists, err := getSlabIndexFromRegisterValue(ledger, address, []byte(domain))
+	domainStorageSlabIndex, domainRegisterExists, err := getSlabIndexFromRegisterValue(
+		ledger,
+		address,
+		[]byte(domain.Identifier()))
 	if err != nil {
 		return nil, err
 	}
@@ -398,8 +355,11 @@ func (s *Storage) isUnmigratedAccount(address common.Address) (bool, error) {
 	}
 
 	// Check most frequently used domains first, such as storage, public, private.
-	for _, domain := range AccountDomains {
-		_, domainExists, err := getSlabIndexFromRegisterValue(s.Ledger, address, []byte(domain))
+	for _, domain := range common.AllStorageDomains {
+		_, domainExists, err := getSlabIndexFromRegisterValue(
+			s.Ledger,
+			address,
+			[]byte(domain.Identifier()))
 		if err != nil {
 			return false, err
 		}
@@ -411,7 +371,6 @@ func (s *Storage) isUnmigratedAccount(address common.Address) (bool, error) {
 	return false, nil
 }
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
 // getSlabIndexFromRegisterValue returns register value as atree.SlabIndex.
 // This function returns error if
 // - underlying ledger panics, or
@@ -429,24 +388,6 @@ func getSlabIndexFromRegisterValue(
 	})
 	if err != nil {
 		return atree.SlabIndex{}, false, interpreter.WrappedExternalError(err)
-=======
-func (s *Storage) loadExistingStorageMap(address atree.Address, slabIndex atree.SlabIndex) *interpreter.StorageMap {
-
-	slabID := atree.NewSlabID(address, slabIndex)
-
-	return interpreter.NewStorageMapWithRootID(s, slabID)
-}
-
-func (s *Storage) StoreNewStorageMap(address atree.Address, domain common.StorageDomain) *interpreter.StorageMap {
-	storageMap := interpreter.NewStorageMap(s.memoryGauge, s, address)
-
-	slabIndex := storageMap.SlabID().Index()
-
-	storageKey := interpreter.NewStorageDomainKey(s.memoryGauge, common.Address(address), domain)
-
-	if s.NewStorageMaps == nil {
-		s.NewStorageMaps = &orderedmap.OrderedMap[interpreter.StorageDomainKey, atree.SlabIndex]{}
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 	}
 
 	dataLength := len(data)
@@ -525,11 +466,7 @@ func (s *Storage) writeContractUpdate(
 	key interpreter.StorageKey,
 	contractValue *interpreter.CompositeValue,
 ) {
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	storageMap := s.GetStorageMap(inter, key.Address, StorageDomainContract, true)
-=======
-	storageMap := s.GetStorageMap(key.Address, common.StorageDomainContract, true)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+	storageMap := s.GetStorageMap(inter, key.Address, common.StorageDomainContract, true)
 	// NOTE: pass nil instead of allocating a Value-typed  interface that points to nil
 	storageMapKey := interpreter.StringStorageMapKey(key.Key)
 	if contractValue == nil {
@@ -598,7 +535,7 @@ func (s *Storage) commitNewStorageMaps() error {
 		errors.WrapPanic(func() {
 			err = s.Ledger.SetValue(
 				pair.Key.Address[:],
-				[]byte(pair.Key.Domain.Identifier()),
+				[]byte(pair.Key.Key),
 				pair.Value[:],
 			)
 		})
@@ -630,9 +567,9 @@ func (s *Storage) migrateAccounts(inter *interpreter.Interpreter) error {
 		ledger atree.Ledger,
 		storage atree.SlabStorage,
 		address common.Address,
-		domain string,
+		domain common.StorageDomain,
 	) (*interpreter.DomainStorageMap, error) {
-		domainStorageKey := interpreter.NewStorageKey(s.memoryGauge, address, domain)
+		domainStorageKey := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain)
 
 		// Get cached domain storage map if available.
 		domainStorageMap := s.cachedDomainStorageMaps[domainStorageKey]
@@ -785,23 +722,3 @@ func (UnreferencedRootSlabsError) IsInternalError() {}
 func (e UnreferencedRootSlabsError) Error() string {
 	return fmt.Sprintf("slabs not referenced: %s", e.UnreferencedRootSlabIDs)
 }
-
-var AccountDomains = []string{
-	common.PathDomainStorage.Identifier(),
-	common.PathDomainPrivate.Identifier(),
-	common.PathDomainPublic.Identifier(),
-	StorageDomainContract,
-	stdlib.InboxStorageDomain,
-	stdlib.CapabilityControllerStorageDomain,
-	stdlib.CapabilityControllerTagStorageDomain,
-	stdlib.PathCapabilityStorageDomain,
-	stdlib.AccountCapabilityStorageDomain,
-}
-
-var accountDomainsSet = func() map[string]struct{} {
-	m := make(map[string]struct{})
-	for _, domain := range AccountDomains {
-		m[domain] = struct{}{}
-	}
-	return m
-}()
diff --git a/runtime/storage_test.go b/runtime/storage_test.go
remerge CONFLICT (content): Merge conflict in runtime/storage_test.go
index 9610bbc47..5c1299df9 100644
--- a/runtime/storage_test.go
+++ b/runtime/storage_test.go
@@ -39,7 +39,6 @@ import (
 	"github.com/onflow/cadence/encoding/json"
 	"github.com/onflow/cadence/interpreter"
 	. "github.com/onflow/cadence/runtime"
-	"github.com/onflow/cadence/stdlib"
 	. "github.com/onflow/cadence/test_utils/common_utils"
 	. "github.com/onflow/cadence/test_utils/interpreter_utils"
 	. "github.com/onflow/cadence/test_utils/runtime_utils"
@@ -64,21 +63,16 @@ func withWritesToStorage(
 		var address common.Address
 		random.Read(address[:])
 
-		storageKey := interpreter.StorageDomainKey{
+		storageKey := interpreter.StorageKey{
 			Address: address,
-			Domain:  common.StorageDomainPathStorage,
+			Key:     AccountStorageKey,
 		}
 
 		var slabIndex atree.SlabIndex
 		binary.BigEndian.PutUint32(slabIndex[:], randomIndex)
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
 		if storage.NewAccountStorageMapSlabIndices == nil {
 			storage.NewAccountStorageMapSlabIndices = &orderedmap.OrderedMap[interpreter.StorageKey, atree.SlabIndex]{}
-=======
-		if storage.NewStorageMaps == nil {
-			storage.NewStorageMaps = &orderedmap.OrderedMap[interpreter.StorageDomainKey, atree.SlabIndex]{}
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
 		}
 		storage.NewAccountStorageMapSlabIndices.Set(storageKey, slabIndex)
 	}
@@ -3116,11 +3110,7 @@ func TestRuntimeStorageInternalAccess(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-<<<<<<< b13fdba74 (Merge pull request #3664 from onflow/fxamacker/use-atree-map-to-replace-domain-regsiters)
-	storageMap := storage.GetStorageMap(inter, address, common.PathDomainStorage.Identifier(), false)
-=======
-	storageMap := storage.GetStorageMap(address, common.PathDomainStorage.StorageDomain(), false)
->>>>>>> 33c0dede6 (Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains)
+	storageMap := storage.GetStorageMap(inter, address, common.PathDomainStorage.StorageDomain(), false)
 	require.NotNil(t, storageMap)
 
 	// Read first
@@ -6252,7 +6242,7 @@ func TestRuntimeStorageReferenceAccess(t *testing.T) {
 
 type (
 	domainStorageMapValues  map[interpreter.StorageMapKey]interpreter.Value
-	accountStorageMapValues map[string]domainStorageMapValues
+	accountStorageMapValues map[common.StorageDomain]domainStorageMapValues
 )
 
 func TestRuntimeStorageForNewAccount(t *testing.T) {
@@ -6274,7 +6264,7 @@ func TestRuntimeStorageForNewAccount(t *testing.T) {
 
 		inter := NewTestInterpreterWithStorage(t, storage)
 
-		domain := common.PathDomainStorage.Identifier()
+		domain := common.PathDomainStorage.StorageDomain()
 
 		// Get non-existent domain storage map
 		const createIfNotExists = false
@@ -6303,13 +6293,13 @@ func TestRuntimeStorageForNewAccount(t *testing.T) {
 	// migration: no migraiton for new account.
 	createDomainTestCases := []struct {
 		name                  string
-		newDomains            []string
+		newDomains            []common.StorageDomain
 		domainStorageMapCount int
 		inlined               bool
 	}{
-		{name: "empty domain storage map", newDomains: []string{common.PathDomainStorage.Identifier()}, domainStorageMapCount: 0, inlined: true},
-		{name: "small domain storage map", newDomains: []string{common.PathDomainStorage.Identifier()}, domainStorageMapCount: 10, inlined: true},
-		{name: "large domain storage map", newDomains: []string{common.PathDomainStorage.Identifier()}, domainStorageMapCount: 20, inlined: false},
+		{name: "empty domain storage map", newDomains: []common.StorageDomain{common.PathDomainStorage.StorageDomain()}, domainStorageMapCount: 0, inlined: true},
+		{name: "small domain storage map", newDomains: []common.StorageDomain{common.PathDomainStorage.StorageDomain()}, domainStorageMapCount: 10, inlined: true},
+		{name: "large domain storage map", newDomains: []common.StorageDomain{common.PathDomainStorage.StorageDomain()}, domainStorageMapCount: 20, inlined: false},
 	}
 
 	for _, tc := range createDomainTestCases {
@@ -6405,9 +6395,9 @@ func TestRuntimeStorageForNewAccount(t *testing.T) {
 
 		accountValues := make(accountStorageMapValues)
 
-		domains := []string{
-			common.PathDomainStorage.Identifier(),
-			common.PathDomainPublic.Identifier(),
+		domains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
+			common.PathDomainPublic.StorageDomain(),
 		}
 
 		// Create empty domain storage map and commit
@@ -6527,7 +6517,7 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 		onRead LedgerOnRead,
 		onWrite LedgerOnWrite,
 		address common.Address,
-		domains []string,
+		domains []common.StorageDomain,
 		domainStorageMapCount int,
 	) (TestLedger, accountStorageMapValues) {
 		ledger := NewTestLedger(nil, nil)
@@ -6549,11 +6539,11 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 	// post-condition: no change
 	// migration: none
 	t.Run("read non-existent domain storage map", func(t *testing.T) {
-		existingDomains := []string{
-			common.PathDomainStorage.Identifier(),
+		existingDomains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
 		}
 
-		nonexistentDomain := common.PathDomainPublic.Identifier()
+		nonexistentDomain := common.PathDomainPublic.StorageDomain()
 
 		var writeCount int
 
@@ -6598,7 +6588,7 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 	for _, tc := range readExistingDomainTestCases {
 		t.Run("read existing domain storage map "+tc.name, func(t *testing.T) {
 
-			existingDomains := []string{common.PathDomainStorage.Identifier()}
+			existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 
 			var writeCount int
 
@@ -6650,33 +6640,33 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 	//  - account storage map with new domain storage map.
 	createDomainTestCases := []struct {
 		name                          string
-		existingDomains               []string
-		newDomains                    []string
+		existingDomains               []common.StorageDomain
+		newDomains                    []common.StorageDomain
 		existingDomainStorageMapCount int
 		newDomainStorageMapCount      int
 		isNewDomainStorageMapInlined  bool
 	}{
 		{
 			name:                          "empty domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      0,
 			isNewDomainStorageMapInlined:  true,
 		},
 		{
 			name:                          "small domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      10,
 			isNewDomainStorageMapInlined:  true,
 		},
 		{
 			name:                          "large domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      20,
 			isNewDomainStorageMapInlined:  false,
 		},
@@ -6769,7 +6759,7 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 
 		var writeEntries []OwnerKeyValue
 
-		existingDomains := []string{common.PathDomainStorage.Identifier()}
+		existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 		const existingDomainStorageMapCount = 5
 
 		// Create storage with account storage map
@@ -6859,9 +6849,9 @@ func TestRuntimeStorageForMigratedAccount(t *testing.T) {
 	// - read domain storage map and commit
 	t.Run("read, commit, update, commit, remove, commit", func(t *testing.T) {
 
-		domains := []string{
-			common.PathDomainStorage.Identifier(),
-			common.PathDomainPublic.Identifier(),
+		domains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
+			common.PathDomainPublic.StorageDomain(),
 		}
 		const domainStorageMapCount = 5
 
@@ -7007,7 +6997,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 		onRead LedgerOnRead,
 		onWrite LedgerOnWrite,
 		address common.Address,
-		domains []string,
+		domains []common.StorageDomain,
 		domainStorageMapCount int,
 	) (TestLedger, accountStorageMapValues) {
 		ledger := NewTestLedger(nil, nil)
@@ -7027,7 +7017,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 
 			// Write domain register
 			domainStorageMapValueID := domainStorageMap.ValueID()
-			err := ledger.SetValue(address[:], []byte(domain), domainStorageMapValueID[8:])
+			err := ledger.SetValue(address[:], []byte(domain.Identifier()), domainStorageMapValueID[8:])
 			require.NoError(t, err)
 
 			// Write elements to to domain storage map
@@ -7058,8 +7048,8 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 	// post-condition: no change
 	// migration: none because only read ops.
 	t.Run("read non-existent domain storage map", func(t *testing.T) {
-		existingDomains := []string{
-			common.PathDomainStorage.Identifier(),
+		existingDomains := []common.StorageDomain{
+			common.PathDomainStorage.StorageDomain(),
 		}
 
 		var writeCount int
@@ -7078,7 +7068,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 
 		// Get non-existent domain storage map
 		const createIfNotExists = false
-		nonexistingDomain := common.PathDomainPublic.Identifier()
+		nonexistingDomain := common.PathDomainPublic.StorageDomain()
 		domainStorageMap := storage.GetStorageMap(inter, address, nonexistingDomain, createIfNotExists)
 		require.Nil(t, domainStorageMap)
 
@@ -7108,7 +7098,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 
 			var writeCount int
 
-			existingDomains := []string{common.PathDomainStorage.Identifier()}
+			existingDomains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 			const existingDomainStorageMapCount = 5
 
 			// Create storage with existing domain storage map
@@ -7162,33 +7152,33 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 	// migration: yes
 	createDomainTestCases := []struct {
 		name                          string
-		existingDomains               []string
-		newDomains                    []string
+		existingDomains               []common.StorageDomain
+		newDomains                    []common.StorageDomain
 		existingDomainStorageMapCount int
 		newDomainStorageMapCount      int
 		isNewDomainStorageMapInlined  bool
 	}{
 		{
 			name:                          "empty domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      0,
 			isNewDomainStorageMapInlined:  true,
 		},
 		{
 			name:                          "small domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      10,
 			isNewDomainStorageMapInlined:  true,
 		},
 		{
 			name:                          "large domain storage map",
-			existingDomains:               []string{common.PathDomainStorage.Identifier()},
+			existingDomains:               []common.StorageDomain{common.PathDomainStorage.StorageDomain()},
 			existingDomainStorageMapCount: 5,
-			newDomains:                    []string{common.PathDomainPublic.Identifier()},
+			newDomains:                    []common.StorageDomain{common.PathDomainPublic.StorageDomain()},
 			newDomainStorageMapCount:      20,
 			isNewDomainStorageMapInlined:  false,
 		},
@@ -7242,7 +7232,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 			require.True(t, len(writeEntries) > 1+len(tc.existingDomains)+len(tc.newDomains))
 
 			i := 0
-			for _, domain := range AccountDomains {
+			for _, domain := range common.AllStorageDomains {
 
 				if slices.Contains(tc.existingDomains, domain) ||
 					slices.Contains(tc.newDomains, domain) {
@@ -7250,7 +7240,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 					// Existing and new domain registers are removed.
 					// Removing new (non-existent) domain registers is no-op.
 					require.Equal(t, address[:], writeEntries[i].Owner)
-					require.Equal(t, []byte(domain), writeEntries[i].Key)
+					require.Equal(t, []byte(domain.Identifier()), writeEntries[i].Key)
 					require.True(t, len(writeEntries[i].Value) == 0)
 
 					i++
@@ -7287,7 +7277,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 
 		var writeEntries []OwnerKeyValue
 
-		domains := []string{common.PathDomainStorage.Identifier()}
+		domains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 		const existingDomainStorageMapCount = 5
 
 		// Create storage with existing domain storage maps
@@ -7396,7 +7386,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) {
 
 		var writeEntries []OwnerKeyValue
 
-		domains := []string{common.PathDomainStorage.Identifier()}
+		domains := []common.StorageDomain{common.PathDomainStorage.StorageDomain()}
 		const domainStorageMapCount = 5
 
 		// Create storage with existing account storage map
@@ -7612,10 +7602,10 @@ func TestRuntimeStorageDomainStorageMapInlinedState(t *testing.T) {
 
 	inter := NewTestInterpreterWithStorage(t, storage)
 
-	domains := []string{
-		common.PathDomainStorage.Identifier(),
-		common.PathDomainPublic.Identifier(),
-		common.PathDomainPrivate.Identifier(),
+	domains := []common.StorageDomain{
+		common.PathDomainStorage.StorageDomain(),
+		common.PathDomainPublic.StorageDomain(),
+		common.PathDomainPrivate.StorageDomain(),
 	}
 
 	const domainStorageMapCount = 500
@@ -7734,10 +7724,10 @@ func TestRuntimeStorageLargeDomainValues(t *testing.T) {
 
 	inter := NewTestInterpreterWithStorage(t, storage)
 
-	domains := []string{
-		common.PathDomainStorage.Identifier(),
-		common.PathDomainPublic.Identifier(),
-		common.PathDomainPrivate.Identifier(),
+	domains := []common.StorageDomain{
+		common.PathDomainStorage.StorageDomain(),
+		common.PathDomainPublic.StorageDomain(),
+		common.PathDomainPrivate.StorageDomain(),
 	}
 
 	const domainStorageMapCount = 5
@@ -7853,9 +7843,9 @@ func TestDomainRegisterMigrationForLargeAccount(t *testing.T) {
 		{
 			address: address,
 			domains: []domainInfo{
-				{domain: common.PathDomainStorage.Identifier(), domainStorageMapCount: 100, maxDepth: 3},
-				{domain: common.PathDomainPublic.Identifier(), domainStorageMapCount: 100, maxDepth: 3},
-				{domain: common.PathDomainPrivate.Identifier(), domainStorageMapCount: 100, maxDepth: 3},
+				{domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 100, maxDepth: 3},
+				{domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 100, maxDepth: 3},
+				{domain: common.PathDomainPrivate.StorageDomain(), domainStorageMapCount: 100, maxDepth: 3},
 			},
 		},
 	}
@@ -7874,7 +7864,7 @@ func TestDomainRegisterMigrationForLargeAccount(t *testing.T) {
 
 	// Create new domain storage map
 	const createIfNotExists = true
-	domain := stdlib.InboxStorageDomain
+	domain := common.StorageDomainInbox
 	domainStorageMap := storage.GetStorageMap(inter, address, domain, createIfNotExists)
 	require.NotNil(t, domainStorageMap)
 
@@ -7921,7 +7911,7 @@ func createAndWriteAccountStorageMap(
 	storage *Storage,
 	inter *interpreter.Interpreter,
 	address common.Address,
-	domains []string,
+	domains []common.StorageDomain,
 	count int,
 	random *rand.Rand,
 ) accountStorageMapValues {
@@ -8006,7 +7996,7 @@ func checkAccountStorageMapData(
 	iter := accountStorageMap.Iterator()
 	for {
 		domain, domainStorageMap := iter.Next()
-		if domain == "" {
+		if domain == common.StorageDomainUnknown {
 			break
 		}
  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@fxamacker
Refactor storage domains to prevent import cycles
d3bfdb8 
Currently, various storage domains are defined in
different packages, such as:
- common
- runtime
- stdlib

This commit moves storage domains from different
packages to a single place: common/storagedomain.go.

This change will help us avoid import cycles when
using domains across different packages.
@fxamacker
Refactor to get storage map with common.StorageDomain
2f8c298 
Currently, we use domain string to get StorageMap.  Even though
we use pre-defined domain strings, this can still be error-prone.

This commit modifies GetStorageMap() to use common.StorageDomain
instead of string.
@fxamacker
Remove unused TypeInfo.Identifier()
28a1f00 
atree.TypeInfo interface used to include Identifier() function.
However, Identifier() was removed later and is no longer part of
atree.TypeInfo interface.

This commit removes Identifier() function from structs
implementing atree.TypeInfo interface.
@fxamacker
Merge pull request #3675 from onflow/fxamacker/remove-unused-function…
44376e4 
…s-for-atree-typeinfo

Remove unused Identifier() function from structs implementing atree.TypeInfo
@fxamacker
Merge pull request #3673 from onflow/fxamacker/refactor-storage-domains
33c0ded 
Refactor storage domains to prevent import cycles and simplify maintenance
@fxamacker
Merge branch 'master' into fxamacker/sync-feature-branch-combine-doma…
7660d65 
…in-payloads
@fxamacker fxamacker self-assigned this Nov 12, 2024
@github-actions GitHub Actions
Copy link

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/combine-domain-payloads-and-domain-storage-maps commit b13fdba
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

@fxamacker fxamacker merged commit cc95223 into feature/combine-domain-payloads-and-domain-storage-maps Nov 12, 2024
7 of 9 checks passed
@fxamacker fxamacker mentioned this pull request Nov 14, 2024
Open
@BrewTestBot BrewTestBot mentioned this pull request Dec 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turbolent turbolent turbolent approved these changes

@SupunS SupunS Awaiting requested review from SupunS SupunS is a code owner

Assignees

@fxamacker fxamacker

Labels
Chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fxamacker @turbolent