From 7f0f77b41c6c74ba46848f9c5f3a5efe5586abd4 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 12 Aug 2024 17:02:01 -0700 Subject: [PATCH] Improve storage path capabiloty migration reporting --- migrations/capcons/capabilities.go | 31 ++++++++-- migrations/capcons/migration_test.go | 73 ++++++++++++++++------- migrations/capcons/storagecapmigration.go | 12 ++-- 3 files changed, 84 insertions(+), 32 deletions(-) diff --git a/migrations/capcons/capabilities.go b/migrations/capcons/capabilities.go index e6805386e7..f33f1e9bd2 100644 --- a/migrations/capcons/capabilities.go +++ b/migrations/capcons/capabilities.go @@ -19,6 +19,7 @@ package capcons import ( + "fmt" "sync" "github.com/onflow/cadence/runtime/common" @@ -26,20 +27,35 @@ import ( ) type AccountCapability struct { - Path interpreter.PathValue + TargetPath interpreter.PathValue BorrowType interpreter.StaticType + StoredPath Path +} + +type Path struct { + Domain string + Path string } type AccountCapabilities struct { Capabilities []AccountCapability } -func (c *AccountCapabilities) Record(path interpreter.PathValue, borrowType interpreter.StaticType) { +func (c *AccountCapabilities) Record( + path interpreter.PathValue, + borrowType interpreter.StaticType, + storageKey interpreter.StorageKey, + storageMapKey interpreter.StorageMapKey, +) { c.Capabilities = append( c.Capabilities, AccountCapability{ - Path: path, + TargetPath: path, BorrowType: borrowType, + StoredPath: Path{ + Domain: storageKey.Key, + Path: fmt.Sprintf("%s", storageMapKey), + }, }, ) } @@ -52,6 +68,8 @@ type AccountsCapabilities struct { func (m *AccountsCapabilities) Record( addressPath interpreter.AddressPath, borrowType interpreter.StaticType, + storageKey interpreter.StorageKey, + storageMapKey interpreter.StorageMapKey, ) { var accountCapabilities *AccountCapabilities rawAccountCapabilities, ok := m.accountCapabilities.Load(addressPath.Address) @@ -61,7 +79,12 @@ func (m *AccountsCapabilities) Record( accountCapabilities = &AccountCapabilities{} m.accountCapabilities.Store(addressPath.Address, accountCapabilities) } - accountCapabilities.Record(addressPath.Path, borrowType) + accountCapabilities.Record( + addressPath.Path, + borrowType, + storageKey, + storageMapKey, + ) } func (m *AccountsCapabilities) ForEach( diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 6b62867e3b..9834cbb1e3 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -3178,22 +3178,34 @@ func TestStorageCapMigration(t *testing.T) { { address: testAddress, capability: AccountCapability{ - Path: testPath, + TargetPath: testPath, BorrowType: testBorrowType1, + StoredPath: Path{ + Domain: "storage", + Path: "cap1", + }, }, }, { address: testAddress, capability: AccountCapability{ - Path: testPath, + TargetPath: testPath, BorrowType: testBorrowType2, + StoredPath: Path{ + Domain: "storage", + Path: "cap3", + }, }, }, { address: testAddress, capability: AccountCapability{ - Path: testPath, + TargetPath: testPath, BorrowType: testBorrowType2, + StoredPath: Path{ + Domain: "storage", + Path: "cap2", + }, }, }, }, @@ -3442,7 +3454,11 @@ func TestUntypedStorageCapMigration(t *testing.T) { { address: testAddress, capability: AccountCapability{ - Path: testPath, + TargetPath: testPath, + StoredPath: Path{ + Domain: "storage", + Path: "cap", + }, }, }, }, @@ -3453,22 +3469,23 @@ func TestUntypedStorageCapMigration(t *testing.T) { func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { t.Parallel() - testPath := interpreter.PathValue{ + addressA := common.MustBytesToAddress([]byte{0x1}) + addressB := common.MustBytesToAddress([]byte{0x2}) + + targetPath := interpreter.PathValue{ Domain: common.PathDomainStorage, Identifier: testPathIdentifier, } - testAddressPath := interpreter.AddressPath{ - Address: testAddress, - Path: testPath, - } + // Capability targets `addressB`. + // Capability itself is stored in `addressA` capabilityValue := &interpreter.PathCapabilityValue{ //nolint:staticcheck // Borrow type must be nil. BorrowType: nil, - Path: testPath, - Address: interpreter.AddressValue(testAddress), + Path: targetPath, + Address: interpreter.AddressValue(addressB), } rt := NewTestInterpreterRuntime() @@ -3478,7 +3495,7 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { runtimeInterface := &TestRuntimeInterface{ Storage: NewTestLedger(nil, nil), OnGetSigningAccounts: func() ([]runtime.Address, error) { - return []runtime.Address{testAddress}, nil + return []runtime.Address{addressA}, nil }, OnEmitEvent: func(event cadence.Event) error { events = append(events, event) @@ -3540,7 +3557,7 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { }) require.NoError(t, err) - migration, err := migrations.NewStorageMigration(inter, storage, "test", testAddress) + migration, err := migrations.NewStorageMigration(inter, storage, "test", addressA) require.NoError(t, err) reporter := &testMigrationReporter{} @@ -3558,14 +3575,14 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { ), ) - storageCapabilities := storageDomainCapabilities.Get(testAddress) + storageCapabilities := storageDomainCapabilities.Get(addressB) require.NotNil(t, storageCapabilities) IssueAccountCapabilities( inter, storage, reporter, - testAddress, + addressA, storageCapabilities, handler, typedStorageCapabilityMapping, @@ -3601,8 +3618,11 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { t, []testCapConsMissingCapabilityID{ { - accountAddress: testAddress, - addressPath: testAddressPath, + accountAddress: addressA, + addressPath: interpreter.AddressPath{ + Address: addressB, + Path: targetPath, + }, }, }, reporter.missingCapabilityIDs, @@ -3612,8 +3632,11 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { t, []testStorageCapConsMissingBorrowType{ { - accountAddress: testAddress, - addressPath: testAddressPath, + accountAddress: addressA, + addressPath: interpreter.AddressPath{ + Address: addressA, + Path: targetPath, + }, }, }, reporter.missingStorageCapConBorrowTypes, @@ -3630,12 +3653,12 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { var actuals []actual storageDomainCapabilities.ForEach( - testAddress, + addressB, func(accountCapability AccountCapability) bool { actuals = append( actuals, actual{ - address: testAddress, + address: addressA, capability: accountCapability, }, ) @@ -3646,9 +3669,13 @@ func TestUntypedStorageCapWithMissingTargetMigration(t *testing.T) { assert.Equal(t, []actual{ { - address: testAddress, + address: addressA, capability: AccountCapability{ - Path: testPath, + TargetPath: targetPath, + StoredPath: Path{ + Domain: "storage", + Path: "cap", + }, }, }, }, diff --git a/migrations/capcons/storagecapmigration.go b/migrations/capcons/storagecapmigration.go index 48fb94fe6b..73787112fa 100644 --- a/migrations/capcons/storagecapmigration.go +++ b/migrations/capcons/storagecapmigration.go @@ -63,8 +63,8 @@ func (*StorageCapMigration) Domains() map[string]struct{} { // Migrate records path capabilities with storage domain target. // It does not actually migrate any values. func (m *StorageCapMigration) Migrate( - _ interpreter.StorageKey, - _ interpreter.StorageMapKey, + storageKey interpreter.StorageKey, + storageMapKey interpreter.StorageMapKey, value interpreter.Value, _ *interpreter.Interpreter, _ migrations.ValueMigrationPosition, @@ -79,6 +79,8 @@ func (m *StorageCapMigration) Migrate( m.StorageDomainCapabilities.Record( pathCapabilityValue.AddressPath(), pathCapabilityValue.BorrowType, + storageKey, + storageMapKey, ) } @@ -111,7 +113,7 @@ func IssueAccountCapabilities( addressPath := interpreter.AddressPath{ Address: address, - Path: capability.Path, + Path: capability.TargetPath, } capabilityBorrowType := capability.BorrowType @@ -132,7 +134,7 @@ func IssueAccountCapabilities( } // If the borrow type is missing, then borrow it as the type of the value. - path := capability.Path.Identifier + path := capability.TargetPath.Identifier value := storageMap.ReadValue(nil, interpreter.StringStorageMapKey(path)) // However, if there is no value at the target, @@ -159,7 +161,7 @@ func IssueAccountCapabilities( handler, address, borrowType, - capability.Path, + capability.TargetPath, ) if hasBorrowType {