From bf541979a20dd8e0f4718c14e66a6a663ba9ef54 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 20 Oct 2023 16:06:35 +0200 Subject: [PATCH 01/33] add test for getting newly created user --- integration/tests/access/access_api_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index a252c86ab2a..1979b7df3dd 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,20 +5,19 @@ import ( "testing" "time" - "github.com/rs/zerolog" - "github.com/stretchr/testify/suite" - "github.com/onflow/cadence" - sdk "github.com/onflow/flow-go-sdk" client "github.com/onflow/flow-go-sdk/access/grpc" - "github.com/onflow/flow-go/engine/access/rpc/backend" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" + "github.com/rs/zerolog" + "github.com/stretchr/testify/suite" + "github.com/onflow/flow-go/integration/testnet" "github.com/onflow/flow-go/integration/tests/lib" "github.com/onflow/flow-go/integration/tests/mvp" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/utils/unittest" + "github.com/onflow/flow-go/integration/utils" ) // This is a collection of tests that validate various Access API endpoints work as expected. @@ -193,6 +192,14 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { s.Assert().Equal(serviceAddress, account.Address) s.Assert().NotZero(serviceAddress, account.Balance) }) + + s.Run("get newly created account", func() { + addr, err := utils.CreateFlowAccount(s.ctx, s.serviceClient) + s.Require().NoError(err) + acc, err := client.GetAccount(s.ctx, addr) + s.Require().NoError(err) + s.Assert().Equal(addr, acc.Address) + }) } func (s *AccessAPISuite) testExecuteScriptWithSimpleScript(client *client.Client) { From 7485fe5667ff1b58912f8342592029e6ff85beb4 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 20 Oct 2023 18:31:32 +0200 Subject: [PATCH 02/33] add create account test --- module/execution/scripts_test.go | 84 +++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 0bcbb8b02d7..6d3c2d33aa2 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/onflow/cadence" + "github.com/onflow/cadence/encoding/ccf" jsoncdc "github.com/onflow/cadence/encoding/json" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" @@ -34,7 +35,7 @@ func Test_ExecuteScript(t *testing.T) { t.Run("Simple Script Execution", func(t *testing.T) { blockchain := unittest.BlockchainFixture(10) first := blockchain[0] - tree := bootstrapFVM() + tree := bootstrapFVM(t) scripts := newScripts( t, @@ -76,7 +77,7 @@ func Test_ExecuteScript(t *testing.T) { func Test_GetAccount(t *testing.T) { blockchain := unittest.BlockchainFixture(10) first := blockchain[0] - tree := bootstrapFVM() + tree := bootstrapFVM(t) scripts := newScripts( t, @@ -92,6 +93,26 @@ func Test_GetAccount(t *testing.T) { assert.NotZero(t, len(account.Contracts)) } +func Test_NewlyCreatedAccount(t *testing.T) { + blockchain := unittest.BlockchainFixture(10) + first := blockchain[0] + tree := bootstrapFVM(t) + tree, newAddress := createAccount(t, tree) + + scripts := newScripts( + t, + newBlockHeadersStorage(blockchain), + treeToRegisterAdapter(tree), + ) + + address := chain.ServiceAddress() + account, err := scripts.GetAccountAtBlockHeight(context.Background(), newAddress, first.Header.Height) + require.NoError(t, err) + assert.Equal(t, address, account.Address) + assert.NotZero(t, account.Balance) + assert.NotZero(t, len(account.Contracts)) +} + func newScripts( t *testing.T, headers storage.Headers, @@ -121,11 +142,8 @@ func newBlockHeadersStorage(blocks []*flow.Block) storage.Headers { } // bootstrapFVM starts up an FVM and run bootstrap procedures and returns the snapshot tree of the state. -func bootstrapFVM() snapshot.SnapshotTree { - opts := []fvm.Option{ - fvm.WithChain(chain), - } - ctx := fvm.NewContext(opts...) +func bootstrapFVM(t *testing.T) snapshot.SnapshotTree { + ctx := fvm.NewContext(fvm.WithChain(chain)) vm := fvm.NewVirtualMachine() snapshotTree := snapshot.NewSnapshotTree(nil) @@ -134,14 +152,64 @@ func bootstrapFVM() snapshot.SnapshotTree { fvm.WithInitialTokenSupply(unittest.GenesisTokenSupply), } - executionSnapshot, _, _ := vm.Run( + executionSnapshot, out, err := vm.Run( ctx, fvm.Bootstrap(unittest.ServiceAccountPublicKey, bootstrapOpts...), snapshotTree) + require.NoError(t, err) + require.NoError(t, out.Err) + return snapshotTree.Append(executionSnapshot) } +// createAccount on an existing bootstrapped snapshot and return a new snapshot as well as the newly created account address +func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotTree, flow.Address) { + const createAccountTransaction = ` + transaction { + prepare(signer: AuthAccount) { + let account = AuthAccount(payer: signer) + } + } + ` + + ctx := fvm.NewContext( + fvm.WithAuthorizationChecksEnabled(false), + fvm.WithSequenceNumberCheckAndIncrementEnabled(false), + ) + vm := fvm.NewVirtualMachine() + + txBody := flow.NewTransactionBody(). + SetScript([]byte(createAccountTransaction)). + AddAuthorizer(chain.ServiceAddress()) + + executionSnapshot, output, err := vm.Run( + ctx, + fvm.Transaction(txBody, 0), + tree, + ) + require.NoError(t, err) + require.NoError(t, output.Err) + + tree = tree.Append(executionSnapshot) + + var accountCreatedEvents []flow.Event + for _, event := range output.Events { + if event.Type != flow.EventAccountCreated { + continue + } + accountCreatedEvents = append(accountCreatedEvents, event) + break + } + require.Len(t, accountCreatedEvents, 1) + + data, err := ccf.Decode(nil, accountCreatedEvents[0].Payload) + require.NoError(t, err) + address := flow.ConvertAddress(data.(cadence.Event).Fields[0].(cadence.Address)) + + return tree, address +} + // converts tree get register function to the required script get register function func treeToRegisterAdapter(tree snapshot.SnapshotTree) func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { return func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { From 9daae1b16d35a8405069b3551549b728b3e3bca2 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 20 Oct 2023 19:38:51 +0200 Subject: [PATCH 03/33] validate an issue in integration test --- module/execution/scripts_test.go | 45 ++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 6d3c2d33aa2..8ec2656dbdc 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -23,6 +23,7 @@ import ( synctest "github.com/onflow/flow-go/module/state_synchronization/requester/unittest" "github.com/onflow/flow-go/module/trace" "github.com/onflow/flow-go/storage" + pebbleStorage "github.com/onflow/flow-go/storage/pebble" "github.com/onflow/flow-go/utils/unittest" ) @@ -97,7 +98,7 @@ func Test_NewlyCreatedAccount(t *testing.T) { blockchain := unittest.BlockchainFixture(10) first := blockchain[0] tree := bootstrapFVM(t) - tree, newAddress := createAccount(t, tree) + tree, _, newAddress := createAccount(t, tree) scripts := newScripts( t, @@ -105,12 +106,39 @@ func Test_NewlyCreatedAccount(t *testing.T) { treeToRegisterAdapter(tree), ) - address := chain.ServiceAddress() account, err := scripts.GetAccountAtBlockHeight(context.Background(), newAddress, first.Header.Height) require.NoError(t, err) - assert.Equal(t, address, account.Address) - assert.NotZero(t, account.Balance) - assert.NotZero(t, len(account.Contracts)) + assert.Equal(t, newAddress, account.Address) +} + +func Test_IntegrationStorage(t *testing.T) { + entropyProvider := testutil.EntropyProviderFixture(nil) + entropyBlock := mock.NewEntropyProviderPerBlock(t) + blockchain := unittest.BlockchainFixture(10) + headers := newBlockHeadersStorage(blockchain) + + pebbleStorage.RunWithRegistersStorageAtInitialHeights(t, 0, blockchain[0].Header.Height, func(registers *pebbleStorage.Registers) { + entropyBlock. + On("AtBlockID", mocks.AnythingOfType("flow.Identifier")). + Return(entropyProvider). + Maybe() + + snap := bootstrapFVM(t) + snap, exeSnap, newAddress := createAccount(t, snap) + + newHeight := blockchain[1].Header.Height + err := registers.Store(exeSnap.UpdatedRegisters(), newHeight) + require.NoError(t, err) + + scripts, err := NewScripts(zerolog.Nop(), collector, flow.Emulator, entropyBlock, headers, func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { + return registers.Get(ID, height) + }) + require.NoError(t, err) + + acc, err := scripts.GetAccountAtBlockHeight(context.Background(), newAddress, newHeight) + require.NoError(t, err) + assert.Equal(t, acc.Address, newAddress) + }) } func newScripts( @@ -163,8 +191,8 @@ func bootstrapFVM(t *testing.T) snapshot.SnapshotTree { return snapshotTree.Append(executionSnapshot) } -// createAccount on an existing bootstrapped snapshot and return a new snapshot as well as the newly created account address -func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotTree, flow.Address) { +// createAccount on an existing bootstrapped snapshot tree, execution snapshot and return a new snapshot as well as the newly created account address +func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotTree, *snapshot.ExecutionSnapshot, flow.Address) { const createAccountTransaction = ` transaction { prepare(signer: AuthAccount) { @@ -174,6 +202,7 @@ func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotT ` ctx := fvm.NewContext( + fvm.WithChain(chain), fvm.WithAuthorizationChecksEnabled(false), fvm.WithSequenceNumberCheckAndIncrementEnabled(false), ) @@ -207,7 +236,7 @@ func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotT require.NoError(t, err) address := flow.ConvertAddress(data.(cadence.Event).Fields[0].(cadence.Address)) - return tree, address + return tree, executionSnapshot, address } // converts tree get register function to the required script get register function From a23665d3a84c81f1d551f09f1369497ebe1080f2 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 20 Oct 2023 19:44:14 +0200 Subject: [PATCH 04/33] change register values handling of nil values --- module/state_synchronization/indexer/indexer_core.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/module/state_synchronization/indexer/indexer_core.go b/module/state_synchronization/indexer/indexer_core.go index c30297ac91d..660aef867df 100644 --- a/module/state_synchronization/indexer/indexer_core.go +++ b/module/state_synchronization/indexer/indexer_core.go @@ -1,6 +1,7 @@ package indexer import ( + "errors" "fmt" "time" @@ -62,15 +63,18 @@ func New( // RegisterValues retrieves register values by the register IDs at the provided block height. // Even if the register wasn't indexed at the provided height, returns the highest height the register was indexed at. +// If a register is not found it will return a nil value and not an error. // Expected errors: -// - storage.ErrNotFound if the register by the ID was never indexed // - storage.ErrHeightNotIndexed if the given height was not indexed yet or lower than the first indexed height. func (c *IndexerCore) RegisterValues(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { values := make([]flow.RegisterValue, len(IDs)) for j, id := range IDs { value, err := c.registers.Get(id, height) - if err != nil { + // only return an error if the error doesn't match the not found error, since we have + // to gracefully handle not found values and instead assign nil, that is because the script executor + // expects that behaviour + if err != nil && !errors.Is(err, storage.ErrNotFound) { return nil, err } From bf1daa0c40568b1b33744c08fc0cc28fd8a5273e Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 16:54:16 +0200 Subject: [PATCH 05/33] change the type for metrics --- module/execution/scripts.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/module/execution/scripts.go b/module/execution/scripts.go index b8157f6cd86..8d75e360c2b 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -13,7 +13,7 @@ import ( "github.com/onflow/flow-go/fvm/storage/derived" "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/module/metrics" + "github.com/onflow/flow-go/module" "github.com/onflow/flow-go/storage" ) @@ -25,8 +25,8 @@ var ErrDataNotAvailable = errors.New("data for block is not available") // RegistersAtHeight returns register value for provided register ID at the block height. // Even if the register wasn't indexed at the provided height, returns the highest height the register was indexed at. +// If the register with the ID was not indexed at all return nil value and no error. // Expected errors: -// - storage.ErrNotFound if the register by the ID was never indexed // - storage.ErrHeightNotIndexed if the given height was not indexed yet or lower than the first indexed height. type RegistersAtHeight func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) @@ -61,7 +61,7 @@ type Scripts struct { func NewScripts( log zerolog.Logger, - metrics *metrics.ExecutionCollector, + metrics module.ExecutionMetrics, chainID flow.ChainID, entropy query.EntropyProviderPerBlock, header storage.Headers, From 9beeb8c2d8f0fdd805929774563a90b1cacd7221 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 16:54:25 +0200 Subject: [PATCH 06/33] rewrite test suite --- module/execution/scripts_test.go | 272 ++++++++++++++----------------- 1 file changed, 118 insertions(+), 154 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 8ec2656dbdc..f0406917168 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -3,6 +3,7 @@ package execution import ( "context" "fmt" + "os" "strings" "testing" @@ -10,9 +11,8 @@ import ( "github.com/onflow/cadence/encoding/ccf" jsoncdc "github.com/onflow/cadence/encoding/json" "github.com/rs/zerolog" - "github.com/stretchr/testify/assert" mocks "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" "github.com/onflow/flow-go/engine/execution/computation/query/mock" "github.com/onflow/flow-go/engine/execution/testutil" @@ -20,207 +20,169 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/metrics" + "github.com/onflow/flow-go/module/state_synchronization/indexer" synctest "github.com/onflow/flow-go/module/state_synchronization/requester/unittest" - "github.com/onflow/flow-go/module/trace" "github.com/onflow/flow-go/storage" pebbleStorage "github.com/onflow/flow-go/storage/pebble" "github.com/onflow/flow-go/utils/unittest" ) -var ( - chain = flow.Emulator.Chain() - collector = metrics.NewExecutionCollector(&trace.NoopTracer{}) -) - -func Test_ExecuteScript(t *testing.T) { - t.Run("Simple Script Execution", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM(t) +func TestScripts(t *testing.T) { + suite.Run(t, new(scriptTestSuite)) +} - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) +type scriptTestSuite struct { + suite.Suite + scripts *Scripts + registerIndex storage.RegisterIndex + vm *fvm.VirtualMachine + vmCtx fvm.Context + chain flow.Chain + height uint64 + snapshot snapshot.SnapshotTree + dbDir string +} +func (s *scriptTestSuite) TestScriptExecution() { + s.Run("Simple Script Execution", func() { number := int64(42) code := []byte(fmt.Sprintf("pub fun main(): Int { return %d; }", number)) - result, err := scripts.ExecuteAtBlockHeight(context.Background(), code, nil, first.Header.Height) - require.NoError(t, err) - value, err := jsoncdc.Decode(nil, result) - require.NoError(t, err) - assert.Equal(t, number, value.(cadence.Int).Value.Int64()) + result, err := s.scripts.ExecuteAtBlockHeight(context.Background(), code, nil, s.height) + s.Require().NoError(err) + val, err := jsoncdc.Decode(nil, result) + s.Require().NoError(err) + s.Assert().Equal(number, val.(cadence.Int).Value.Int64()) }) - t.Run("Handle not found Register", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - IndexRegisterAdapter( - func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { - return nil, nil // intentionally return nil to check edge case - }), + s.Run("Handle Not Found Register", func() { + s.scripts.registersAtHeight = IndexRegisterAdapter( // using the adapter checks the error handling in the adapter + func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { + return nil, nil // intentionally return nil to check edge case + }, ) // use a non-existing address to trigger register get function code := []byte("import Foo from 0x01; pub fun main() { }") - result, err := scripts.ExecuteAtBlockHeight(context.Background(), code, nil, first.Header.Height) - require.True(t, strings.Contains(err.Error(), "invalid number of returned values for a single register")) - require.Nil(t, result) + result, err := s.scripts.ExecuteAtBlockHeight(context.Background(), code, nil, s.height) + s.Assert().True(strings.Contains(err.Error(), "invalid number of returned values for a single register")) + s.Assert().Nil(result) }) } -func Test_GetAccount(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM(t) - - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) - - address := chain.ServiceAddress() - account, err := scripts.GetAccountAtBlockHeight(context.Background(), address, first.Header.Height) - require.NoError(t, err) - assert.Equal(t, address, account.Address) - assert.NotZero(t, account.Balance) - assert.NotZero(t, len(account.Contracts)) -} - -func Test_NewlyCreatedAccount(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM(t) - tree, _, newAddress := createAccount(t, tree) - - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) +func (s *scriptTestSuite) TestGetAccount() { + s.Run("Get Service Account", func() { + address := s.chain.ServiceAddress() + account, err := s.scripts.GetAccountAtBlockHeight(context.Background(), address, s.height) + s.Require().NoError(err) + s.Assert().Equal(address, account.Address) + s.Assert().NotZero(account.Balance) + s.Assert().NotZero(len(account.Contracts)) + }) - account, err := scripts.GetAccountAtBlockHeight(context.Background(), newAddress, first.Header.Height) - require.NoError(t, err) - assert.Equal(t, newAddress, account.Address) + s.Run("Get New Account", func() { + address := s.createAccount() + account, err := s.scripts.GetAccountAtBlockHeight(context.Background(), address, s.height) + s.Require().NoError(err) + s.Require().Equal(address, account.Address) + }) } -func Test_IntegrationStorage(t *testing.T) { +func (s *scriptTestSuite) SetupTest() { + logger := unittest.LoggerForTest(s.Suite.T(), zerolog.InfoLevel) entropyProvider := testutil.EntropyProviderFixture(nil) - entropyBlock := mock.NewEntropyProviderPerBlock(t) blockchain := unittest.BlockchainFixture(10) headers := newBlockHeadersStorage(blockchain) - pebbleStorage.RunWithRegistersStorageAtInitialHeights(t, 0, blockchain[0].Header.Height, func(registers *pebbleStorage.Registers) { - entropyBlock. - On("AtBlockID", mocks.AnythingOfType("flow.Identifier")). - Return(entropyProvider). - Maybe() - - snap := bootstrapFVM(t) - snap, exeSnap, newAddress := createAccount(t, snap) - - newHeight := blockchain[1].Header.Height - err := registers.Store(exeSnap.UpdatedRegisters(), newHeight) - require.NoError(t, err) - - scripts, err := NewScripts(zerolog.Nop(), collector, flow.Emulator, entropyBlock, headers, func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { - return registers.Get(ID, height) - }) - require.NoError(t, err) - - acc, err := scripts.GetAccountAtBlockHeight(context.Background(), newAddress, newHeight) - require.NoError(t, err) - assert.Equal(t, acc.Address, newAddress) - }) -} - -func newScripts( - t *testing.T, - headers storage.Headers, - registers func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error), -) *Scripts { - entropyProvider := testutil.EntropyProviderFixture(nil) - entropyBlock := mock.NewEntropyProviderPerBlock(t) + s.chain = flow.Emulator.Chain() + s.snapshot = snapshot.NewSnapshotTree(nil) + s.vm = fvm.NewVirtualMachine() + s.vmCtx = fvm.NewContext( + fvm.WithChain(s.chain), + fvm.WithAuthorizationChecksEnabled(false), + fvm.WithSequenceNumberCheckAndIncrementEnabled(false), + ) + s.height = blockchain[0].Header.Height + entropyBlock := mock.NewEntropyProviderPerBlock(s.T()) entropyBlock. On("AtBlockID", mocks.AnythingOfType("flow.Identifier")). Return(entropyProvider). Maybe() - scripts, err := NewScripts(zerolog.Nop(), collector, flow.Emulator, entropyBlock, headers, registers) - require.NoError(t, err) + s.dbDir = unittest.TempDir(s.T()) + db := pebbleStorage.NewBootstrappedRegistersWithPathForTest(s.T(), s.dbDir, s.height, s.height) + pebbleRegisters, err := pebbleStorage.NewRegisters(db) + s.Require().NoError(err) + s.registerIndex = pebbleRegisters + + index, err := indexer.New(logger, metrics.NewNoopCollector(), nil, s.registerIndex, headers, nil, nil) + s.Require().NoError(err) + + scripts, err := NewScripts( + logger, + metrics.NewNoopCollector(), + s.chain.ChainID(), + entropyBlock, + headers, + IndexRegisterAdapter(index.RegisterValues), + ) + s.Require().NoError(err) + s.scripts = scripts - return scripts + s.bootstrap() } -func newBlockHeadersStorage(blocks []*flow.Block) storage.Headers { - blocksByHeight := make(map[uint64]*flow.Block) - for _, b := range blocks { - blocksByHeight[b.Header.Height] = b - } - - return synctest.MockBlockHeaderStorage(synctest.WithByHeight(blocksByHeight)) +func (s *scriptTestSuite) TearDownTest() { + s.Require().NoError(os.RemoveAll(s.dbDir)) } -// bootstrapFVM starts up an FVM and run bootstrap procedures and returns the snapshot tree of the state. -func bootstrapFVM(t *testing.T) snapshot.SnapshotTree { - ctx := fvm.NewContext(fvm.WithChain(chain)) - vm := fvm.NewVirtualMachine() - - snapshotTree := snapshot.NewSnapshotTree(nil) - +func (s *scriptTestSuite) bootstrap() { bootstrapOpts := []fvm.BootstrapProcedureOption{ fvm.WithInitialTokenSupply(unittest.GenesisTokenSupply), } - executionSnapshot, out, err := vm.Run( - ctx, + executionSnapshot, out, err := s.vm.Run( + s.vmCtx, fvm.Bootstrap(unittest.ServiceAccountPublicKey, bootstrapOpts...), - snapshotTree) + s.snapshot) + + s.Require().NoError(err) + s.Require().NoError(out.Err) - require.NoError(t, err) - require.NoError(t, out.Err) + s.height++ + err = s.registerIndex.Store(executionSnapshot.UpdatedRegisters(), s.height) + s.Require().NoError(err) - return snapshotTree.Append(executionSnapshot) + s.snapshot = s.snapshot.Append(executionSnapshot) } -// createAccount on an existing bootstrapped snapshot tree, execution snapshot and return a new snapshot as well as the newly created account address -func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotTree, *snapshot.ExecutionSnapshot, flow.Address) { +func (s *scriptTestSuite) createAccount() flow.Address { const createAccountTransaction = ` - transaction { - prepare(signer: AuthAccount) { - let account = AuthAccount(payer: signer) - } - } - ` - - ctx := fvm.NewContext( - fvm.WithChain(chain), - fvm.WithAuthorizationChecksEnabled(false), - fvm.WithSequenceNumberCheckAndIncrementEnabled(false), - ) - vm := fvm.NewVirtualMachine() + transaction { + prepare(signer: AuthAccount) { + let account = AuthAccount(payer: signer) + } + }` txBody := flow.NewTransactionBody(). SetScript([]byte(createAccountTransaction)). - AddAuthorizer(chain.ServiceAddress()) + AddAuthorizer(s.chain.ServiceAddress()) - executionSnapshot, output, err := vm.Run( - ctx, + executionSnapshot, output, err := s.vm.Run( + s.vmCtx, fvm.Transaction(txBody, 0), - tree, + s.snapshot, ) - require.NoError(t, err) - require.NoError(t, output.Err) + s.Require().NoError(err) + s.Require().NoError(output.Err) + + s.height++ + err = s.registerIndex.Store(executionSnapshot.UpdatedRegisters(), s.height) + s.Require().NoError(err) - tree = tree.Append(executionSnapshot) + s.snapshot = s.snapshot.Append(executionSnapshot) var accountCreatedEvents []flow.Event for _, event := range output.Events { @@ -230,18 +192,20 @@ func createAccount(t *testing.T, tree snapshot.SnapshotTree) (snapshot.SnapshotT accountCreatedEvents = append(accountCreatedEvents, event) break } - require.Len(t, accountCreatedEvents, 1) + s.Require().Len(accountCreatedEvents, 1) data, err := ccf.Decode(nil, accountCreatedEvents[0].Payload) - require.NoError(t, err) + s.Require().NoError(err) address := flow.ConvertAddress(data.(cadence.Event).Fields[0].(cadence.Address)) - return tree, executionSnapshot, address + return address } -// converts tree get register function to the required script get register function -func treeToRegisterAdapter(tree snapshot.SnapshotTree) func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { - return func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { - return tree.Get(ID) +func newBlockHeadersStorage(blocks []*flow.Block) storage.Headers { + blocksByHeight := make(map[uint64]*flow.Block) + for _, b := range blocks { + blocksByHeight[b.Header.Height] = b } + + return synctest.MockBlockHeaderStorage(synctest.WithByHeight(blocksByHeight)) } From 433d0c5e027da88cc0421a6aa82a9c228b55624a Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 16:54:34 +0200 Subject: [PATCH 07/33] export needed function --- storage/pebble/testutil.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/pebble/testutil.go b/storage/pebble/testutil.go index a68e5c8b466..64bd5a71a21 100644 --- a/storage/pebble/testutil.go +++ b/storage/pebble/testutil.go @@ -11,7 +11,7 @@ import ( func RunWithRegistersStorageAtInitialHeights(tb testing.TB, first uint64, latest uint64, f func(r *Registers)) { unittest.RunWithTempDir(tb, func(dir string) { - db := newBootstrappedRegistersWithPathForTest(tb, dir, first, latest) + db := NewBootstrappedRegistersWithPathForTest(tb, dir, first, latest) r, err := NewRegisters(db) require.NoError(tb, err) @@ -21,7 +21,7 @@ func RunWithRegistersStorageAtInitialHeights(tb testing.TB, first uint64, latest }) } -func newBootstrappedRegistersWithPathForTest(tb testing.TB, dir string, first, latest uint64) *pebble.DB { +func NewBootstrappedRegistersWithPathForTest(tb testing.TB, dir string, first, latest uint64) *pebble.DB { db, err := OpenRegisterPebbleDB(dir) require.NoError(tb, err) From c60d708fc454e36fafb4b049e32b4c37fb8d272c Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 17:58:46 +0200 Subject: [PATCH 08/33] fix imports --- integration/tests/access/access_api_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 1979b7df3dd..7c637558d20 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,19 +5,19 @@ import ( "testing" "time" + "github.com/rs/zerolog" + "github.com/stretchr/testify/suite" + "github.com/onflow/cadence" sdk "github.com/onflow/flow-go-sdk" client "github.com/onflow/flow-go-sdk/access/grpc" "github.com/onflow/flow-go/engine/access/rpc/backend" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/utils/unittest" - "github.com/rs/zerolog" - "github.com/stretchr/testify/suite" - "github.com/onflow/flow-go/integration/testnet" "github.com/onflow/flow-go/integration/tests/lib" "github.com/onflow/flow-go/integration/tests/mvp" "github.com/onflow/flow-go/integration/utils" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) // This is a collection of tests that validate various Access API endpoints work as expected. From 9cdb00bd6b690fb9977d3e304338b439620562f5 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 17:59:14 +0200 Subject: [PATCH 09/33] fix imports --- integration/tests/access/access_api_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 7c637558d20..8b7b16b569a 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,19 +5,21 @@ import ( "testing" "time" - "github.com/rs/zerolog" - "github.com/stretchr/testify/suite" - "github.com/onflow/cadence" + sdk "github.com/onflow/flow-go-sdk" client "github.com/onflow/flow-go-sdk/access/grpc" + "github.com/onflow/flow-go/engine/access/rpc/backend" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" + "github.com/rs/zerolog" + "github.com/stretchr/testify/suite" + "github.com/onflow/flow-go/integration/testnet" "github.com/onflow/flow-go/integration/tests/lib" "github.com/onflow/flow-go/integration/tests/mvp" "github.com/onflow/flow-go/integration/utils" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/utils/unittest" ) // This is a collection of tests that validate various Access API endpoints work as expected. From 11bcfa400b5680a130a6d651f80704ef50224847 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 23 Oct 2023 18:00:37 +0200 Subject: [PATCH 10/33] fix imports --- integration/tests/access/access_api_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 8b7b16b569a..6fa958c3946 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,21 +5,21 @@ import ( "testing" "time" + "github.com/rs/zerolog" + "github.com/stretchr/testify/suite" + "github.com/onflow/cadence" sdk "github.com/onflow/flow-go-sdk" client "github.com/onflow/flow-go-sdk/access/grpc" "github.com/onflow/flow-go/engine/access/rpc/backend" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/utils/unittest" - "github.com/rs/zerolog" - "github.com/stretchr/testify/suite" - "github.com/onflow/flow-go/integration/testnet" "github.com/onflow/flow-go/integration/tests/lib" "github.com/onflow/flow-go/integration/tests/mvp" "github.com/onflow/flow-go/integration/utils" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" ) // This is a collection of tests that validate various Access API endpoints work as expected. From 717f24c1abb714f981a0149ea450fe54d6a9511b Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Tue, 24 Oct 2023 15:35:10 +0200 Subject: [PATCH 11/33] check no zero balance --- integration/tests/access/access_api_test.go | 7 ++++--- module/execution/scripts_test.go | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 6fa958c3946..5488033c5ec 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -178,21 +178,21 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { account, err := client.GetAccount(s.ctx, serviceAddress) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) - s.Assert().NotZero(serviceAddress, account.Balance) + s.Assert().NotZero(account.Balance) }) s.Run("get account block ID", func() { account, err := client.GetAccountAtLatestBlock(s.ctx, serviceAddress) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) - s.Assert().NotZero(serviceAddress, account.Balance) + s.Assert().NotZero(account.Balance) }) s.Run("get account block height", func() { account, err := client.GetAccountAtBlockHeight(s.ctx, serviceAddress, header.Height) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) - s.Assert().NotZero(serviceAddress, account.Balance) + s.Assert().NotZero(account.Balance) }) s.Run("get newly created account", func() { @@ -201,6 +201,7 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { acc, err := client.GetAccount(s.ctx, addr) s.Require().NoError(err) s.Assert().Equal(addr, acc.Address) + s.Assert().NotZero(acc.Balance) }) } diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index f0406917168..3aa74b3b327 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -86,6 +86,7 @@ func (s *scriptTestSuite) TestGetAccount() { account, err := s.scripts.GetAccountAtBlockHeight(context.Background(), address, s.height) s.Require().NoError(err) s.Require().Equal(address, account.Address) + s.Assert().NotZero(account.Balance) }) } From f2d8bc2523a5a86c98b0a729a50e8d7aa49af799 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 14:52:21 +0200 Subject: [PATCH 12/33] change get registers to a single register id --- module/execution/scripts_test.go | 4 ++- .../indexer/indexer_core.go | 27 +++++++++---------- .../indexer/indexer_core_test.go | 22 +++++++-------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 3aa74b3b327..40ea532e939 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -58,7 +58,9 @@ func (s *scriptTestSuite) TestScriptExecution() { s.Run("Handle Not Found Register", func() { s.scripts.registersAtHeight = IndexRegisterAdapter( // using the adapter checks the error handling in the adapter func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { - return nil, nil // intentionally return nil to check edge case + vals := make([]flow.RegisterValue, 1) + vals[0] = nil + return vals, nil // intentionally return nil to check edge case }, ) diff --git a/module/state_synchronization/indexer/indexer_core.go b/module/state_synchronization/indexer/indexer_core.go index 660aef867df..00a67ff8ca8 100644 --- a/module/state_synchronization/indexer/indexer_core.go +++ b/module/state_synchronization/indexer/indexer_core.go @@ -61,27 +61,24 @@ func New( }, nil } -// RegisterValues retrieves register values by the register IDs at the provided block height. +// RegisterValue retrieves register values by the register IDs at the provided block height. // Even if the register wasn't indexed at the provided height, returns the highest height the register was indexed at. // If a register is not found it will return a nil value and not an error. // Expected errors: // - storage.ErrHeightNotIndexed if the given height was not indexed yet or lower than the first indexed height. -func (c *IndexerCore) RegisterValues(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { - values := make([]flow.RegisterValue, len(IDs)) - - for j, id := range IDs { - value, err := c.registers.Get(id, height) - // only return an error if the error doesn't match the not found error, since we have - // to gracefully handle not found values and instead assign nil, that is because the script executor - // expects that behaviour - if err != nil && !errors.Is(err, storage.ErrNotFound) { - return nil, err - } - - values[j] = value +func (c *IndexerCore) RegisterValue(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { + value, err := c.registers.Get(ID, height) + // only return an error if the error doesn't match the not found error, since we have + // to gracefully handle not found values and instead assign nil, that is because the script executor + // expects that behaviour + if errors.Is(err, storage.ErrNotFound) { + return nil, nil + } + if err != nil { + return nil, err } - return values, nil + return value, nil } // IndexBlockData indexes all execution block data by height. diff --git a/module/state_synchronization/indexer/indexer_core_test.go b/module/state_synchronization/indexer/indexer_core_test.go index 5b4289dc4d8..caa4e612da8 100644 --- a/module/state_synchronization/indexer/indexer_core_test.go +++ b/module/state_synchronization/indexer/indexer_core_test.go @@ -171,9 +171,9 @@ func (i *indexCoreTest) runIndexBlockData() error { return i.indexer.IndexBlockData(i.data) } -func (i *indexCoreTest) runGetRegisters(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { +func (i *indexCoreTest) runGetRegister(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { i.initIndexer() - return i.indexer.RegisterValues(IDs, height) + return i.indexer.RegisterValue(ID, height) } func TestExecutionState_IndexBlockData(t *testing.T) { @@ -441,10 +441,10 @@ func TestExecutionState_RegisterValues(t *testing.T) { t.Run("Get value for single register", func(t *testing.T) { blocks := unittest.BlockchainFixture(5) height := blocks[1].Header.Height - ids := []flow.RegisterID{{ + id := flow.RegisterID{ Owner: "1", Key: "2", - }} + } val := flow.RegisterValue("0x1") values, err := newIndexCoreTest(t, blocks, nil). @@ -452,7 +452,7 @@ func TestExecutionState_RegisterValues(t *testing.T) { setGetRegisters(func(t *testing.T, ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { return val, nil }). - runGetRegisters(ids, height) + runGetRegister(id, height) assert.NoError(t, err) assert.Equal(t, values, []flow.RegisterValue{val}) @@ -559,9 +559,9 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { index, err := New(logger, metrics, db, registers, nil, nil, nil) require.NoError(t, err) - values := [][]byte{[]byte("1"), []byte("1"), []byte("2"), []byte("3") /*nil,*/, []byte("4")} + values := [][]byte{[]byte("1"), []byte("1"), []byte("2"), []byte("3"), []byte("4")} - value, err := index.RegisterValues(flow.RegisterIDs{registerID}, 0) + value, err := index.RegisterValue(registerID, 0) require.Nil(t, value) assert.ErrorIs(t, err, storage.ErrNotFound) @@ -571,7 +571,7 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { err := storeRegisterWithValue(index, height, regOwner, regKey, val) assert.NoError(t, err) - results, err := index.RegisterValues(flow.RegisterIDs{registerID}, height) + results, err := index.RegisterValue(registerID, height) require.Nil(t, err, testDesc) assert.Equal(t, val, results[0]) } @@ -592,7 +592,7 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { require.NoError(t, index.indexRegisters(nil, 2)) - value, err := index.RegisterValues(flow.RegisterIDs{registerID}, uint64(2)) + value, err := index.RegisterValue(registerID, uint64(2)) require.Nil(t, err) assert.Equal(t, storeValues[0], value[0]) @@ -601,11 +601,11 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { err = storeRegisterWithValue(index, 4, regOwner, regKey, storeValues[1]) require.NoError(t, err) - value, err = index.RegisterValues(flow.RegisterIDs{registerID}, uint64(4)) + value, err = index.RegisterValue(registerID, uint64(4)) require.Nil(t, err) assert.Equal(t, storeValues[1], value[0]) - value, err = index.RegisterValues(flow.RegisterIDs{registerID}, uint64(3)) + value, err = index.RegisterValue(registerID, uint64(3)) require.Nil(t, err) assert.Equal(t, storeValues[0], value[0]) }) From e6e78d904f71cacb1081a5c9405bbfdb3bcaf3a2 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 14:53:41 +0200 Subject: [PATCH 13/33] replace get registers --- module/execution/scripts.go | 19 ------------------- module/execution/scripts_test.go | 12 ++++-------- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 8d75e360c2b..9efe8907536 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -3,7 +3,6 @@ package execution import ( "context" "errors" - "fmt" "github.com/rs/zerolog" @@ -146,21 +145,3 @@ func (s *Scripts) snapshotWithBlock(height uint64) (snapshot.StorageSnapshot, *f return storageSnapshot, header, nil } - -// IndexRegisterAdapter an adapter for using indexer register values function that takes a slice of IDs in the -// script executor that only uses a single register ID at a time. It also does additional sanity checks if multiple values -// are returned, which shouldn't occur in normal operation. -func IndexRegisterAdapter(registerFun func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error)) func(flow.RegisterID, uint64) (flow.RegisterValue, error) { - return func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { - values, err := registerFun([]flow.RegisterID{ID}, height) - if err != nil { - return nil, err - } - - // even though this shouldn't occur in correct implementation we check that function returned either a single register or error - if len(values) != 1 { - return nil, fmt.Errorf("invalid number of returned values for a single register: %d", len(values)) - } - return values[0], nil - } -} diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 40ea532e939..76f409a8f70 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -56,13 +56,9 @@ func (s *scriptTestSuite) TestScriptExecution() { }) s.Run("Handle Not Found Register", func() { - s.scripts.registersAtHeight = IndexRegisterAdapter( // using the adapter checks the error handling in the adapter - func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { - vals := make([]flow.RegisterValue, 1) - vals[0] = nil - return vals, nil // intentionally return nil to check edge case - }, - ) + s.scripts.registersAtHeight = func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { + return nil, nil // intentionally return nil to check edge case + } // use a non-existing address to trigger register get function code := []byte("import Foo from 0x01; pub fun main() { }") @@ -129,7 +125,7 @@ func (s *scriptTestSuite) SetupTest() { s.chain.ChainID(), entropyBlock, headers, - IndexRegisterAdapter(index.RegisterValues), + index.RegisterValue, ) s.Require().NoError(err) s.scripts = scripts From 1b18d6f76d15f1383ad1fdc3e8b145164dce7cb5 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 14:53:59 +0200 Subject: [PATCH 14/33] replace get registers --- cmd/access/node_builder/access_node_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index b1a97d930ce..e8b97915a11 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -758,7 +758,7 @@ func (builder *FlowAccessNodeBuilder) BuildExecutionSyncComponents() *FlowAccess builder.RootChainID, query.NewProtocolStateWrapper(builder.State), builder.Storage.Headers, - execution.IndexRegisterAdapter(builder.ExecutionIndexerCore.RegisterValues), + builder.ExecutionIndexerCore.RegisterValue, ) if err != nil { return nil, err From 75f696fc43942597b1c2e54939090fef87bfb916 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 15:08:37 +0200 Subject: [PATCH 15/33] change tests to cover the nil value --- module/execution/scripts_test.go | 7 ++--- .../indexer/indexer_core_test.go | 28 ++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 76f409a8f70..c0cfe41a8d7 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "strings" "testing" "github.com/onflow/cadence" @@ -55,7 +54,7 @@ func (s *scriptTestSuite) TestScriptExecution() { s.Assert().Equal(number, val.(cadence.Int).Value.Int64()) }) - s.Run("Handle Not Found Register", func() { + s.Run("Not Found Register", func() { s.scripts.registersAtHeight = func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { return nil, nil // intentionally return nil to check edge case } @@ -64,7 +63,7 @@ func (s *scriptTestSuite) TestScriptExecution() { code := []byte("import Foo from 0x01; pub fun main() { }") result, err := s.scripts.ExecuteAtBlockHeight(context.Background(), code, nil, s.height) - s.Assert().True(strings.Contains(err.Error(), "invalid number of returned values for a single register")) + s.Assert().Error(err) s.Assert().Nil(result) }) } @@ -84,7 +83,7 @@ func (s *scriptTestSuite) TestGetAccount() { account, err := s.scripts.GetAccountAtBlockHeight(context.Background(), address, s.height) s.Require().NoError(err) s.Require().Equal(address, account.Address) - s.Assert().NotZero(account.Balance) + s.Assert().Zero(account.Balance) }) } diff --git a/module/state_synchronization/indexer/indexer_core_test.go b/module/state_synchronization/indexer/indexer_core_test.go index caa4e612da8..7adb771db3e 100644 --- a/module/state_synchronization/indexer/indexer_core_test.go +++ b/module/state_synchronization/indexer/indexer_core_test.go @@ -455,7 +455,7 @@ func TestExecutionState_RegisterValues(t *testing.T) { runGetRegister(id, height) assert.NoError(t, err) - assert.Equal(t, values, []flow.RegisterValue{val}) + assert.Equal(t, values, val) }) } @@ -560,11 +560,6 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { require.NoError(t, err) values := [][]byte{[]byte("1"), []byte("1"), []byte("2"), []byte("3"), []byte("4")} - - value, err := index.RegisterValue(registerID, 0) - require.Nil(t, value) - assert.ErrorIs(t, err, storage.ErrNotFound) - for i, val := range values { testDesc := fmt.Sprintf("test itteration number %d failed with test value %s", i, val) height := uint64(i + 1) @@ -573,11 +568,24 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { results, err := index.RegisterValue(registerID, height) require.Nil(t, err, testDesc) - assert.Equal(t, val, results[0]) + assert.Equal(t, val, results) } }) }) + // this test makes sure if a register is not found the value returned is nil and without an error in order for this to be + // up to the specification script executor requires + t.Run("Missing Register", func(t *testing.T) { + pebbleStorage.RunWithRegistersStorageAtInitialHeights(t, 0, 0, func(registers *pebbleStorage.Registers) { + index, err := New(logger, metrics, db, registers, nil, nil, nil) + require.NoError(t, err) + + value, err := index.RegisterValue(registerID, 0) + require.Nil(t, value) + assert.NoError(t, err) + }) + }) + // this test makes sure that even if indexed values for a specific register are requested with higher height // the correct highest height indexed value is returned. // e.g. we index A{h(1) -> X}, A{h(2) -> Y}, when we request h(4) we get value Y @@ -594,7 +602,7 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { value, err := index.RegisterValue(registerID, uint64(2)) require.Nil(t, err) - assert.Equal(t, storeValues[0], value[0]) + assert.Equal(t, storeValues[0], value) require.NoError(t, index.indexRegisters(nil, 3)) @@ -603,11 +611,11 @@ func TestIndexerIntegration_StoreAndGet(t *testing.T) { value, err = index.RegisterValue(registerID, uint64(4)) require.Nil(t, err) - assert.Equal(t, storeValues[1], value[0]) + assert.Equal(t, storeValues[1], value) value, err = index.RegisterValue(registerID, uint64(3)) require.Nil(t, err) - assert.Equal(t, storeValues[0], value[0]) + assert.Equal(t, storeValues[0], value) }) }) From 81bab7fc6c52867425d0be4162c8eabdcc081b11 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 17:48:45 +0200 Subject: [PATCH 16/33] add output error --- integration/testnet/client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/integration/testnet/client.go b/integration/testnet/client.go index 51bb6ba614a..74675bd310e 100644 --- a/integration/testnet/client.go +++ b/integration/testnet/client.go @@ -386,6 +386,10 @@ func (c *Client) CreateAccount( return sdk.Address{}, fmt.Errorf("failed to wait for create account transaction to seal %w", err) } + if result.Error != nil { + return sdk.Address{}, fmt.Errorf("failed to create new account %w", result.Error) + } + if address, ok := c.UserAddress(result); ok { return address, nil } From 1cc371794a3db40931199f37ffcce487a54c46df Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Thu, 26 Oct 2023 17:56:54 +0200 Subject: [PATCH 17/33] new account has 0 balance --- integration/tests/access/access_api_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 5488033c5ec..34bf7b18763 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -154,13 +154,13 @@ func (s *AccessAPISuite) TestScriptExecutionAndGetAccounts() { // Run tests against Access 2, which uses local storage s.testGetAccount(s.an2Client) - s.testExecuteScriptWithSimpleScript(s.an2Client) - s.testExecuteScriptWithSimpleContract(s.an2Client, targetHeight) + //s.testExecuteScriptWithSimpleScript(s.an2Client) + //s.testExecuteScriptWithSimpleContract(s.an2Client, targetHeight) // Run tests against Access 1, which uses the execution node - s.testGetAccount(s.an1Client) - s.testExecuteScriptWithSimpleScript(s.an1Client) - s.testExecuteScriptWithSimpleContract(s.an1Client, targetHeight) + //s.testGetAccount(s.an1Client) + //s.testExecuteScriptWithSimpleScript(s.an1Client) + //s.testExecuteScriptWithSimpleContract(s.an1Client, targetHeight) // this is a specialized test that creates accounts, deposits funds, deploys contracts, etc, and // uses the provided access node to handle the Access API calls. there is an existing test that @@ -201,7 +201,6 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { acc, err := client.GetAccount(s.ctx, addr) s.Require().NoError(err) s.Assert().Equal(addr, acc.Address) - s.Assert().NotZero(acc.Balance) }) } From 2cd52fe20840a86305b48556848eb1cac0f5fdaa Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 27 Oct 2023 13:13:28 +0200 Subject: [PATCH 18/33] revert commented out code --- integration/tests/access/access_api_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 34bf7b18763..fbeca2708f1 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -154,13 +154,13 @@ func (s *AccessAPISuite) TestScriptExecutionAndGetAccounts() { // Run tests against Access 2, which uses local storage s.testGetAccount(s.an2Client) - //s.testExecuteScriptWithSimpleScript(s.an2Client) - //s.testExecuteScriptWithSimpleContract(s.an2Client, targetHeight) + s.testExecuteScriptWithSimpleScript(s.an2Client) + s.testExecuteScriptWithSimpleContract(s.an2Client, targetHeight) // Run tests against Access 1, which uses the execution node - //s.testGetAccount(s.an1Client) - //s.testExecuteScriptWithSimpleScript(s.an1Client) - //s.testExecuteScriptWithSimpleContract(s.an1Client, targetHeight) + s.testGetAccount(s.an1Client) + s.testExecuteScriptWithSimpleScript(s.an1Client) + s.testExecuteScriptWithSimpleContract(s.an1Client, targetHeight) // this is a specialized test that creates accounts, deposits funds, deploys contracts, etc, and // uses the provided access node to handle the Access API calls. there is an existing test that From bb2bc2ff30652a7dd44511b8c3187491fe2cff61 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Fri, 27 Oct 2023 13:25:15 +0200 Subject: [PATCH 19/33] make sure key sequence number is updated --- integration/testnet/client.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/integration/testnet/client.go b/integration/testnet/client.go index 74675bd310e..feb6481ae22 100644 --- a/integration/testnet/client.go +++ b/integration/testnet/client.go @@ -365,8 +365,12 @@ func (c *Client) CreateAccount( payer sdk.Address, latestBlockID sdk.Identifier, ) (sdk.Address, error) { + updatedPayer, err := c.client.GetAccount(ctx, payerAccount.Address) + if err != nil { + return sdk.Address{}, err + } - payerKey := payerAccount.Keys[0] + payerKey := updatedPayer.Keys[0] tx, err := templates.CreateAccount([]*sdk.AccountKey{accountKey}, nil, payer) if err != nil { return sdk.Address{}, fmt.Errorf("failed cusnctruct create account transaction %w", err) From 1d1eedcbc9fa9ce5d2dfdf8eb778283fd8d5847c Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 12:56:06 +0100 Subject: [PATCH 20/33] wip test fix --- integration/testnet/client.go | 7 +------ integration/tests/access/access_api_test.go | 6 +++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/integration/testnet/client.go b/integration/testnet/client.go index feb6481ae22..9ae3289f27f 100644 --- a/integration/testnet/client.go +++ b/integration/testnet/client.go @@ -365,12 +365,7 @@ func (c *Client) CreateAccount( payer sdk.Address, latestBlockID sdk.Identifier, ) (sdk.Address, error) { - updatedPayer, err := c.client.GetAccount(ctx, payerAccount.Address) - if err != nil { - return sdk.Address{}, err - } - - payerKey := updatedPayer.Keys[0] + payerKey := payerAccount.Keys[0] tx, err := templates.CreateAccount([]*sdk.AccountKey{accountKey}, nil, payer) if err != nil { return sdk.Address{}, fmt.Errorf("failed cusnctruct create account transaction %w", err) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index fbeca2708f1..9a0246a80cf 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -2,6 +2,7 @@ package access import ( "context" + "fmt" "testing" "time" @@ -196,9 +197,12 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { }) s.Run("get newly created account", func() { + acc, err := s.serviceClient.GetAccount(s.serviceClient.SDKServiceAddress()) + fmt.Println("-----------", acc.Keys[0]) + addr, err := utils.CreateFlowAccount(s.ctx, s.serviceClient) s.Require().NoError(err) - acc, err := client.GetAccount(s.ctx, addr) + acc, err = client.GetAccount(s.ctx, addr) s.Require().NoError(err) s.Assert().Equal(addr, acc.Address) }) From bcb28358c143f9236ef009893d8ef13cf15b6122 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 13:39:03 +0100 Subject: [PATCH 21/33] fix the payer as the sequence key needs to be increased --- integration/testnet/client.go | 6 ++---- integration/utils/transactions.go | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/integration/testnet/client.go b/integration/testnet/client.go index 9ae3289f27f..9a75dc427b7 100644 --- a/integration/testnet/client.go +++ b/integration/testnet/client.go @@ -361,18 +361,16 @@ func (c *Client) GetAccount(accountAddress sdk.Address) (*sdk.Account, error) { func (c *Client) CreateAccount( ctx context.Context, accountKey *sdk.AccountKey, - payerAccount *sdk.Account, - payer sdk.Address, latestBlockID sdk.Identifier, ) (sdk.Address, error) { - payerKey := payerAccount.Keys[0] + payer := c.SDKServiceAddress() tx, err := templates.CreateAccount([]*sdk.AccountKey{accountKey}, nil, payer) if err != nil { return sdk.Address{}, fmt.Errorf("failed cusnctruct create account transaction %w", err) } tx.SetGasLimit(1000). SetReferenceBlockID(latestBlockID). - SetProposalKey(payer, 0, payerKey.SequenceNumber). + SetProposalKey(payer, 0, c.GetSeqNumber()). SetPayer(payer) err = c.SignAndSendTransaction(ctx, tx) diff --git a/integration/utils/transactions.go b/integration/utils/transactions.go index 2ae98c87eb5..b08a0f0c385 100644 --- a/integration/utils/transactions.go +++ b/integration/utils/transactions.go @@ -193,11 +193,10 @@ func CreateFlowAccount(ctx context.Context, client *testnet.Client) (sdk.Address } // createAccount will submit a create account transaction and wait for it to be sealed - addr, err := client.CreateAccount(ctx, fullAccountKey, client.Account(), client.SDKServiceAddress(), sdk.Identifier(latestBlockID)) + addr, err := client.CreateAccount(ctx, fullAccountKey, sdk.Identifier(latestBlockID)) if err != nil { return sdk.EmptyAddress, fmt.Errorf("failed to create account: %w", err) } - client.Account().Keys[0].SequenceNumber++ return addr, nil } From 20648ae5317f9c214550619d78c91a98696b0cb8 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 14:04:45 +0100 Subject: [PATCH 22/33] update comment --- module/execution/scripts.go | 1 - 1 file changed, 1 deletion(-) diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 9efe8907536..93905991eae 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -45,7 +45,6 @@ type ScriptExecutor interface { // GetAccountAtBlockHeight returns a Flow account by the provided address and block height. // Expected errors: - // - storage.ErrNotFound if block or register value at height was not found. // - ErrDataNotAvailable if the data for the block height is not available GetAccountAtBlockHeight(ctx context.Context, address flow.Address, height uint64) (*flow.Account, error) } From cc204bd2355c344c4c014546b62d27765ccd9f10 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 14:22:44 +0100 Subject: [PATCH 23/33] change name to singular form --- module/execution/scripts.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 93905991eae..5c2e706e03a 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -22,12 +22,12 @@ import ( // not locally indexed var ErrDataNotAvailable = errors.New("data for block is not available") -// RegistersAtHeight returns register value for provided register ID at the block height. +// RegisterAtHeight returns register value for provided register ID at the block height. // Even if the register wasn't indexed at the provided height, returns the highest height the register was indexed at. // If the register with the ID was not indexed at all return nil value and no error. // Expected errors: // - storage.ErrHeightNotIndexed if the given height was not indexed yet or lower than the first indexed height. -type RegistersAtHeight func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) +type RegisterAtHeight func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) type ScriptExecutor interface { // ExecuteAtBlockHeight executes provided script against the block height. @@ -52,9 +52,9 @@ type ScriptExecutor interface { var _ ScriptExecutor = (*Scripts)(nil) type Scripts struct { - executor *query.QueryExecutor - headers storage.Headers - registersAtHeight RegistersAtHeight + executor *query.QueryExecutor + headers storage.Headers + registerAtHeight RegisterAtHeight } func NewScripts( @@ -63,7 +63,7 @@ func NewScripts( chainID flow.ChainID, entropy query.EntropyProviderPerBlock, header storage.Headers, - registersAtHeight RegistersAtHeight, + registerAtHeight RegisterAtHeight, ) (*Scripts, error) { vm := fvm.NewVirtualMachine() @@ -86,9 +86,9 @@ func NewScripts( ) return &Scripts{ - executor: queryExecutor, - headers: header, - registersAtHeight: registersAtHeight, + executor: queryExecutor, + headers: header, + registerAtHeight: registerAtHeight, }, nil } @@ -135,11 +135,11 @@ func (s *Scripts) snapshotWithBlock(height uint64) (snapshot.StorageSnapshot, *f } storageSnapshot := snapshot.NewReadFuncStorageSnapshot(func(ID flow.RegisterID) (flow.RegisterValue, error) { - registers, err := s.registersAtHeight(ID, height) + register, err := s.registerAtHeight(ID, height) if errors.Is(err, storage.ErrHeightNotIndexed) { return nil, errors.Join(ErrDataNotAvailable, err) } - return registers, err + return register, err }) return storageSnapshot, header, nil From 1832214e985272f96f0a0603f657eb230d42fa5f Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 14:23:27 +0100 Subject: [PATCH 24/33] fix regsiter at height name --- module/execution/scripts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index c0cfe41a8d7..b32aba9c30e 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -55,7 +55,7 @@ func (s *scriptTestSuite) TestScriptExecution() { }) s.Run("Not Found Register", func() { - s.scripts.registersAtHeight = func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { + s.scripts.registerAtHeight = func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error) { return nil, nil // intentionally return nil to check edge case } From 605d70b54fc543d8b37d766e2e785fb3a851da1e Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 14:55:08 +0100 Subject: [PATCH 25/33] fix lint issue --- integration/tests/access/access_api_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 9a0246a80cf..fbeca2708f1 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -2,7 +2,6 @@ package access import ( "context" - "fmt" "testing" "time" @@ -197,12 +196,9 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { }) s.Run("get newly created account", func() { - acc, err := s.serviceClient.GetAccount(s.serviceClient.SDKServiceAddress()) - fmt.Println("-----------", acc.Keys[0]) - addr, err := utils.CreateFlowAccount(s.ctx, s.serviceClient) s.Require().NoError(err) - acc, err = client.GetAccount(s.ctx, addr) + acc, err := client.GetAccount(s.ctx, addr) s.Require().NoError(err) s.Assert().Equal(addr, acc.Address) }) From 83b23d775bcf1722ab8ed1a2017bb1cec5699cda Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 16:55:43 +0100 Subject: [PATCH 26/33] wip test latest height --- cmd/access/node_builder/access_node_builder.go | 1 + engine/access/rpc/backend/backend_accounts.go | 3 ++- module/execution/scripts.go | 11 +++++++++++ module/execution/scripts_test.go | 1 + 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index e8b97915a11..724714d5068 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -759,6 +759,7 @@ func (builder *FlowAccessNodeBuilder) BuildExecutionSyncComponents() *FlowAccess query.NewProtocolStateWrapper(builder.State), builder.Storage.Headers, builder.ExecutionIndexerCore.RegisterValue, + builder.ExecutionIndexer, ) if err != nil { return nil, err diff --git a/engine/access/rpc/backend/backend_accounts.go b/engine/access/rpc/backend/backend_accounts.go index eaa0ded99bc..0c1ed2d9df8 100644 --- a/engine/access/rpc/backend/backend_accounts.go +++ b/engine/access/rpc/backend/backend_accounts.go @@ -41,6 +41,7 @@ func (b *backendAccounts) GetAccount(ctx context.Context, address flow.Address) // GetAccountAtLatestBlock returns the account details at the latest sealed block. func (b *backendAccounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Address) (*flow.Account, error) { + height := b.scriptExecutor.LatestHeight() sealed, err := b.state.Sealed().Head() if err != nil { return nil, status.Errorf(codes.Internal, "failed to get latest sealed header: %v", err) @@ -48,7 +49,7 @@ func (b *backendAccounts) GetAccountAtLatestBlock(ctx context.Context, address f sealedBlockID := sealed.ID() - account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, sealed.Height) + account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, height) if err != nil { b.log.Debug().Err(err).Msgf("failed to get account at blockID: %v", sealedBlockID) return nil, err diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 5c2e706e03a..e1527ef5482 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -4,6 +4,8 @@ import ( "context" "errors" + "github.com/onflow/flow-go/module/state_synchronization/indexer" + "github.com/rs/zerolog" "github.com/onflow/flow-go/engine/execution/computation" @@ -47,6 +49,8 @@ type ScriptExecutor interface { // Expected errors: // - ErrDataNotAvailable if the data for the block height is not available GetAccountAtBlockHeight(ctx context.Context, address flow.Address, height uint64) (*flow.Account, error) + + LatestHeight() uint64 } var _ ScriptExecutor = (*Scripts)(nil) @@ -55,6 +59,7 @@ type Scripts struct { executor *query.QueryExecutor headers storage.Headers registerAtHeight RegisterAtHeight + indexer *indexer.Indexer } func NewScripts( @@ -64,6 +69,7 @@ func NewScripts( entropy query.EntropyProviderPerBlock, header storage.Headers, registerAtHeight RegisterAtHeight, + indexer *indexer.Indexer, ) (*Scripts, error) { vm := fvm.NewVirtualMachine() @@ -89,9 +95,14 @@ func NewScripts( executor: queryExecutor, headers: header, registerAtHeight: registerAtHeight, + indexer: indexer, }, nil } +func (s *Scripts) LatestHeight() uint64 { + return s.indexer.HighestIndexedHeight() +} + // ExecuteAtBlockHeight executes provided script against the block height. // A result value is returned encoded as byte array. An error will be returned if script // doesn't successfully execute. diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index b32aba9c30e..09f1ff53a1b 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -125,6 +125,7 @@ func (s *scriptTestSuite) SetupTest() { entropyBlock, headers, index.RegisterValue, + nil, ) s.Require().NoError(err) s.scripts = scripts From 7aecceaadb34c0b28bd35b40b7ff5400f773989b Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 17:01:03 +0100 Subject: [PATCH 27/33] wip test latest height --- engine/access/rpc/backend/backend_accounts.go | 4 +++- engine/access/rpc/backend/backend_scripts.go | 4 +++- engine/access/rpc/backend/script_executor.go | 4 ++++ module/execution/scripts.go | 6 ++++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/engine/access/rpc/backend/backend_accounts.go b/engine/access/rpc/backend/backend_accounts.go index eaa0ded99bc..0668e66063d 100644 --- a/engine/access/rpc/backend/backend_accounts.go +++ b/engine/access/rpc/backend/backend_accounts.go @@ -48,7 +48,9 @@ func (b *backendAccounts) GetAccountAtLatestBlock(ctx context.Context, address f sealedBlockID := sealed.ID() - account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, sealed.Height) + height := b.scriptExecutor.LatestHeight() + + account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, height) if err != nil { b.log.Debug().Err(err).Msgf("failed to get account at blockID: %v", sealedBlockID) return nil, err diff --git a/engine/access/rpc/backend/backend_scripts.go b/engine/access/rpc/backend/backend_scripts.go index 7ebb04a0e73..0947470f3e3 100644 --- a/engine/access/rpc/backend/backend_scripts.go +++ b/engine/access/rpc/backend/backend_scripts.go @@ -52,7 +52,9 @@ func (b *backendScripts) ExecuteScriptAtLatestBlock( return nil, status.Errorf(codes.Internal, "failed to get latest sealed header: %v", err) } - return b.executeScript(ctx, latestHeader.ID(), latestHeader.Height, script, arguments) + height := b.scriptExecutor.LatestHeight() + + return b.executeScript(ctx, latestHeader.ID(), height, script, arguments) } // ExecuteScriptAtBlockID executes provided script at the provided block ID. diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index 12d64a0daa9..8411ae566d7 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -31,6 +31,10 @@ func NewScriptExecutor() *ScriptExecutor { } } +func (s *ScriptExecutor) LatestHeight() uint64 { + return s.indexReporter.HighestIndexedHeight() +} + // InitReporter initializes the indexReporter and script executor // This method can be called at any time after the ScriptExecutor object is created. Any requests // made to the other methods will return execution.ErrDataNotAvailable until this method is called. diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 5c2e706e03a..53bdf4e1677 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -43,6 +43,8 @@ type ScriptExecutor interface { height uint64, ) ([]byte, error) + LatestHeight() uint64 + // GetAccountAtBlockHeight returns a Flow account by the provided address and block height. // Expected errors: // - ErrDataNotAvailable if the data for the block height is not available @@ -92,6 +94,10 @@ func NewScripts( }, nil } +func (s *Scripts) LatestHeight() uint64 { + panic(0) +} + // ExecuteAtBlockHeight executes provided script against the block height. // A result value is returned encoded as byte array. An error will be returned if script // doesn't successfully execute. From 13f4e928c4d17520f4d531e19b3cc8d2fb059db5 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 17:14:07 +0100 Subject: [PATCH 28/33] wip test latest height --- cmd/access/node_builder/access_node_builder.go | 1 - module/execution/scripts_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 724714d5068..e8b97915a11 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -759,7 +759,6 @@ func (builder *FlowAccessNodeBuilder) BuildExecutionSyncComponents() *FlowAccess query.NewProtocolStateWrapper(builder.State), builder.Storage.Headers, builder.ExecutionIndexerCore.RegisterValue, - builder.ExecutionIndexer, ) if err != nil { return nil, err diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 09f1ff53a1b..b32aba9c30e 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -125,7 +125,6 @@ func (s *scriptTestSuite) SetupTest() { entropyBlock, headers, index.RegisterValue, - nil, ) s.Require().NoError(err) s.scripts = scripts From b4258da91c7413b74e78ebb20cb9526c58098635 Mon Sep 17 00:00:00 2001 From: Gregor Gololicic Date: Mon, 30 Oct 2023 19:16:37 +0100 Subject: [PATCH 29/33] add wait for index to be synced --- integration/tests/access/access_api_test.go | 59 ++++++++++++++++++--- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index fbeca2708f1..11b0be6ba4f 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,6 +5,10 @@ import ( "testing" "time" + "google.golang.org/grpc/codes" + + "google.golang.org/grpc/status" + "github.com/rs/zerolog" "github.com/stretchr/testify/suite" @@ -175,21 +179,27 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) { serviceAddress := s.serviceClient.SDKServiceAddress() s.Run("get account at latest block", func() { - account, err := client.GetAccount(s.ctx, serviceAddress) + account, err := s.waitAccountsUntilIndexed(func() (*sdk.Account, error) { + return client.GetAccount(s.ctx, serviceAddress) + }) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) s.Assert().NotZero(account.Balance) }) s.Run("get account block ID", func() { - account, err := client.GetAccountAtLatestBlock(s.ctx, serviceAddress) + account, err := s.waitAccountsUntilIndexed(func() (*sdk.Account, error) { + return client.GetAccountAtLatestBlock(s.ctx, serviceAddress) + }) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) s.Assert().NotZero(account.Balance) }) s.Run("get account block height", func() { - account, err := client.GetAccountAtBlockHeight(s.ctx, serviceAddress, header.Height) + account, err := s.waitAccountsUntilIndexed(func() (*sdk.Account, error) { + return client.GetAccountAtBlockHeight(s.ctx, serviceAddress, header.Height) + }) s.Require().NoError(err) s.Assert().Equal(serviceAddress, account.Address) s.Assert().NotZero(account.Balance) @@ -209,19 +219,25 @@ func (s *AccessAPISuite) testExecuteScriptWithSimpleScript(client *client.Client s.Require().NoError(err) s.Run("execute at latest block", func() { - result, err := client.ExecuteScriptAtLatestBlock(s.ctx, []byte(simpleScript), nil) + result, err := s.waitScriptExecutionUntilIndexed(func() (cadence.Value, error) { + return client.ExecuteScriptAtLatestBlock(s.ctx, []byte(simpleScript), nil) + }) s.Require().NoError(err) s.Assert().Equal(simpleScriptResult, result) }) s.Run("execute at block height", func() { - result, err := client.ExecuteScriptAtBlockHeight(s.ctx, header.Height, []byte(simpleScript), nil) + result, err := s.waitScriptExecutionUntilIndexed(func() (cadence.Value, error) { + return client.ExecuteScriptAtBlockHeight(s.ctx, header.Height, []byte(simpleScript), nil) + }) s.Require().NoError(err) s.Assert().Equal(simpleScriptResult, result) }) s.Run("execute at block ID", func() { - result, err := client.ExecuteScriptAtBlockID(s.ctx, header.ID, []byte(simpleScript), nil) + result, err := s.waitScriptExecutionUntilIndexed(func() (cadence.Value, error) { + return client.ExecuteScriptAtBlockID(s.ctx, header.ID, []byte(simpleScript), nil) + }) s.Require().NoError(err) s.Assert().Equal(simpleScriptResult, result) }) @@ -297,6 +313,37 @@ func (s *AccessAPISuite) deployContract() *sdk.TransactionResult { return result } +type getAccount func() (*sdk.Account, error) +type executeScript func() (cadence.Value, error) + +var indexDelay = 10 * time.Second +var indexRetry = 100 * time.Millisecond + +// wait for sealed block to get indexed, as there is a delay in syncing blocks between nodes +func (s *AccessAPISuite) waitAccountsUntilIndexed(get getAccount) (*sdk.Account, error) { + var account *sdk.Account + var err error + s.Require().Eventually(func() bool { + account, err = get() + statusErr, ok := status.FromError(err) + return ok && statusErr.Code() != codes.OutOfRange + }, indexDelay, indexRetry) + + return account, err +} + +func (s *AccessAPISuite) waitScriptExecutionUntilIndexed(execute executeScript) (cadence.Value, error) { + var val cadence.Value + var err error + s.Require().Eventually(func() bool { + val, err = execute() + statusErr, ok := status.FromError(err) + return ok && statusErr.Code() != codes.OutOfRange + }, indexDelay, indexRetry) + + return val, err +} + func (s *AccessAPISuite) waitUntilIndexed(height uint64) { // wait until the block is indexed // This relying on the fact that the API is configured to only use the local db, and will return From e4f32b9144551ca3361bd94545fbee3ea5a0524d Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:50:54 -0700 Subject: [PATCH 30/33] fix lint --- integration/tests/access/access_api_test.go | 6 ++---- module/execution/mock/script_executor.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 6598805311f..b4779c4f56d 100644 --- a/integration/tests/access/access_api_test.go +++ b/integration/tests/access/access_api_test.go @@ -5,12 +5,10 @@ import ( "testing" "time" - "google.golang.org/grpc/codes" - - "google.golang.org/grpc/status" - "github.com/rs/zerolog" "github.com/stretchr/testify/suite" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/onflow/cadence" diff --git a/module/execution/mock/script_executor.go b/module/execution/mock/script_executor.go index ad716744d72..b3520d40656 100644 --- a/module/execution/mock/script_executor.go +++ b/module/execution/mock/script_executor.go @@ -67,6 +67,20 @@ func (_m *ScriptExecutor) GetAccountAtBlockHeight(ctx context.Context, address f return r0, r1 } +// LatestHeight provides a mock function with given fields: +func (_m *ScriptExecutor) LatestHeight() uint64 { + ret := _m.Called() + + var r0 uint64 + if rf, ok := ret.Get(0).(func() uint64); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint64) + } + + return r0 +} + type mockConstructorTestingTNewScriptExecutor interface { mock.TestingT Cleanup(func()) From bbb455ebe940299278f73cf6ed8288eda6c39bef Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Mon, 30 Oct 2023 16:04:12 -0700 Subject: [PATCH 31/33] Remove LatestHeight --- engine/access/rpc/backend/backend_accounts.go | 4 +--- engine/access/rpc/backend/backend_scripts.go | 4 +--- engine/access/rpc/backend/script_executor.go | 4 ---- module/execution/mock/script_executor.go | 14 -------------- module/execution/scripts.go | 6 ------ 5 files changed, 2 insertions(+), 30 deletions(-) diff --git a/engine/access/rpc/backend/backend_accounts.go b/engine/access/rpc/backend/backend_accounts.go index 0668e66063d..eaa0ded99bc 100644 --- a/engine/access/rpc/backend/backend_accounts.go +++ b/engine/access/rpc/backend/backend_accounts.go @@ -48,9 +48,7 @@ func (b *backendAccounts) GetAccountAtLatestBlock(ctx context.Context, address f sealedBlockID := sealed.ID() - height := b.scriptExecutor.LatestHeight() - - account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, height) + account, err := b.getAccountAtBlock(ctx, address, sealedBlockID, sealed.Height) if err != nil { b.log.Debug().Err(err).Msgf("failed to get account at blockID: %v", sealedBlockID) return nil, err diff --git a/engine/access/rpc/backend/backend_scripts.go b/engine/access/rpc/backend/backend_scripts.go index 0947470f3e3..7ebb04a0e73 100644 --- a/engine/access/rpc/backend/backend_scripts.go +++ b/engine/access/rpc/backend/backend_scripts.go @@ -52,9 +52,7 @@ func (b *backendScripts) ExecuteScriptAtLatestBlock( return nil, status.Errorf(codes.Internal, "failed to get latest sealed header: %v", err) } - height := b.scriptExecutor.LatestHeight() - - return b.executeScript(ctx, latestHeader.ID(), height, script, arguments) + return b.executeScript(ctx, latestHeader.ID(), latestHeader.Height, script, arguments) } // ExecuteScriptAtBlockID executes provided script at the provided block ID. diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index 8411ae566d7..12d64a0daa9 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -31,10 +31,6 @@ func NewScriptExecutor() *ScriptExecutor { } } -func (s *ScriptExecutor) LatestHeight() uint64 { - return s.indexReporter.HighestIndexedHeight() -} - // InitReporter initializes the indexReporter and script executor // This method can be called at any time after the ScriptExecutor object is created. Any requests // made to the other methods will return execution.ErrDataNotAvailable until this method is called. diff --git a/module/execution/mock/script_executor.go b/module/execution/mock/script_executor.go index b3520d40656..ad716744d72 100644 --- a/module/execution/mock/script_executor.go +++ b/module/execution/mock/script_executor.go @@ -67,20 +67,6 @@ func (_m *ScriptExecutor) GetAccountAtBlockHeight(ctx context.Context, address f return r0, r1 } -// LatestHeight provides a mock function with given fields: -func (_m *ScriptExecutor) LatestHeight() uint64 { - ret := _m.Called() - - var r0 uint64 - if rf, ok := ret.Get(0).(func() uint64); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint64) - } - - return r0 -} - type mockConstructorTestingTNewScriptExecutor interface { mock.TestingT Cleanup(func()) diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 53bdf4e1677..5c2e706e03a 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -43,8 +43,6 @@ type ScriptExecutor interface { height uint64, ) ([]byte, error) - LatestHeight() uint64 - // GetAccountAtBlockHeight returns a Flow account by the provided address and block height. // Expected errors: // - ErrDataNotAvailable if the data for the block height is not available @@ -94,10 +92,6 @@ func NewScripts( }, nil } -func (s *Scripts) LatestHeight() uint64 { - panic(0) -} - // ExecuteAtBlockHeight executes provided script against the block height. // A result value is returned encoded as byte array. An error will be returned if script // doesn't successfully execute. From c42490c0d8b747bc529027045dbe8e4bb2a1657f Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Mon, 30 Oct 2023 16:28:45 -0700 Subject: [PATCH 32/33] Fix unittests to use suite --- module/execution/scripts_test.go | 77 ++++++++------------------------ 1 file changed, 18 insertions(+), 59 deletions(-) diff --git a/module/execution/scripts_test.go b/module/execution/scripts_test.go index 1d6dfcecf8f..97a63a20d1b 100644 --- a/module/execution/scripts_test.go +++ b/module/execution/scripts_test.go @@ -56,42 +56,21 @@ func (s *scriptTestSuite) TestScriptExecution() { s.Assert().Equal(number, val.(cadence.Int).Value.Int64()) }) - t.Run("Get Block", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM() - - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) - + s.Run("Get Block", func() { code := []byte(fmt.Sprintf(`pub fun main(): UInt64 { getBlock(at: %d)! return getCurrentBlock().height - }`, first.Header.Height)) + }`, s.height)) - result, err := scripts.ExecuteAtBlockHeight(context.Background(), code, nil, first.Header.Height) - require.NoError(t, err) + result, err := s.scripts.ExecuteAtBlockHeight(context.Background(), code, nil, s.height) + s.Require().NoError(err) val, err := jsoncdc.Decode(nil, result) - require.NoError(t, err) + s.Require().NoError(err) // make sure that the returned block height matches the current one set - assert.Equal(t, first.Header.Height, val.(cadence.UInt64).ToGoValue()) + s.Assert().Equal(s.height, val.(cadence.UInt64).ToGoValue()) }) - t.Run("Handle not found Register", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - IndexRegisterAdapter( - func(IDs flow.RegisterIDs, height uint64) ([]flow.RegisterValue, error) { - return nil, nil // intentionally return nil to check edge case - }), - ) - + s.Run("Handle not found Register", func() { // use a non-existing address to trigger register get function code := []byte("import Foo from 0x01; pub fun main() { }") @@ -100,52 +79,32 @@ func (s *scriptTestSuite) TestScriptExecution() { s.Assert().Nil(result) }) - t.Run("Valid Argument", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM() - - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) - + s.Run("Valid Argument", func() { code := []byte("pub fun main(foo: Int): Int { return foo }") arg := cadence.NewInt(2) encoded, err := jsoncdc.Encode(arg) - require.NoError(t, err) + s.Require().NoError(err) - result, err := scripts.ExecuteAtBlockHeight( + result, err := s.scripts.ExecuteAtBlockHeight( context.Background(), code, [][]byte{encoded}, - first.Header.Height, + s.height, ) - require.NoError(t, err) - assert.Equal(t, encoded, result) + s.Require().NoError(err) + s.Assert().Equal(encoded, result) }) - t.Run("Invalid Argument", func(t *testing.T) { - blockchain := unittest.BlockchainFixture(10) - first := blockchain[0] - tree := bootstrapFVM() - - scripts := newScripts( - t, - newBlockHeadersStorage(blockchain), - treeToRegisterAdapter(tree), - ) - + s.Run("Invalid Argument", func() { code := []byte("pub fun main(foo: Int): Int { return foo }") invalid := [][]byte{[]byte("i")} - result, err := scripts.ExecuteAtBlockHeight(context.Background(), code, invalid, first.Header.Height) - assert.Nil(t, result) + result, err := s.scripts.ExecuteAtBlockHeight(context.Background(), code, invalid, s.height) + s.Assert().Nil(result) var coded errors.CodedError - require.True(t, errors.As(err, &coded)) + s.Require().True(errors.As(err, &coded)) fmt.Println(coded.Code(), coded.Error()) - assert.Equal(t, errors.ErrCodeInvalidArgumentError, coded.Code()) + s.Assert().Equal(errors.ErrCodeInvalidArgumentError, coded.Code()) }) } From 68709052e052723db3b9aeae826625bbebea5bef Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Mon, 30 Oct 2023 20:14:13 -0700 Subject: [PATCH 33/33] fix test seqNum --- integration/testnet/client.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/integration/testnet/client.go b/integration/testnet/client.go index 9a75dc427b7..fe4138b666e 100644 --- a/integration/testnet/client.go +++ b/integration/testnet/client.go @@ -32,7 +32,6 @@ type Client struct { accountKey *sdk.AccountKey accountKeyPriv sdkcrypto.PrivateKey signer sdkcrypto.InMemorySigner - seqNo uint64 Chain flow.Chain account *sdk.Account } @@ -63,7 +62,6 @@ func NewClientWithKey(accessAddr string, accountAddr sdk.Address, key sdkcrypto. accountKeyPriv: key, signer: mySigner, Chain: chain, - seqNo: accountKey.SequenceNumber, account: acc, } return tc, nil @@ -101,8 +99,8 @@ func (c *Client) AccountKeyPriv() sdkcrypto.PrivateKey { } func (c *Client) GetSeqNumber() uint64 { - n := c.seqNo - c.seqNo++ + n := c.accountKey.SequenceNumber + c.accountKey.SequenceNumber++ return n }