From a0193b152e1d92738f98d7d152da4172a04c0057 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 9 Aug 2024 11:26:43 -0700 Subject: [PATCH 1/2] Report and skip storage caps with missing borrow type --- migrations/capcons/capabilitymigration.go | 10 + migrations/capcons/migration_test.go | 233 +++++++++++++++++++++- migrations/capcons/storagecapmigration.go | 26 ++- runtime/stdlib/account.go | 2 +- 4 files changed, 257 insertions(+), 14 deletions(-) diff --git a/migrations/capcons/capabilitymigration.go b/migrations/capcons/capabilitymigration.go index 21916bf231..9aa3fb697f 100644 --- a/migrations/capcons/capabilitymigration.go +++ b/migrations/capcons/capabilitymigration.go @@ -37,6 +37,16 @@ type CapabilityMigrationReporter interface { accountAddress common.Address, addressPath interpreter.AddressPath, ) + MissingBorrowType( + accountAddress common.Address, + addressPath interpreter.AddressPath, + ) + IssuedStorageCapability( + accountAddress common.Address, + addressPath interpreter.AddressPath, + borrowType *interpreter.ReferenceStaticType, + capabilityID interpreter.UInt64Value, + ) } // CapabilityValueMigration migrates all path capabilities to ID capabilities, diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 22d3a8eaee..f658d322af 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -86,6 +86,18 @@ type testCapConsMissingCapabilityID struct { addressPath interpreter.AddressPath } +type testStorageCapConIssued struct { + accountAddress common.Address + addressPath interpreter.AddressPath + borrowType *interpreter.ReferenceStaticType + capabilityID interpreter.UInt64Value +} + +type testStorageCapConsMissingBorrowType struct { + accountAddress common.Address + addressPath interpreter.AddressPath +} + type testMigration struct { storageKey interpreter.StorageKey storageMapKey interpreter.StorageMapKey @@ -93,13 +105,15 @@ type testMigration struct { } type testMigrationReporter struct { - migrations []testMigration - errors []error - linkMigrations []testCapConsLinkMigration - pathCapabilityMigrations []testCapConsPathCapabilityMigration - missingCapabilityIDs []testCapConsMissingCapabilityID - cyclicLinkErrors []CyclicLinkError - missingTargets []interpreter.AddressPath + migrations []testMigration + errors []error + linkMigrations []testCapConsLinkMigration + pathCapabilityMigrations []testCapConsPathCapabilityMigration + missingCapabilityIDs []testCapConsMissingCapabilityID + issuedStorageCapCons []testStorageCapConIssued + missingStorageCapConBorrowTypes []testStorageCapConsMissingBorrowType + cyclicLinkErrors []CyclicLinkError + missingTargets []interpreter.AddressPath } var _ migrations.Reporter = &testMigrationReporter{} @@ -167,6 +181,36 @@ func (t *testMigrationReporter) MissingCapabilityID( ) } +func (t *testMigrationReporter) MissingBorrowType( + accountAddress common.Address, + addressPath interpreter.AddressPath, +) { + t.missingStorageCapConBorrowTypes = append( + t.missingStorageCapConBorrowTypes, + testStorageCapConsMissingBorrowType{ + accountAddress: accountAddress, + addressPath: addressPath, + }, + ) +} + +func (t *testMigrationReporter) IssuedStorageCapability( + accountAddress common.Address, + addressPath interpreter.AddressPath, + borrowType *interpreter.ReferenceStaticType, + capabilityID interpreter.UInt64Value, +) { + t.issuedStorageCapCons = append( + t.issuedStorageCapCons, + testStorageCapConIssued{ + accountAddress: accountAddress, + addressPath: addressPath, + borrowType: borrowType, + capabilityID: capabilityID, + }, + ) +} + func (t *testMigrationReporter) CyclicLink(cyclicLinkError CyclicLinkError) { t.cyclicLinkErrors = append( t.cyclicLinkErrors, @@ -509,6 +553,7 @@ func testPathCapabilityValueMigration( if storageCapabilities != nil { IssueAccountCapabilities( inter, + reporter, testAddress, storageCapabilities, handler, @@ -2964,3 +3009,177 @@ func TestStorageCapMigration(t *testing.T) { actuals, ) } + +func TestStorageCapWithoutBorrowTypeMigration(t *testing.T) { + t.Parallel() + + capabilityValue := &interpreter.PathCapabilityValue{ //nolint:staticcheck + // Borrow type must be nil. + BorrowType: nil, + + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: testPathIdentifier, + }, + Address: interpreter.AddressValue(testAddress), + } + + rt := NewTestInterpreterRuntime() + + var events []cadence.Event + + runtimeInterface := &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]runtime.Address, error) { + return []runtime.Address{testAddress}, nil + }, + OnEmitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + } + + nextTransactionLocation := NewTransactionLocationGenerator() + + // Setup + + setupTransactionLocation := nextTransactionLocation() + + environment := runtime.NewScriptInterpreterEnvironment(runtime.Config{}) + + // Inject the path capability value. + // + // We don't have a way to create a path capability value in a Cadence program anymore, + // so we have to inject it manually. + + environment.DeclareValue( + stdlib.StandardLibraryValue{ + Name: "cap", + Type: &sema.CapabilityType{}, + Kind: common.DeclarationKindConstant, + Value: capabilityValue, + }, + setupTransactionLocation, + ) + + // Save capability value into account + + // language=cadence + setupTx := ` + transaction { + prepare(signer: auth(SaveValue) &Account) { + signer.storage.save(cap, to: /storage/cap) + } + } + ` + + err := rt.ExecuteTransaction( + runtime.Script{ + Source: []byte(setupTx), + }, + runtime.Context{ + Interface: runtimeInterface, + Environment: environment, + Location: setupTransactionLocation, + }, + ) + require.NoError(t, err) + + // Migrate + + storage, inter, err := rt.Storage(runtime.Context{ + Interface: runtimeInterface, + }) + require.NoError(t, err) + + migration, err := migrations.NewStorageMigration(inter, storage, "test", testAddress) + require.NoError(t, err) + + reporter := &testMigrationReporter{} + capabilityMapping := &CapabilityMapping{} + handler := &testCapConHandler{} + storageDomainCapabilities := &AccountsCapabilities{} + + migration.Migrate( + migration.NewValueMigrationsPathMigrator( + reporter, + &StorageCapMigration{ + StorageDomainCapabilities: storageDomainCapabilities, + }, + ), + ) + + storageCapabilities := storageDomainCapabilities.Get(testAddress) + require.NotNil(t, storageCapabilities) + IssueAccountCapabilities( + inter, + reporter, + testAddress, + storageCapabilities, + handler, + capabilityMapping, + ) + + err = migration.Commit() + require.NoError(t, err) + + // Assert + + require.Empty(t, reporter.migrations) + require.Empty(t, reporter.errors) + + require.Empty(t, reporter.missingCapabilityIDs) + require.Empty(t, reporter.issuedStorageCapCons) + require.Equal( + t, + []testStorageCapConsMissingBorrowType{ + { + accountAddress: testAddress, + addressPath: interpreter.AddressPath{ + Address: testAddress, + Path: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "test"), + }, + }, + }, + reporter.missingStorageCapConBorrowTypes, + ) + + err = storage.CheckHealth() + require.NoError(t, err) + + type actual struct { + address common.Address + capability AccountCapability + } + + var actuals []actual + + storageDomainCapabilities.ForEach( + testAddress, + func(accountCapability AccountCapability) bool { + actuals = append( + actuals, + actual{ + address: testAddress, + capability: accountCapability, + }, + ) + return true + }, + ) + + assert.Equal(t, + []actual{ + { + address: testAddress, + capability: AccountCapability{ + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: testPathIdentifier, + }, + }, + }, + }, + actuals, + ) +} diff --git a/migrations/capcons/storagecapmigration.go b/migrations/capcons/storagecapmigration.go index ff13157962..811f4a4c26 100644 --- a/migrations/capcons/storagecapmigration.go +++ b/migrations/capcons/storagecapmigration.go @@ -73,6 +73,7 @@ func (m *StorageCapMigration) CanSkip(valueType interpreter.StaticType) bool { func IssueAccountCapabilities( inter *interpreter.Interpreter, + reporter CapabilityMigrationReporter, address common.Address, capabilities *AccountCapabilities, handler stdlib.CapabilityControllerIssueHandler, @@ -80,7 +81,18 @@ func IssueAccountCapabilities( ) { for _, capability := range capabilities.Capabilities { - borrowType := inter.MustConvertStaticToSemaType(capability.BorrowType).(*sema.ReferenceType) + addressPath := interpreter.AddressPath{ + Address: address, + Path: capability.Path, + } + + borrowStaticType := capability.BorrowType + if borrowStaticType == nil { + reporter.MissingBorrowType(address, addressPath) + continue + } + + borrowType := inter.MustConvertStaticToSemaType(borrowStaticType).(*sema.ReferenceType) capabilityID, _ := stdlib.IssueStorageCapabilityController( inter, @@ -91,11 +103,13 @@ func IssueAccountCapabilities( capability.Path, ) - addressPath := interpreter.AddressPath{ - Address: address, - Path: capability.Path, - } - mapping.Record(addressPath, capabilityID, borrowType) + + reporter.IssuedStorageCapability( + address, + addressPath, + borrowStaticType.(*interpreter.ReferenceStaticType), + capabilityID, + ) } } diff --git a/runtime/stdlib/account.go b/runtime/stdlib/account.go index 4d0a02c96c..704b9c7129 100644 --- a/runtime/stdlib/account.go +++ b/runtime/stdlib/account.go @@ -2602,7 +2602,7 @@ func newAccountStorageCapabilitiesIssueFunction( panic(errors.NewUnreachableError()) } - // Get borrow type type argument + // Get borrow-type type-argument typeParameterPair := invocation.TypeParameterTypes.Oldest() ty := typeParameterPair.Value From 97ddc3129f3cb6a6ed90f24887f77c21042ddcd1 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 9 Aug 2024 12:40:12 -0700 Subject: [PATCH 2/2] Refactor code --- migrations/capcons/capabilitymigration.go | 10 ---------- migrations/capcons/migration_test.go | 3 ++- migrations/capcons/storagecapmigration.go | 17 +++++++++++++++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/migrations/capcons/capabilitymigration.go b/migrations/capcons/capabilitymigration.go index 9aa3fb697f..21916bf231 100644 --- a/migrations/capcons/capabilitymigration.go +++ b/migrations/capcons/capabilitymigration.go @@ -37,16 +37,6 @@ type CapabilityMigrationReporter interface { accountAddress common.Address, addressPath interpreter.AddressPath, ) - MissingBorrowType( - accountAddress common.Address, - addressPath interpreter.AddressPath, - ) - IssuedStorageCapability( - accountAddress common.Address, - addressPath interpreter.AddressPath, - borrowType *interpreter.ReferenceStaticType, - capabilityID interpreter.UInt64Value, - ) } // CapabilityValueMigration migrates all path capabilities to ID capabilities, diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index f658d322af..4e4268637f 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -119,6 +119,7 @@ type testMigrationReporter struct { var _ migrations.Reporter = &testMigrationReporter{} var _ LinkMigrationReporter = &testMigrationReporter{} var _ CapabilityMigrationReporter = &testMigrationReporter{} +var _ StorageCapabilityMigrationReporter = &testMigrationReporter{} func (t *testMigrationReporter) Migrated( storageKey interpreter.StorageKey, @@ -194,7 +195,7 @@ func (t *testMigrationReporter) MissingBorrowType( ) } -func (t *testMigrationReporter) IssuedStorageCapability( +func (t *testMigrationReporter) IssuedStorageCapabilityController( accountAddress common.Address, addressPath interpreter.AddressPath, borrowType *interpreter.ReferenceStaticType, diff --git a/migrations/capcons/storagecapmigration.go b/migrations/capcons/storagecapmigration.go index 811f4a4c26..6ab7b74c46 100644 --- a/migrations/capcons/storagecapmigration.go +++ b/migrations/capcons/storagecapmigration.go @@ -26,6 +26,19 @@ import ( "github.com/onflow/cadence/runtime/stdlib" ) +type StorageCapabilityMigrationReporter interface { + MissingBorrowType( + accountAddress common.Address, + addressPath interpreter.AddressPath, + ) + IssuedStorageCapabilityController( + accountAddress common.Address, + addressPath interpreter.AddressPath, + borrowType *interpreter.ReferenceStaticType, + capabilityID interpreter.UInt64Value, + ) +} + // StorageCapMigration records path capabilities with storage domain target. // It does not actually migrate any values. type StorageCapMigration struct { @@ -73,7 +86,7 @@ func (m *StorageCapMigration) CanSkip(valueType interpreter.StaticType) bool { func IssueAccountCapabilities( inter *interpreter.Interpreter, - reporter CapabilityMigrationReporter, + reporter StorageCapabilityMigrationReporter, address common.Address, capabilities *AccountCapabilities, handler stdlib.CapabilityControllerIssueHandler, @@ -105,7 +118,7 @@ func IssueAccountCapabilities( mapping.Record(addressPath, capabilityID, borrowType) - reporter.IssuedStorageCapability( + reporter.IssuedStorageCapabilityController( address, addressPath, borrowStaticType.(*interpreter.ReferenceStaticType),