Skip to content

Commit

Permalink
Merge pull request #4862 from onflow/gregor/script-execution/local-ac…
Browse files Browse the repository at this point in the history
…count-bugfix

[Access] Get account bugfix with tests
  • Loading branch information
peterargue authored Oct 31, 2023
2 parents d300106 + 6870905 commit 3704b14
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 224 deletions.
2 changes: 1 addition & 1 deletion cmd/access/node_builder/access_node_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,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
Expand Down
17 changes: 8 additions & 9 deletions integration/testnet/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type Client struct {
accountKey *sdk.AccountKey
accountKeyPriv sdkcrypto.PrivateKey
signer sdkcrypto.InMemorySigner
seqNo uint64
Chain flow.Chain
account *sdk.Account
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -361,19 +359,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)
Expand All @@ -386,6 +381,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
}
Expand Down
21 changes: 14 additions & 7 deletions integration/tests/access/access_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -21,6 +19,7 @@ import (
"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"
)
Expand Down Expand Up @@ -183,7 +182,7 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) {
})
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() {
Expand All @@ -192,7 +191,7 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) {
})
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() {
Expand All @@ -201,7 +200,15 @@ func (s *AccessAPISuite) testGetAccount(client *client.Client) {
})
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() {
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)
})
}

Expand Down
3 changes: 1 addition & 2 deletions integration/utils/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
48 changes: 14 additions & 34 deletions module/execution/scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package execution
import (
"context"
"errors"
"fmt"

"github.com/onflow/flow-go/fvm/environment"

Expand All @@ -15,7 +14,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"
)

Expand All @@ -25,12 +24,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.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)
type RegisterAtHeight func(ID flow.RegisterID, height uint64) (flow.RegisterValue, error)

type ScriptExecutor interface {
// ExecuteAtBlockHeight executes provided script against the block height.
Expand All @@ -48,26 +47,25 @@ 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)
}

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(
log zerolog.Logger,
metrics *metrics.ExecutionCollector,
metrics module.ExecutionMetrics,
chainID flow.ChainID,
entropy query.EntropyProviderPerBlock,
header storage.Headers,
registersAtHeight RegistersAtHeight,
registerAtHeight RegisterAtHeight,
) (*Scripts, error) {
vm := fvm.NewVirtualMachine()

Expand All @@ -92,9 +90,9 @@ func NewScripts(
)

return &Scripts{
executor: queryExecutor,
headers: header,
registersAtHeight: registersAtHeight,
executor: queryExecutor,
headers: header,
registerAtHeight: registerAtHeight,
}, nil
}

Expand Down Expand Up @@ -141,30 +139,12 @@ 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
}

// 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
}
}
Loading

0 comments on commit 3704b14

Please sign in to comment.