From 136a144c6211370c4d76d8b37ada090ef1253ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 30 Jul 2024 12:48:46 -0700 Subject: [PATCH 1/3] migrate capability values targeting storage paths by issuing cap cons --- migrations/capcons/capabilitymigration.go | 107 +++++++++++++++------- migrations/capcons/migration_test.go | 35 +++++++ 2 files changed, 110 insertions(+), 32 deletions(-) diff --git a/migrations/capcons/capabilitymigration.go b/migrations/capcons/capabilitymigration.go index e81b09cd58..117b2e101f 100644 --- a/migrations/capcons/capabilitymigration.go +++ b/migrations/capcons/capabilitymigration.go @@ -24,6 +24,7 @@ import ( "github.com/onflow/cadence/runtime/errors" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" + "github.com/onflow/cadence/runtime/stdlib" ) type CapabilityMigrationReporter interface { @@ -42,6 +43,7 @@ type CapabilityMigrationReporter interface { // using the path to ID capability controller mapping generated by LinkValueMigration. type CapabilityValueMigration struct { CapabilityMapping *CapabilityMapping + IssueHandler stdlib.CapabilityControllerIssueHandler Reporter CapabilityMigrationReporter } @@ -67,24 +69,42 @@ func (m *CapabilityValueMigration) Migrate( storageKey interpreter.StorageKey, _ interpreter.StorageMapKey, value interpreter.Value, - _ *interpreter.Interpreter, + inter *interpreter.Interpreter, _ migrations.ValueMigrationPosition, ) ( interpreter.Value, error, ) { + // Migrate path capabilities to ID capabilities + if pathCapabilityValue, ok := value.(*interpreter.PathCapabilityValue); ok { //nolint:staticcheck + return m.migratePathCapabilityValue(pathCapabilityValue, storageKey, inter) + } + + return nil, nil +} + +func (m *CapabilityValueMigration) migratePathCapabilityValue( + oldCapability *interpreter.PathCapabilityValue, + storageKey interpreter.StorageKey, + inter *interpreter.Interpreter, +) (interpreter.Value, error) { + reporter := m.Reporter - switch value := value.(type) { - case *interpreter.PathCapabilityValue: //nolint:staticcheck + capabilityAddressPath := oldCapability.AddressPath() + + var capabilityID interpreter.UInt64Value + var controllerBorrowType sema.Type - // Migrate the path capability to an ID capability + targetPath := capabilityAddressPath.Path - oldCapability := value + switch targetPath.Domain { + case common.PathDomainPublic, + common.PathDomainPrivate: - capabilityAddressPath := oldCapability.AddressPath() - capabilityID, controllerBorrowType, ok := m.CapabilityMapping.Get(capabilityAddressPath) + var ok bool + capabilityID, controllerBorrowType, ok = m.CapabilityMapping.Get(capabilityAddressPath) if !ok { if reporter != nil { reporter.MissingCapabilityID( @@ -92,40 +112,63 @@ func (m *CapabilityValueMigration) Migrate( capabilityAddressPath, ) } - break + return nil, nil } - oldBorrowType := oldCapability.BorrowType + case common.PathDomainStorage: + // The capability may incorrectly target a storage path, + // due to an incorrect implementation of the link function in an early version of Cadence. + // This bug was fixed in https://github.com/onflow/cadence/pull/865, + // and subsequent capabilities created by the link function have correct targets, + // but networks may still have capabilities with incorrect targets. + // + // In this case, there is no corresponding capability controller in the mapping, + // so issue a new capability controller. + + borrowType := inter.MustConvertStaticToSemaType(oldCapability.BorrowType).(*sema.ReferenceType) + controllerBorrowType = borrowType + + capabilityID, _ = stdlib.IssueStorageCapabilityController( + inter, + interpreter.EmptyLocationRange, + m.IssueHandler, + common.Address(oldCapability.Address), + borrowType, + targetPath, + ) - // Convert untyped path capability value to typed ID capability value - // by using capability controller's borrow type - if oldBorrowType == nil { - oldBorrowType = interpreter.ConvertSemaToStaticType(nil, controllerBorrowType) - } + default: + panic(errors.NewUnexpectedError("unexpected capability target path: %s", targetPath)) + } - newBorrowType, ok := oldBorrowType.(*interpreter.ReferenceStaticType) - if !ok { - panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", oldBorrowType)) - } + oldBorrowType := oldCapability.BorrowType - newCapability := interpreter.NewUnmeteredCapabilityValue( - capabilityID, - oldCapability.Address, - newBorrowType, - ) + // Convert untyped path capability value to typed ID capability value + // by using capability controller's borrow type + if oldBorrowType == nil { + oldBorrowType = interpreter.ConvertSemaToStaticType(nil, controllerBorrowType) + } - if reporter != nil { - reporter.MigratedPathCapability( - storageKey.Address, - capabilityAddressPath, - newBorrowType, - ) - } + newBorrowType, ok := oldBorrowType.(*interpreter.ReferenceStaticType) + if !ok { + panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", oldBorrowType)) + } - return newCapability, nil + newCapability := interpreter.NewUnmeteredCapabilityValue( + capabilityID, + oldCapability.Address, + newBorrowType, + ) + + if reporter != nil { + reporter.MigratedPathCapability( + storageKey.Address, + capabilityAddressPath, + newBorrowType, + ) } - return nil, nil + return newCapability, nil } func (m *CapabilityValueMigration) CanSkip(valueType interpreter.StaticType) bool { diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 879c78fba9..2fdfdfc446 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -508,6 +508,7 @@ func testPathCapabilityValueMigration( reporter, &CapabilityValueMigration{ CapabilityMapping: capabilityMapping, + IssueHandler: handler, Reporter: reporter, }, ), @@ -1065,6 +1066,38 @@ func TestPathCapabilityValueMigration(t *testing.T) { }, expectedEvents: []string{}, }, + { + name: "Path link, storage path", + // Equivalent to: getCapability<&Test.R>(/storage/test) + capabilityValue: &interpreter.PathCapabilityValue{ //nolint:staticcheck + BorrowType: testRReferenceStaticType, + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: testPathIdentifier, + }, + Address: interpreter.AddressValue(testAddress), + }, + pathLinks: nil, + expectedMigrations: []testMigration{ + expectedWrappedCapabilityValueMigration, + }, + expectedPathMigrations: []testCapConsPathCapabilityMigration{ + { + accountAddress: testAddress, + addressPath: interpreter.AddressPath{ + Address: testAddress, + Path: interpreter.NewUnmeteredPathValue( + common.PathDomainStorage, + testPathIdentifier, + ), + }, + borrowType: testRReferenceStaticType, + }, + }, + expectedEvents: []string{ + `flow.StorageCapabilityControllerIssued(id: 1, address: 0x0000000000000001, type: Type<&A.0000000000000001.Test.R>(), path: /storage/test)`, + }, + }, { name: "Account link, working chain (public), unauthorized", // Equivalent to: getCapability<&Account>(/public/test) @@ -2322,6 +2355,7 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) { reporter, &CapabilityValueMigration{ CapabilityMapping: capabilityMapping, + IssueHandler: handler, Reporter: reporter, }, ), @@ -2573,6 +2607,7 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) { reporter, &CapabilityValueMigration{ CapabilityMapping: capabilityMapping, + IssueHandler: handler, Reporter: reporter, }, ), From 571adf1508cceea2cab90c07e8e518519290fac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 30 Jul 2024 12:53:24 -0700 Subject: [PATCH 2/3] report capability ID --- migrations/capcons/capabilitymigration.go | 2 ++ migrations/capcons/migration_test.go | 39 ++++++++++++++++------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/migrations/capcons/capabilitymigration.go b/migrations/capcons/capabilitymigration.go index 117b2e101f..e07beec13d 100644 --- a/migrations/capcons/capabilitymigration.go +++ b/migrations/capcons/capabilitymigration.go @@ -32,6 +32,7 @@ type CapabilityMigrationReporter interface { accountAddress common.Address, addressPath interpreter.AddressPath, borrowType *interpreter.ReferenceStaticType, + capabilityID interpreter.UInt64Value, ) MissingCapabilityID( accountAddress common.Address, @@ -165,6 +166,7 @@ func (m *CapabilityValueMigration) migratePathCapabilityValue( storageKey.Address, capabilityAddressPath, newBorrowType, + capabilityID, ) } diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 2fdfdfc446..531a9a60dd 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -78,6 +78,7 @@ type testCapConsPathCapabilityMigration struct { accountAddress common.Address addressPath interpreter.AddressPath borrowType *interpreter.ReferenceStaticType + capabilityID interpreter.UInt64Value } type testCapConsMissingCapabilityID struct { @@ -140,6 +141,7 @@ func (t *testMigrationReporter) MigratedPathCapability( accountAddress common.Address, addressPath interpreter.AddressPath, borrowType *interpreter.ReferenceStaticType, + capabilityID interpreter.UInt64Value, ) { t.pathCapabilityMigrations = append( t.pathCapabilityMigrations, @@ -147,6 +149,7 @@ func (t *testMigrationReporter) MigratedPathCapability( accountAddress: accountAddress, addressPath: addressPath, borrowType: borrowType, + capabilityID: capabilityID, }, ) } @@ -690,7 +693,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 2, }, }, expectedEvents: []string{ @@ -745,7 +749,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -799,7 +804,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -879,7 +885,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 2, }, }, expectedEvents: []string{ @@ -936,7 +943,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -1091,7 +1099,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: testRReferenceStaticType, + borrowType: testRReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -1138,7 +1147,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: unauthorizedAccountReferenceStaticType, + borrowType: unauthorizedAccountReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -1189,7 +1199,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: fullyEntitledAccountReferenceStaticType, + borrowType: fullyEntitledAccountReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -1236,7 +1247,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: unauthorizedAccountReferenceStaticType, + borrowType: unauthorizedAccountReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -1283,7 +1295,8 @@ func TestPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: fullyEntitledAccountReferenceStaticType, + borrowType: fullyEntitledAccountReferenceStaticType, + capabilityID: 1, }, }, expectedEvents: []string{ @@ -2249,7 +2262,8 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) { testPathIdentifier, ), }, - borrowType: borrowType, + borrowType: borrowType, + capabilityID: 2, }, } @@ -2500,7 +2514,8 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) { ), }, // NOTE: link / cap con's borrow type is used - borrowType: linkBorrowType, + borrowType: linkBorrowType, + capabilityID: 2, }, } From 6f6ee81d9c5d7da638b665cca3b3fa8744bfd18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 30 Jul 2024 12:59:42 -0700 Subject: [PATCH 3/3] lint --- migrations/capcons/capabilitymigration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/capcons/capabilitymigration.go b/migrations/capcons/capabilitymigration.go index e07beec13d..d1a7f9c96d 100644 --- a/migrations/capcons/capabilitymigration.go +++ b/migrations/capcons/capabilitymigration.go @@ -86,7 +86,7 @@ func (m *CapabilityValueMigration) Migrate( } func (m *CapabilityValueMigration) migratePathCapabilityValue( - oldCapability *interpreter.PathCapabilityValue, + oldCapability *interpreter.PathCapabilityValue, //nolint:staticcheck storageKey interpreter.StorageKey, inter *interpreter.Interpreter, ) (interpreter.Value, error) {