From d3bfdb8bc5711e4cb0e1f298d1b35d6b4c237c43 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Thu, 7 Nov 2024 17:37:32 -0600 Subject: [PATCH] Refactor storage domains to prevent import cycles 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. --- common/storagedomain.go | 117 ++++++++++++++++++++++++ interpreter/interpreter.go | 4 +- interpreter/value_composite.go | 2 +- runtime/capabilitycontrollers_test.go | 2 +- runtime/contract_test.go | 2 +- runtime/environment.go | 2 +- runtime/ft_test.go | 2 +- runtime/runtime_memory_metering_test.go | 2 +- runtime/sharedstate_test.go | 4 +- runtime/storage.go | 4 +- stdlib/account.go | 50 ++++------ stdlib/account_test.go | 11 ++- 12 files changed, 150 insertions(+), 52 deletions(-) create mode 100644 common/storagedomain.go diff --git a/common/storagedomain.go b/common/storagedomain.go new file mode 100644 index 0000000000..741756c44e --- /dev/null +++ b/common/storagedomain.go @@ -0,0 +1,117 @@ +/* + * Cadence - The resource-oriented smart contract programming language + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package common + +import ( + "github.com/onflow/cadence/errors" +) + +type StorageDomain uint8 + +const ( + StorageDomainUnknown StorageDomain = iota + + StorageDomainStorage + + StorageDomainPrivate + + StorageDomainPublic + + StorageDomainContract + + StorageDomainInbox + + // StorageDomainCapabilityController is the storage domain which stores + // capability controllers by capability ID + StorageDomainCapabilityController + + // StorageDomainCapabilityControllerTag is the storage domain which stores + // capability controller tags by capability ID + StorageDomainCapabilityControllerTag + + // StorageDomainPathCapability is the storage domain which stores + // capability ID dictionaries (sets) by storage path identifier + StorageDomainPathCapability + + // StorageDomainAccountCapability is the storage domain which + // records active account capability controller IDs + StorageDomainAccountCapability +) + +var AllStorageDomains = []StorageDomain{ + StorageDomainStorage, + StorageDomainPrivate, + StorageDomainPublic, + StorageDomainContract, + StorageDomainInbox, + StorageDomainCapabilityController, + StorageDomainCapabilityControllerTag, + StorageDomainPathCapability, + StorageDomainAccountCapability, +} + +var AllStorageDomainsByIdentifier = map[string]StorageDomain{} + +func init() { + for _, domain := range AllStorageDomains { + identifier := domain.Identifier() + AllStorageDomainsByIdentifier[identifier] = domain + } +} + +func StorageDomainFromIdentifier(domain string) (StorageDomain, bool) { + result, ok := AllStorageDomainsByIdentifier[domain] + if !ok { + return StorageDomainUnknown, false + } + return result, true +} + +func (d StorageDomain) Identifier() string { + switch d { + case StorageDomainStorage: + return PathDomainStorage.Identifier() + + case StorageDomainPrivate: + return PathDomainPrivate.Identifier() + + case StorageDomainPublic: + return PathDomainPublic.Identifier() + + case StorageDomainContract: + return "contract" + + case StorageDomainInbox: + return "inbox" + + case StorageDomainCapabilityController: + return "cap_con" + + case StorageDomainCapabilityControllerTag: + return "cap_tag" + + case StorageDomainPathCapability: + return "path_cap" + + case StorageDomainAccountCapability: + return "acc_cap" + } + + panic(errors.NewUnreachableError()) +} diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index a163efb01f..1c6533ba76 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -967,7 +967,7 @@ func (interpreter *Interpreter) declareSelfVariable(value Value, locationRange L } func (interpreter *Interpreter) visitAssignment( - transferOperation ast.TransferOperation, + _ ast.TransferOperation, targetGetterSetter getterSetter, targetType sema.Type, valueExpression ast.Expression, valueType sema.Type, position ast.HasPosition, @@ -1271,7 +1271,7 @@ func (declarationInterpreter *Interpreter) declareNonEnumCompositeValue( functions.Set(resourceDefaultDestroyEventName(compositeType), destroyEventConstructor) } - applyDefaultFunctions := func(ty *sema.InterfaceType, code WrapperCode) { + applyDefaultFunctions := func(_ *sema.InterfaceType, code WrapperCode) { // Apply default functions, if conforming type does not provide the function diff --git a/interpreter/value_composite.go b/interpreter/value_composite.go index d11311d543..cf460d466d 100644 --- a/interpreter/value_composite.go +++ b/interpreter/value_composite.go @@ -1658,7 +1658,7 @@ func (v *CompositeValue) getBaseValue( return NewEphemeralReferenceValue(interpreter, functionAuthorization, v.base, baseType, locationRange) } -func (v *CompositeValue) setBaseValue(interpreter *Interpreter, base *CompositeValue) { +func (v *CompositeValue) setBaseValue(_ *Interpreter, base *CompositeValue) { v.base = base } diff --git a/runtime/capabilitycontrollers_test.go b/runtime/capabilitycontrollers_test.go index fc6f692fe4..0e3427d97c 100644 --- a/runtime/capabilitycontrollers_test.go +++ b/runtime/capabilitycontrollers_test.go @@ -3253,7 +3253,7 @@ func TestRuntimeCapabilityControllers(t *testing.T) { storageMap := storage.GetStorageMap( common.MustBytesToAddress([]byte{0x1}), - stdlib.PathCapabilityStorageDomain, + common.StorageDomainPathCapability.Identifier(), false, ) require.Zero(t, storageMap.Count()) diff --git a/runtime/contract_test.go b/runtime/contract_test.go index 2bb89a2cb5..7cd3e72d49 100644 --- a/runtime/contract_test.go +++ b/runtime/contract_test.go @@ -223,7 +223,7 @@ func TestRuntimeContract(t *testing.T) { getContractValueExists := func() bool { storageMap := NewStorage(storage, nil). - GetStorageMap(signerAddress, StorageDomainContract, false) + GetStorageMap(signerAddress, common.StorageDomainContract.Identifier(), false) if storageMap == nil { return false } diff --git a/runtime/environment.go b/runtime/environment.go index 7873d4e990..ca6d48f01a 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -1108,7 +1108,7 @@ func (e *interpreterEnvironment) loadContract( if addressLocation, ok := location.(common.AddressLocation); ok { storageMap := e.storage.GetStorageMap( addressLocation.Address, - StorageDomainContract, + common.StorageDomainContract.Identifier(), false, ) if storageMap != nil { diff --git a/runtime/ft_test.go b/runtime/ft_test.go index b9374d986e..44602a00f0 100644 --- a/runtime/ft_test.go +++ b/runtime/ft_test.go @@ -1085,7 +1085,7 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { contractStorage := storage.GetStorageMap( contractsAddress, - StorageDomainContract, + common.StorageDomainContract.Identifier(), true, ) contractStorage.SetValue( diff --git a/runtime/runtime_memory_metering_test.go b/runtime/runtime_memory_metering_test.go index d6d389ff73..64512a1091 100644 --- a/runtime/runtime_memory_metering_test.go +++ b/runtime/runtime_memory_metering_test.go @@ -930,7 +930,7 @@ func TestRuntimeMemoryMeteringErrors(t *testing.T) { type memoryMeter map[common.MemoryKind]uint64 - runtimeInterface := func(meter memoryMeter) *TestRuntimeInterface { + runtimeInterface := func(memoryMeter) *TestRuntimeInterface { return &TestRuntimeInterface{ OnMeterMemory: func(usage common.MemoryUsage) error { if usage.Kind == common.MemoryKindStringValue || diff --git a/runtime/sharedstate_test.go b/runtime/sharedstate_test.go index 3008c85fff..4e3749a944 100644 --- a/runtime/sharedstate_test.go +++ b/runtime/sharedstate_test.go @@ -213,11 +213,11 @@ func TestRuntimeSharedState(t *testing.T) { []ownerKeyPair{ { owner: signerAddress[:], - key: []byte(StorageDomainContract), + key: []byte(common.StorageDomainContract.Identifier()), }, { owner: signerAddress[:], - key: []byte(StorageDomainContract), + key: []byte(common.StorageDomainContract.Identifier()), }, { owner: signerAddress[:], diff --git a/runtime/storage.go b/runtime/storage.go index 7b9a567285..94cb9431b9 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -32,8 +32,6 @@ import ( "github.com/onflow/cadence/interpreter" ) -const StorageDomainContract = "contract" - type Storage struct { *atree.PersistentSlabStorage NewStorageMaps *orderedmap.OrderedMap[interpreter.StorageKey, atree.SlabIndex] @@ -216,7 +214,7 @@ func (s *Storage) writeContractUpdate( key interpreter.StorageKey, contractValue *interpreter.CompositeValue, ) { - storageMap := s.GetStorageMap(key.Address, StorageDomainContract, true) + storageMap := s.GetStorageMap(key.Address, common.StorageDomainContract.Identifier(), true) // NOTE: pass nil instead of allocating a Value-typed interface that points to nil storageMapKey := interpreter.StringStorageMapKey(key.Key) if contractValue == nil { diff --git a/stdlib/account.go b/stdlib/account.go index 97a8e4d0f3..c1a303a2c5 100644 --- a/stdlib/account.go +++ b/stdlib/account.go @@ -938,8 +938,6 @@ func newAccountKeysRevokeFunction( } } -const InboxStorageDomain = "inbox" - func newAccountInboxPublishFunction( inter *interpreter.Interpreter, handler EventEmitter, @@ -996,7 +994,7 @@ func newAccountInboxPublishFunction( inter.WriteStored( provider, - InboxStorageDomain, + common.StorageDomainInbox.Identifier(), storageMapKey, publishedValue, ) @@ -1029,7 +1027,7 @@ func newAccountInboxUnpublishFunction( storageMapKey := interpreter.StringStorageMapKey(nameValue.Str) - readValue := inter.ReadStored(provider, InboxStorageDomain, storageMapKey) + readValue := inter.ReadStored(provider, common.StorageDomainInbox.Identifier(), storageMapKey) if readValue == nil { return interpreter.Nil } @@ -1065,7 +1063,7 @@ func newAccountInboxUnpublishFunction( inter.WriteStored( provider, - InboxStorageDomain, + common.StorageDomainInbox.Identifier(), storageMapKey, nil, ) @@ -1114,7 +1112,7 @@ func newAccountInboxClaimFunction( storageMapKey := interpreter.StringStorageMapKey(nameValue.Str) - readValue := inter.ReadStored(providerAddress, InboxStorageDomain, storageMapKey) + readValue := inter.ReadStored(providerAddress, common.StorageDomainInbox.Identifier(), storageMapKey) if readValue == nil { return interpreter.Nil } @@ -1155,7 +1153,7 @@ func newAccountInboxClaimFunction( inter.WriteStored( providerAddress, - InboxStorageDomain, + common.StorageDomainInbox.Identifier(), storageMapKey, nil, ) @@ -2983,10 +2981,6 @@ func IssueAccountCapabilityController( return capabilityIDValue } -// CapabilityControllerStorageDomain is the storage domain which stores -// capability controllers by capability ID -const CapabilityControllerStorageDomain = "cap_con" - // storeCapabilityController stores a capability controller in the account's capability ID to controller storage map func storeCapabilityController( inter *interpreter.Interpreter, @@ -2998,7 +2992,7 @@ func storeCapabilityController( existed := inter.WriteStored( address, - CapabilityControllerStorageDomain, + common.StorageDomainCapabilityController.Identifier(), storageMapKey, controller, ) @@ -3017,7 +3011,7 @@ func removeCapabilityController( existed := inter.WriteStored( address, - CapabilityControllerStorageDomain, + common.StorageDomainCapabilityController.Identifier(), storageMapKey, nil, ) @@ -3045,7 +3039,7 @@ func getCapabilityController( readValue := inter.ReadStored( address, - CapabilityControllerStorageDomain, + common.StorageDomainCapabilityController.Identifier(), storageMapKey, ) if readValue == nil { @@ -3225,10 +3219,6 @@ var capabilityIDSetStaticType = &interpreter.DictionaryStaticType{ ValueType: interpreter.NilStaticType, } -// PathCapabilityStorageDomain is the storage domain which stores -// capability ID dictionaries (sets) by storage path identifier -const PathCapabilityStorageDomain = "path_cap" - func recordStorageCapabilityController( inter *interpreter.Interpreter, locationRange interpreter.LocationRange, @@ -3254,7 +3244,7 @@ func recordStorageCapabilityController( storageMap := inter.Storage().GetStorageMap( address, - PathCapabilityStorageDomain, + common.StorageDomainPathCapability.Identifier(), true, ) @@ -3296,7 +3286,7 @@ func getPathCapabilityIDSet( storageMap := inter.Storage().GetStorageMap( address, - PathCapabilityStorageDomain, + common.StorageDomainPathCapability.Identifier(), false, ) if storageMap == nil { @@ -3346,7 +3336,7 @@ func unrecordStorageCapabilityController( if capabilityIDSet.Count() == 0 { storageMap := inter.Storage().GetStorageMap( address, - PathCapabilityStorageDomain, + common.StorageDomainPathCapability.Identifier(), true, ) if storageMap == nil { @@ -3397,10 +3387,6 @@ func getStorageCapabilityControllerIDsIterator( return } -// AccountCapabilityStorageDomain is the storage domain which -// records active account capability controller IDs -const AccountCapabilityStorageDomain = "acc_cap" - func recordAccountCapabilityController( inter *interpreter.Interpreter, locationRange interpreter.LocationRange, @@ -3418,7 +3404,7 @@ func recordAccountCapabilityController( storageMap := inter.Storage().GetStorageMap( address, - AccountCapabilityStorageDomain, + common.StorageDomainAccountCapability.Identifier(), true, ) @@ -3445,7 +3431,7 @@ func unrecordAccountCapabilityController( storageMap := inter.Storage().GetStorageMap( address, - AccountCapabilityStorageDomain, + common.StorageDomainAccountCapability.Identifier(), true, ) @@ -3464,7 +3450,7 @@ func getAccountCapabilityControllerIDsIterator( ) { storageMap := inter.Storage().GetStorageMap( address, - AccountCapabilityStorageDomain, + common.StorageDomainAccountCapability.Identifier(), false, ) if storageMap == nil { @@ -4427,10 +4413,6 @@ func newAccountCapabilityControllerDeleteFunction( } } -// CapabilityControllerTagStorageDomain is the storage domain which stores -// capability controller tags by capability ID -const CapabilityControllerTagStorageDomain = "cap_tag" - func getCapabilityControllerTag( inter *interpreter.Interpreter, address common.Address, @@ -4439,7 +4421,7 @@ func getCapabilityControllerTag( value := inter.ReadStored( address, - CapabilityControllerTagStorageDomain, + common.StorageDomainCapabilityControllerTag.Identifier(), interpreter.Uint64StorageMapKey(capabilityID), ) if value == nil { @@ -4501,7 +4483,7 @@ func setCapabilityControllerTag( inter.WriteStored( address, - CapabilityControllerTagStorageDomain, + common.StorageDomainCapabilityControllerTag.Identifier(), interpreter.Uint64StorageMapKey(capabilityID), value, ) diff --git a/stdlib/account_test.go b/stdlib/account_test.go index 2429663cc2..2218655c86 100644 --- a/stdlib/account_test.go +++ b/stdlib/account_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/onflow/cadence/common" "github.com/onflow/cadence/sema" . "github.com/onflow/cadence/test_utils/common_utils" ) @@ -33,11 +34,11 @@ func TestSemaCheckPathLiteralForInternalStorageDomains(t *testing.T) { t.Parallel() internalStorageDomains := []string{ - InboxStorageDomain, - AccountCapabilityStorageDomain, - CapabilityControllerStorageDomain, - PathCapabilityStorageDomain, - CapabilityControllerTagStorageDomain, + common.StorageDomainInbox.Identifier(), + common.StorageDomainAccountCapability.Identifier(), + common.StorageDomainCapabilityController.Identifier(), + common.StorageDomainPathCapability.Identifier(), + common.StorageDomainCapabilityControllerTag.Identifier(), } test := func(domain string) {