From f65ca60387ff9f93c2e7f6da5c182033cb45a6ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 13 Nov 2024 16:56:33 -0800 Subject: [PATCH 1/6] simplify domain register migration: only migrate one account specifically --- runtime/migrate_domain_registers.go | 127 +++++----------- runtime/migrate_domain_registers_test.go | 183 ++++------------------- runtime/storage.go | 56 ------- 3 files changed, 69 insertions(+), 297 deletions(-) diff --git a/runtime/migrate_domain_registers.go b/runtime/migrate_domain_registers.go index 2ffd38830..d5b7f7aa0 100644 --- a/runtime/migrate_domain_registers.go +++ b/runtime/migrate_domain_registers.go @@ -22,26 +22,16 @@ import ( "github.com/onflow/atree" "github.com/onflow/cadence/common" - "github.com/onflow/cadence/common/orderedmap" "github.com/onflow/cadence/errors" "github.com/onflow/cadence/interpreter" ) -type AccountStorageMaps = *orderedmap.OrderedMap[common.Address, *interpreter.AccountStorageMap] - -type GetDomainStorageMapFunc func( - ledger atree.Ledger, - storage atree.SlabStorage, - address common.Address, - domain common.StorageDomain, -) (*interpreter.DomainStorageMap, error) - +// DomainRegisterMigration migrates domain registers to account storage maps. type DomainRegisterMigration struct { - ledger atree.Ledger - storage atree.SlabStorage - inter *interpreter.Interpreter - memoryGauge common.MemoryGauge - getDomainStorageMap GetDomainStorageMapFunc + ledger atree.Ledger + storage atree.SlabStorage + inter *interpreter.Interpreter + memoryGauge common.MemoryGauge } func NewDomainRegisterMigration( @@ -51,74 +41,30 @@ func NewDomainRegisterMigration( memoryGauge common.MemoryGauge, ) *DomainRegisterMigration { return &DomainRegisterMigration{ - ledger: ledger, - storage: storage, - inter: inter, - memoryGauge: memoryGauge, - getDomainStorageMap: getDomainStorageMapFromV1DomainRegister, + ledger: ledger, + storage: storage, + inter: inter, + memoryGauge: memoryGauge, } } -// SetGetDomainStorageMapFunc allows user to provide custom GetDomainStorageMap function. -func (m *DomainRegisterMigration) SetGetDomainStorageMapFunc( - getDomainStorageMapFunc GetDomainStorageMapFunc, -) { - m.getDomainStorageMap = getDomainStorageMapFunc -} - -// MigrateAccounts migrates given accounts. -func (m *DomainRegisterMigration) MigrateAccounts( - accounts *orderedmap.OrderedMap[common.Address, struct{}], - pred func(common.Address) bool, +func (m *DomainRegisterMigration) MigrateAccount( + address common.Address, ) ( - AccountStorageMaps, + *interpreter.AccountStorageMap, error, ) { - if accounts == nil || accounts.Len() == 0 { - return nil, nil + exists, err := hasAccountStorageMap(m.ledger, address) + if err != nil { + return nil, err } - - var migratedAccounts AccountStorageMaps - - for pair := accounts.Oldest(); pair != nil; pair = pair.Next() { - address := pair.Key - - if !pred(address) { - continue - } - - migrated, err := isMigrated(m.ledger, address) - if err != nil { - return nil, err - } - if migrated { - continue - } - - accountStorageMap, err := m.MigrateAccount(address) - if err != nil { - return nil, err - } - - if accountStorageMap == nil { - continue - } - - if migratedAccounts == nil { - migratedAccounts = &orderedmap.OrderedMap[common.Address, *interpreter.AccountStorageMap]{} - } - migratedAccounts.Set(address, accountStorageMap) + if exists { + // Account storage map already exists + return nil, nil } - return migratedAccounts, nil -} - -func (m *DomainRegisterMigration) MigrateAccount( - address common.Address, -) (*interpreter.AccountStorageMap, error) { - // Migrate existing domains - accountStorageMap, err := m.migrateDomains(address) + accountStorageMap, err := m.migrateDomainRegisters(address) if err != nil { return nil, err } @@ -128,14 +74,14 @@ func (m *DomainRegisterMigration) MigrateAccount( return nil, nil } - accountStorageMapSlabIndex := accountStorageMap.SlabID().Index() + slabIndex := accountStorageMap.SlabID().Index() // Write account register errors.WrapPanic(func() { err = m.ledger.SetValue( address[:], []byte(AccountStorageKey), - accountStorageMapSlabIndex[:], + slabIndex[:], ) }) if err != nil { @@ -145,16 +91,25 @@ func (m *DomainRegisterMigration) MigrateAccount( return accountStorageMap, nil } -// migrateDomains migrates existing domain storage maps and removes domain registers. -func (m *DomainRegisterMigration) migrateDomains( +// migrateDomainRegisters migrates all existing domain storage maps to a new account storage map, +// and removes the domain registers. +func (m *DomainRegisterMigration) migrateDomainRegisters( address common.Address, -) (*interpreter.AccountStorageMap, error) { +) ( + *interpreter.AccountStorageMap, + error, +) { var accountStorageMap *interpreter.AccountStorageMap for _, domain := range common.AllStorageDomains { - domainStorageMap, err := m.getDomainStorageMap(m.ledger, m.storage, address, domain) + domainStorageMap, err := getDomainStorageMapFromV1DomainRegister( + m.ledger, + m.storage, + address, + domain, + ) if err != nil { return nil, err } @@ -165,7 +120,11 @@ func (m *DomainRegisterMigration) migrateDomains( } if accountStorageMap == nil { - accountStorageMap = interpreter.NewAccountStorageMap(m.memoryGauge, m.storage, atree.Address(address)) + accountStorageMap = interpreter.NewAccountStorageMap( + m.memoryGauge, + m.storage, + atree.Address(address), + ) } // Migrate (insert) existing domain storage map to account storage map @@ -194,11 +153,3 @@ func (m *DomainRegisterMigration) migrateDomains( return accountStorageMap, nil } - -func isMigrated(ledger atree.Ledger, address common.Address) (bool, error) { - _, registerExists, err := getSlabIndexFromRegisterValue(ledger, address, []byte(AccountStorageKey)) - if err != nil { - return false, err - } - return registerExists, nil -} diff --git a/runtime/migrate_domain_registers_test.go b/runtime/migrate_domain_registers_test.go index 75f016748..db1038f61 100644 --- a/runtime/migrate_domain_registers_test.go +++ b/runtime/migrate_domain_registers_test.go @@ -31,7 +31,6 @@ import ( "github.com/onflow/atree" "github.com/onflow/cadence/common" - "github.com/onflow/cadence/common/orderedmap" "github.com/onflow/cadence/errors" "github.com/onflow/cadence/interpreter" "github.com/onflow/cadence/runtime" @@ -42,20 +41,6 @@ import ( func TestMigrateDomainRegisters(t *testing.T) { t.Parallel() - alwaysMigrate := func(common.Address) bool { - return true - } - - neverMigrate := func(common.Address) bool { - return false - } - - migrateSpecificAccount := func(addressToMigrate common.Address) func(common.Address) bool { - return func(address common.Address) bool { - return address == addressToMigrate - } - } - isAtreeRegister := func(key string) bool { return key[0] == '$' && len(key) == 9 } @@ -74,31 +59,7 @@ func TestMigrateDomainRegisters(t *testing.T) { address1 := common.MustBytesToAddress([]byte{0x1}) address2 := common.MustBytesToAddress([]byte{0x2}) - t.Run("no accounts", func(t *testing.T) { - t.Parallel() - - ledger := NewTestLedger(nil, nil) - storage := runtime.NewStorage( - ledger, - nil, - runtime.StorageConfig{ - StorageFormatV2Enabled: true, - }, - ) - - inter := NewTestInterpreterWithStorage(t, storage) - - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - - migratedAccounts, err := migrator.MigrateAccounts(nil, alwaysMigrate) - require.NoError(t, err) - require.True(t, migratedAccounts == nil || migratedAccounts.Len() == 0) - - err = storage.FastCommit(goruntime.NumCPU()) - require.NoError(t, err) - - require.Equal(t, 0, len(ledger.StoredValues)) - }) + addresses := []common.Address{address2, address1} t.Run("accounts without domain registers", func(t *testing.T) { t.Parallel() @@ -116,15 +77,13 @@ func TestMigrateDomainRegisters(t *testing.T) { migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - accounts := &orderedmap.OrderedMap[common.Address, struct{}]{} - accounts.Set(address2, struct{}{}) - accounts.Set(address1, struct{}{}) - - migratedAccounts, err := migrator.MigrateAccounts(accounts, alwaysMigrate) - require.NoError(t, err) - require.True(t, migratedAccounts == nil || migratedAccounts.Len() == 0) + for _, address := range addresses { + accountStorageMap, err := migrator.MigrateAccount(address) + require.Nil(t, accountStorageMap) + require.NoError(t, err) + } - err = storage.FastCommit(goruntime.NumCPU()) + err := storage.FastCommit(goruntime.NumCPU()) require.NoError(t, err) require.Equal(t, 0, len(ledger.StoredValues)) @@ -162,30 +121,26 @@ func TestMigrateDomainRegisters(t *testing.T) { migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - accounts := &orderedmap.OrderedMap[common.Address, struct{}]{} - accounts.Set(address2, struct{}{}) - accounts.Set(address1, struct{}{}) - - migratedAccounts, err := migrator.MigrateAccounts(accounts, alwaysMigrate) - require.NoError(t, err) - require.NotNil(t, migratedAccounts) - require.Equal(t, accounts.Len(), migratedAccounts.Len()) - require.Equal(t, address2, migratedAccounts.Oldest().Key) - require.Equal(t, address1, migratedAccounts.Newest().Key) + var accountStorageMaps []*interpreter.AccountStorageMap + for _, address := range addresses { + accountStorageMap, err := migrator.MigrateAccount(address) + require.NotNil(t, accountStorageMap) + require.NoError(t, err) + accountStorageMaps = append(accountStorageMaps, accountStorageMap) + } - err = storage.FastCommit(goruntime.NumCPU()) + err := storage.FastCommit(goruntime.NumCPU()) require.NoError(t, err) // Check non-atree registers nonAtreeRegisters := getNonAtreeRegisters(ledger.StoredValues) - require.Equal(t, accounts.Len(), len(nonAtreeRegisters)) + require.Equal(t, len(addresses), len(nonAtreeRegisters)) require.Contains(t, nonAtreeRegisters, string(address1[:])+"|"+runtime.AccountStorageKey) require.Contains(t, nonAtreeRegisters, string(address2[:])+"|"+runtime.AccountStorageKey) // Check atree storage - expectedRootSlabIDs := make([]atree.SlabID, 0, migratedAccounts.Len()) - for pair := migratedAccounts.Oldest(); pair != nil; pair = pair.Next() { - accountStorageMap := pair.Value + expectedRootSlabIDs := make([]atree.SlabID, 0, len(accountStorageMaps)) + for _, accountStorageMap := range accountStorageMaps { expectedRootSlabIDs = append(expectedRootSlabIDs, accountStorageMap.SlabID()) } @@ -229,100 +184,22 @@ func TestMigrateDomainRegisters(t *testing.T) { migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - accounts := &orderedmap.OrderedMap[common.Address, struct{}]{} - accounts.Set(address2, struct{}{}) - accounts.Set(address1, struct{}{}) - - migratedAccounts, err := migrator.MigrateAccounts(accounts, alwaysMigrate) - require.NoError(t, err) - require.True(t, migratedAccounts == nil || migratedAccounts.Len() == 0) + for _, address := range addresses { + accountStorageMap, err := migrator.MigrateAccount(address) + require.Nil(t, accountStorageMap) + require.NoError(t, err) + } // Check account storage map data for address, accountValues := range accountsValues { - checkAccountStorageMapData(t, ledger.StoredValues, ledger.StorageIndices, address, accountValues) - } - }) - - t.Run("never migration predicate", func(t *testing.T) { - t.Parallel() - - accountsInfo := []accountInfo{ - { - address: address1, - domains: []domainInfo{ - {domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3}, - }, - }, - { - address: address2, - domains: []domainInfo{ - {domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3}, - }, - }, - } - - ledger, _ := newTestLedgerWithUnmigratedAccounts(t, nil, nil, accountsInfo) - storage := runtime.NewStorage( - ledger, - nil, - runtime.StorageConfig{ - StorageFormatV2Enabled: true, - }, - ) - - inter := NewTestInterpreterWithStorage(t, storage) - - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - - accounts := &orderedmap.OrderedMap[common.Address, struct{}]{} - accounts.Set(address2, struct{}{}) - accounts.Set(address1, struct{}{}) - - migratedAccounts, err := migrator.MigrateAccounts(accounts, neverMigrate) - require.NoError(t, err) - require.True(t, migratedAccounts == nil || migratedAccounts.Len() == 0) - }) - - t.Run("selective migration predicate", func(t *testing.T) { - t.Parallel() - - accountsInfo := []accountInfo{ - { - address: address1, - domains: []domainInfo{ - {domain: common.PathDomainStorage.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3}, - }, - }, - { - address: address2, - domains: []domainInfo{ - {domain: common.PathDomainPublic.StorageDomain(), domainStorageMapCount: 10, maxDepth: 3}, - }, - }, + checkAccountStorageMapData( + t, + ledger.StoredValues, + ledger.StorageIndices, + address, + accountValues, + ) } - - ledger, _ := newTestLedgerWithUnmigratedAccounts(t, nil, nil, accountsInfo) - storage := runtime.NewStorage( - ledger, - nil, - runtime.StorageConfig{ - StorageFormatV2Enabled: true, - }, - ) - - inter := NewTestInterpreterWithStorage(t, storage) - - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) - - accounts := &orderedmap.OrderedMap[common.Address, struct{}]{} - accounts.Set(address2, struct{}{}) - accounts.Set(address1, struct{}{}) - - migratedAccounts, err := migrator.MigrateAccounts(accounts, migrateSpecificAccount(address2)) - require.NoError(t, err) - require.NotNil(t, migratedAccounts) - require.Equal(t, 1, migratedAccounts.Len()) - require.Equal(t, address2, migratedAccounts.Oldest().Key) }) } diff --git a/runtime/storage.go b/runtime/storage.go index 7460c8c4b..8bd8be96e 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -399,62 +399,6 @@ func (s *Storage) commit(inter *interpreter.Interpreter, commitContractUpdates b } } -// TODO: -//func (s *Storage) migrateAccounts(inter *interpreter.Interpreter) error { -// // Predicate function allows migration for accounts with write ops. -// migrateAccountPred := func(address common.Address) bool { -// return s.PersistentSlabStorage.HasUnsavedChanges(atree.Address(address)) -// } -// -// // getDomainStorageMap function returns cached domain storage map if it is available -// // before loading domain storage map from storage. -// // This is necessary to migrate uncommitted (new) but cached domain storage map. -// getDomainStorageMap := func( -// ledger atree.Ledger, -// storage atree.SlabStorage, -// address common.Address, -// domain common.StorageDomain, -// ) (*interpreter.DomainStorageMap, error) { -// domainStorageKey := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain) -// -// // Get cached domain storage map if available. -// domainStorageMap := s.cachedDomainStorageMaps[domainStorageKey] -// -// if domainStorageMap != nil { -// return domainStorageMap, nil -// } -// -// return getDomainStorageMapFromV1DomainRegister(ledger, storage, address, domain) -// } -// -// migrator := NewDomainRegisterMigration(s.Ledger, s.PersistentSlabStorage, inter, s.memoryGauge) -// migrator.SetGetDomainStorageMapFunc(getDomainStorageMap) -// -// migratedAccounts, err := migrator.MigrateAccounts(s.unmigratedAccounts, migrateAccountPred) -// if err != nil { -// return err -// } -// -// if migratedAccounts == nil { -// return nil -// } -// -// // Update internal state with migrated accounts -// for pair := migratedAccounts.Oldest(); pair != nil; pair = pair.Next() { -// address := pair.Key -// accountStorageMap := pair.Value -// -// // Cache migrated account storage map -// accountStorageKey := interpreter.NewStorageKey(s.memoryGauge, address, AccountStorageKey) -// s.cachedAccountStorageMaps[accountStorageKey] = accountStorageMap -// -// // Remove migrated accounts from unmigratedAccounts -// s.unmigratedAccounts.Delete(address) -// } -// -// return nil -//} - func (s *Storage) CheckHealth() error { // Check slab storage health From cfe3db0e9d3639d76ac703a79387723118ae6570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 14 Nov 2024 16:06:20 -0800 Subject: [PATCH 2/6] try to run manual migration --- runtime/storage.go | 28 +++++++++++++++++++++------- runtime/storage_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/runtime/storage.go b/runtime/storage.go index 8bd8be96e..c07fff17e 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -356,7 +356,7 @@ func (s *Storage) Commit(inter *interpreter.Interpreter, commitContractUpdates b return s.commit(inter, commitContractUpdates, true) } -// NondeterministicCommit serializes and commits all values in the deltas storage +// Deprecated: NondeterministicCommit serializes and commits all values in the deltas storage // in nondeterministic order. This function is used when commit ordering isn't // required (e.g. migration programs). func (s *Storage) NondeterministicCommit(inter *interpreter.Interpreter, commitContractUpdates bool) error { @@ -381,21 +381,35 @@ func (s *Storage) commit(inter *interpreter.Interpreter, commitContractUpdates b // Commit the underlying slab storage's writes - size := s.PersistentSlabStorage.DeltasSizeWithoutTempAddresses() + slabStorage := s.PersistentSlabStorage + + return CommitSlabStorage( + slabStorage, + inter, + deterministic, + ) +} + +func CommitSlabStorage( + slabStorage *atree.PersistentSlabStorage, + inter *interpreter.Interpreter, + deterministic bool, +) error { + size := slabStorage.DeltasSizeWithoutTempAddresses() if size > 0 { inter.ReportComputation(common.ComputationKindEncodeValue, uint(size)) usage := common.NewBytesMemoryUsage(int(size)) - common.UseMemory(s.memoryGauge, usage) + common.UseMemory(inter, usage) } - deltas := s.PersistentSlabStorage.DeltasWithoutTempAddresses() - common.UseMemory(s.memoryGauge, common.NewAtreeEncodedSlabMemoryUsage(deltas)) + deltas := slabStorage.DeltasWithoutTempAddresses() + common.UseMemory(inter, common.NewAtreeEncodedSlabMemoryUsage(deltas)) // TODO: report encoding metric for all encoded slabs if deterministic { - return s.PersistentSlabStorage.FastCommit(runtime.NumCPU()) + return slabStorage.FastCommit(runtime.NumCPU()) } else { - return s.PersistentSlabStorage.NondeterministicFastCommit(runtime.NumCPU()) + return slabStorage.NondeterministicFastCommit(runtime.NumCPU()) } } diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 3b1622012..066189a08 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -7140,6 +7140,18 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { err := storage.Commit(inter, commitContractUpdates) require.NoError(t, err) + migration := NewDomainRegisterMigration(ledger, storage, inter, nil) + accountStorageMap, err := migration.MigrateAccount(address) + require.NotNil(t, accountStorageMap) + require.NoError(t, err) + + err = CommitSlabStorage( + storage.PersistentSlabStorage, + inter, + true, + ) + require.NoError(t, err) + // Create a new storage newLedger := NewTestLedgerWithData(onRead, onWrite, ledger.StoredValues, ledger.StorageIndices) @@ -8029,6 +8041,18 @@ func TestDomainRegisterMigrationForLargeAccount(t *testing.T) { // Check there are writes to underlying storage require.True(t, writeCount > 0) + migration := NewDomainRegisterMigration(ledger, storage, inter, nil) + accountStorageMap, err := migration.MigrateAccount(address) + require.NotNil(t, accountStorageMap) + require.NoError(t, err) + + err = CommitSlabStorage( + storage.PersistentSlabStorage, + inter, + true, + ) + require.NoError(t, err) + // Check there isn't any domain registers nonAtreeRegisters := make(map[string][]byte) for k, v := range ledger.StoredValues { From 996ba7f04a2369df82e71f55a1b3a21bc9538274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 14 Nov 2024 16:35:51 -0800 Subject: [PATCH 3/6] bring back logic to automatically migrate accounts with writes on commit --- runtime/account_storage_v2.go | 17 ++--- runtime/migrate_domain_registers.go | 34 ++++++--- runtime/migrate_domain_registers_test.go | 24 ++++++- runtime/storage.go | 88 +++++++++++++++++++----- runtime/storage_test.go | 24 ------- 5 files changed, 125 insertions(+), 62 deletions(-) diff --git a/runtime/account_storage_v2.go b/runtime/account_storage_v2.go index 4c76d3a4a..8767056b9 100644 --- a/runtime/account_storage_v2.go +++ b/runtime/account_storage_v2.go @@ -33,8 +33,7 @@ type AccountStorageV2 struct { memoryGauge common.MemoryGauge // cachedAccountStorageMaps is a cache of account storage maps. - // Key is StorageKey{address, accountStorageKey} and value is account storage map. - cachedAccountStorageMaps map[interpreter.StorageKey]*interpreter.AccountStorageMap + cachedAccountStorageMaps map[common.Address]*interpreter.AccountStorageMap // newAccountStorageMapSlabIndices contains root slab index of new account storage maps. // The indices are saved using Ledger.SetValue() during Commit(). @@ -94,12 +93,10 @@ func (s *AccountStorageV2) getAccountStorageMap( ) ( accountStorageMap *interpreter.AccountStorageMap, ) { - accountStorageKey := s.accountStorageKey(address) - // Return cached account storage map if it exists. if s.cachedAccountStorageMaps != nil { - accountStorageMap = s.cachedAccountStorageMaps[accountStorageKey] + accountStorageMap = s.cachedAccountStorageMaps[address] if accountStorageMap != nil { return accountStorageMap } @@ -108,7 +105,7 @@ func (s *AccountStorageV2) getAccountStorageMap( defer func() { if accountStorageMap != nil { s.cacheAccountStorageMap( - accountStorageKey, + address, accountStorageMap, ) } @@ -130,13 +127,13 @@ func (s *AccountStorageV2) getAccountStorageMap( } func (s *AccountStorageV2) cacheAccountStorageMap( - accountStorageKey interpreter.StorageKey, + address common.Address, accountStorageMap *interpreter.AccountStorageMap, ) { if s.cachedAccountStorageMaps == nil { - s.cachedAccountStorageMaps = map[interpreter.StorageKey]*interpreter.AccountStorageMap{} + s.cachedAccountStorageMaps = map[common.Address]*interpreter.AccountStorageMap{} } - s.cachedAccountStorageMaps[accountStorageKey] = accountStorageMap + s.cachedAccountStorageMaps[address] = accountStorageMap } func (s *AccountStorageV2) storeNewAccountStorageMap( @@ -156,7 +153,7 @@ func (s *AccountStorageV2) storeNewAccountStorageMap( s.SetNewAccountStorageMapSlabIndex(accountStorageKey, slabIndex) s.cacheAccountStorageMap( - accountStorageKey, + address, accountStorageMap, ) diff --git a/runtime/migrate_domain_registers.go b/runtime/migrate_domain_registers.go index d5b7f7aa0..08d6afa55 100644 --- a/runtime/migrate_domain_registers.go +++ b/runtime/migrate_domain_registers.go @@ -26,12 +26,23 @@ import ( "github.com/onflow/cadence/interpreter" ) +type GetDomainStorageMapFunc func( + ledger atree.Ledger, + storage atree.SlabStorage, + address common.Address, + domain common.StorageDomain, +) ( + *interpreter.DomainStorageMap, + error, +) + // DomainRegisterMigration migrates domain registers to account storage maps. type DomainRegisterMigration struct { - ledger atree.Ledger - storage atree.SlabStorage - inter *interpreter.Interpreter - memoryGauge common.MemoryGauge + ledger atree.Ledger + storage atree.SlabStorage + inter *interpreter.Interpreter + memoryGauge common.MemoryGauge + getDomainStorageMap GetDomainStorageMapFunc } func NewDomainRegisterMigration( @@ -39,12 +50,17 @@ func NewDomainRegisterMigration( storage atree.SlabStorage, inter *interpreter.Interpreter, memoryGauge common.MemoryGauge, + getDomainStorageMap GetDomainStorageMapFunc, ) *DomainRegisterMigration { + if getDomainStorageMap == nil { + getDomainStorageMap = getDomainStorageMapFromV1DomainRegister + } return &DomainRegisterMigration{ - ledger: ledger, - storage: storage, - inter: inter, - memoryGauge: memoryGauge, + ledger: ledger, + storage: storage, + inter: inter, + memoryGauge: memoryGauge, + getDomainStorageMap: getDomainStorageMap, } } @@ -104,7 +120,7 @@ func (m *DomainRegisterMigration) migrateDomainRegisters( for _, domain := range common.AllStorageDomains { - domainStorageMap, err := getDomainStorageMapFromV1DomainRegister( + domainStorageMap, err := m.getDomainStorageMap( m.ledger, m.storage, address, diff --git a/runtime/migrate_domain_registers_test.go b/runtime/migrate_domain_registers_test.go index db1038f61..f1d6e4304 100644 --- a/runtime/migrate_domain_registers_test.go +++ b/runtime/migrate_domain_registers_test.go @@ -75,7 +75,13 @@ func TestMigrateDomainRegisters(t *testing.T) { inter := NewTestInterpreterWithStorage(t, storage) - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) + migrator := runtime.NewDomainRegisterMigration( + ledger, + storage, + inter, + nil, + nil, + ) for _, address := range addresses { accountStorageMap, err := migrator.MigrateAccount(address) @@ -119,7 +125,13 @@ func TestMigrateDomainRegisters(t *testing.T) { inter := NewTestInterpreterWithStorage(t, storage) - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) + migrator := runtime.NewDomainRegisterMigration( + ledger, + storage, + inter, + nil, + nil, + ) var accountStorageMaps []*interpreter.AccountStorageMap for _, address := range addresses { @@ -182,7 +194,13 @@ func TestMigrateDomainRegisters(t *testing.T) { inter := NewTestInterpreterWithStorage(t, storage) - migrator := runtime.NewDomainRegisterMigration(ledger, storage, inter, nil) + migrator := runtime.NewDomainRegisterMigration( + ledger, + storage, + inter, + nil, + nil, + ) for _, address := range addresses { accountStorageMap, err := migrator.MigrateAccount(address) diff --git a/runtime/storage.go b/runtime/storage.go index c07fff17e..506fdf4ff 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -49,7 +49,7 @@ type Storage struct { // cachedV1Accounts contains the cached result of determining // if the account is in storage format v1 or not. - cachedV1Accounts map[common.Address]bool + cachedV1Accounts *orderedmap.OrderedMap[common.Address, bool] // contractUpdates is a cache of contract updates. // Key is StorageKey{contract_address, contract_name} and value is contract composite value. @@ -185,7 +185,7 @@ func (s *Storage) IsV1Account(address common.Address) (isV1 bool) { if s.cachedV1Accounts != nil { var present bool - isV1, present = s.cachedV1Accounts[address] + isV1, present = s.cachedV1Accounts.Get(address) if present { return isV1 } @@ -230,9 +230,9 @@ func (s *Storage) IsV1Account(address common.Address) (isV1 bool) { func (s *Storage) cacheIsV1Account(address common.Address, isV1 bool) { if s.cachedV1Accounts == nil { - s.cachedV1Accounts = map[common.Address]bool{} + s.cachedV1Accounts = &orderedmap.OrderedMap[common.Address, bool]{} } - s.cachedV1Accounts[address] = isV1 + s.cachedV1Accounts.Set(address, isV1) } func (s *Storage) cacheDomainStorageMap( @@ -379,22 +379,17 @@ func (s *Storage) commit(inter *interpreter.Interpreter, commitContractUpdates b return err } + if s.Config.StorageFormatV2Enabled { + err = s.migrateV1AccountsToV2(inter) + if err != nil { + return err + } + } + // Commit the underlying slab storage's writes slabStorage := s.PersistentSlabStorage - return CommitSlabStorage( - slabStorage, - inter, - deterministic, - ) -} - -func CommitSlabStorage( - slabStorage *atree.PersistentSlabStorage, - inter *interpreter.Interpreter, - deterministic bool, -) error { size := slabStorage.DeltasSizeWithoutTempAddresses() if size > 0 { inter.ReportComputation(common.ComputationKindEncodeValue, uint(size)) @@ -413,6 +408,67 @@ func CommitSlabStorage( } } +func (s *Storage) migrateV1AccountsToV2(inter *interpreter.Interpreter) error { + + if s.cachedV1Accounts == nil { + return nil + } + + // getDomainStorageMap function returns cached domain storage map if it is available + // before loading domain storage map from storage. + // This is necessary to migrate uncommitted (new) but cached domain storage map. + getDomainStorageMap := func( + ledger atree.Ledger, + storage atree.SlabStorage, + address common.Address, + domain common.StorageDomain, + ) (*interpreter.DomainStorageMap, error) { + domainStorageKey := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain) + + // Get cached domain storage map if available. + domainStorageMap := s.cachedDomainStorageMaps[domainStorageKey] + + if domainStorageMap != nil { + return domainStorageMap, nil + } + + return getDomainStorageMapFromV1DomainRegister(ledger, storage, address, domain) + } + + migrator := NewDomainRegisterMigration( + s.Ledger, + s.PersistentSlabStorage, + inter, + s.memoryGauge, + getDomainStorageMap, + ) + + for pair := s.cachedV1Accounts.Oldest(); pair != nil; pair = pair.Next() { + address := pair.Key + isV1 := pair.Value + + if !isV1 || !s.PersistentSlabStorage.HasUnsavedChanges(atree.Address(address)) { + continue + } + + accountStorageMap, err := migrator.MigrateAccount(address) + if err != nil { + return err + } + + // TODO: is this all that is needed? + + s.AccountStorageV2.cacheAccountStorageMap( + address, + accountStorageMap, + ) + + s.cacheIsV1Account(address, false) + } + + return nil +} + func (s *Storage) CheckHealth() error { // Check slab storage health diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 066189a08..3b1622012 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -7140,18 +7140,6 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { err := storage.Commit(inter, commitContractUpdates) require.NoError(t, err) - migration := NewDomainRegisterMigration(ledger, storage, inter, nil) - accountStorageMap, err := migration.MigrateAccount(address) - require.NotNil(t, accountStorageMap) - require.NoError(t, err) - - err = CommitSlabStorage( - storage.PersistentSlabStorage, - inter, - true, - ) - require.NoError(t, err) - // Create a new storage newLedger := NewTestLedgerWithData(onRead, onWrite, ledger.StoredValues, ledger.StorageIndices) @@ -8041,18 +8029,6 @@ func TestDomainRegisterMigrationForLargeAccount(t *testing.T) { // Check there are writes to underlying storage require.True(t, writeCount > 0) - migration := NewDomainRegisterMigration(ledger, storage, inter, nil) - accountStorageMap, err := migration.MigrateAccount(address) - require.NotNil(t, accountStorageMap) - require.NoError(t, err) - - err = CommitSlabStorage( - storage.PersistentSlabStorage, - inter, - true, - ) - require.NoError(t, err) - // Check there isn't any domain registers nonAtreeRegisters := make(map[string][]byte) for k, v := range ledger.StoredValues { From 54c23eefe8d63a228f6978ddc34cb2d8f799bf42 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Thu, 14 Nov 2024 19:04:07 -0600 Subject: [PATCH 4/6] Fix storage test --- runtime/storage_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 3b1622012..d01458284 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -7102,7 +7102,7 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { ledger, nil, StorageConfig{ - StorageFormatV2Enabled: true, + StorageFormatV2Enabled: false, }, ) @@ -7353,12 +7353,28 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { require.True(t, len(writeEntries) > 1+len(tc.existingDomains)+len(tc.newDomains)) i := 0 + + // Check new domain register committed in V1 format. + for _, domain := range common.AllStorageDomains { + + if slices.Contains(tc.newDomains, domain) { + + // New domains are committed in V1 format (with domain register). + require.Equal(t, address[:], writeEntries[i].Owner) + require.Equal(t, []byte(domain.Identifier()), writeEntries[i].Key) + require.True(t, len(writeEntries[i].Value) > 0) + + i++ + } + } + + // Check modified registers in migration. for _, domain := range common.AllStorageDomains { if slices.Contains(tc.existingDomains, domain) || slices.Contains(tc.newDomains, domain) { - // Existing and new domain registers are removed. + // Existing and new domain registers are removed (migrated). // Removing new (non-existent) domain registers is no-op. require.Equal(t, address[:], writeEntries[i].Owner) require.Equal(t, []byte(domain.Identifier()), writeEntries[i].Key) From efe2f3aa384ea15899b5f3ec7d02575e215dbd69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 15 Nov 2024 10:21:27 -0800 Subject: [PATCH 5/6] allow scheduling migrations of accounts to V2 --- common/address.go | 5 ++++ runtime/storage.go | 54 +++++++++++++++++++++++++++-------------- runtime/storage_test.go | 14 +++++++++++ 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/common/address.go b/common/address.go index 5c1a354f5..04c2bade7 100644 --- a/common/address.go +++ b/common/address.go @@ -19,6 +19,7 @@ package common import ( + "bytes" "encoding/hex" goErrors "errors" "fmt" @@ -112,6 +113,10 @@ func (a Address) HexWithPrefix() string { return fmt.Sprintf("0x%x", [AddressLength]byte(a)) } +func (a Address) Compare(other Address) int { + return bytes.Compare(a[:], other[:]) +} + // HexToAddress converts a hex string to an Address after // ensuring that the hex string starts with the prefix 0x. func HexToAddressAssertPrefix(h string) (Address, error) { diff --git a/runtime/storage.go b/runtime/storage.go index 506fdf4ff..843c3e9e0 100644 --- a/runtime/storage.go +++ b/runtime/storage.go @@ -49,7 +49,7 @@ type Storage struct { // cachedV1Accounts contains the cached result of determining // if the account is in storage format v1 or not. - cachedV1Accounts *orderedmap.OrderedMap[common.Address, bool] + cachedV1Accounts map[common.Address]bool // contractUpdates is a cache of contract updates. // Key is StorageKey{contract_address, contract_name} and value is contract composite value. @@ -61,8 +61,9 @@ type Storage struct { Config StorageConfig - AccountStorageV1 *AccountStorageV1 - AccountStorageV2 *AccountStorageV2 + AccountStorageV1 *AccountStorageV1 + AccountStorageV2 *AccountStorageV2 + scheduledV2Migrations []common.Address } var _ atree.SlabStorage = &Storage{} @@ -183,12 +184,8 @@ func (s *Storage) IsV1Account(address common.Address) (isV1 bool) { // Check cache - if s.cachedV1Accounts != nil { - var present bool - isV1, present = s.cachedV1Accounts.Get(address) - if present { - return isV1 - } + if isV1, present := s.cachedV1Accounts[address]; present { + return isV1 } // Cache result @@ -230,9 +227,9 @@ func (s *Storage) IsV1Account(address common.Address) (isV1 bool) { func (s *Storage) cacheIsV1Account(address common.Address, isV1 bool) { if s.cachedV1Accounts == nil { - s.cachedV1Accounts = &orderedmap.OrderedMap[common.Address, bool]{} + s.cachedV1Accounts = map[common.Address]bool{} } - s.cachedV1Accounts.Set(address, isV1) + s.cachedV1Accounts[address] = isV1 } func (s *Storage) cacheDomainStorageMap( @@ -408,9 +405,23 @@ func (s *Storage) commit(inter *interpreter.Interpreter, commitContractUpdates b } } +func (s *Storage) ScheduleV2Migration(address common.Address) { + s.scheduledV2Migrations = append(s.scheduledV2Migrations, address) +} + +func (s *Storage) ScheduleV2MigrationForModifiedAccounts() { + for address, isV1 := range s.cachedV1Accounts { //nolint:maprange + if !isV1 || !s.PersistentSlabStorage.HasUnsavedChanges(atree.Address(address)) { + continue + } + + s.ScheduleV2Migration(address) + } +} + func (s *Storage) migrateV1AccountsToV2(inter *interpreter.Interpreter) error { - if s.cachedV1Accounts == nil { + if len(s.scheduledV2Migrations) == 0 { return nil } @@ -443,13 +454,18 @@ func (s *Storage) migrateV1AccountsToV2(inter *interpreter.Interpreter) error { getDomainStorageMap, ) - for pair := s.cachedV1Accounts.Oldest(); pair != nil; pair = pair.Next() { - address := pair.Key - isV1 := pair.Value + // Ensure the scheduled accounts are migrated in a deterministic order - if !isV1 || !s.PersistentSlabStorage.HasUnsavedChanges(atree.Address(address)) { - continue - } + sort.Slice( + s.scheduledV2Migrations, + func(i, j int) bool { + address1 := s.scheduledV2Migrations[i] + address2 := s.scheduledV2Migrations[j] + return address1.Compare(address2) < 0 + }, + ) + + for _, address := range s.scheduledV2Migrations { accountStorageMap, err := migrator.MigrateAccount(address) if err != nil { @@ -466,6 +482,8 @@ func (s *Storage) migrateV1AccountsToV2(inter *interpreter.Interpreter) error { s.cacheIsV1Account(address, false) } + s.scheduledV2Migrations = nil + return nil } diff --git a/runtime/storage_test.go b/runtime/storage_test.go index d01458284..3ebe7d88f 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -7181,6 +7181,8 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { domainStorageMap := storage.GetDomainStorageMap(inter, address, nonExistingDomain, createIfNotExists) require.Nil(t, domainStorageMap) + storage.ScheduleV2MigrationForModifiedAccounts() + // Commit changes const commitContractUpdates = false err := storage.Commit(inter, commitContractUpdates) @@ -7335,6 +7337,9 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { accountValues[domain] = writeToDomainStorageMap(inter, domainStorageMap, tc.newDomainStorageMapCount, random) } + // TODO: + storage.ScheduleV2MigrationForModifiedAccounts() + // Commit changes const commitContractUpdates = false err := storage.Commit(inter, commitContractUpdates) @@ -7480,6 +7485,9 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { } } + // TODO: + storage.ScheduleV2MigrationForModifiedAccounts() + // Commit changes const commitContractUpdates = false err := storage.Commit(inter, commitContractUpdates) @@ -7610,6 +7618,9 @@ func TestRuntimeStorageForUnmigratedAccount(t *testing.T) { } } + // TODO: + storage.ScheduleV2MigrationForModifiedAccounts() + // Commit changes const commitContractUpdates = false err := storage.Commit(inter, commitContractUpdates) @@ -8037,6 +8048,9 @@ func TestDomainRegisterMigrationForLargeAccount(t *testing.T) { accountValues[domain] = make(domainStorageMapValues) + // TODO: + storage.ScheduleV2MigrationForModifiedAccounts() + // Commit changes const commitContractUpdates = false err := storage.Commit(inter, commitContractUpdates) From bff61870dbe4187b9d153555c1995c053f0d8570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 15 Nov 2024 14:12:56 -0800 Subject: [PATCH 6/6] simplify ordered maps to simple maps. sort keys on commit --- interpreter/storage.go | 14 +++++++ runtime/account_storage_v1.go | 47 +++++++++++++++++++---- runtime/account_storage_v2.go | 70 +++++++++++++++++++++++------------ runtime/storage_test.go | 7 +--- 4 files changed, 101 insertions(+), 37 deletions(-) diff --git a/interpreter/storage.go b/interpreter/storage.go index 3b0951efc..1927cdfa0 100644 --- a/interpreter/storage.go +++ b/interpreter/storage.go @@ -20,6 +20,7 @@ package interpreter import ( "bytes" + "cmp" "io" "math" "strings" @@ -106,6 +107,19 @@ type StorageDomainKey struct { Address common.Address } +func (k StorageDomainKey) Compare(o StorageDomainKey) int { + switch bytes.Compare(k.Address[:], o.Address[:]) { + case -1: + return -1 + case 0: + return cmp.Compare(k.Domain, o.Domain) + case 1: + return 1 + default: + panic(errors.NewUnreachableError()) + } +} + func NewStorageDomainKey( memoryGauge common.MemoryGauge, address common.Address, diff --git a/runtime/account_storage_v1.go b/runtime/account_storage_v1.go index 0c5253772..a0ababd3c 100644 --- a/runtime/account_storage_v1.go +++ b/runtime/account_storage_v1.go @@ -19,10 +19,11 @@ package runtime import ( + "sort" + "github.com/onflow/atree" "github.com/onflow/cadence/common" - "github.com/onflow/cadence/common/orderedmap" "github.com/onflow/cadence/errors" "github.com/onflow/cadence/interpreter" ) @@ -35,7 +36,7 @@ type AccountStorageV1 struct { // newDomainStorageMapSlabIndices contains root slab indices of new domain storage maps. // The indices are saved using Ledger.SetValue() during commit(). // Key is StorageDomainKey{common.StorageDomain, Address} and value is 8-byte slab index. - newDomainStorageMapSlabIndices *orderedmap.OrderedMap[interpreter.StorageDomainKey, atree.SlabIndex] + newDomainStorageMapSlabIndices map[interpreter.StorageDomainKey]atree.SlabIndex } func NewAccountStorageV1( @@ -91,9 +92,9 @@ func (s *AccountStorageV1) storeNewDomainStorageMap( storageKey := interpreter.NewStorageDomainKey(s.memoryGauge, address, domain) if s.newDomainStorageMapSlabIndices == nil { - s.newDomainStorageMapSlabIndices = &orderedmap.OrderedMap[interpreter.StorageDomainKey, atree.SlabIndex]{} + s.newDomainStorageMapSlabIndices = map[interpreter.StorageDomainKey]atree.SlabIndex{} } - s.newDomainStorageMapSlabIndices.Set(storageKey, slabIndex) + s.newDomainStorageMapSlabIndices[storageKey] = slabIndex return domainStorageMap } @@ -103,13 +104,41 @@ func (s *AccountStorageV1) commit() error { return nil } - for pair := s.newDomainStorageMapSlabIndices.Oldest(); pair != nil; pair = pair.Next() { + type domainStorageMapSlabIndex struct { + StorageDomainKey interpreter.StorageDomainKey + SlabIndex atree.SlabIndex + } + + slabIndices := make([]domainStorageMapSlabIndex, 0, len(s.newDomainStorageMapSlabIndices)) + for storageDomainKey, slabIndex := range s.newDomainStorageMapSlabIndices { //nolint:maprange + slabIndices = append( + slabIndices, + domainStorageMapSlabIndex{ + StorageDomainKey: storageDomainKey, + SlabIndex: slabIndex, + }, + ) + } + sort.Slice( + slabIndices, + func(i, j int) bool { + slabIndex1 := slabIndices[i] + slabIndex2 := slabIndices[j] + domainKey1 := slabIndex1.StorageDomainKey + domainKey2 := slabIndex2.StorageDomainKey + return domainKey1.Compare(domainKey2) < 0 + }, + ) + + for _, slabIndex := range slabIndices { + storageDomainKey := slabIndex.StorageDomainKey + var err error errors.WrapPanic(func() { err = s.ledger.SetValue( - pair.Key.Address[:], - []byte(pair.Key.Domain.Identifier()), - pair.Value[:], + storageDomainKey.Address[:], + []byte(storageDomainKey.Domain.Identifier()), + slabIndex.SlabIndex[:], ) }) if err != nil { @@ -117,6 +146,8 @@ func (s *AccountStorageV1) commit() error { } } + s.newDomainStorageMapSlabIndices = nil + return nil } diff --git a/runtime/account_storage_v2.go b/runtime/account_storage_v2.go index 8767056b9..9e737053d 100644 --- a/runtime/account_storage_v2.go +++ b/runtime/account_storage_v2.go @@ -19,10 +19,11 @@ package runtime import ( + "sort" + "github.com/onflow/atree" "github.com/onflow/cadence/common" - "github.com/onflow/cadence/common/orderedmap" "github.com/onflow/cadence/errors" "github.com/onflow/cadence/interpreter" ) @@ -35,10 +36,9 @@ type AccountStorageV2 struct { // cachedAccountStorageMaps is a cache of account storage maps. cachedAccountStorageMaps map[common.Address]*interpreter.AccountStorageMap - // newAccountStorageMapSlabIndices contains root slab index of new account storage maps. - // The indices are saved using Ledger.SetValue() during Commit(). - // Key is StorageKey{address, accountStorageKey} and value is 8-byte slab index. - newAccountStorageMapSlabIndices *orderedmap.OrderedMap[interpreter.StorageKey, atree.SlabIndex] + // newAccountStorageMapSlabIndices contains root slab indices of new account storage maps. + // The indices are saved using Ledger.SetValue() during commit(). + newAccountStorageMapSlabIndices map[common.Address]atree.SlabIndex } func NewAccountStorageV2( @@ -53,14 +53,6 @@ func NewAccountStorageV2( } } -func (s *AccountStorageV2) accountStorageKey(address common.Address) interpreter.StorageKey { - return interpreter.NewStorageKey( - s.memoryGauge, - address, - AccountStorageKey, - ) -} - func (s *AccountStorageV2) GetDomainStorageMap( inter *interpreter.Interpreter, address common.Address, @@ -148,9 +140,10 @@ func (s *AccountStorageV2) storeNewAccountStorageMap( slabIndex := accountStorageMap.SlabID().Index() - accountStorageKey := s.accountStorageKey(address) - - s.SetNewAccountStorageMapSlabIndex(accountStorageKey, slabIndex) + s.SetNewAccountStorageMapSlabIndex( + address, + slabIndex, + ) s.cacheAccountStorageMap( address, @@ -161,13 +154,13 @@ func (s *AccountStorageV2) storeNewAccountStorageMap( } func (s *AccountStorageV2) SetNewAccountStorageMapSlabIndex( - accountStorageKey interpreter.StorageKey, + address common.Address, slabIndex atree.SlabIndex, ) { if s.newAccountStorageMapSlabIndices == nil { - s.newAccountStorageMapSlabIndices = &orderedmap.OrderedMap[interpreter.StorageKey, atree.SlabIndex]{} + s.newAccountStorageMapSlabIndices = map[common.Address]atree.SlabIndex{} } - s.newAccountStorageMapSlabIndices.Set(accountStorageKey, slabIndex) + s.newAccountStorageMapSlabIndices[address] = slabIndex } func (s *AccountStorageV2) commit() error { @@ -175,13 +168,42 @@ func (s *AccountStorageV2) commit() error { return nil } - for pair := s.newAccountStorageMapSlabIndices.Oldest(); pair != nil; pair = pair.Next() { + type accountStorageMapSlabIndex struct { + Address common.Address + SlabIndex atree.SlabIndex + } + + slabIndices := make([]accountStorageMapSlabIndex, 0, len(s.newAccountStorageMapSlabIndices)) + for address, slabIndex := range s.newAccountStorageMapSlabIndices { //nolint:maprange + slabIndices = append( + slabIndices, + accountStorageMapSlabIndex{ + Address: address, + SlabIndex: slabIndex, + }, + ) + } + sort.Slice( + slabIndices, + func(i, j int) bool { + slabIndex1 := slabIndices[i] + slabIndex2 := slabIndices[j] + address1 := slabIndex1.Address + address2 := slabIndex2.Address + return address1.Compare(address2) < 0 + }, + ) + + storageKey := []byte(AccountStorageKey) + + for _, slabIndex := range slabIndices { + var err error errors.WrapPanic(func() { err = s.ledger.SetValue( - pair.Key.Address[:], - []byte(pair.Key.Key), - pair.Value[:], + slabIndex.Address[:], + storageKey, + slabIndex.SlabIndex[:], ) }) if err != nil { @@ -189,6 +211,8 @@ func (s *AccountStorageV2) commit() error { } } + s.newAccountStorageMapSlabIndices = nil + return nil } diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 3ebe7d88f..4e4dc261b 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -68,15 +68,10 @@ func withWritesToStorage( var address common.Address random.Read(address[:]) - storageKey := interpreter.StorageKey{ - Address: address, - Key: AccountStorageKey, - } - var slabIndex atree.SlabIndex binary.BigEndian.PutUint32(slabIndex[:], randomIndex) - storage.AccountStorageV2.SetNewAccountStorageMapSlabIndex(storageKey, slabIndex) + storage.AccountStorageV2.SetNewAccountStorageMapSlabIndex(address, slabIndex) } handler(storage, inter)