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

Improve storage path capability migration reporting #3522

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Improve storage path capability migration reporting #3522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions migrations/capcons/capabilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,43 @@
package capcons

import (
"fmt"
"sync"

"github.com/onflow/cadence/runtime/common"
"github.com/onflow/cadence/runtime/interpreter"
)

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),
},
},
)
}
Expand All @@ -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)
Expand All @@ -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(
Expand Down
73 changes: 50 additions & 23 deletions migrations/capcons/migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
},
Expand Down Expand Up @@ -3442,7 +3454,11 @@ func TestUntypedStorageCapMigration(t *testing.T) {
{
address: testAddress,
capability: AccountCapability{
Path: testPath,
TargetPath: testPath,
StoredPath: Path{
Domain: "storage",
Path: "cap",
},
},
},
},
Expand All @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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{}
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
},
)
Expand All @@ -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",
},
},
},
},
Expand Down
12 changes: 7 additions & 5 deletions migrations/capcons/storagecapmigration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -79,6 +79,8 @@ func (m *StorageCapMigration) Migrate(
m.StorageDomainCapabilities.Record(
pathCapabilityValue.AddressPath(),
pathCapabilityValue.BorrowType,
storageKey,
storageMapKey,
)
}

Expand Down Expand Up @@ -111,7 +113,7 @@ func IssueAccountCapabilities(

addressPath := interpreter.AddressPath{
Address: address,
Path: capability.Path,
Path: capability.TargetPath,
}

capabilityBorrowType := capability.BorrowType
Expand All @@ -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,
Expand All @@ -159,7 +161,7 @@ func IssueAccountCapabilities(
handler,
address,
borrowType,
capability.Path,
capability.TargetPath,
)

if hasBorrowType {
Expand Down
Loading