Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FVM] reducing register touches #2502

Merged
merged 35 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
05e96a6
add account status
ramtinms May 26, 2022
33a5e1a
remove old migration codes
ramtinms May 26, 2022
fb2cb71
add account status migration
ramtinms May 26, 2022
0db4cd0
remove onhold status
ramtinms May 26, 2022
3ee8b00
no need for account freeze enabler anymore
ramtinms May 26, 2022
001695e
account freezing disabled by the context
ramtinms May 26, 2022
64a43b1
update migrations and remove depricated keys
ramtinms May 26, 2022
a85a535
merge FrozenChecker into TxVerifier
ramtinms May 26, 2022
8ba25f1
optimize get contract to load less register on happy path
ramtinms May 26, 2022
9284151
remove unused methods
ramtinms May 27, 2022
480b19d
format imports
ramtinms May 30, 2022
58b7f5d
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms May 30, 2022
15a2be8
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms May 30, 2022
aceb91a
update tests
ramtinms Jun 2, 2022
8295c41
reduce key prefix size
ramtinms Jun 2, 2022
a0cda92
fix linter
ramtinms Jun 3, 2022
80005ad
update tests
ramtinms Jun 4, 2022
055be62
update storage used after migration
ramtinms Jun 4, 2022
5f63bc3
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 4, 2022
5c1d879
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 7, 2022
6672c31
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 9, 2022
d9fd0a1
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 21, 2022
65dc33f
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 21, 2022
27e2cec
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 21, 2022
c38f3e9
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 22, 2022
70cbbf8
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 22, 2022
e412b5d
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 22, 2022
eea22d7
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 22, 2022
c53cf6c
Merge branch 'master' into ramtin/2492-revamp-account-registers
m4ksio Jun 23, 2022
964cbf4
Ensure transaction executed successfully
m4ksio Jun 23, 2022
36fb7f2
comment out the regression part for now
ramtinms Jun 24, 2022
2b25ed2
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 24, 2022
228f280
update test to support Epoch Tx
ramtinms Jun 27, 2022
9d429be
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 27, 2022
02c16e8
Merge branch 'master' into ramtin/2492-revamp-account-registers
ramtinms Jun 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ func extractExecutionState(
OutputDir: outputDir,
}

accountStatusMigration := mgr.AccountStatusMigration{
Logger: log,
}

migrations = []ledger.Migration{
accountStatusMigration.Migrate,
storageUsedUpdateMigration.Migrate,
ramtinms marked this conversation as resolved.
Show resolved Hide resolved
mgr.PruneMigration,
}
Expand Down
50 changes: 50 additions & 0 deletions cmd/util/ledger/migrations/account_status_migration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package migrations

import (
"encoding/hex"

"github.com/rs/zerolog"

"github.com/onflow/flow-go/fvm/state"
"github.com/onflow/flow-go/ledger"
)

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
// 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) == 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) == 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
}
55 changes: 55 additions & 0 deletions cmd/util/ledger/migrations/account_status_migration_test.go
Original file line number Diff line number Diff line change
@@ -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, KeyExists), Value: []byte{1}},
{Key: createAccountPayloadKey(address1, 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))
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -35,12 +35,12 @@ 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(52), migratedSize)
})

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)
Expand All @@ -50,12 +50,12 @@ 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(52), migratedSize)
})

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)
Expand All @@ -65,12 +65,12 @@ 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(52), migratedSize)
})

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)
Expand Down
19 changes: 14 additions & 5 deletions engine/execution/computation/computer/computer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}

Expand Down Expand Up @@ -407,6 +408,9 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) {
return nil, nil
})

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)
assert.Len(t, result.StateSnapshots, collectionCount+1) // +1 system chunk
Expand All @@ -423,8 +427,10 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) {
),
)

address := common.Address{0x1}

contractLocation := common.AddressLocation{
Address: common.Address{0x1},
Address: address,
Name: "Test",
}

Expand Down Expand Up @@ -477,6 +483,9 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) {
return nil, nil
})

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)
assert.Len(t, result.StateSnapshots, collectionCount+1) // +1 system chunk
Expand Down Expand Up @@ -594,7 +603,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}
Expand Down Expand Up @@ -633,11 +642,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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
33 changes: 28 additions & 5 deletions engine/execution/computation/programs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package computation

import (
"context"
"fmt"
"testing"
"time"

"github.com/onflow/cadence"
jsoncdc "github.com/onflow/cadence/encoding/json"
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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},
Expand All @@ -409,6 +427,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
}

Expand Down
2 changes: 1 addition & 1 deletion engine/execution/state/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestBootstrapLedger(t *testing.T) {
}

func TestBootstrapLedger_ZeroTokenSupply(t *testing.T) {
expectedStateCommitmentBytes, _ := hex.DecodeString("cae2f2d6c53582503dad30dba8a8bba098ff8654d19b820a49e1961c1f459a41")
expectedStateCommitmentBytes, _ := hex.DecodeString("09a41556866f7756a6a348e4fbf0c585b8f43870dfab30e98abdbb5635c0416b")
expectedStateCommitment, err := flow.ToStateCommitment(expectedStateCommitmentBytes)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion fvm/accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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_2550), script.Value)
}),
)

Expand Down
14 changes: 6 additions & 8 deletions fvm/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Context struct {
CadenceLoggingEnabled bool
EventCollectionEnabled bool
ServiceEventCollectionEnabled bool
AccountFreezeAvailable bool
AccountFreezeEnabled bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed it to match other ones.

ExtensiveTracing bool
TransactionProcessors []TransactionProcessor
ScriptProcessors []ScriptProcessor
Expand Down Expand Up @@ -89,13 +89,11 @@ func defaultContext(logger zerolog.Logger) Context {
CadenceLoggingEnabled: false,
EventCollectionEnabled: true,
ServiceEventCollectionEnabled: false,
AccountFreezeAvailable: false,
AccountFreezeEnabled: true,
janezpodhostnik marked this conversation as resolved.
Show resolved Hide resolved
ExtensiveTracing: false,
TransactionProcessors: []TransactionProcessor{
NewTransactionAccountFrozenChecker(),
NewTransactionSignatureVerifier(AccountKeyWeightThreshold),
NewTransactionVerifier(AccountKeyWeightThreshold),
NewTransactionSequenceNumberChecker(),
NewTransactionAccountFrozenEnabler(),
NewTransactionInvoker(logger),
},
ScriptProcessors: []ScriptProcessor{
Expand Down Expand Up @@ -185,12 +183,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
}
}
Expand Down
2 changes: 0 additions & 2 deletions fvm/fvm_blockcontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1102,8 +1102,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.NewTransactionAccountFrozenEnabler(),
fvm.NewTransactionInvoker(zerolog.Nop()),
),
).
Expand Down
Loading