From 05e96a6f8b7e2455095258182d99f6ed7f377339 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 12:00:56 -0700 Subject: [PATCH 01/19] add account status --- fvm/state/accounts.go | 66 +++++++++++++++++-------------- fvm/state/accounts_status.go | 63 +++++++++++++++++++++++++++++ fvm/state/accounts_status_test.go | 45 +++++++++++++++++++++ fvm/state/state.go | 9 +++-- 4 files changed, 150 insertions(+), 33 deletions(-) create mode 100644 fvm/state/accounts_status.go create mode 100644 fvm/state/accounts_status_test.go diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index d7e869504d5..55ec145b4c1 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -17,22 +17,21 @@ import ( ) const ( - KeyExists = "exists" - KeyCode = "code" - KeyContractNames = "contract_names" - KeyPublicKeyCount = "public_key_count" - KeyStorageUsed = "storage_used" - KeyAccountFrozen = "frozen" - KeyStorageIndex = "storage_index" - uint64StorageSize = 8 - AccountFrozenValue = 1 - AccountNotFrozenValue = 0 + KeyAccountStatus = "account_state" + KeyPublicKeyCount = "public_key_count" + KeyCode = "code" + KeyContractNames = "contract_names" + KeyStorageUsed = "storage_used" + KeyStorageIndex = "storage_index" + uint64StorageSize = 8 + + // TODO remove depricated keys when migrations don't need them any more + // depricated, please use account status instead + KeyExists = "exists" + // depricated, please use account status instead + KeyAccountFrozen = "frozen" ) -func keyPublicKey(index uint64) string { - return fmt.Sprintf("public_key_%d", index) -} - type Accounts interface { Exists(address flow.Address) (bool, error) Get(address flow.Address) (*flow.Account, error) @@ -141,16 +140,17 @@ func (a *StatefulAccounts) Get(address flow.Address) (*flow.Account, error) { } func (a *StatefulAccounts) Exists(address flow.Address) (bool, error) { - exists, err := a.getValue(address, false, KeyExists) + accStatusBytes, err := a.getValue(address, false, KeyAccountStatus) if err != nil { return false, err } - if len(exists) != 0 { - return true, nil + accStatus, err := AccountStatusFromBytes(accStatusBytes) + if err != nil { + return false, err } - return false, nil + return accStatus.AccountExists(), nil } // Create account sets all required registers on an address. @@ -170,7 +170,7 @@ func (a *StatefulAccounts) Create(publicKeys []flow.AccountPublicKey, newAddress } // mark that this account exists - err = a.setValue(newAddress, false, KeyExists, []byte{1}) + err = a.setValue(newAddress, false, KeyAccountStatus, NewAccountStatus().ToBytes()) if err != nil { return err } @@ -618,26 +618,28 @@ func readUint64(input []byte) (value uint64, rest []byte, err error) { } func (a *StatefulAccounts) GetAccountFrozen(address flow.Address) (bool, error) { - frozen, err := a.getValue(address, false, KeyAccountFrozen) + accStatusBytes, err := a.getValue(address, false, KeyAccountStatus) if err != nil { return false, err } - - if len(frozen) == 0 { + accStatus, err := AccountStatusFromBytes(accStatusBytes) + if err != nil { return false, err } - - return frozen[0] != AccountNotFrozenValue, nil + return accStatus.IsAccountFrozen(), nil } func (a *StatefulAccounts) SetAccountFrozen(address flow.Address, frozen bool) error { - - val := make([]byte, 1) //zero value for byte is 0 - if frozen { - val[0] = AccountFrozenValue + accStatusBytes, err := a.getValue(address, false, KeyAccountStatus) + if err != nil { + return err } - - return a.setValue(address, false, KeyAccountFrozen, val) + accStatus, err := AccountStatusFromBytes(accStatusBytes) + if err != nil { + return err + } + accStatus = SetAccountStatusFrozenFlag(accStatus, frozen) + return a.setValue(address, false, KeyAccountStatus, accStatus.ToBytes()) } // handy function to error out if account is frozen @@ -678,3 +680,7 @@ func (l *contractNames) remove(contractName string) { } *l = append((*l)[:i], (*l)[i+1:]...) } + +func keyPublicKey(index uint64) string { + return fmt.Sprintf("public_key_%d", index) +} diff --git a/fvm/state/accounts_status.go b/fvm/state/accounts_status.go new file mode 100644 index 00000000000..feb2b45f7ff --- /dev/null +++ b/fvm/state/accounts_status.go @@ -0,0 +1,63 @@ +package state + +import ( + "encoding/hex" + + "github.com/onflow/flow-go/fvm/errors" +) + +type AccountStatus uint8 + +const ( + maskExist byte = 0b0000_0001 + maskFrozen byte = 0b1000_0000 + maskOnHold byte = 0b0100_0000 +) + +// NewAccountStatus sets exist flag and return an AccountStatus +func NewAccountStatus() AccountStatus { + return AccountStatus(maskExist) +} + +func (a AccountStatus) ToBytes() []byte { + b := make([]byte, 1) + b[0] = byte(a) + return b +} + +func AccountStatusFromBytes(inp []byte) (AccountStatus, error) { + // if len of inp is zero, account does not exist + if len(inp) == 0 { + return 0, nil + } + if len(inp) > 1 { + return 0, errors.NewValueErrorf(hex.EncodeToString(inp), "invalid account state") + } + return AccountStatus(inp[0]), nil +} + +func (a AccountStatus) AccountExists() bool { + return a > 0 +} + +func (a AccountStatus) IsAccountFrozen() bool { + return uint8(a)&maskFrozen > 0 +} + +func SetAccountStatusFrozenFlag(inp AccountStatus, frozen bool) AccountStatus { + if frozen { + return AccountStatus(uint8(inp) | maskFrozen) + } + return AccountStatus(uint8(inp) & (0xFF - maskFrozen)) +} + +func (a AccountStatus) IsAccountOnHold() bool { + return uint8(a)&maskOnHold > 0 +} + +func SetAccountStatusOnHoldFlag(inp AccountStatus, onHold bool) AccountStatus { + if onHold { + return AccountStatus(uint8(inp) | maskOnHold) + } + return AccountStatus(uint8(inp) & (0xFF - maskOnHold)) +} diff --git a/fvm/state/accounts_status_test.go b/fvm/state/accounts_status_test.go new file mode 100644 index 00000000000..24d2117a2d9 --- /dev/null +++ b/fvm/state/accounts_status_test.go @@ -0,0 +1,45 @@ +package state_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/onflow/flow-go/fvm/state" +) + +func TestAccountStatus(t *testing.T) { + + s := state.NewAccountStatus() + require.True(t, s.AccountExists()) + require.False(t, s.IsAccountFrozen()) + require.False(t, s.IsAccountOnHold()) + + s = state.SetAccountStatusOnHoldFlag(s, true) + require.True(t, s.AccountExists()) + require.False(t, s.IsAccountFrozen()) + require.True(t, s.IsAccountOnHold()) + + s = state.SetAccountStatusFrozenFlag(s, true) + require.True(t, s.AccountExists()) + require.True(t, s.IsAccountFrozen()) + require.True(t, s.IsAccountOnHold()) + + s = state.SetAccountStatusOnHoldFlag(s, false) + require.True(t, s.AccountExists()) + require.True(t, s.IsAccountFrozen()) + require.False(t, s.IsAccountOnHold()) + + s = state.SetAccountStatusFrozenFlag(s, false) + require.True(t, s.AccountExists()) + require.False(t, s.IsAccountFrozen()) + require.False(t, s.IsAccountOnHold()) + + var err error + s, err = state.AccountStatusFromBytes(s.ToBytes()) + require.NoError(t, err) + require.True(t, s.AccountExists()) + require.False(t, s.IsAccountFrozen()) + require.False(t, s.IsAccountOnHold()) + +} diff --git a/fvm/state/state.go b/fvm/state/state.go index 199cffb96c0..c1a450614e1 100644 --- a/fvm/state/state.go +++ b/fvm/state/state.go @@ -366,13 +366,16 @@ func IsFVMStateKey(owner, controller, key string) bool { if key == KeyExists { return true } - if key == KeyStorageUsed { + if key == KeyAccountFrozen { return true } - if key == KeyStorageIndex { + if key == KeyAccountStatus { return true } - if key == KeyAccountFrozen { + if key == KeyStorageUsed { + return true + } + if key == KeyStorageIndex { return true } } From 33a5e1acccb03ccdea91da7272b35a7e37495afc Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 14:56:48 -0700 Subject: [PATCH 02/19] remove old migration codes --- .../migrations/ordered_map_migration.go | 258 ------------------ .../migrations/ordered_map_migration_test.go | 246 ----------------- .../ledger/reporters/account_reporter_test.go | 66 ----- 3 files changed, 570 deletions(-) delete mode 100644 cmd/util/ledger/migrations/ordered_map_migration.go delete mode 100644 cmd/util/ledger/migrations/ordered_map_migration_test.go delete mode 100644 cmd/util/ledger/reporters/account_reporter_test.go diff --git a/cmd/util/ledger/migrations/ordered_map_migration.go b/cmd/util/ledger/migrations/ordered_map_migration.go deleted file mode 100644 index 902e84be439..00000000000 --- a/cmd/util/ledger/migrations/ordered_map_migration.go +++ /dev/null @@ -1,258 +0,0 @@ -package migrations - -import ( - "fmt" - "math" - "os" - "path" - "strings" - "time" - - "github.com/onflow/atree" - "github.com/onflow/cadence/runtime" - "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/interpreter" - "github.com/rs/zerolog" - "github.com/schollz/progressbar/v3" - - "github.com/onflow/flow-go/fvm/state" - "github.com/onflow/flow-go/ledger" -) - -type OrderedMapMigration struct { - Log zerolog.Logger - OutputDir string - reportFile *os.File - NewStorage *runtime.Storage - Interpreter *interpreter.Interpreter - ledgerView *view - - progress *progressbar.ProgressBar -} - -func (m *OrderedMapMigration) filename() string { - return path.Join(m.OutputDir, fmt.Sprintf("migration_report_%d.txt", int32(time.Now().Unix()))) -} - -func (m *OrderedMapMigration) Migrate(payloads []ledger.Payload) ([]ledger.Payload, error) { - - filename := m.filename() - m.Log.Info().Msgf("Running ordered map storage migration. Saving report to %s.", filename) - - reportFile, err := os.Create(filename) - if err != nil { - return nil, err - } - defer func() { - err = reportFile.Close() - if err != nil { - panic(err) - } - }() - - m.reportFile = reportFile - - total := int64(len(payloads)) - m.progress = progressbar.Default(total, "Migrating:") - - storagePayloads, err := m.initialize(payloads) - if err != nil { - panic(err) - } - return m.migrate(storagePayloads) -} - -func (m *OrderedMapMigration) initPersistentSlabStorage(v *view) { - st := state.NewState( - v, - state.WithMaxInteractionSizeAllowed(math.MaxUint64), - ) - stateHolder := state.NewStateHolder(st) - accounts := state.NewAccounts(stateHolder) - - m.NewStorage = runtime.NewStorage( - NewAccountsAtreeLedger(accounts), - nil, - ) -} - -func (m *OrderedMapMigration) initIntepreter() { - inter, err := interpreter.NewInterpreter( - nil, - nil, - interpreter.WithStorage(m.NewStorage), - ) - - if err != nil { - panic(fmt.Errorf( - "failed to create interpreter: %w", - err, - )) - } - - m.Interpreter = inter -} - -type Pair = struct { - Key string - Value []byte -} - -var _ interpreter.Value = RawStorable{} - -type RawStorable []byte - -func (RawStorable) IsValue() {} - -func (v RawStorable) Accept(interpreter *interpreter.Interpreter, visitor interpreter.Visitor) { - panic("unreachable") -} - -func (RawStorable) Walk(*interpreter.Interpreter, func(interpreter.Value)) { - // NO-OP -} - -func (RawStorable) StaticType(_ *interpreter.Interpreter) interpreter.StaticType { - panic("unreachable") -} - -func (RawStorable) IsImportable(_ *interpreter.Interpreter) bool { - panic("unreachable") -} - -func (RawStorable) ConformsToStaticType(_ *interpreter.Interpreter, - _ func() interpreter.LocationRange, - _ interpreter.StaticType, - _ interpreter.TypeConformanceResults, -) bool { - panic("unreachable") -} - -func (RawStorable) String() string { - panic("unreachable") -} - -func (v RawStorable) RecursiveString(_ interpreter.SeenReferences) string { - panic("unreachable") -} - -func (v RawStorable) MeteredString(common.MemoryGauge, interpreter.SeenReferences) string { - panic("unreachable") -} - -func (v RawStorable) Equal(_ *interpreter.Interpreter, _ func() interpreter.LocationRange, other interpreter.Value) bool { - panic("unreachable") -} - -func (RawStorable) NeedsStoreTo(_ atree.Address) bool { - panic("unreachable") -} - -func (RawStorable) IsResourceKinded(_ *interpreter.Interpreter) bool { - panic("unreachable") -} - -func (v RawStorable) Transfer( - interpreter *interpreter.Interpreter, - _ func() interpreter.LocationRange, - _ atree.Address, - remove bool, - storable atree.Storable, -) interpreter.Value { - panic("unreachable") -} - -func (v RawStorable) Clone(_ *interpreter.Interpreter) interpreter.Value { - panic("unreachable") -} - -func (RawStorable) DeepRemove(_ *interpreter.Interpreter) { - // NO-OP -} - -func (r RawStorable) Encode(enc *atree.Encoder) error { - return enc.CBOR.EncodeRawBytes(r) -} - -func (r RawStorable) ByteSize() uint32 { - return uint32(len(r)) -} - -func (r RawStorable) StoredValue(storage atree.SlabStorage) (atree.Value, error) { - return r, nil -} - -func (r RawStorable) ChildStorables() []atree.Storable { - return nil -} - -func (r RawStorable) Storable(_ atree.SlabStorage, _ atree.Address, _ uint64) (atree.Storable, error) { - return r, nil -} - -func (m *OrderedMapMigration) initialize(payload []ledger.Payload) ([]ledger.Payload, error) { - fvmPayloads, storagePayloads, slabPayloads := splitPayloads(payload) - - m.ledgerView = NewView(append(fvmPayloads, slabPayloads...)) - m.initPersistentSlabStorage(m.ledgerView) - m.initIntepreter() - return storagePayloads, nil -} - -func (m *OrderedMapMigration) migrate(storagePayloads []ledger.Payload) ([]ledger.Payload, error) { - groupedByOwnerAndDomain := make(map[string](map[string][]Pair)) - - for _, p := range storagePayloads { - owner, entry := - string(p.Key.KeyParts[0].Value), - string(p.Key.KeyParts[2].Value) - // if the entry doesn't contain a separator, we just ignore it - // since it is storage metadata and does not need migration - if !strings.HasPrefix(entry, "storage\x1f") && - !strings.HasPrefix(entry, "public\x1f") && - !strings.HasPrefix(entry, "contract\x1f") && - !strings.HasPrefix(entry, "private\x1f") { - m.Log.Warn().Msgf("Ignoring key in storage payloads: %s", entry) - continue - } - splitEntry := strings.Split(entry, "\x1f") - domain, key := splitEntry[0], splitEntry[1] - value := p.Value - - domainMap, domainOk := groupedByOwnerAndDomain[owner] - if !domainOk { - domainMap = make(map[string][]Pair) - } - keyValuePairs, orderedOk := domainMap[domain] - if !orderedOk { - keyValuePairs = make([]Pair, 0) - } - domainMap[domain] = append(keyValuePairs, Pair{Key: key, Value: value}) - groupedByOwnerAndDomain[owner] = domainMap - } - - for owner, domainMaps := range groupedByOwnerAndDomain { - for domain, keyValuePairs := range domainMaps { - address, err := common.BytesToAddress([]byte(owner)) - if err != nil { - panic(err) - } - storageMap := m.NewStorage.GetStorageMap(address, domain, true) - for _, pair := range keyValuePairs { - storageMap.SetValue( - m.Interpreter, - pair.Key, - RawStorable(pair.Value), - ) - } - } - } - - // we don't need to update any contracts in this migration - err := m.NewStorage.Commit(m.Interpreter, false) - if err != nil { - return nil, fmt.Errorf("failed to migrate payloads: %w", err) - } - - return m.ledgerView.Payloads(), nil -} diff --git a/cmd/util/ledger/migrations/ordered_map_migration_test.go b/cmd/util/ledger/migrations/ordered_map_migration_test.go deleted file mode 100644 index 1b3db724cec..00000000000 --- a/cmd/util/ledger/migrations/ordered_map_migration_test.go +++ /dev/null @@ -1,246 +0,0 @@ -package migrations - -import ( - "math" - "testing" - - "github.com/onflow/atree" - "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/interpreter" - "github.com/rs/zerolog" - "github.com/stretchr/testify/require" - - "github.com/onflow/flow-go-sdk" - - "github.com/onflow/flow-go/engine/execution/state" - state2 "github.com/onflow/flow-go/fvm/state" - "github.com/onflow/flow-go/ledger" - "github.com/onflow/flow-go/ledger/common/utils" -) - -func createAccountPayloadKey(a flow.Address, key string) ledger.Key { - return ledger.Key{ - KeyParts: []ledger.KeyPart{ - ledger.NewKeyPart(state.KeyPartOwner, a.Bytes()), - ledger.NewKeyPart(state.KeyPartController, []byte("")), - ledger.NewKeyPart(state.KeyPartKey, []byte(key)), - }, - } -} - -func TestOrderedMapMigration(t *testing.T) { - dir := t.TempDir() - mig := OrderedMapMigration{ - Log: zerolog.Logger{}, - OutputDir: dir, - } - - address1 := flow.HexToAddress("0x1") - cadenceAddress, _ := common.HexToAddress("0x1") - - _, _ = mig.initialize([]ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - }) - - encodeValue := func(v interpreter.Value) ledger.Value { - storable, err := v.Storable(mig.NewStorage, atree.Address(address1), math.MaxUint64) - require.NoError(t, err) - encodedInt, err := atree.Encode(storable, interpreter.CBOREncMode) - require.NoError(t, err) - return encodedInt - } - - t.Run("sort values", func(t *testing.T) { - - one := interpreter.NewUnmeteredIntValueFromInt64(1) - - str := interpreter.NewUnmeteredStringValue("test") - - array := interpreter.NewArrayValue( - mig.Interpreter, - interpreter.VariableSizedStaticType{ - Type: interpreter.PrimitiveStaticTypeAnyStruct, - }, - cadenceAddress, - str, - interpreter.BoolValue(true), - ) - - dict := interpreter.NewDictionaryValueWithAddress( - mig.Interpreter, - interpreter.DictionaryStaticType{ - KeyType: interpreter.PrimitiveStaticTypeBool, - ValueType: interpreter.PrimitiveStaticTypeString, - }, - cadenceAddress, - interpreter.BoolValue(true), - str, - ) - - payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, "storage\x1fFoo"), Value: encodeValue(array)}, - {Key: createAccountPayloadKey(address1, "public\x1fBar"), Value: encodeValue(one)}, - {Key: createAccountPayloadKey(address1, "public\x1fBar"), Value: encodeValue(one)}, - {Key: createAccountPayloadKey(address1, "storage\x1fBar"), Value: encodeValue(str)}, - {Key: createAccountPayloadKey(address1, "private\x1fBar"), Value: encodeValue(dict)}, - } - migratedPayload, err := mig.migrate(payload) - require.NoError(t, err) - require.Equal(t, len(migratedPayload), 11) - - migrated := &OrderedMapMigration{} - migrated.initPersistentSlabStorage(NewView(migratedPayload)) - migrated.initIntepreter() - - stored := migrated.Interpreter.ReadStored(cadenceAddress, "public", "Bar") - require.Equal(t, stored, one) - - stored = migrated.Interpreter.ReadStored(cadenceAddress, "storage", "Foo") - require.IsType(t, &interpreter.ArrayValue{}, stored) - - stored = migrated.Interpreter.ReadStored(cadenceAddress, "storage", "Bar") - require.Equal(t, stored, str) - - stored = migrated.Interpreter.ReadStored(cadenceAddress, "private", "Bar") - require.IsType(t, &interpreter.DictionaryValue{}, stored) - - stored = migrated.Interpreter.ReadStored(cadenceAddress, "storage", "Baz") - require.Equal(t, stored, nil) - }) -} - -func TestMultipleAccounts(t *testing.T) { - dir := t.TempDir() - mig := OrderedMapMigration{ - Log: zerolog.Logger{}, - OutputDir: dir, - } - - address1 := flow.HexToAddress("0x1") - cadenceAddress1, _ := common.HexToAddress("0x1") - - address2 := flow.HexToAddress("0x2") - cadenceAddress2, _ := common.HexToAddress("0x2") - - address3 := flow.HexToAddress("0x3") - cadenceAddress3, _ := common.HexToAddress("0x3") - - _, _ = mig.initialize([]ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - {Key: createAccountPayloadKey(address2, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address2, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - {Key: createAccountPayloadKey(address3, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address3, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - }) - - encodeValue := func(v interpreter.Value, address flow.Address) ledger.Value { - storable, _ := v.Storable(mig.NewStorage, atree.Address(address), math.MaxUint64) - encodedInt, _ := atree.Encode(storable, interpreter.CBOREncMode) - return encodedInt - } - - t.Run("sort values", func(t *testing.T) { - - one := interpreter.NewUnmeteredIntValueFromInt64(1) - two := interpreter.NewUnmeteredIntValueFromInt64(1) - three := interpreter.NewUnmeteredIntValueFromInt64(1) - four := interpreter.NewUnmeteredIntValueFromInt64(1) - five := interpreter.NewUnmeteredIntValueFromInt64(1) - six := interpreter.NewUnmeteredIntValueFromInt64(1) - - payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, "storage\x1fFoo"), Value: encodeValue(one, address1)}, - {Key: createAccountPayloadKey(address2, "storage\x1fFoo"), Value: encodeValue(two, address2)}, - {Key: createAccountPayloadKey(address3, "storage\x1fFoo"), Value: encodeValue(three, address3)}, - {Key: createAccountPayloadKey(address1, "storage\x1fBar"), Value: encodeValue(four, address1)}, - {Key: createAccountPayloadKey(address1, "storage\x1fBaz"), Value: encodeValue(five, address1)}, - {Key: createAccountPayloadKey(address3, "public\x1fFoo"), Value: encodeValue(six, address3)}, - } - migratedPayload, err := mig.migrate(payload) - require.NoError(t, err) - require.Equal(t, len(migratedPayload), 17) - - migrated := &OrderedMapMigration{} - migrated.initPersistentSlabStorage(NewView(migratedPayload)) - migrated.initIntepreter() - - stored := migrated.Interpreter.ReadStored(cadenceAddress1, "storage", "Foo") - require.Equal(t, stored, one) - - stored = migrated.Interpreter.ReadStored(cadenceAddress2, "storage", "Foo") - require.Equal(t, stored, two) - - stored = migrated.Interpreter.ReadStored(cadenceAddress3, "storage", "Foo") - require.Equal(t, stored, three) - - stored = migrated.Interpreter.ReadStored(cadenceAddress1, "storage", "Bar") - require.Equal(t, stored, four) - - stored = migrated.Interpreter.ReadStored(cadenceAddress2, "storage", "Bar") - require.Equal(t, stored, nil) - - stored = migrated.Interpreter.ReadStored(cadenceAddress1, "storage", "Baz") - require.Equal(t, stored, five) - - stored = migrated.Interpreter.ReadStored(cadenceAddress3, "public", "Foo") - require.Equal(t, stored, six) - - stored = migrated.Interpreter.ReadStored(cadenceAddress1, "public", "Foo") - require.Equal(t, stored, nil) - - }) -} - -func TestContractValue(t *testing.T) { - dir := t.TempDir() - mig := OrderedMapMigration{ - Log: zerolog.Logger{}, - OutputDir: dir, - } - - address1 := flow.HexToAddress("0x1") - cadenceAddress, _ := common.HexToAddress("0x1") - - _, _ = mig.initialize([]ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - }) - - encodeValue := func(v interpreter.Value) ledger.Value { - storable, err := v.Storable(mig.NewStorage, atree.Address(address1), math.MaxUint64) - require.NoError(t, err) - encodedInt, err := atree.Encode(storable, interpreter.CBOREncMode) - require.NoError(t, err) - return encodedInt - } - - t.Run("migrate contract values", func(t *testing.T) { - c := interpreter.NewCompositeValue( - mig.Interpreter, - common.AddressLocation{}, - "C", - common.CompositeKindContract, - []interpreter.CompositeField{}, - cadenceAddress, - ) - - payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, "contract\x1fFoo"), Value: encodeValue(c)}, - } - migratedPayload, err := mig.migrate(payload) - require.NoError(t, err) - require.Equal(t, len(migratedPayload), 6) - - migrated := &OrderedMapMigration{} - migrated.initPersistentSlabStorage(NewView(migratedPayload)) - migrated.initIntepreter() - - stored := migrated.Interpreter.ReadStored(cadenceAddress, "contract", "Foo") - require.IsType(t, &interpreter.CompositeValue{}, stored) - composite := stored.(*interpreter.CompositeValue) - require.Equal(t, composite.Kind, common.CompositeKindContract) - require.Equal(t, composite.QualifiedIdentifier, "C") - }) -} diff --git a/cmd/util/ledger/reporters/account_reporter_test.go b/cmd/util/ledger/reporters/account_reporter_test.go deleted file mode 100644 index 9689a2e0bd3..00000000000 --- a/cmd/util/ledger/reporters/account_reporter_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package reporters_test - -import ( - "math" - "math/big" - "testing" - - "github.com/onflow/atree" - "github.com/onflow/cadence" - "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/interpreter" - "github.com/rs/zerolog" - "github.com/stretchr/testify/require" - - "github.com/onflow/flow-go/cmd/util/ledger/migrations" - "github.com/onflow/flow-go/cmd/util/ledger/reporters" - "github.com/onflow/flow-go/engine/execution/state" - state2 "github.com/onflow/flow-go/fvm/state" - "github.com/onflow/flow-go/ledger" - "github.com/onflow/flow-go/ledger/common/utils" - "github.com/onflow/flow-go/model/flow" -) - -func createAccountPayloadKey(a flow.Address, key string) ledger.Key { - return ledger.Key{ - KeyParts: []ledger.KeyPart{ - ledger.NewKeyPart(state.KeyPartOwner, a.Bytes()), - ledger.NewKeyPart(state.KeyPartController, []byte("")), - ledger.NewKeyPart(state.KeyPartKey, []byte(key)), - }, - } -} - -func TestLookupValues(t *testing.T) { - dir := t.TempDir() - mig := migrations.OrderedMapMigration{ - Log: zerolog.Logger{}, - OutputDir: dir, - } - - address1 := flow.HexToAddress("0x1") - - encodeValue := func(v interpreter.Value) ledger.Value { - storable, err := v.Storable(mig.NewStorage, atree.Address(address1), math.MaxUint64) - require.NoError(t, err) - encodedInt, err := atree.Encode(storable, interpreter.CBOREncMode) - require.NoError(t, err) - return encodedInt - } - - one := interpreter.NewUnmeteredIntValueFromInt64(1) - - payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, - {Key: createAccountPayloadKey(address1, "storage\x1fFoo"), Value: encodeValue(one)}, - } - migratedPayload, _ := mig.Migrate(payload) - - l := migrations.NewView(migratedPayload) - bp := reporters.NewBalanceReporter(flow.Testnet.Chain(), l) - - stored, err := bp.ReadStored(address1, common.PathDomainStorage, "Foo") - require.NoError(t, err) - require.Equal(t, stored, cadence.Int{Value: big.NewInt(1)}) -} From fb2cb71c6f442b4bdedfedcf60d4c85f06829840 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 14:57:03 -0700 Subject: [PATCH 03/19] add account status migration --- .../migrations/account_status_migration.go | 44 +++++++++++++++ .../account_status_migration_test.go | 55 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 cmd/util/ledger/migrations/account_status_migration.go create mode 100644 cmd/util/ledger/migrations/account_status_migration_test.go diff --git a/cmd/util/ledger/migrations/account_status_migration.go b/cmd/util/ledger/migrations/account_status_migration.go new file mode 100644 index 00000000000..bc66571c837 --- /dev/null +++ b/cmd/util/ledger/migrations/account_status_migration.go @@ -0,0 +1,44 @@ +package migrations + +import ( + "encoding/hex" + + "github.com/onflow/flow-go/fvm/state" + "github.com/onflow/flow-go/ledger" + "github.com/rs/zerolog" +) + +// AccountStatusMigration migrates previous registers under +// key of Exists which were used for checking existance of accounts. +// the new register AccountStatus also captures frozen and all future states +// of the accounts. Frozen state is used when an account is set +// by the network governance for furture investigation and prevents operations on the account until +// furthure investigation by the community. +// This migration assumes no account has been frozen until now, and would warn if +// find any account with frozen flags. +type AccountStatusMigration struct { + Logger zerolog.Logger +} + +func (as *AccountStatusMigration) Migrate(payload []ledger.Payload) ([]ledger.Payload, error) { + newPayloads := make([]ledger.Payload, 0, len(payload)) + for _, p := range payload { + owner := p.Key.KeyParts[0].Value + controller := p.Key.KeyParts[1].Value + key := p.Key.KeyParts[2].Value + if len(controller) == 0 && string(key) == state.KeyExists { + newPayload := p.DeepCopy() + newPayload.Key.KeyParts[2].Value = []byte(state.KeyAccountStatus) + newPayload.Value = state.NewAccountStatus().ToBytes() + newPayloads = append(newPayloads, *newPayload) + continue + } + if len(controller) == 0 && string(key) == state.KeyAccountFrozen { + as.Logger.Warn().Msgf("frozen account found: %s", hex.EncodeToString(owner)) + continue + } + // else just append and continue + newPayloads = append(newPayloads, p) + } + return newPayloads, nil +} diff --git a/cmd/util/ledger/migrations/account_status_migration_test.go b/cmd/util/ledger/migrations/account_status_migration_test.go new file mode 100644 index 00000000000..ccf7125015f --- /dev/null +++ b/cmd/util/ledger/migrations/account_status_migration_test.go @@ -0,0 +1,55 @@ +package migrations + +import ( + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" + + "github.com/onflow/flow-go-sdk" + + state "github.com/onflow/flow-go/fvm/state" + "github.com/onflow/flow-go/ledger" + "github.com/onflow/flow-go/ledger/common/utils" +) + +func createAccountPayloadKey(a flow.Address, key string) ledger.Key { + return ledger.Key{ + KeyParts: []ledger.KeyPart{ + ledger.NewKeyPart(0, a.Bytes()), + ledger.NewKeyPart(1, []byte("")), + ledger.NewKeyPart(2, []byte(key)), + }, + } +} + +func TestAccountStatusMigration(t *testing.T) { + mig := AccountStatusMigration{ + Logger: zerolog.Logger{}, + } + + address1 := flow.HexToAddress("0x1") + address2 := flow.HexToAddress("0x2") + + payloads := []ledger.Payload{ + {Key: createAccountPayloadKey(address1, state.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, + {Key: createAccountPayloadKey(address1, "other registers"), Value: utils.Uint64ToBinary(2)}, + {Key: createAccountPayloadKey(address2, "other registers2"), Value: utils.Uint64ToBinary(3)}, + {Key: createAccountPayloadKey(address1, state.KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, state.KeyAccountFrozen), Value: []byte{1}}, + } + + newPayloads, err := mig.Migrate(payloads) + require.NoError(t, err) + require.Equal(t, 4, len(newPayloads)) // no more frozen register + + require.True(t, newPayloads[0].Equals(&payloads[0])) + require.True(t, newPayloads[1].Equals(&payloads[1])) + require.True(t, newPayloads[2].Equals(&payloads[2])) + + expectedPayload := &ledger.Payload{ + Key: createAccountPayloadKey(address1, state.KeyAccountStatus), + Value: state.NewAccountStatus().ToBytes(), + } + require.True(t, newPayloads[3].Equals(expectedPayload)) +} From 0db4cd0dddfbe89d8907e431ba34bd460417da3a Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 14:57:21 -0700 Subject: [PATCH 04/19] remove onhold status --- fvm/state/accounts_status.go | 12 ------------ fvm/state/accounts_status_test.go | 15 --------------- fvm/state/state_test.go | 1 + 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/fvm/state/accounts_status.go b/fvm/state/accounts_status.go index feb2b45f7ff..9ee2442887c 100644 --- a/fvm/state/accounts_status.go +++ b/fvm/state/accounts_status.go @@ -11,7 +11,6 @@ type AccountStatus uint8 const ( maskExist byte = 0b0000_0001 maskFrozen byte = 0b1000_0000 - maskOnHold byte = 0b0100_0000 ) // NewAccountStatus sets exist flag and return an AccountStatus @@ -50,14 +49,3 @@ func SetAccountStatusFrozenFlag(inp AccountStatus, frozen bool) AccountStatus { } return AccountStatus(uint8(inp) & (0xFF - maskFrozen)) } - -func (a AccountStatus) IsAccountOnHold() bool { - return uint8(a)&maskOnHold > 0 -} - -func SetAccountStatusOnHoldFlag(inp AccountStatus, onHold bool) AccountStatus { - if onHold { - return AccountStatus(uint8(inp) | maskOnHold) - } - return AccountStatus(uint8(inp) & (0xFF - maskOnHold)) -} diff --git a/fvm/state/accounts_status_test.go b/fvm/state/accounts_status_test.go index 24d2117a2d9..2765fa15b72 100644 --- a/fvm/state/accounts_status_test.go +++ b/fvm/state/accounts_status_test.go @@ -13,33 +13,18 @@ func TestAccountStatus(t *testing.T) { s := state.NewAccountStatus() require.True(t, s.AccountExists()) require.False(t, s.IsAccountFrozen()) - require.False(t, s.IsAccountOnHold()) - - s = state.SetAccountStatusOnHoldFlag(s, true) - require.True(t, s.AccountExists()) - require.False(t, s.IsAccountFrozen()) - require.True(t, s.IsAccountOnHold()) s = state.SetAccountStatusFrozenFlag(s, true) require.True(t, s.AccountExists()) require.True(t, s.IsAccountFrozen()) - require.True(t, s.IsAccountOnHold()) - - s = state.SetAccountStatusOnHoldFlag(s, false) - require.True(t, s.AccountExists()) - require.True(t, s.IsAccountFrozen()) - require.False(t, s.IsAccountOnHold()) s = state.SetAccountStatusFrozenFlag(s, false) require.True(t, s.AccountExists()) require.False(t, s.IsAccountFrozen()) - require.False(t, s.IsAccountOnHold()) var err error s, err = state.AccountStatusFromBytes(s.ToBytes()) require.NoError(t, err) require.True(t, s.AccountExists()) require.False(t, s.IsAccountFrozen()) - require.False(t, s.IsAccountOnHold()) - } diff --git a/fvm/state/state_test.go b/fvm/state/state_test.go index 203eda1fe82..68e9d116b82 100644 --- a/fvm/state/state_test.go +++ b/fvm/state/state_test.go @@ -205,6 +205,7 @@ func TestState_IsFVMStateKey(t *testing.T) { require.True(t, state.IsFVMStateKey("Address", "Address", state.KeyContractNames)) require.True(t, state.IsFVMStateKey("Address", "Address", state.KeyContractNames)) require.True(t, state.IsFVMStateKey("Address", "Address", "code.MYCODE")) + require.True(t, state.IsFVMStateKey("Address", "", state.KeyAccountStatus)) require.True(t, state.IsFVMStateKey("Address", "", state.KeyExists)) require.True(t, state.IsFVMStateKey("Address", "", state.KeyStorageUsed)) require.True(t, state.IsFVMStateKey("Address", "", state.KeyAccountFrozen)) From 3ee8b00ffe7651e28bd0ebaea03b353dbd8cb08d Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 15:06:12 -0700 Subject: [PATCH 05/19] no need for account freeze enabler anymore --- fvm/context.go | 11 +++++------ fvm/fvm_test.go | 1 - fvm/transactionAccountFrozen.go | 32 -------------------------------- fvm/transactionInvoker.go | 2 +- fvm/transaction_test.go | 6 ++---- 5 files changed, 8 insertions(+), 44 deletions(-) diff --git a/fvm/context.go b/fvm/context.go index 9ac0686063d..72549fe684d 100644 --- a/fvm/context.go +++ b/fvm/context.go @@ -34,7 +34,7 @@ type Context struct { CadenceLoggingEnabled bool EventCollectionEnabled bool ServiceEventCollectionEnabled bool - AccountFreezeAvailable bool + AccountFreezeEnabled bool ExtensiveTracing bool TransactionProcessors []TransactionProcessor ScriptProcessors []ScriptProcessor @@ -87,13 +87,12 @@ func defaultContext(logger zerolog.Logger) Context { CadenceLoggingEnabled: false, EventCollectionEnabled: true, ServiceEventCollectionEnabled: false, - AccountFreezeAvailable: false, + AccountFreezeEnabled: true, ExtensiveTracing: false, TransactionProcessors: []TransactionProcessor{ NewTransactionAccountFrozenChecker(), NewTransactionSignatureVerifier(AccountKeyWeightThreshold), NewTransactionSequenceNumberChecker(), - NewTransactionAccountFrozenEnabler(), NewTransactionInvoker(logger), }, ScriptProcessors: []ScriptProcessor{ @@ -183,12 +182,12 @@ func WithBlockHeader(header *flow.Header) Option { } } -// WithAccountFreezeAvailable sets availability of account freeze function for a virtual machine context. +// WithAccountFreezeEnabled enable/disable of account freeze functionality for a virtual machine context. // // With this option set to true, a setAccountFreeze function will be enabled for transactions processed by the VM -func WithAccountFreezeAvailable(accountFreezeAvailable bool) Option { +func WithAccountFreezeEnabled(accountFreezeEnabled bool) Option { return func(ctx Context) Context { - ctx.AccountFreezeAvailable = accountFreezeAvailable + ctx.AccountFreezeEnabled = accountFreezeEnabled return ctx } } diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 960aacec33a..4d2d197800d 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -1156,7 +1156,6 @@ func TestBlockContext_ExecuteTransaction_InteractionLimitReached(t *testing.T) { withContextOptions( fvm.WithTransactionProcessors( fvm.NewTransactionAccountFrozenChecker(), - fvm.NewTransactionAccountFrozenEnabler(), fvm.NewTransactionInvoker(zerolog.Nop()), ), ). diff --git a/fvm/transactionAccountFrozen.go b/fvm/transactionAccountFrozen.go index 91955e63b65..5a8b1d93a56 100644 --- a/fvm/transactionAccountFrozen.go +++ b/fvm/transactionAccountFrozen.go @@ -6,7 +6,6 @@ import ( "github.com/onflow/flow-go/fvm/programs" "github.com/onflow/flow-go/fvm/state" "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/module/trace" ) type TransactionAccountFrozenChecker struct{} @@ -52,34 +51,3 @@ func (c *TransactionAccountFrozenChecker) checkAccountNotFrozen( return nil } - -type TransactionAccountFrozenEnabler struct{} - -func NewTransactionAccountFrozenEnabler() *TransactionAccountFrozenEnabler { - return &TransactionAccountFrozenEnabler{} -} - -func (c *TransactionAccountFrozenEnabler) Process( - _ *VirtualMachine, - ctx *Context, - proc *TransactionProcedure, - _ *state.StateHolder, - _ *programs.Programs, -) error { - - if ctx.Tracer != nil && proc.TraceSpan != nil { - span := ctx.Tracer.StartSpanFromParent(proc.TraceSpan, trace.FVMFrozenAccountCheckTransaction) - defer span.Finish() - } - - serviceAddress := ctx.Chain.ServiceAddress() - - for _, signature := range proc.Transaction.EnvelopeSignatures { - if signature.Address == serviceAddress { - ctx.AccountFreezeAvailable = true - return nil // we can bail out and save maybe some loops - } - } - - return nil -} diff --git a/fvm/transactionInvoker.go b/fvm/transactionInvoker.go index a0b49565588..e704f6db0c8 100644 --- a/fvm/transactionInvoker.go +++ b/fvm/transactionInvoker.go @@ -307,7 +307,7 @@ var setAccountFrozenFunctionType = &sema.FunctionType{ func valueDeclarations(ctx *Context, env Environment) []runtime.ValueDeclaration { var predeclaredValues []runtime.ValueDeclaration - if ctx.AccountFreezeAvailable { + if ctx.AccountFreezeEnabled { // TODO return the errors instead of panicing setAccountFrozen := runtime.ValueDeclaration{ diff --git a/fvm/transaction_test.go b/fvm/transaction_test.go index 8f89a68b322..96e7b3477a1 100644 --- a/fvm/transaction_test.go +++ b/fvm/transaction_test.go @@ -76,14 +76,14 @@ func TestAccountFreezing(t *testing.T) { tx.AddAuthorizer(chain.ServiceAddress()) proc := fvm.Transaction(&tx, 0) - context := fvm.NewContext(log, fvm.WithAccountFreezeAvailable(false), fvm.WithChain(chain)) + context := fvm.NewContext(log, fvm.WithAccountFreezeEnabled(false), fvm.WithChain(chain)) err = txInvoker.Process(vm, &context, proc, st, programsStorage) require.Error(t, err) require.Contains(t, err.Error(), "cannot find") require.Contains(t, err.Error(), "setAccountFrozen") - context = fvm.NewContext(log, fvm.WithAccountFreezeAvailable(true), fvm.WithChain(chain)) + context = fvm.NewContext(log, fvm.WithAccountFreezeEnabled(true), fvm.WithChain(chain)) err = txInvoker.Process(vm, &context, proc, st, programsStorage) require.NoError(t, err) @@ -193,7 +193,6 @@ func TestAccountFreezing(t *testing.T) { fvm.WithCadenceLogging(true), fvm.WithTransactionProcessors( // run with limited processor to test just core of freezing, but still inside FVM fvm.NewTransactionAccountFrozenChecker(), - fvm.NewTransactionAccountFrozenEnabler(), fvm.NewTransactionInvoker(zerolog.Nop()))) err := vm.Run(context, procFrozen, st.State().View(), programsStorage) @@ -452,7 +451,6 @@ func TestAccountFreezing(t *testing.T) { fvm.WithCadenceLogging(true), fvm.WithTransactionProcessors( // run with limited processor to test just core of freezing, but still inside FVM fvm.NewTransactionAccountFrozenChecker(), - fvm.NewTransactionAccountFrozenEnabler(), fvm.NewTransactionInvoker(zerolog.Nop()))) err := accounts.SetAccountFrozen(frozenAddress, true) From 001695e569cd22b22c3efb88dda876774aa06bd2 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 15:13:04 -0700 Subject: [PATCH 06/19] account freezing disabled by the context --- fvm/transactionEnv.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fvm/transactionEnv.go b/fvm/transactionEnv.go index 963c36225ad..93dcff7944b 100644 --- a/fvm/transactionEnv.go +++ b/fvm/transactionEnv.go @@ -744,6 +744,10 @@ func (e *TransactionEnv) MemoryUsed() uint64 { func (e *TransactionEnv) SetAccountFrozen(address common.Address, frozen bool) error { + if !e.ctx.AccountFreezeEnabled { + return errors.NewOperationNotSupportedError("SetAccountFrozen") + } + flowAddress := flow.Address(address) if flowAddress == e.ctx.Chain.ServiceAddress() { From 64a43b152e3b951f34a5d86456b0cfb1786f1fa1 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 15:22:29 -0700 Subject: [PATCH 07/19] update migrations and remove depricated keys --- .../execution_state_extract.go | 13 +++---------- .../ledger/migrations/account_status_migration.go | 9 +++++++-- .../migrations/account_status_migration_test.go | 4 ++-- .../storage_used_update_migration_test.go | 8 ++++---- .../execution/computation/computer/computer_test.go | 6 +++--- fvm/state/accounts.go | 6 ------ fvm/state/state.go | 6 ------ fvm/state/state_test.go | 2 -- utils/debug/remoteDebugger.go | 1 - 9 files changed, 19 insertions(+), 36 deletions(-) diff --git a/cmd/util/cmd/execution-state-extract/execution_state_extract.go b/cmd/util/cmd/execution-state-extract/execution_state_extract.go index 67a70866f2c..961b7d667b8 100644 --- a/cmd/util/cmd/execution-state-extract/execution_state_extract.go +++ b/cmd/util/cmd/execution-state-extract/execution_state_extract.go @@ -63,19 +63,12 @@ func extractExecutionState( newState := ledger.State(targetHash) if migrate { - storageUsedUpdateMigration := mgr.StorageUsedUpdateMigration{ - Log: log, - OutputDir: outputDir, - } - - orderedMapMigration := mgr.OrderedMapMigration{ - Log: log, - OutputDir: dir, + accountStatusMigration := mgr.AccountStatusMigration{ + Logger: log, } migrations = []ledger.Migration{ - orderedMapMigration.Migrate, - storageUsedUpdateMigration.Migrate, + accountStatusMigration.Migrate, mgr.PruneMigration, } diff --git a/cmd/util/ledger/migrations/account_status_migration.go b/cmd/util/ledger/migrations/account_status_migration.go index bc66571c837..259972d0630 100644 --- a/cmd/util/ledger/migrations/account_status_migration.go +++ b/cmd/util/ledger/migrations/account_status_migration.go @@ -8,6 +8,11 @@ import ( "github.com/rs/zerolog" ) +const ( + KeyExists = "exists" + KeyAccountFrozen = "frozen" +) + // AccountStatusMigration migrates previous registers under // key of Exists which were used for checking existance of accounts. // the new register AccountStatus also captures frozen and all future states @@ -26,14 +31,14 @@ func (as *AccountStatusMigration) Migrate(payload []ledger.Payload) ([]ledger.Pa owner := p.Key.KeyParts[0].Value controller := p.Key.KeyParts[1].Value key := p.Key.KeyParts[2].Value - if len(controller) == 0 && string(key) == state.KeyExists { + if len(controller) == 0 && string(key) == KeyExists { newPayload := p.DeepCopy() newPayload.Key.KeyParts[2].Value = []byte(state.KeyAccountStatus) newPayload.Value = state.NewAccountStatus().ToBytes() newPayloads = append(newPayloads, *newPayload) continue } - if len(controller) == 0 && string(key) == state.KeyAccountFrozen { + if len(controller) == 0 && string(key) == KeyAccountFrozen { as.Logger.Warn().Msgf("frozen account found: %s", hex.EncodeToString(owner)) continue } diff --git a/cmd/util/ledger/migrations/account_status_migration_test.go b/cmd/util/ledger/migrations/account_status_migration_test.go index ccf7125015f..9e50de4ef5f 100644 --- a/cmd/util/ledger/migrations/account_status_migration_test.go +++ b/cmd/util/ledger/migrations/account_status_migration_test.go @@ -35,8 +35,8 @@ func TestAccountStatusMigration(t *testing.T) { {Key: createAccountPayloadKey(address1, state.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, {Key: createAccountPayloadKey(address1, "other registers"), Value: utils.Uint64ToBinary(2)}, {Key: createAccountPayloadKey(address2, "other registers2"), Value: utils.Uint64ToBinary(3)}, - {Key: createAccountPayloadKey(address1, state.KeyExists), Value: []byte{1}}, - {Key: createAccountPayloadKey(address1, state.KeyAccountFrozen), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, KeyAccountFrozen), Value: []byte{1}}, } newPayloads, err := mig.Migrate(payloads) diff --git a/cmd/util/ledger/migrations/storage_used_update_migration_test.go b/cmd/util/ledger/migrations/storage_used_update_migration_test.go index c6a8b6a722d..4ca55940953 100644 --- a/cmd/util/ledger/migrations/storage_used_update_migration_test.go +++ b/cmd/util/ledger/migrations/storage_used_update_migration_test.go @@ -25,7 +25,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { t.Run("fix storage used", func(t *testing.T) { payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, state2.KeyAccountStatus), Value: []byte{1}}, {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(1)}, } migratedPayload, err := mig.Migrate(payload) @@ -40,7 +40,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { t.Run("fix storage used if used to high", func(t *testing.T) { payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, state2.KeyAccountStatus), Value: []byte{1}}, {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(10000)}, } migratedPayload, err := mig.Migrate(payload) @@ -55,7 +55,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { t.Run("do not fix storage used if storage used ok", func(t *testing.T) { payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, state2.KeyAccountStatus), Value: []byte{1}}, {Key: createAccountPayloadKey(address1, state2.KeyStorageUsed), Value: utils.Uint64ToBinary(55)}, } migratedPayload, err := mig.Migrate(payload) @@ -70,7 +70,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { t.Run("error is storage used does not exist", func(t *testing.T) { payload := []ledger.Payload{ - {Key: createAccountPayloadKey(address1, state2.KeyExists), Value: []byte{1}}, + {Key: createAccountPayloadKey(address1, state2.KeyAccountStatus), Value: []byte{1}}, } _, err := mig.Migrate(payload) require.Error(t, err) diff --git a/engine/execution/computation/computer/computer_test.go b/engine/execution/computation/computer/computer_test.go index b865cd3fcdd..136ef768177 100644 --- a/engine/execution/computation/computer/computer_test.go +++ b/engine/execution/computation/computer/computer_test.go @@ -590,7 +590,7 @@ func (f *FixedAddressGenerator) AddressCount() uint64 { panic("not implemented") } -func Test_FreezeAccountChecksAreIncluded(t *testing.T) { +func Test_AccountStatusRegistersAreIncluded(t *testing.T) { address := flow.HexToAddress("1234") fag := &FixedAddressGenerator{Address: address} @@ -629,11 +629,11 @@ func Test_FreezeAccountChecksAreIncluded(t *testing.T) { registerTouches := view.Interactions().RegisterTouches() - // make sure check for frozen account has been registered + // make sure check for account status has been registered id := flow.RegisterID{ Owner: string(address.Bytes()), Controller: "", - Key: state.KeyAccountFrozen, + Key: state.KeyAccountStatus, } require.Contains(t, registerTouches, id.String()) diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index 55ec145b4c1..62ea6753674 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -24,12 +24,6 @@ const ( KeyStorageUsed = "storage_used" KeyStorageIndex = "storage_index" uint64StorageSize = 8 - - // TODO remove depricated keys when migrations don't need them any more - // depricated, please use account status instead - KeyExists = "exists" - // depricated, please use account status instead - KeyAccountFrozen = "frozen" ) type Accounts interface { diff --git a/fvm/state/state.go b/fvm/state/state.go index c1a450614e1..f4485904282 100644 --- a/fvm/state/state.go +++ b/fvm/state/state.go @@ -363,12 +363,6 @@ func IsFVMStateKey(owner, controller, key string) bool { } if len(controller) == 0 { - if key == KeyExists { - return true - } - if key == KeyAccountFrozen { - return true - } if key == KeyAccountStatus { return true } diff --git a/fvm/state/state_test.go b/fvm/state/state_test.go index 68e9d116b82..c8b84352d91 100644 --- a/fvm/state/state_test.go +++ b/fvm/state/state_test.go @@ -206,9 +206,7 @@ func TestState_IsFVMStateKey(t *testing.T) { require.True(t, state.IsFVMStateKey("Address", "Address", state.KeyContractNames)) require.True(t, state.IsFVMStateKey("Address", "Address", "code.MYCODE")) require.True(t, state.IsFVMStateKey("Address", "", state.KeyAccountStatus)) - require.True(t, state.IsFVMStateKey("Address", "", state.KeyExists)) require.True(t, state.IsFVMStateKey("Address", "", state.KeyStorageUsed)) - require.True(t, state.IsFVMStateKey("Address", "", state.KeyAccountFrozen)) require.False(t, state.IsFVMStateKey("Address", "", "anything else")) } diff --git a/utils/debug/remoteDebugger.go b/utils/debug/remoteDebugger.go index a325f6272a8..c5cfe50f5b3 100644 --- a/utils/debug/remoteDebugger.go +++ b/utils/debug/remoteDebugger.go @@ -30,7 +30,6 @@ func NewRemoteDebugger(grpcAddress string, fvm.WithTransactionProcessors( fvm.NewTransactionAccountFrozenChecker(), fvm.NewTransactionSequenceNumberChecker(), - fvm.NewTransactionAccountFrozenEnabler(), fvm.NewTransactionInvoker(logger), ), ) From a85a535e7b7cfe40e895ae223a415c0fe0151aaa Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 16:23:43 -0700 Subject: [PATCH 08/19] merge FrozenChecker into TxVerifier --- fvm/context.go | 3 +- fvm/fvm_test.go | 1 - fvm/state/accounts.go | 4 + fvm/transactionAccountFrozen.go | 53 ---------- fvm/transactionVerifier.go | 62 +++++++++--- fvm/transactionVerifier_test.go | 91 ++++++++++++++++- fvm/transaction_test.go | 170 +++++++------------------------- utils/debug/remoteDebugger.go | 1 - 8 files changed, 182 insertions(+), 203 deletions(-) delete mode 100644 fvm/transactionAccountFrozen.go diff --git a/fvm/context.go b/fvm/context.go index 72549fe684d..c605d58c84e 100644 --- a/fvm/context.go +++ b/fvm/context.go @@ -90,8 +90,7 @@ func defaultContext(logger zerolog.Logger) Context { AccountFreezeEnabled: true, ExtensiveTracing: false, TransactionProcessors: []TransactionProcessor{ - NewTransactionAccountFrozenChecker(), - NewTransactionSignatureVerifier(AccountKeyWeightThreshold), + NewTransactionVerifier(AccountKeyWeightThreshold), NewTransactionSequenceNumberChecker(), NewTransactionInvoker(logger), }, diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 4d2d197800d..9fcd91c89d2 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -1155,7 +1155,6 @@ func TestBlockContext_ExecuteTransaction_InteractionLimitReached(t *testing.T) { t.Run("Using to much interaction fails but does not panic", newVMTest().withBootstrapProcedureOptions(bootstrapOptions...). withContextOptions( fvm.WithTransactionProcessors( - fvm.NewTransactionAccountFrozenChecker(), fvm.NewTransactionInvoker(zerolog.Nop()), ), ). diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index 62ea6753674..8bed380929c 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -620,6 +620,10 @@ func (a *StatefulAccounts) GetAccountFrozen(address flow.Address) (bool, error) if err != nil { return false, err } + // if account does not exist, frozen is not meaningful + if !accStatus.AccountExists() { + return false, errors.NewAccountNotFoundError(address) + } return accStatus.IsAccountFrozen(), nil } diff --git a/fvm/transactionAccountFrozen.go b/fvm/transactionAccountFrozen.go deleted file mode 100644 index 5a8b1d93a56..00000000000 --- a/fvm/transactionAccountFrozen.go +++ /dev/null @@ -1,53 +0,0 @@ -package fvm - -import ( - "fmt" - - "github.com/onflow/flow-go/fvm/programs" - "github.com/onflow/flow-go/fvm/state" - "github.com/onflow/flow-go/model/flow" -) - -type TransactionAccountFrozenChecker struct{} - -func NewTransactionAccountFrozenChecker() *TransactionAccountFrozenChecker { - return &TransactionAccountFrozenChecker{} -} - -func (c *TransactionAccountFrozenChecker) Process( - _ *VirtualMachine, - _ *Context, - proc *TransactionProcedure, - sth *state.StateHolder, - _ *programs.Programs, -) error { - sth.DisableAllLimitEnforcements() - defer sth.EnableAllLimitEnforcements() - return c.checkAccountNotFrozen(proc.Transaction, sth) -} - -func (c *TransactionAccountFrozenChecker) checkAccountNotFrozen( - tx *flow.TransactionBody, - sth *state.StateHolder, -) error { - accounts := state.NewAccounts(sth) - - for _, authorizer := range tx.Authorizers { - err := accounts.CheckAccountNotFrozen(authorizer) - if err != nil { - return fmt.Errorf("checking frozen account failed: %w", err) - } - } - - err := accounts.CheckAccountNotFrozen(tx.ProposalKey.Address) - if err != nil { - return fmt.Errorf("checking frozen account failed: %w", err) - } - - err = accounts.CheckAccountNotFrozen(tx.Payer) - if err != nil { - return fmt.Errorf("checking frozen account failed: %w", err) - } - - return nil -} diff --git a/fvm/transactionVerifier.go b/fvm/transactionVerifier.go index 1c9f3921f2a..d3b3d67839e 100644 --- a/fvm/transactionVerifier.go +++ b/fvm/transactionVerifier.go @@ -13,24 +13,31 @@ import ( "github.com/onflow/flow-go/module/trace" ) -type TransactionSignatureVerifier struct { +// TransactionVerifier verifies the content of the transaction by +// checking accounts (authorizers, payer, proposer) are not frozen +// checking there is no double signature +// all signatures are valid +// all accounts provides enoguh weights +// +// if KeyWeightThreshold is set to a negative number, signature verification is skipped +type TransactionVerifier struct { KeyWeightThreshold int } -func NewTransactionSignatureVerifier(keyWeightThreshold int) *TransactionSignatureVerifier { - return &TransactionSignatureVerifier{ +func NewTransactionVerifier(keyWeightThreshold int) *TransactionVerifier { + return &TransactionVerifier{ KeyWeightThreshold: keyWeightThreshold, } } -func (v *TransactionSignatureVerifier) Process( +func (v *TransactionVerifier) Process( _ *VirtualMachine, ctx *Context, proc *TransactionProcedure, sth *state.StateHolder, _ *programs.Programs, ) error { - return v.verifyTransactionSignatures(proc, *ctx, sth) + return v.verifyTransaction(proc, *ctx, sth) } func newInvalidEnvelopeSignatureError(txSig flow.TransactionSignature, err error) error { @@ -41,7 +48,7 @@ func newInvalidPayloadSignatureError(txSig flow.TransactionSignature, err error) return errors.NewInvalidPayloadSignatureError(txSig.Address, txSig.KeyIndex, err) } -func (v *TransactionSignatureVerifier) verifyTransactionSignatures( +func (v *TransactionVerifier) verifyTransaction( proc *TransactionProcedure, ctx Context, sth *state.StateHolder, @@ -70,6 +77,15 @@ func (v *TransactionSignatureVerifier) verifyTransactionSignatures( return fmt.Errorf("transaction verification failed: %w", err) } + err = v.checkAccountsAreNotFrozen(tx, accounts) + if err != nil { + return fmt.Errorf("transaction verification failed: %w", err) + } + + if v.KeyWeightThreshold < 0 { + return nil + } + payloadWeights, proposalKeyVerifiedInPayload, err = v.verifyAccountSignatures( accounts, tx.PayloadSignatures, @@ -125,7 +141,7 @@ func (v *TransactionSignatureVerifier) verifyTransactionSignatures( return nil } -func (v *TransactionSignatureVerifier) verifyAccountSignatures( +func (v *TransactionVerifier) verifyAccountSignatures( accounts state.Accounts, signatures []flow.TransactionSignature, message []byte, @@ -165,7 +181,7 @@ func (v *TransactionSignatureVerifier) verifyAccountSignatures( // // An error is returned if the account does not contain a public key that // correctly verifies the signature against the given message. -func (v *TransactionSignatureVerifier) verifyAccountSignature( +func (v *TransactionVerifier) verifyAccountSignature( accountKey flow.AccountPublicKey, txSig flow.TransactionSignature, message []byte, @@ -193,21 +209,21 @@ func (v *TransactionSignatureVerifier) verifyAccountSignature( return errorBuilder(txSig, fmt.Errorf("signature is not valid")) } -func (v *TransactionSignatureVerifier) hasSufficientKeyWeight( +func (v *TransactionVerifier) hasSufficientKeyWeight( weights map[flow.Address]int, address flow.Address, ) bool { return weights[address] >= v.KeyWeightThreshold } -func (v *TransactionSignatureVerifier) sigIsForProposalKey( +func (v *TransactionVerifier) sigIsForProposalKey( txSig flow.TransactionSignature, proposalKey flow.ProposalKey, ) bool { return txSig.Address == proposalKey.Address && txSig.KeyIndex == proposalKey.KeyIndex } -func (v *TransactionSignatureVerifier) checkSignatureDuplications(tx *flow.TransactionBody) error { +func (v *TransactionVerifier) checkSignatureDuplications(tx *flow.TransactionBody) error { type uniqueKey struct { address flow.Address index uint64 @@ -230,3 +246,27 @@ func (v *TransactionSignatureVerifier) checkSignatureDuplications(tx *flow.Trans } return nil } + +func (c *TransactionVerifier) checkAccountsAreNotFrozen( + tx *flow.TransactionBody, + accounts state.Accounts, +) error { + for _, authorizer := range tx.Authorizers { + err := accounts.CheckAccountNotFrozen(authorizer) + if err != nil { + return fmt.Errorf("checking frozen account failed: %w", err) + } + } + + err := accounts.CheckAccountNotFrozen(tx.ProposalKey.Address) + if err != nil { + return fmt.Errorf("checking frozen account failed: %w", err) + } + + err = accounts.CheckAccountNotFrozen(tx.Payer) + if err != nil { + return fmt.Errorf("checking frozen account failed: %w", err) + } + + return nil +} diff --git a/fvm/transactionVerifier_test.go b/fvm/transactionVerifier_test.go index b243476d511..6be43171de5 100644 --- a/fvm/transactionVerifier_test.go +++ b/fvm/transactionVerifier_test.go @@ -47,7 +47,7 @@ func TestTransactionVerification(t *testing.T) { tx.PayloadSignatures = []flow.TransactionSignature{sig1, sig2} proc := fvm.Transaction(&tx, 0) - txVerifier := fvm.NewTransactionSignatureVerifier(1000) + txVerifier := fvm.NewTransactionVerifier(1000) err = txVerifier.Process(nil, &fvm.Context{}, proc, sth, programs.NewEmptyPrograms()) require.Error(t, err) require.True(t, strings.Contains(err.Error(), "duplicate signatures are provided for the same key")) @@ -61,9 +61,96 @@ func TestTransactionVerification(t *testing.T) { tx.EnvelopeSignatures = []flow.TransactionSignature{sig2} proc := fvm.Transaction(&tx, 0) - txVerifier := fvm.NewTransactionSignatureVerifier(1000) + txVerifier := fvm.NewTransactionVerifier(1000) err = txVerifier.Process(nil, &fvm.Context{}, proc, sth, programs.NewEmptyPrograms()) require.Error(t, err) require.True(t, strings.Contains(err.Error(), "duplicate signatures are provided for the same key")) }) + + t.Run("frozen account is rejected", func(t *testing.T) { + + txChecker := fvm.NewTransactionVerifier(-1) + + frozenAddress, notFrozenAddress, st := makeTwoAccounts(t, nil, nil) + accounts := state.NewAccounts(st) + programsStorage := programs.NewEmptyPrograms() + + // freeze account + err := accounts.SetAccountFrozen(frozenAddress, true) + require.NoError(t, err) + + // make sure freeze status is correct + frozen, err := accounts.GetAccountFrozen(frozenAddress) + require.NoError(t, err) + require.True(t, frozen) + + frozen, err = accounts.GetAccountFrozen(notFrozenAddress) + require.NoError(t, err) + require.False(t, frozen) + + // Authorizers + tx := fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.NoError(t, err) + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Authorizers: []flow.Address{notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.NoError(t, err) + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Authorizers: []flow.Address{frozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.Error(t, err) + + // all addresses must not be frozen + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Authorizers: []flow.Address{frozenAddress, notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.Error(t, err) + + // Payer should be part of authorizers account, but lets check it separately for completeness + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.NoError(t, err) + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: frozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.Error(t, err) + + // Proposal account + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: frozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.Error(t, err) + + tx = fvm.Transaction(&flow.TransactionBody{ + Payer: notFrozenAddress, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + }, 0) + err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) + require.NoError(t, err) + }) } diff --git a/fvm/transaction_test.go b/fvm/transaction_test.go index 96e7b3477a1..3bbda1c20ee 100644 --- a/fvm/transaction_test.go +++ b/fvm/transaction_test.go @@ -94,68 +94,6 @@ func TestAccountFreezing(t *testing.T) { require.True(t, frozen) }) - t.Run("frozen account is rejected", func(t *testing.T) { - - txChecker := fvm.NewTransactionAccountFrozenChecker() - - frozenAddress, notFrozenAddress, st := makeTwoAccounts(t, nil, nil) - accounts := state.NewAccounts(st) - programsStorage := programs.NewEmptyPrograms() - - // freeze account - err := accounts.SetAccountFrozen(frozenAddress, true) - require.NoError(t, err) - - // make sure freeze status is correct - frozen, err := accounts.GetAccountFrozen(frozenAddress) - require.NoError(t, err) - require.True(t, frozen) - - frozen, err = accounts.GetAccountFrozen(notFrozenAddress) - require.NoError(t, err) - require.False(t, frozen) - - // Authorizers - - // no account associated with tx so it should work - tx := fvm.Transaction(&flow.TransactionBody{}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.NoError(t, err) - - tx = fvm.Transaction(&flow.TransactionBody{Authorizers: []flow.Address{notFrozenAddress}}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.NoError(t, err) - - tx = fvm.Transaction(&flow.TransactionBody{Authorizers: []flow.Address{frozenAddress}}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.Error(t, err) - - // all addresses must not be frozen - tx = fvm.Transaction(&flow.TransactionBody{Authorizers: []flow.Address{frozenAddress, notFrozenAddress}}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.Error(t, err) - - // Payer should be part of authorizers account, but lets check it separately for completeness - - tx = fvm.Transaction(&flow.TransactionBody{Payer: notFrozenAddress}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.NoError(t, err) - - tx = fvm.Transaction(&flow.TransactionBody{Payer: frozenAddress}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.Error(t, err) - - // Proposal account - - tx = fvm.Transaction(&flow.TransactionBody{ProposalKey: flow.ProposalKey{Address: frozenAddress}}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.Error(t, err) - - tx = fvm.Transaction(&flow.TransactionBody{ProposalKey: flow.ProposalKey{Address: notFrozenAddress}}, 0) - err = txChecker.Process(nil, &fvm.Context{}, tx, st, programsStorage) - require.NoError(t, err) - }) - t.Run("code from frozen account cannot be loaded", func(t *testing.T) { frozenAddress, notFrozenAddress, st := makeTwoAccounts(t, nil, nil) @@ -192,7 +130,6 @@ func TestAccountFreezing(t *testing.T) { fvm.WithRestrictedDeployment(false), fvm.WithCadenceLogging(true), fvm.WithTransactionProcessors( // run with limited processor to test just core of freezing, but still inside FVM - fvm.NewTransactionAccountFrozenChecker(), fvm.NewTransactionInvoker(zerolog.Nop()))) err := vm.Run(context, procFrozen, st.State().View(), programsStorage) @@ -278,68 +215,6 @@ func TestAccountFreezing(t *testing.T) { require.Equal(t, frozenAddress, accountFrozenError.Address()) }) - t.Run("default settings allow only service account to freeze accounts", func(t *testing.T) { - - rt := fvm.NewInterpreterRuntime() - log := zerolog.Nop() - vm := fvm.NewVirtualMachine(rt) - // create default context - context := fvm.NewContext(log) - programsStorage := programs.NewEmptyPrograms() - - ledger := testutil.RootBootstrappedLedger(vm, context) - - privateKeys, err := testutil.GenerateAccountPrivateKeys(1) - require.NoError(t, err) - - // Bootstrap a ledger, creating accounts with the provided private keys and the root account. - accounts, err := testutil.CreateAccounts(vm, ledger, programsStorage, privateKeys, context.Chain) - require.NoError(t, err) - - address := accounts[0] - - code := fmt.Sprintf(` - transaction { - prepare(auth: AuthAccount) {} - execute { - setAccountFrozen(0x%s, true) - } - } - `, address.String()) - - txBody := &flow.TransactionBody{Script: []byte(code)} - txBody.SetPayer(accounts[0]) - txBody.SetProposalKey(accounts[0], 0, 0) - - err = testutil.SignEnvelope(txBody, accounts[0], privateKeys[0]) - require.NoError(t, err) - - tx := fvm.Transaction(txBody, 0) - err = vm.Run(context, tx, ledger, programsStorage) - require.NoError(t, err) - require.Error(t, tx.Err) - - require.Contains(t, tx.Err.Error(), "cannot find") - require.Contains(t, tx.Err.Error(), "setAccountFrozen") - require.Equal(t, (&errors.CadenceRuntimeError{}).Code(), tx.Err.Code()) - - // sign tx by service account now - txBody = &flow.TransactionBody{Script: []byte(code)} - txBody.AddAuthorizer(serviceAddress) - txBody.SetPayer(serviceAddress) - txBody.SetProposalKey(serviceAddress, 0, 0) - - err = testutil.SignEnvelope(txBody, serviceAddress, unittest.ServiceAccountPrivateKey) - require.NoError(t, err) - - tx = fvm.Transaction(txBody, 0) - err = vm.Run(context, tx, ledger, programsStorage) - require.NoError(t, err) - - require.NoError(t, tx.Err) - - }) - t.Run("service account cannot freeze itself", func(t *testing.T) { rt := fvm.NewInterpreterRuntime() @@ -450,7 +325,7 @@ func TestAccountFreezing(t *testing.T) { fvm.WithRestrictedDeployment(false), fvm.WithCadenceLogging(true), fvm.WithTransactionProcessors( // run with limited processor to test just core of freezing, but still inside FVM - fvm.NewTransactionAccountFrozenChecker(), + fvm.NewTransactionVerifier(-1), fvm.NewTransactionInvoker(zerolog.Nop()))) err := accounts.SetAccountFrozen(frozenAddress, true) @@ -467,9 +342,18 @@ func TestAccountFreezing(t *testing.T) { t.Run("authorizer", func(t *testing.T) { - notFrozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{notFrozenAddress}}, 0) - frozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{frozenAddress}}, 0) - + notFrozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{notFrozenAddress}, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Payer: notFrozenAddress}, + 0) + frozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{frozenAddress}, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Payer: notFrozenAddress}, + 0) // tx run OK by nonfrozen account err = vm.Run(context, notFrozenProc, st.State().View(), programsStorage) require.NoError(t, err) @@ -487,8 +371,18 @@ func TestAccountFreezing(t *testing.T) { t.Run("proposal", func(t *testing.T) { - notFrozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{serviceAddress}, ProposalKey: flow.ProposalKey{Address: notFrozenAddress}}, 0) - frozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{serviceAddress}, ProposalKey: flow.ProposalKey{Address: frozenAddress}}, 0) + notFrozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{notFrozenAddress}, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Payer: notFrozenAddress, + }, 0) + frozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{notFrozenAddress}, + ProposalKey: flow.ProposalKey{Address: frozenAddress}, + Payer: notFrozenAddress, + }, 0) // tx run OK by nonfrozen account err = vm.Run(context, notFrozenProc, st.State().View(), programsStorage) @@ -507,8 +401,18 @@ func TestAccountFreezing(t *testing.T) { t.Run("payer", func(t *testing.T) { - notFrozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{serviceAddress}, Payer: notFrozenAddress}, 0) - frozenProc := fvm.Transaction(&flow.TransactionBody{Script: whateverCode, Authorizers: []flow.Address{serviceAddress}, Payer: frozenAddress}, 0) + notFrozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{notFrozenAddress}, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Payer: notFrozenAddress, + }, 0) + frozenProc := fvm.Transaction(&flow.TransactionBody{ + Script: whateverCode, + Authorizers: []flow.Address{notFrozenAddress}, + ProposalKey: flow.ProposalKey{Address: notFrozenAddress}, + Payer: frozenAddress, + }, 0) // tx run OK by nonfrozen account err = vm.Run(context, notFrozenProc, st.State().View(), programsStorage) diff --git a/utils/debug/remoteDebugger.go b/utils/debug/remoteDebugger.go index c5cfe50f5b3..f465d7a47e7 100644 --- a/utils/debug/remoteDebugger.go +++ b/utils/debug/remoteDebugger.go @@ -28,7 +28,6 @@ func NewRemoteDebugger(grpcAddress string, logger, fvm.WithChain(chain), fvm.WithTransactionProcessors( - fvm.NewTransactionAccountFrozenChecker(), fvm.NewTransactionSequenceNumberChecker(), fvm.NewTransactionInvoker(logger), ), From 8ba25f13c511a0fd1decfdca81f4f8ea5f8c143d Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 16:37:48 -0700 Subject: [PATCH 09/19] optimize get contract to load less register on happy path --- fvm/state/accounts.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index 8bed380929c..3e94f41c353 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -546,14 +546,19 @@ func (a *StatefulAccounts) ContractExists(contractName string, address flow.Addr } func (a *StatefulAccounts) GetContract(contractName string, address flow.Address) ([]byte, error) { - exists, err := a.ContractExists(contractName, address) - if err != nil { - return nil, err - } - if !exists { - return nil, nil + // we optimized the happy case here, we look up for the content of the contract + // and if its not there we check if contract exists or if this is another problem. + code, err := a.getContract(contractName, address) + if err != nil || len(code) == 0 { + exists, err := a.ContractExists(contractName, address) + if err != nil { + return nil, err + } + if !exists { + return nil, nil + } } - return a.getContract(contractName, address) + return code, err } func (a *StatefulAccounts) SetContract(contractName string, address flow.Address, contract []byte) error { From 9284151a485b596fabefadf8ddcb2f5361dcf2d4 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 26 May 2022 17:26:48 -0700 Subject: [PATCH 10/19] remove unused methods --- cmd/util/ledger/migrations/utils.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/cmd/util/ledger/migrations/utils.go b/cmd/util/ledger/migrations/utils.go index 87bba9ab151..558cb5eadd3 100644 --- a/cmd/util/ledger/migrations/utils.go +++ b/cmd/util/ledger/migrations/utils.go @@ -1,7 +1,6 @@ package migrations import ( - "bytes" "fmt" "github.com/onflow/atree" @@ -96,23 +95,3 @@ func (a *AccountsAtreeLedger) AllocateStorageIndex(owner []byte) (atree.StorageI } return v, nil } - -func splitPayloads(inp []ledger.Payload) (fvmPayloads []ledger.Payload, storagePayloads []ledger.Payload, slabPayloads []ledger.Payload) { - for _, p := range inp { - if fvmState.IsFVMStateKey( - string(p.Key.KeyParts[0].Value), - string(p.Key.KeyParts[1].Value), - string(p.Key.KeyParts[2].Value), - ) { - fvmPayloads = append(fvmPayloads, p) - continue - } - if bytes.HasPrefix(p.Key.KeyParts[2].Value, []byte(atree.LedgerBaseStorageSlabPrefix)) { - slabPayloads = append(slabPayloads, p) - continue - } - // otherwise this is a storage payload - storagePayloads = append(storagePayloads, p) - } - return -} From 480b19d262e552831bf638daf6750ccd2a778ad0 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Mon, 30 May 2022 10:05:55 -0700 Subject: [PATCH 11/19] format imports --- cmd/util/ledger/migrations/account_status_migration.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/util/ledger/migrations/account_status_migration.go b/cmd/util/ledger/migrations/account_status_migration.go index 259972d0630..c1b26aecc11 100644 --- a/cmd/util/ledger/migrations/account_status_migration.go +++ b/cmd/util/ledger/migrations/account_status_migration.go @@ -3,9 +3,10 @@ package migrations import ( "encoding/hex" + "github.com/rs/zerolog" + "github.com/onflow/flow-go/fvm/state" "github.com/onflow/flow-go/ledger" - "github.com/rs/zerolog" ) const ( From aceb91a5024b833959b6e149aa243cb8f9799b0b Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 2 Jun 2022 09:38:43 -0700 Subject: [PATCH 12/19] update tests --- .../storage_used_update_migration_test.go | 6 +++--- .../computation/computer/computer_test.go | 11 +++++++++-- .../computation/execution_verification_test.go | 2 +- engine/execution/state/bootstrap/bootstrap_test.go | 2 +- fvm/accounts_test.go | 2 +- fvm/state/accounts.go | 2 +- fvm/state/accounts_test.go | 14 +++++++------- utils/unittest/execution_state.go | 2 +- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/cmd/util/ledger/migrations/storage_used_update_migration_test.go b/cmd/util/ledger/migrations/storage_used_update_migration_test.go index 4ca55940953..dab5d354ad6 100644 --- a/cmd/util/ledger/migrations/storage_used_update_migration_test.go +++ b/cmd/util/ledger/migrations/storage_used_update_migration_test.go @@ -35,7 +35,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(55), migratedSize) + require.Equal(t, uint64(62), migratedSize) }) t.Run("fix storage used if used to high", func(t *testing.T) { @@ -50,7 +50,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(55), migratedSize) + require.Equal(t, uint64(62), migratedSize) }) t.Run("do not fix storage used if storage used ok", func(t *testing.T) { @@ -65,7 +65,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(55), migratedSize) + require.Equal(t, uint64(62), migratedSize) }) t.Run("error is storage used does not exist", func(t *testing.T) { diff --git a/engine/execution/computation/computer/computer_test.go b/engine/execution/computation/computer/computer_test.go index 136ef768177..15155dea3ac 100644 --- a/engine/execution/computation/computer/computer_test.go +++ b/engine/execution/computation/computer/computer_test.go @@ -367,8 +367,9 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { execCtx := fvm.NewContext(zerolog.Nop()) + address := common.Address{0x1} contractLocation := common.AddressLocation{ - Address: common.Address{0x1}, + Address: address, Name: "Test", } @@ -407,6 +408,8 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { return nil, nil }) + view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + result, err := exe.ExecuteBlock(context.Background(), block, view, programs.NewEmptyPrograms()) assert.NoError(t, err) assert.Len(t, result.StateSnapshots, collectionCount+1) // +1 system chunk @@ -423,8 +426,10 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { ), ) + address := common.Address{0x1} + contractLocation := common.AddressLocation{ - Address: common.Address{0x1}, + Address: address, Name: "Test", } @@ -477,6 +482,8 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { return nil, nil }) + view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + result, err := exe.ExecuteBlock(context.Background(), block, view, programs.NewEmptyPrograms()) require.NoError(t, err) assert.Len(t, result.StateSnapshots, collectionCount+1) // +1 system chunk diff --git a/engine/execution/computation/execution_verification_test.go b/engine/execution/computation/execution_verification_test.go index b331ddb27ec..a12fb53b85a 100644 --- a/engine/execution/computation/execution_verification_test.go +++ b/engine/execution/computation/execution_verification_test.go @@ -160,7 +160,7 @@ func Test_ExecutionMatchesVerification(t *testing.T) { err = testutil.SignTransaction(addKeyTx, accountAddress, accountPrivKey, 0) require.NoError(t, err) - minimumStorage, err := cadence.NewUFix64("0.00008164") + minimumStorage, err := cadence.NewUFix64("0.00008171") require.NoError(t, err) cr := executeBlockAndVerify(t, [][]*flow.TransactionBody{ diff --git a/engine/execution/state/bootstrap/bootstrap_test.go b/engine/execution/state/bootstrap/bootstrap_test.go index 33613c6909b..96356f204c7 100644 --- a/engine/execution/state/bootstrap/bootstrap_test.go +++ b/engine/execution/state/bootstrap/bootstrap_test.go @@ -47,7 +47,7 @@ func TestBootstrapLedger(t *testing.T) { } func TestBootstrapLedger_ZeroTokenSupply(t *testing.T) { - expectedStateCommitmentBytes, _ := hex.DecodeString("cae2f2d6c53582503dad30dba8a8bba098ff8654d19b820a49e1961c1f459a41") + expectedStateCommitmentBytes, _ := hex.DecodeString("5c59bcf9dd6b91ec41a50ba7a16c6069bb5add83615511e50d708be0334f0fd2") expectedStateCommitment, err := flow.ToStateCommitment(expectedStateCommitmentBytes) require.NoError(t, err) diff --git a/fvm/accounts_test.go b/fvm/accounts_test.go index 7ae1c81b60a..523d69ffe35 100644 --- a/fvm/accounts_test.go +++ b/fvm/accounts_test.go @@ -1284,7 +1284,7 @@ func TestAccountBalanceFields(t *testing.T) { assert.NoError(t, err) assert.NoError(t, script.Err) - assert.Equal(t, cadence.UFix64(9999_2520), script.Value) + assert.Equal(t, cadence.UFix64(9999_2450), script.Value) }), ) diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index 3e94f41c353..c2b6921ba18 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -17,7 +17,7 @@ import ( ) const ( - KeyAccountStatus = "account_state" + KeyAccountStatus = "account_status" KeyPublicKeyCount = "public_key_count" KeyCode = "code" KeyContractNames = "contract_names" diff --git a/fvm/state/accounts_test.go b/fvm/state/accounts_test.go index 96fc0398628..f5ff59ed8eb 100644 --- a/fvm/state/accounts_test.go +++ b/fvm/state/accounts_test.go @@ -270,7 +270,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55), storageUsed) + require.Equal(t, uint64(62), storageUsed) }) t.Run("Storage used on register set increases", func(t *testing.T) { @@ -287,7 +287,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+34), storageUsed) + require.Equal(t, uint64(62+34), storageUsed) }) t.Run("Storage used, set twice on same register to same value, stays the same", func(t *testing.T) { @@ -306,7 +306,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+34), storageUsed) + require.Equal(t, uint64(62+34), storageUsed) }) t.Run("Storage used, set twice on same register to larger value, increases", func(t *testing.T) { @@ -325,7 +325,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+35), storageUsed) + require.Equal(t, uint64(62+35), storageUsed) }) t.Run("Storage used, set twice on same register to smaller value, decreases", func(t *testing.T) { @@ -344,7 +344,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+33), storageUsed) + require.Equal(t, uint64(62+33), storageUsed) }) t.Run("Storage used, after register deleted, decreases", func(t *testing.T) { @@ -363,7 +363,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+0), storageUsed) + require.Equal(t, uint64(62+0), storageUsed) }) t.Run("Storage used on a complex scenario has correct value", func(t *testing.T) { @@ -392,7 +392,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(55+33+46), storageUsed) + require.Equal(t, uint64(62+33+46), storageUsed) }) } diff --git a/utils/unittest/execution_state.go b/utils/unittest/execution_state.go index 3a10537863b..69ce9657475 100644 --- a/utils/unittest/execution_state.go +++ b/utils/unittest/execution_state.go @@ -24,7 +24,7 @@ const ServiceAccountPrivateKeySignAlgo = crypto.ECDSAP256 const ServiceAccountPrivateKeyHashAlgo = hash.SHA2_256 // Pre-calculated state commitment with root account with the above private key -const GenesisStateCommitmentHex = "5b8f8283d5e719672cb53c0e20a822bf0782f4345d09df076c14fba4d9e21da0" +const GenesisStateCommitmentHex = "82d5e109f74c336325a270d180cd760f625665ee3675f36659ba9b4e2d7ab9ce" var GenesisStateCommitment flow.StateCommitment From 8295c41eb43d9afd00b17597bd321724fe11b238 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 2 Jun 2022 14:10:29 -0700 Subject: [PATCH 13/19] reduce key prefix size --- .../storage_used_update_migration_test.go | 6 +++--- fvm/state/accounts.go | 3 ++- fvm/state/accounts_test.go | 2 +- fvm/transactionInvoker_test.go | 16 ++++++++++++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/cmd/util/ledger/migrations/storage_used_update_migration_test.go b/cmd/util/ledger/migrations/storage_used_update_migration_test.go index dab5d354ad6..3fa28c23237 100644 --- a/cmd/util/ledger/migrations/storage_used_update_migration_test.go +++ b/cmd/util/ledger/migrations/storage_used_update_migration_test.go @@ -35,7 +35,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(62), migratedSize) + require.Equal(t, uint64(52), migratedSize) }) t.Run("fix storage used if used to high", func(t *testing.T) { @@ -50,7 +50,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(62), migratedSize) + require.Equal(t, uint64(52), migratedSize) }) t.Run("do not fix storage used if storage used ok", func(t *testing.T) { @@ -65,7 +65,7 @@ func TestStorageUsedUpdateMigrationMigration(t *testing.T) { require.NoError(t, err) require.Equal(t, len(migratedPayload), len(payload)) - require.Equal(t, uint64(62), migratedSize) + require.Equal(t, uint64(52), migratedSize) }) t.Run("error is storage used does not exist", func(t *testing.T) { diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index c2b6921ba18..8a6259fd9cb 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -17,7 +17,8 @@ import ( ) const ( - KeyAccountStatus = "account_status" + AccountKeyPrefix = "a." + KeyAccountStatus = AccountKeyPrefix + "s" KeyPublicKeyCount = "public_key_count" KeyCode = "code" KeyContractNames = "contract_names" diff --git a/fvm/state/accounts_test.go b/fvm/state/accounts_test.go index f5ff59ed8eb..4504f3d9b9d 100644 --- a/fvm/state/accounts_test.go +++ b/fvm/state/accounts_test.go @@ -270,7 +270,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62), storageUsed) + require.Equal(t, uint64(52), storageUsed) }) t.Run("Storage used on register set increases", func(t *testing.T) { diff --git a/fvm/transactionInvoker_test.go b/fvm/transactionInvoker_test.go index c80cc05c26f..d0a50e04c34 100644 --- a/fvm/transactionInvoker_test.go +++ b/fvm/transactionInvoker_test.go @@ -65,6 +65,14 @@ func TestSafetyCheck(t *testing.T) { encodedName, err := encodeContractNames([]string{"TestContract"}) require.NoError(t, err) + err = view.Set( + string(contractAddress.Bytes()), + "", + state.KeyAccountStatus, + []byte{1}, + ) + require.NoError(t, err) + err = view.Set( string(contractAddress.Bytes()), string(contractAddress.Bytes()), @@ -72,6 +80,7 @@ func TestSafetyCheck(t *testing.T) { encodedName, ) require.NoError(t, err) + err = view.Set( string(contractAddress.Bytes()), string(contractAddress.Bytes()), @@ -135,6 +144,13 @@ func TestSafetyCheck(t *testing.T) { encodedName, err := encodeContractNames([]string{"TestContract"}) require.NoError(t, err) + err = view.Set( + string(contractAddress.Bytes()), + "", + state.KeyAccountStatus, + []byte{1}, + ) + require.NoError(t, err) err = view.Set( string(contractAddress.Bytes()), string(contractAddress.Bytes()), From a0cda92aaf53ed88fdd78475d7da9299f8ca00e8 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Fri, 3 Jun 2022 16:42:40 -0700 Subject: [PATCH 14/19] fix linter --- engine/execution/computation/computer/computer_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/engine/execution/computation/computer/computer_test.go b/engine/execution/computation/computer/computer_test.go index 15155dea3ac..2fc605382f1 100644 --- a/engine/execution/computation/computer/computer_test.go +++ b/engine/execution/computation/computer/computer_test.go @@ -408,7 +408,8 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { return nil, nil }) - view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + err = view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + require.NoError(t, err) result, err := exe.ExecuteBlock(context.Background(), block, view, programs.NewEmptyPrograms()) assert.NoError(t, err) @@ -482,7 +483,8 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { return nil, nil }) - view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + err = view.Set(string(address.Bytes()), "", state.KeyAccountStatus, []byte{1}) + require.NoError(t, err) result, err := exe.ExecuteBlock(context.Background(), block, view, programs.NewEmptyPrograms()) require.NoError(t, err) From 80005ad7f497fc138b379cff3fdabb51eac0d078 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Fri, 3 Jun 2022 17:06:47 -0700 Subject: [PATCH 15/19] update tests --- engine/execution/state/bootstrap/bootstrap_test.go | 2 +- fvm/accounts_test.go | 2 +- fvm/state/accounts_test.go | 12 ++++++------ utils/unittest/execution_state.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/engine/execution/state/bootstrap/bootstrap_test.go b/engine/execution/state/bootstrap/bootstrap_test.go index 96356f204c7..933004c7687 100644 --- a/engine/execution/state/bootstrap/bootstrap_test.go +++ b/engine/execution/state/bootstrap/bootstrap_test.go @@ -47,7 +47,7 @@ func TestBootstrapLedger(t *testing.T) { } func TestBootstrapLedger_ZeroTokenSupply(t *testing.T) { - expectedStateCommitmentBytes, _ := hex.DecodeString("5c59bcf9dd6b91ec41a50ba7a16c6069bb5add83615511e50d708be0334f0fd2") + expectedStateCommitmentBytes, _ := hex.DecodeString("09a41556866f7756a6a348e4fbf0c585b8f43870dfab30e98abdbb5635c0416b") expectedStateCommitment, err := flow.ToStateCommitment(expectedStateCommitmentBytes) require.NoError(t, err) diff --git a/fvm/accounts_test.go b/fvm/accounts_test.go index 523d69ffe35..a4a77f28da5 100644 --- a/fvm/accounts_test.go +++ b/fvm/accounts_test.go @@ -1284,7 +1284,7 @@ func TestAccountBalanceFields(t *testing.T) { assert.NoError(t, err) assert.NoError(t, script.Err) - assert.Equal(t, cadence.UFix64(9999_2450), script.Value) + assert.Equal(t, cadence.UFix64(9999_2550), script.Value) }), ) diff --git a/fvm/state/accounts_test.go b/fvm/state/accounts_test.go index 4504f3d9b9d..54d11b9fffb 100644 --- a/fvm/state/accounts_test.go +++ b/fvm/state/accounts_test.go @@ -287,7 +287,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+34), storageUsed) + require.Equal(t, uint64(52+34), storageUsed) }) t.Run("Storage used, set twice on same register to same value, stays the same", func(t *testing.T) { @@ -306,7 +306,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+34), storageUsed) + require.Equal(t, uint64(52+34), storageUsed) }) t.Run("Storage used, set twice on same register to larger value, increases", func(t *testing.T) { @@ -325,7 +325,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+35), storageUsed) + require.Equal(t, uint64(52+35), storageUsed) }) t.Run("Storage used, set twice on same register to smaller value, decreases", func(t *testing.T) { @@ -344,7 +344,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+33), storageUsed) + require.Equal(t, uint64(52+33), storageUsed) }) t.Run("Storage used, after register deleted, decreases", func(t *testing.T) { @@ -363,7 +363,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+0), storageUsed) + require.Equal(t, uint64(52+0), storageUsed) }) t.Run("Storage used on a complex scenario has correct value", func(t *testing.T) { @@ -392,7 +392,7 @@ func TestAccount_StorageUsed(t *testing.T) { storageUsed, err := accounts.GetStorageUsed(address) require.NoError(t, err) - require.Equal(t, uint64(62+33+46), storageUsed) + require.Equal(t, uint64(52+33+46), storageUsed) }) } diff --git a/utils/unittest/execution_state.go b/utils/unittest/execution_state.go index 69ce9657475..512ecef31b3 100644 --- a/utils/unittest/execution_state.go +++ b/utils/unittest/execution_state.go @@ -24,7 +24,7 @@ const ServiceAccountPrivateKeySignAlgo = crypto.ECDSAP256 const ServiceAccountPrivateKeyHashAlgo = hash.SHA2_256 // Pre-calculated state commitment with root account with the above private key -const GenesisStateCommitmentHex = "82d5e109f74c336325a270d180cd760f625665ee3675f36659ba9b4e2d7ab9ce" +const GenesisStateCommitmentHex = "632e3a7d35c1b70c66bf9669ae691f866fd81989bbc0f8cfeb93bf4072f0cc7a" var GenesisStateCommitment flow.StateCommitment From 055be6225595507be3167d80f8ed3995f2b6e4fc Mon Sep 17 00:00:00 2001 From: ramtinms Date: Fri, 3 Jun 2022 17:09:51 -0700 Subject: [PATCH 16/19] update storage used after migration --- .../cmd/execution-state-extract/execution_state_extract.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/util/cmd/execution-state-extract/execution_state_extract.go b/cmd/util/cmd/execution-state-extract/execution_state_extract.go index 961b7d667b8..540038156a4 100644 --- a/cmd/util/cmd/execution-state-extract/execution_state_extract.go +++ b/cmd/util/cmd/execution-state-extract/execution_state_extract.go @@ -63,12 +63,18 @@ func extractExecutionState( newState := ledger.State(targetHash) if migrate { + storageUsedUpdateMigration := mgr.StorageUsedUpdateMigration{ + Log: log, + OutputDir: outputDir, + } + accountStatusMigration := mgr.AccountStatusMigration{ Logger: log, } migrations = []ledger.Migration{ accountStatusMigration.Migrate, + storageUsedUpdateMigration.Migrate, mgr.PruneMigration, } From 964cbf4bf94a664e863a41d54e8eca4517ffe2de Mon Sep 17 00:00:00 2001 From: Maks Pawlak <120831+m4ksio@users.noreply.github.com> Date: Wed, 22 Jun 2022 21:22:17 -0700 Subject: [PATCH 17/19] Ensure transaction executed successfully --- engine/execution/computation/programs_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/engine/execution/computation/programs_test.go b/engine/execution/computation/programs_test.go index 0a27a788e3a..4c3676538e2 100644 --- a/engine/execution/computation/programs_test.go +++ b/engine/execution/computation/programs_test.go @@ -407,6 +407,11 @@ func createTestBlockAndRun(t *testing.T, engine *Manager, parentBlock *flow.Bloc } returnedComputationResult, err := engine.ComputeBlock(context.Background(), executableBlock, view) require.NoError(t, err) + + for _, txResult := range returnedComputationResult.TransactionResults { + require.Empty(t, txResult.ErrorMessage) + } + return block, returnedComputationResult } From 36fb7f2c44389eea6e2a3452405d023146c2a497 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Fri, 24 Jun 2022 09:58:03 -0700 Subject: [PATCH 18/19] comment out the regression part for now --- fvm/state/accounts.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fvm/state/accounts.go b/fvm/state/accounts.go index 8a6259fd9cb..e21cfebd77b 100644 --- a/fvm/state/accounts.go +++ b/fvm/state/accounts.go @@ -626,10 +626,11 @@ func (a *StatefulAccounts) GetAccountFrozen(address flow.Address) (bool, error) if err != nil { return false, err } - // if account does not exist, frozen is not meaningful - if !accStatus.AccountExists() { - return false, errors.NewAccountNotFoundError(address) - } + // TODO introduce this logic later + // // if account does not exist, frozen is not meaningful + // if !accStatus.AccountExists() { + // return false, errors.NewAccountNotFoundError(address) + // } return accStatus.IsAccountFrozen(), nil } From 228f28097e044e2cf9bb770c696749c3a8d9f083 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Mon, 27 Jun 2022 11:30:47 -0700 Subject: [PATCH 19/19] update test to support Epoch Tx --- engine/execution/computation/programs_test.go | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/engine/execution/computation/programs_test.go b/engine/execution/computation/programs_test.go index d79fc020c98..3b47dd1d955 100644 --- a/engine/execution/computation/programs_test.go +++ b/engine/execution/computation/programs_test.go @@ -2,7 +2,9 @@ package computation import ( "context" + "fmt" "testing" + "time" "github.com/onflow/cadence" jsoncdc "github.com/onflow/cadence/encoding/json" @@ -146,6 +148,18 @@ func TestPrograms_TestContractUpdates(t *testing.T) { hasValidEventValue(t, returnedComputationResult.Events[0][4], 2) } +type blockProvider struct { + blocks map[uint64]*flow.Block +} + +func (b blockProvider) ByHeightFrom(height uint64, _ *flow.Header) (*flow.Header, error) { + block, has := b.blocks[height] + if has { + return block.Header, nil + } + return nil, fmt.Errorf("block for height (%d) is not available", height) +} + // TestPrograms_TestBlockForks tests the functionality of // programsCache under contract deployment and contract updates on // different block forks @@ -161,11 +175,14 @@ func TestPrograms_TestContractUpdates(t *testing.T) { // -> Block121 (emit event - version should be 2) // -> Block1211 (emit event - version should be 2) func TestPrograms_TestBlockForks(t *testing.T) { - // setup + block := unittest.BlockFixture() rt := fvm.NewInterpreterRuntime() - chain := flow.Mainnet.Chain() + chain := flow.Emulator.Chain() vm := fvm.NewVirtualMachine(rt) - execCtx := fvm.NewContext(zerolog.Nop(), fvm.WithChain(chain)) + execCtx := fvm.NewContext(zerolog.Nop(), + fvm.WithBlockHeader(block.Header), + fvm.WithBlocks(blockProvider{map[uint64]*flow.Block{0: &block}}), + fvm.WithChain(chain)) privateKeys, err := testutil.GenerateAccountPrivateKeys(1) require.NoError(t, err) @@ -389,8 +406,9 @@ func createTestBlockAndRun(t *testing.T, engine *Manager, parentBlock *flow.Bloc block := &flow.Block{ Header: &flow.Header{ - ParentID: parentBlock.ID(), - View: parentBlock.Header.Height + 1, + ParentID: parentBlock.ID(), + View: parentBlock.Header.Height + 1, + Timestamp: time.Now(), }, Payload: &flow.Payload{ Guarantees: []*flow.CollectionGuarantee{&guarantee},