From 1030bcd997cdc1bca43defd05aea2941793de7f9 Mon Sep 17 00:00:00 2001 From: radu chis Date: Fri, 19 Apr 2024 10:48:37 +0300 Subject: [PATCH 1/5] add withKeys option on account --- api/groups/addressGroup.go | 1 + api/groups/addressGroupOptions.go | 6 ++ facade/interface.go | 2 +- facade/mock/nodeStub.go | 6 +- facade/nodeFacade.go | 14 +++- facade/nodeFacade_test.go | 14 ++-- go.mod | 2 +- go.sum | 4 +- .../node/getAccount/getAccount_test.go | 5 +- node/node.go | 34 +++++--- node/nodeLoadAccounts_test.go | 3 +- node/node_test.go | 79 +++++++++++++++++-- 12 files changed, 135 insertions(+), 35 deletions(-) diff --git a/api/groups/addressGroup.go b/api/groups/addressGroup.go index 1866c3bf022..da2adc0ab56 100644 --- a/api/groups/addressGroup.go +++ b/api/groups/addressGroup.go @@ -38,6 +38,7 @@ const ( urlParamBlockHash = "blockHash" urlParamBlockRootHash = "blockRootHash" urlParamHintEpoch = "hintEpoch" + urlParamWithKeys = "withKeys" ) // addressFacadeHandler defines the methods to be implemented by a facade for handling address requests diff --git a/api/groups/addressGroupOptions.go b/api/groups/addressGroupOptions.go index 5cd4fc6a11f..c3841f7e7fd 100644 --- a/api/groups/addressGroupOptions.go +++ b/api/groups/addressGroupOptions.go @@ -54,6 +54,11 @@ func parseAccountQueryOptions(c *gin.Context) (api.AccountQueryOptions, error) { return api.AccountQueryOptions{}, err } + withKeys, err := parseBoolUrlParam(c, urlParamWithKeys) + if err != nil { + return api.AccountQueryOptions{}, err + } + options := api.AccountQueryOptions{ OnFinalBlock: onFinalBlock, OnStartOfEpoch: onStartOfEpoch, @@ -61,6 +66,7 @@ func parseAccountQueryOptions(c *gin.Context) (api.AccountQueryOptions, error) { BlockHash: blockHash, BlockRootHash: blockRootHash, HintEpoch: hintEpoch, + WithKeys: withKeys, } return options, nil } diff --git a/facade/interface.go b/facade/interface.go index 07488622a96..b7deebb17e7 100644 --- a/facade/interface.go +++ b/facade/interface.go @@ -74,7 +74,7 @@ type NodeHandler interface { // GetAccount returns an accountResponse containing information // about the account correlated with provided address - GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) + GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) // GetCode returns the code for the given code hash GetCode(codeHash []byte, options api.AccountQueryOptions) ([]byte, api.BlockInfo) diff --git a/facade/mock/nodeStub.go b/facade/mock/nodeStub.go index 74c9cbea536..d03d847b151 100644 --- a/facade/mock/nodeStub.go +++ b/facade/mock/nodeStub.go @@ -25,7 +25,7 @@ type NodeStub struct { ValidateTransactionHandler func(tx *transaction.Transaction) error ValidateTransactionForSimulationCalled func(tx *transaction.Transaction, bypassSignature bool) error SendBulkTransactionsHandler func(txs []*transaction.Transaction) (uint64, error) - GetAccountCalled func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) + GetAccountCalled func(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) GetCodeCalled func(codeHash []byte, options api.AccountQueryOptions) ([]byte, api.BlockInfo) GetCurrentPublicKeyHandler func() string GenerateAndSendBulkTransactionsHandler func(destination string, value *big.Int, nrTransactions uint64) error @@ -181,9 +181,9 @@ func (ns *NodeStub) SendBulkTransactions(txs []*transaction.Transaction) (uint64 } // GetAccount - -func (ns *NodeStub) GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { +func (ns *NodeStub) GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { if ns.GetAccountCalled != nil { - return ns.GetAccountCalled(address, options) + return ns.GetAccountCalled(address, options, ctx) } return api.AccountResponse{}, api.BlockInfo{}, nil diff --git a/facade/nodeFacade.go b/facade/nodeFacade.go index 03dd77c76b7..4e328b5462a 100644 --- a/facade/nodeFacade.go +++ b/facade/nodeFacade.go @@ -321,7 +321,10 @@ func (nf *nodeFacade) GetLastPoolNonceForSender(sender string) (uint64, error) { // GetTransactionsPoolNonceGapsForSender will return the nonce gaps from pool for sender, if exists, that is to be returned on API calls func (nf *nodeFacade) GetTransactionsPoolNonceGapsForSender(sender string) (*common.TransactionsPoolNonceGapsForSenderApiResponse, error) { - accountResponse, _, err := nf.node.GetAccount(sender, apiData.AccountQueryOptions{}) + ctx, cancel := nf.getContextForApiTrieRangeOperations() + defer cancel() + + accountResponse, _, err := nf.node.GetAccount(sender, apiData.AccountQueryOptions{}, ctx) if err != nil { return &common.TransactionsPoolNonceGapsForSenderApiResponse{}, err } @@ -336,7 +339,10 @@ func (nf *nodeFacade) ComputeTransactionGasLimit(tx *transaction.Transaction) (* // GetAccount returns a response containing information about the account correlated with provided address func (nf *nodeFacade) GetAccount(address string, options apiData.AccountQueryOptions) (apiData.AccountResponse, apiData.BlockInfo, error) { - accountResponse, blockInfo, err := nf.node.GetAccount(address, options) + ctx, cancel := nf.getContextForApiTrieRangeOperations() + defer cancel() + + accountResponse, blockInfo, err := nf.node.GetAccount(address, options, ctx) if err != nil { return apiData.AccountResponse{}, apiData.BlockInfo{}, err } @@ -358,9 +364,11 @@ func (nf *nodeFacade) GetAccounts(addresses []string, options apiData.AccountQue response := make(map[string]*apiData.AccountResponse) var blockInfo apiData.BlockInfo + ctx, cancel := nf.getContextForApiTrieRangeOperations() + defer cancel() for _, address := range addresses { - accountResponse, blockInfoForAccount, err := nf.node.GetAccount(address, options) + accountResponse, blockInfoForAccount, err := nf.node.GetAccount(address, options, ctx) if err != nil { return nil, apiData.BlockInfo{}, err } diff --git a/facade/nodeFacade_test.go b/facade/nodeFacade_test.go index 21823b60b6e..f2eaff6025e 100644 --- a/facade/nodeFacade_test.go +++ b/facade/nodeFacade_test.go @@ -338,7 +338,7 @@ func TestNodeFacade_GetAccount(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(_ string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(_ string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr }, } @@ -352,7 +352,7 @@ func TestNodeFacade_GetAccount(t *testing.T) { getAccountCalled := false node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { getAccountCalled = true return api.AccountResponse{}, api.BlockInfo{}, nil } @@ -387,7 +387,7 @@ func TestNodeFacade_GetAccounts(t *testing.T) { t.Parallel() node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr } @@ -407,7 +407,7 @@ func TestNodeFacade_GetAccounts(t *testing.T) { expectedAcount := api.AccountResponse{Address: "test"} node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return expectedAcount, api.BlockInfo{}, nil } @@ -2008,7 +2008,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr }, } @@ -2030,7 +2030,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, nil }, } @@ -2062,7 +2062,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { } providedNonce := uint64(10) arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{Nonce: providedNonce}, api.BlockInfo{}, nil }, } diff --git a/go.mod b/go.mod index aafbc51ec02..b5d8fd684c6 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/klauspost/cpuid/v2 v2.2.5 github.com/mitchellh/mapstructure v1.5.0 github.com/multiversx/mx-chain-communication-go v1.0.14 - github.com/multiversx/mx-chain-core-go v1.2.19 + github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9 github.com/multiversx/mx-chain-crypto-go v1.2.11 github.com/multiversx/mx-chain-es-indexer-go v1.4.21 github.com/multiversx/mx-chain-logger-go v1.0.14 diff --git a/go.sum b/go.sum index 09c6f9ea503..64517c9b404 100644 --- a/go.sum +++ b/go.sum @@ -387,8 +387,8 @@ github.com/multiversx/concurrent-map v0.1.4 h1:hdnbM8VE4b0KYJaGY5yJS2aNIW9TFFsUY github.com/multiversx/concurrent-map v0.1.4/go.mod h1:8cWFRJDOrWHOTNSqgYCUvwT7c7eFQ4U2vKMOp4A/9+o= github.com/multiversx/mx-chain-communication-go v1.0.14 h1:YhAUDjBBpc5h5W0A7LHLXUMIMeCgwgGvkqfAPbFqsno= github.com/multiversx/mx-chain-communication-go v1.0.14/go.mod h1:qYCqgk0h+YpcTA84jHIpCBy6UShRwmXzHSCcdfwNrkw= -github.com/multiversx/mx-chain-core-go v1.2.19 h1:2BaVHkB0tro3cjs5ay2pmLup1loCV0e1p9jV5QW0xqc= -github.com/multiversx/mx-chain-core-go v1.2.19/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE= +github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9 h1:iUGEUzmxh2xrgaHS9Lq5CpnKGDsR02v9gEWroc/jbpA= +github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE= github.com/multiversx/mx-chain-crypto-go v1.2.11 h1:MNPJoiTJA5/tedYrI0N22OorbsKDESWG0SF8MCJwcJI= github.com/multiversx/mx-chain-crypto-go v1.2.11/go.mod h1:pcZutPdfLiAFytzCU3LxU3s8cXkvpNqquyitFSfoF3o= github.com/multiversx/mx-chain-es-indexer-go v1.4.21 h1:rzxXCkgOsqj67GRYtqzKuf9XgHwnZLTZhU90Ck3VbrE= diff --git a/integrationTests/node/getAccount/getAccount_test.go b/integrationTests/node/getAccount/getAccount_test.go index 487c8b1a15a..c48a1017a4e 100644 --- a/integrationTests/node/getAccount/getAccount_test.go +++ b/integrationTests/node/getAccount/getAccount_test.go @@ -1,6 +1,7 @@ package getAccount import ( + "context" "math/big" "testing" @@ -57,7 +58,7 @@ func TestNode_GetAccountAccountDoesNotExistsShouldRetEmpty(t *testing.T) { encodedAddress, err := integrationTests.TestAddressPubkeyConverter.Encode(integrationTests.CreateRandomBytes(32)) require.Nil(t, err) - recovAccnt, _, err := n.GetAccount(encodedAddress, api.AccountQueryOptions{}) + recovAccnt, _, err := n.GetAccount(encodedAddress, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) assert.Equal(t, uint64(0), recovAccnt.Nonce) @@ -99,7 +100,7 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { testAddress, err := coreComponents.AddressPubKeyConverter().Encode(testPubkey) require.Nil(t, err) - recovAccnt, _, err := n.GetAccount(testAddress, api.AccountQueryOptions{}) + recovAccnt, _, err := n.GetAccount(testAddress, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) require.Equal(t, testNonce, recovAccnt.Nonce) diff --git a/node/node.go b/node/node.go index 978fd45dc99..29cea2671d6 100644 --- a/node/node.go +++ b/node/node.go @@ -291,13 +291,26 @@ func (n *Node) GetKeyValuePairs(address string, options api.AccountQueryOptions, return map[string]string{}, api.BlockInfo{}, nil } + mapToReturn, err := n.getKeys(userAccount, ctx) + if err != nil { + return nil, api.BlockInfo{}, err + } + + if common.IsContextDone(ctx) { + return nil, api.BlockInfo{}, ErrTrieOperationsTimeout + } + + return mapToReturn, blockInfo, nil +} + +func (n *Node) getKeys(userAccount state.UserAccountHandler, ctx context.Context) (map[string]string, error) { chLeaves := &common.TrieIteratorChannels{ LeavesChan: make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity), ErrChan: errChan.NewErrChanWrapper(), } - err = userAccount.GetAllLeaves(chLeaves, ctx) + err := userAccount.GetAllLeaves(chLeaves, ctx) if err != nil { - return nil, api.BlockInfo{}, err + return nil, err } mapToReturn := make(map[string]string) @@ -307,14 +320,9 @@ func (n *Node) GetKeyValuePairs(address string, options api.AccountQueryOptions, err = chLeaves.ErrChan.ReadFromChanNonBlocking() if err != nil { - return nil, api.BlockInfo{}, err - } - - if common.IsContextDone(ctx) { - return nil, api.BlockInfo{}, ErrTrieOperationsTimeout + return nil, err } - - return mapToReturn, blockInfo, nil + return mapToReturn, nil } // GetValueForKey will return the value for a key from a given account @@ -930,7 +938,7 @@ func (n *Node) setTxGuardianData(guardian string, guardianSigHex string, tx *tra } // GetAccount will return account details for a given address -func (n *Node) GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { +func (n *Node) GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) if err != nil { adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) @@ -954,6 +962,11 @@ func (n *Node) GetAccount(address string, options api.AccountQueryOptions) (api. } } + var keys map[string]string + if options.WithKeys { + keys, _ = n.getKeys(account, ctx) + } + return api.AccountResponse{ Address: address, Nonce: account.GetNonce(), @@ -964,6 +977,7 @@ func (n *Node) GetAccount(address string, options api.AccountQueryOptions) (api. CodeMetadata: account.GetCodeMetadata(), DeveloperReward: account.GetDeveloperReward().String(), OwnerAddress: ownerAddress, + Pairs: keys, }, blockInfo, nil } diff --git a/node/nodeLoadAccounts_test.go b/node/nodeLoadAccounts_test.go index e7e03c2d05b..7ef831bca0f 100644 --- a/node/nodeLoadAccounts_test.go +++ b/node/nodeLoadAccounts_test.go @@ -2,6 +2,7 @@ package node_test import ( "bytes" + "context" "errors" "math/big" "testing" @@ -45,7 +46,7 @@ func TestNode_GetAccountWithOptionsShouldWork(t *testing.T) { node.WithStateComponents(stateComponents), ) - account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) + account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) require.Equal(t, "100", account.Balance) require.Equal(t, uint64(1), blockInfo.Nonce) diff --git a/node/node_test.go b/node/node_test.go index 2cde11d08a0..d10f79b67be 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -3293,7 +3293,7 @@ func TestNode_GetAccountPubkeyConverterFailsShouldErr(t *testing.T) { node.WithCoreComponents(coreComponents), ) - recovAccnt, _, err := n.GetAccount(createDummyHexAddress(64), api.AccountQueryOptions{}) + recovAccnt, _, err := n.GetAccount(createDummyHexAddress(64), api.AccountQueryOptions{}, context.Background()) assert.Empty(t, recovAccnt) assert.ErrorIs(t, err, errExpected) @@ -3320,7 +3320,7 @@ func TestNode_GetAccountAccountDoesNotExistsShouldRetEmpty(t *testing.T) { node.WithStateComponents(stateComponents), ) - account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) + account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) require.Equal(t, uint64(0), account.Nonce) @@ -3355,7 +3355,7 @@ func TestNode_GetAccountAccountsRepositoryFailsShouldErr(t *testing.T) { node.WithStateComponents(stateComponents), ) - recovAccnt, _, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) + recovAccnt, _, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) assert.Empty(t, recovAccnt) assert.NotNil(t, err) @@ -3394,7 +3394,7 @@ func TestNode_GetAccountAccNotFoundShouldReturnEmpty(t *testing.T) { node.WithStateComponents(stateComponents), ) - acc, bInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) + acc, bInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) require.Equal(t, dummyBlockInfo.apiResult(), bInfo) require.Equal(t, api.AccountResponse{Address: testscommon.TestAddressAlice, Balance: "0", DeveloperReward: "0"}, acc) @@ -3436,7 +3436,7 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { node.WithStateComponents(stateComponents), ) - recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{}) + recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{}, context.Background()) require.Nil(t, err) require.Equal(t, uint64(2), recovAccnt.Nonce) @@ -3447,6 +3447,75 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { require.Equal(t, testscommon.TestAddressAlice, recovAccnt.OwnerAddress) } +func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { + t.Parallel() + + accnt := createAcc(testscommon.TestPubKeyBob) + _ = accnt.AddToBalance(big.NewInt(1)) + + k1, v1 := []byte("key1"), []byte("value1") + k2, v2 := []byte("key2"), []byte("value2") + + accnt.SetDataTrie( + &trieMock.TrieStub{ + GetAllLeavesOnChannelCalled: func(leavesChannels *common.TrieIteratorChannels, ctx context.Context, rootHash []byte, _ common.KeyBuilder, tlp common.TrieLeafParser) error { + go func() { + suffix := append(k1, accnt.AddressBytes()...) + trieLeaf, _ := tlp.ParseLeaf(k1, append(v1, suffix...), core.NotSpecified) + leavesChannels.LeavesChan <- trieLeaf + + suffix = append(k2, accnt.AddressBytes()...) + trieLeaf2, _ := tlp.ParseLeaf(k2, append(v2, suffix...), core.NotSpecified) + leavesChannels.LeavesChan <- trieLeaf2 + + close(leavesChannels.LeavesChan) + leavesChannels.ErrChan.Close() + }() + + return nil + }, + RootCalled: func() ([]byte, error) { + return nil, nil + }, + }) + + accDB := &stateMock.AccountsStub{ + GetAccountWithBlockInfoCalled: func(address []byte, options common.RootHashHolder) (vmcommon.AccountHandler, common.BlockInfo, error) { + return accnt, nil, nil + }, + RecreateTrieCalled: func(rootHash []byte) error { + return nil + }, + } + + coreComponents := getDefaultCoreComponents() + dataComponents := getDefaultDataComponents() + stateComponents := getDefaultStateComponents() + args := state.ArgsAccountsRepository{ + FinalStateAccountsWrapper: accDB, + CurrentStateAccountsWrapper: accDB, + HistoricalStateAccountsWrapper: accDB, + } + stateComponents.AccountsRepo, _ = state.NewAccountsRepository(args) + n, _ := node.NewNode( + node.WithCoreComponents(coreComponents), + node.WithDataComponents(dataComponents), + node.WithStateComponents(stateComponents), + ) + + recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) + require.Nil(t, err) + require.Nil(t, recovAccnt.Pairs) + + recovAccnt, _, err = n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) + + require.Nil(t, err) + require.NotNil(t, recovAccnt.Pairs) + require.Equal(t, 2, len(recovAccnt.Pairs)) + require.Equal(t, hex.EncodeToString(v1), recovAccnt.Pairs[hex.EncodeToString(k1)]) + require.Equal(t, hex.EncodeToString(v2), recovAccnt.Pairs[hex.EncodeToString(k2)]) +} + func TestNode_AppStatusHandlersShouldIncrement(t *testing.T) { t.Parallel() From bcd6efca6ce8150da9f5f895d06e80d59caf4d86 Mon Sep 17 00:00:00 2001 From: radu chis Date: Mon, 22 Apr 2024 15:33:18 +0300 Subject: [PATCH 2/5] refactored withKeys --- api/groups/addressGroup.go | 8 ++++ api/groups/addressGroupOptions.go | 7 +-- facade/interface.go | 6 ++- facade/mock/nodeStub.go | 16 +++++-- facade/nodeFacade.go | 36 +++++++++------ facade/nodeFacade_test.go | 14 +++--- go.mod | 2 +- go.sum | 4 +- .../node/getAccount/getAccount_test.go | 5 +-- node/node.go | 45 ++++++++++++++++++- node/nodeLoadAccounts_test.go | 3 +- node/node_test.go | 14 +++--- 12 files changed, 113 insertions(+), 47 deletions(-) diff --git a/api/groups/addressGroup.go b/api/groups/addressGroup.go index da2adc0ab56..a059d3a4388 100644 --- a/api/groups/addressGroup.go +++ b/api/groups/addressGroup.go @@ -186,6 +186,14 @@ func (ag *addressGroup) getAccount(c *gin.Context) { return } + withKeys, err := parseBoolUrlParam(c, urlParamWithKeys) + if err != nil { + shared.RespondWithValidationError(c, errors.ErrCouldNotGetAccount, err) + return + } + + options.WithKeys = withKeys + accountResponse, blockInfo, err := ag.getFacade().GetAccount(addr, options) if err != nil { shared.RespondWithInternalError(c, errors.ErrCouldNotGetAccount, err) diff --git a/api/groups/addressGroupOptions.go b/api/groups/addressGroupOptions.go index c3841f7e7fd..e21dc6d361a 100644 --- a/api/groups/addressGroupOptions.go +++ b/api/groups/addressGroupOptions.go @@ -54,11 +54,6 @@ func parseAccountQueryOptions(c *gin.Context) (api.AccountQueryOptions, error) { return api.AccountQueryOptions{}, err } - withKeys, err := parseBoolUrlParam(c, urlParamWithKeys) - if err != nil { - return api.AccountQueryOptions{}, err - } - options := api.AccountQueryOptions{ OnFinalBlock: onFinalBlock, OnStartOfEpoch: onStartOfEpoch, @@ -66,7 +61,7 @@ func parseAccountQueryOptions(c *gin.Context) (api.AccountQueryOptions, error) { BlockHash: blockHash, BlockRootHash: blockRootHash, HintEpoch: hintEpoch, - WithKeys: withKeys, + WithKeys: false, } return options, nil } diff --git a/facade/interface.go b/facade/interface.go index b7deebb17e7..e3be7b76cb0 100644 --- a/facade/interface.go +++ b/facade/interface.go @@ -74,7 +74,11 @@ type NodeHandler interface { // GetAccount returns an accountResponse containing information // about the account correlated with provided address - GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) + GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) + + // GetAccountWithKeys returns an accountResponse containing information + // about the account correlated with provided address and all keys + GetAccountWithKeys(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) // GetCode returns the code for the given code hash GetCode(codeHash []byte, options api.AccountQueryOptions) ([]byte, api.BlockInfo) diff --git a/facade/mock/nodeStub.go b/facade/mock/nodeStub.go index d03d847b151..1e779e0ebce 100644 --- a/facade/mock/nodeStub.go +++ b/facade/mock/nodeStub.go @@ -25,7 +25,8 @@ type NodeStub struct { ValidateTransactionHandler func(tx *transaction.Transaction) error ValidateTransactionForSimulationCalled func(tx *transaction.Transaction, bypassSignature bool) error SendBulkTransactionsHandler func(txs []*transaction.Transaction) (uint64, error) - GetAccountCalled func(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) + GetAccountCalled func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) + GetAccountWithKeysCalled func(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) GetCodeCalled func(codeHash []byte, options api.AccountQueryOptions) ([]byte, api.BlockInfo) GetCurrentPublicKeyHandler func() string GenerateAndSendBulkTransactionsHandler func(destination string, value *big.Int, nrTransactions uint64) error @@ -181,9 +182,18 @@ func (ns *NodeStub) SendBulkTransactions(txs []*transaction.Transaction) (uint64 } // GetAccount - -func (ns *NodeStub) GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { +func (ns *NodeStub) GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { if ns.GetAccountCalled != nil { - return ns.GetAccountCalled(address, options, ctx) + return ns.GetAccountCalled(address, options) + } + + return api.AccountResponse{}, api.BlockInfo{}, nil +} + +// GetAccountWithKeys - +func (ns *NodeStub) GetAccountWithKeys(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { + if ns.GetAccountWithKeysCalled != nil { + return ns.GetAccountWithKeysCalled(address, options, ctx) } return api.AccountResponse{}, api.BlockInfo{}, nil diff --git a/facade/nodeFacade.go b/facade/nodeFacade.go index 4e328b5462a..8bc696b6adc 100644 --- a/facade/nodeFacade.go +++ b/facade/nodeFacade.go @@ -321,10 +321,7 @@ func (nf *nodeFacade) GetLastPoolNonceForSender(sender string) (uint64, error) { // GetTransactionsPoolNonceGapsForSender will return the nonce gaps from pool for sender, if exists, that is to be returned on API calls func (nf *nodeFacade) GetTransactionsPoolNonceGapsForSender(sender string) (*common.TransactionsPoolNonceGapsForSenderApiResponse, error) { - ctx, cancel := nf.getContextForApiTrieRangeOperations() - defer cancel() - - accountResponse, _, err := nf.node.GetAccount(sender, apiData.AccountQueryOptions{}, ctx) + accountResponse, _, err := nf.node.GetAccount(sender, apiData.AccountQueryOptions{}) if err != nil { return &common.TransactionsPoolNonceGapsForSenderApiResponse{}, err } @@ -339,10 +336,19 @@ func (nf *nodeFacade) ComputeTransactionGasLimit(tx *transaction.Transaction) (* // GetAccount returns a response containing information about the account correlated with provided address func (nf *nodeFacade) GetAccount(address string, options apiData.AccountQueryOptions) (apiData.AccountResponse, apiData.BlockInfo, error) { - ctx, cancel := nf.getContextForApiTrieRangeOperations() - defer cancel() + var accountResponse apiData.AccountResponse + var blockInfo apiData.BlockInfo + var err error + + if options.WithKeys { + ctx, cancel := nf.getContextForApiTrieRangeOperations() + defer cancel() + + accountResponse, blockInfo, err = nf.node.GetAccountWithKeys(address, options, ctx) + } else { + accountResponse, blockInfo, err = nf.node.GetAccount(address, options) + } - accountResponse, blockInfo, err := nf.node.GetAccount(address, options, ctx) if err != nil { return apiData.AccountResponse{}, apiData.BlockInfo{}, err } @@ -364,16 +370,20 @@ func (nf *nodeFacade) GetAccounts(addresses []string, options apiData.AccountQue response := make(map[string]*apiData.AccountResponse) var blockInfo apiData.BlockInfo - ctx, cancel := nf.getContextForApiTrieRangeOperations() - defer cancel() - for _, address := range addresses { - accountResponse, blockInfoForAccount, err := nf.node.GetAccount(address, options, ctx) + for i, address := range addresses { + accountResponse, blockInfoForAccount, err := nf.node.GetAccount(address, options) if err != nil { return nil, apiData.BlockInfo{}, err } - - blockInfo = blockInfoForAccount + // Use the first block info as the block info for the whole bulk + if i == 0 { + blockInfo = blockInfoForAccount + blockRootHash, errBlockRootHash := hex.DecodeString(blockInfoForAccount.RootHash) + if errBlockRootHash == nil { + options.BlockRootHash = blockRootHash + } + } codeHash := accountResponse.CodeHash code, _ := nf.node.GetCode(codeHash, options) diff --git a/facade/nodeFacade_test.go b/facade/nodeFacade_test.go index f2eaff6025e..21823b60b6e 100644 --- a/facade/nodeFacade_test.go +++ b/facade/nodeFacade_test.go @@ -338,7 +338,7 @@ func TestNodeFacade_GetAccount(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(_ string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(_ string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr }, } @@ -352,7 +352,7 @@ func TestNodeFacade_GetAccount(t *testing.T) { getAccountCalled := false node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { getAccountCalled = true return api.AccountResponse{}, api.BlockInfo{}, nil } @@ -387,7 +387,7 @@ func TestNodeFacade_GetAccounts(t *testing.T) { t.Parallel() node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr } @@ -407,7 +407,7 @@ func TestNodeFacade_GetAccounts(t *testing.T) { expectedAcount := api.AccountResponse{Address: "test"} node := &mock.NodeStub{} - node.GetAccountCalled = func(address string, _ api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + node.GetAccountCalled = func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return expectedAcount, api.BlockInfo{}, nil } @@ -2008,7 +2008,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, expectedErr }, } @@ -2030,7 +2030,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { arg := createMockArguments() arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{}, api.BlockInfo{}, nil }, } @@ -2062,7 +2062,7 @@ func TestNodeFacade_GetTransactionsPoolNonceGapsForSender(t *testing.T) { } providedNonce := uint64(10) arg.Node = &mock.NodeStub{ - GetAccountCalled: func(address string, options api.AccountQueryOptions, _ context.Context) (api.AccountResponse, api.BlockInfo, error) { + GetAccountCalled: func(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { return api.AccountResponse{Nonce: providedNonce}, api.BlockInfo{}, nil }, } diff --git a/go.mod b/go.mod index b5d8fd684c6..f8454ef4f3a 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/klauspost/cpuid/v2 v2.2.5 github.com/mitchellh/mapstructure v1.5.0 github.com/multiversx/mx-chain-communication-go v1.0.14 - github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9 + github.com/multiversx/mx-chain-core-go v1.2.20 github.com/multiversx/mx-chain-crypto-go v1.2.11 github.com/multiversx/mx-chain-es-indexer-go v1.4.21 github.com/multiversx/mx-chain-logger-go v1.0.14 diff --git a/go.sum b/go.sum index 64517c9b404..03321b70303 100644 --- a/go.sum +++ b/go.sum @@ -387,8 +387,8 @@ github.com/multiversx/concurrent-map v0.1.4 h1:hdnbM8VE4b0KYJaGY5yJS2aNIW9TFFsUY github.com/multiversx/concurrent-map v0.1.4/go.mod h1:8cWFRJDOrWHOTNSqgYCUvwT7c7eFQ4U2vKMOp4A/9+o= github.com/multiversx/mx-chain-communication-go v1.0.14 h1:YhAUDjBBpc5h5W0A7LHLXUMIMeCgwgGvkqfAPbFqsno= github.com/multiversx/mx-chain-communication-go v1.0.14/go.mod h1:qYCqgk0h+YpcTA84jHIpCBy6UShRwmXzHSCcdfwNrkw= -github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9 h1:iUGEUzmxh2xrgaHS9Lq5CpnKGDsR02v9gEWroc/jbpA= -github.com/multiversx/mx-chain-core-go v1.2.20-0.20240418121049-6970013b49d9/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE= +github.com/multiversx/mx-chain-core-go v1.2.20 h1:jOQ10LxxUqECnuqUYeBBT6VoZcpJDdYgOvsSGtifDdI= +github.com/multiversx/mx-chain-core-go v1.2.20/go.mod h1:B5zU4MFyJezmEzCsAHE9YNULmGCm2zbPHvl9hazNxmE= github.com/multiversx/mx-chain-crypto-go v1.2.11 h1:MNPJoiTJA5/tedYrI0N22OorbsKDESWG0SF8MCJwcJI= github.com/multiversx/mx-chain-crypto-go v1.2.11/go.mod h1:pcZutPdfLiAFytzCU3LxU3s8cXkvpNqquyitFSfoF3o= github.com/multiversx/mx-chain-es-indexer-go v1.4.21 h1:rzxXCkgOsqj67GRYtqzKuf9XgHwnZLTZhU90Ck3VbrE= diff --git a/integrationTests/node/getAccount/getAccount_test.go b/integrationTests/node/getAccount/getAccount_test.go index c48a1017a4e..487c8b1a15a 100644 --- a/integrationTests/node/getAccount/getAccount_test.go +++ b/integrationTests/node/getAccount/getAccount_test.go @@ -1,7 +1,6 @@ package getAccount import ( - "context" "math/big" "testing" @@ -58,7 +57,7 @@ func TestNode_GetAccountAccountDoesNotExistsShouldRetEmpty(t *testing.T) { encodedAddress, err := integrationTests.TestAddressPubkeyConverter.Encode(integrationTests.CreateRandomBytes(32)) require.Nil(t, err) - recovAccnt, _, err := n.GetAccount(encodedAddress, api.AccountQueryOptions{}, context.Background()) + recovAccnt, _, err := n.GetAccount(encodedAddress, api.AccountQueryOptions{}) require.Nil(t, err) assert.Equal(t, uint64(0), recovAccnt.Nonce) @@ -100,7 +99,7 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { testAddress, err := coreComponents.AddressPubKeyConverter().Encode(testPubkey) require.Nil(t, err) - recovAccnt, _, err := n.GetAccount(testAddress, api.AccountQueryOptions{}, context.Background()) + recovAccnt, _, err := n.GetAccount(testAddress, api.AccountQueryOptions{}) require.Nil(t, err) require.Equal(t, testNonce, recovAccnt.Nonce) diff --git a/node/node.go b/node/node.go index 29cea2671d6..db2031a37ca 100644 --- a/node/node.go +++ b/node/node.go @@ -938,7 +938,45 @@ func (n *Node) setTxGuardianData(guardian string, guardianSigHex string, tx *tra } // GetAccount will return account details for a given address -func (n *Node) GetAccount(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { +func (n *Node) GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { + account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) + if err != nil { + adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) + if isEmptyAccount { + return api.AccountResponse{ + Address: address, + Balance: "0", + DeveloperReward: "0", + }, adaptedBlockInfo, nil + } + + return api.AccountResponse{}, api.BlockInfo{}, err + } + + ownerAddress := "" + if len(account.GetOwnerAddress()) > 0 { + addressPubkeyConverter := n.coreComponents.AddressPubKeyConverter() + ownerAddress, err = addressPubkeyConverter.Encode(account.GetOwnerAddress()) + if err != nil { + return api.AccountResponse{}, api.BlockInfo{}, err + } + } + + return api.AccountResponse{ + Address: address, + Nonce: account.GetNonce(), + Balance: account.GetBalance().String(), + Username: string(account.GetUserName()), + CodeHash: account.GetCodeHash(), + RootHash: account.GetRootHash(), + CodeMetadata: account.GetCodeMetadata(), + DeveloperReward: account.GetDeveloperReward().String(), + OwnerAddress: ownerAddress, + }, blockInfo, nil +} + +// GetAccountWithKeys will return account details for a given address including the keys +func (n *Node) GetAccountWithKeys(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) if err != nil { adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) @@ -964,7 +1002,10 @@ func (n *Node) GetAccount(address string, options api.AccountQueryOptions, ctx c var keys map[string]string if options.WithKeys { - keys, _ = n.getKeys(account, ctx) + keys, err = n.getKeys(account, ctx) + if err != nil { + return api.AccountResponse{}, api.BlockInfo{}, err + } } return api.AccountResponse{ diff --git a/node/nodeLoadAccounts_test.go b/node/nodeLoadAccounts_test.go index 7ef831bca0f..e7e03c2d05b 100644 --- a/node/nodeLoadAccounts_test.go +++ b/node/nodeLoadAccounts_test.go @@ -2,7 +2,6 @@ package node_test import ( "bytes" - "context" "errors" "math/big" "testing" @@ -46,7 +45,7 @@ func TestNode_GetAccountWithOptionsShouldWork(t *testing.T) { node.WithStateComponents(stateComponents), ) - account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) + account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) require.Nil(t, err) require.Equal(t, "100", account.Balance) require.Equal(t, uint64(1), blockInfo.Nonce) diff --git a/node/node_test.go b/node/node_test.go index d10f79b67be..dc8b012734e 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -3293,7 +3293,7 @@ func TestNode_GetAccountPubkeyConverterFailsShouldErr(t *testing.T) { node.WithCoreComponents(coreComponents), ) - recovAccnt, _, err := n.GetAccount(createDummyHexAddress(64), api.AccountQueryOptions{}, context.Background()) + recovAccnt, _, err := n.GetAccount(createDummyHexAddress(64), api.AccountQueryOptions{}) assert.Empty(t, recovAccnt) assert.ErrorIs(t, err, errExpected) @@ -3320,7 +3320,7 @@ func TestNode_GetAccountAccountDoesNotExistsShouldRetEmpty(t *testing.T) { node.WithStateComponents(stateComponents), ) - account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) + account, blockInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) require.Nil(t, err) require.Equal(t, uint64(0), account.Nonce) @@ -3355,7 +3355,7 @@ func TestNode_GetAccountAccountsRepositoryFailsShouldErr(t *testing.T) { node.WithStateComponents(stateComponents), ) - recovAccnt, _, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) + recovAccnt, _, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) assert.Empty(t, recovAccnt) assert.NotNil(t, err) @@ -3394,7 +3394,7 @@ func TestNode_GetAccountAccNotFoundShouldReturnEmpty(t *testing.T) { node.WithStateComponents(stateComponents), ) - acc, bInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}, context.Background()) + acc, bInfo, err := n.GetAccount(testscommon.TestAddressAlice, api.AccountQueryOptions{}) require.Nil(t, err) require.Equal(t, dummyBlockInfo.apiResult(), bInfo) require.Equal(t, api.AccountResponse{Address: testscommon.TestAddressAlice, Balance: "0", DeveloperReward: "0"}, acc) @@ -3436,7 +3436,7 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { node.WithStateComponents(stateComponents), ) - recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{}, context.Background()) + recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{}) require.Nil(t, err) require.Equal(t, uint64(2), recovAccnt.Nonce) @@ -3503,11 +3503,11 @@ func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { node.WithStateComponents(stateComponents), ) - recovAccnt, _, err := n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) + recovAccnt, _, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) require.Nil(t, err) require.Nil(t, recovAccnt.Pairs) - recovAccnt, _, err = n.GetAccount(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) + recovAccnt, _, err = n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) require.Nil(t, err) require.NotNil(t, recovAccnt.Pairs) From 4cae25d8bdff39ef737874f72334ee83ddd60655 Mon Sep 17 00:00:00 2001 From: radu chis Date: Tue, 23 Apr 2024 11:12:34 +0300 Subject: [PATCH 3/5] added test for withKeys error --- api/groups/addressGroupOptions.go | 1 - node/node_test.go | 46 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/api/groups/addressGroupOptions.go b/api/groups/addressGroupOptions.go index e21dc6d361a..5cd4fc6a11f 100644 --- a/api/groups/addressGroupOptions.go +++ b/api/groups/addressGroupOptions.go @@ -61,7 +61,6 @@ func parseAccountQueryOptions(c *gin.Context) (api.AccountQueryOptions, error) { BlockHash: blockHash, BlockRootHash: blockRootHash, HintEpoch: hintEpoch, - WithKeys: false, } return options, nil } diff --git a/node/node_test.go b/node/node_test.go index dc8b012734e..71eba9f5467 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -3447,6 +3447,51 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { require.Equal(t, testscommon.TestAddressAlice, recovAccnt.OwnerAddress) } +func TestNode_GetAccountAccountWithKeysErrorShouldErr(t *testing.T) { + accnt := createAcc(testscommon.TestPubKeyBob) + _ = accnt.AddToBalance(big.NewInt(1)) + expectedErr := errors.New("expected error") + accnt.SetDataTrie( + &trieMock.TrieStub{ + GetAllLeavesOnChannelCalled: func(leavesChannels *common.TrieIteratorChannels, ctx context.Context, rootHash []byte, _ common.KeyBuilder, tlp common.TrieLeafParser) error { + return expectedErr + }, + RootCalled: func() ([]byte, error) { + return nil, nil + }, + }) + + accDB := &stateMock.AccountsStub{ + GetAccountWithBlockInfoCalled: func(address []byte, options common.RootHashHolder) (vmcommon.AccountHandler, common.BlockInfo, error) { + return accnt, nil, nil + }, + RecreateTrieCalled: func(rootHash []byte) error { + return nil + }, + } + + coreComponents := getDefaultCoreComponents() + dataComponents := getDefaultDataComponents() + stateComponents := getDefaultStateComponents() + args := state.ArgsAccountsRepository{ + FinalStateAccountsWrapper: accDB, + CurrentStateAccountsWrapper: accDB, + HistoricalStateAccountsWrapper: accDB, + } + stateComponents.AccountsRepo, _ = state.NewAccountsRepository(args) + n, _ := node.NewNode( + node.WithCoreComponents(coreComponents), + node.WithDataComponents(dataComponents), + node.WithStateComponents(stateComponents), + ) + + recovAccnt, blockInfo, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) + + require.Equal(t, expectedErr, err) + require.Equal(t, api.AccountResponse{}, recovAccnt) + require.Equal(t, api.BlockInfo{}, blockInfo) +} + func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { t.Parallel() @@ -3504,6 +3549,7 @@ func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { ) recovAccnt, _, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) + require.Nil(t, err) require.Nil(t, recovAccnt.Pairs) From 5a774a56faf2d0cfc1c9f884834a9e4f9a34d7cf Mon Sep 17 00:00:00 2001 From: radu chis Date: Tue, 23 Apr 2024 13:16:01 +0300 Subject: [PATCH 4/5] fix after review --- facade/interface.go | 2 +- node/node.go | 116 +++++++++++++++++++++----------------------- node/node_test.go | 50 ++++++++----------- 3 files changed, 76 insertions(+), 92 deletions(-) diff --git a/facade/interface.go b/facade/interface.go index e3be7b76cb0..35f185874ed 100644 --- a/facade/interface.go +++ b/facade/interface.go @@ -77,7 +77,7 @@ type NodeHandler interface { GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) // GetAccountWithKeys returns an accountResponse containing information - // about the account correlated with provided address and all keys + // about the account correlated with provided address and all keys GetAccountWithKeys(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) // GetCode returns the code for the given code hash diff --git a/node/node.go b/node/node.go index db2031a37ca..9cdf368c57e 100644 --- a/node/node.go +++ b/node/node.go @@ -62,6 +62,12 @@ type filter interface { filter(tokenIdentifier string, esdtData *systemSmartContracts.ESDTDataV2) bool } +type accountInfo struct { + account state.UserAccountHandler + block api.BlockInfo + accountResponse api.AccountResponse +} + // Node is a structure that holds all managed components type Node struct { initialNodesPubkeys map[uint32][]string @@ -939,87 +945,32 @@ func (n *Node) setTxGuardianData(guardian string, guardianSigHex string, tx *tra // GetAccount will return account details for a given address func (n *Node) GetAccount(address string, options api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { - account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) + accInfo, err := n.getAccountInfo(address, options) if err != nil { - adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) - if isEmptyAccount { - return api.AccountResponse{ - Address: address, - Balance: "0", - DeveloperReward: "0", - }, adaptedBlockInfo, nil - } - return api.AccountResponse{}, api.BlockInfo{}, err } - ownerAddress := "" - if len(account.GetOwnerAddress()) > 0 { - addressPubkeyConverter := n.coreComponents.AddressPubKeyConverter() - ownerAddress, err = addressPubkeyConverter.Encode(account.GetOwnerAddress()) - if err != nil { - return api.AccountResponse{}, api.BlockInfo{}, err - } - } - - return api.AccountResponse{ - Address: address, - Nonce: account.GetNonce(), - Balance: account.GetBalance().String(), - Username: string(account.GetUserName()), - CodeHash: account.GetCodeHash(), - RootHash: account.GetRootHash(), - CodeMetadata: account.GetCodeMetadata(), - DeveloperReward: account.GetDeveloperReward().String(), - OwnerAddress: ownerAddress, - }, blockInfo, nil + return accInfo.accountResponse, accInfo.block, nil } // GetAccountWithKeys will return account details for a given address including the keys func (n *Node) GetAccountWithKeys(address string, options api.AccountQueryOptions, ctx context.Context) (api.AccountResponse, api.BlockInfo, error) { - account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) + accInfo, err := n.getAccountInfo(address, options) if err != nil { - adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) - if isEmptyAccount { - return api.AccountResponse{ - Address: address, - Balance: "0", - DeveloperReward: "0", - }, adaptedBlockInfo, nil - } - return api.AccountResponse{}, api.BlockInfo{}, err } - ownerAddress := "" - if len(account.GetOwnerAddress()) > 0 { - addressPubkeyConverter := n.coreComponents.AddressPubKeyConverter() - ownerAddress, err = addressPubkeyConverter.Encode(account.GetOwnerAddress()) - if err != nil { - return api.AccountResponse{}, api.BlockInfo{}, err - } - } - var keys map[string]string if options.WithKeys { - keys, err = n.getKeys(account, ctx) + keys, err = n.getKeys(accInfo.account, ctx) if err != nil { return api.AccountResponse{}, api.BlockInfo{}, err } } - return api.AccountResponse{ - Address: address, - Nonce: account.GetNonce(), - Balance: account.GetBalance().String(), - Username: string(account.GetUserName()), - CodeHash: account.GetCodeHash(), - RootHash: account.GetRootHash(), - CodeMetadata: account.GetCodeMetadata(), - DeveloperReward: account.GetDeveloperReward().String(), - OwnerAddress: ownerAddress, - Pairs: keys, - }, blockInfo, nil + accInfo.accountResponse.Pairs = keys + + return accInfo.accountResponse, accInfo.block, nil } func extractBlockInfoIfNewAccount(err error) (api.BlockInfo, bool) { @@ -1039,6 +990,47 @@ func extractBlockInfoIfNewAccount(err error) (api.BlockInfo, bool) { return api.BlockInfo{}, false } +func (n *Node) getAccountInfo(address string, options api.AccountQueryOptions) (accountInfo, error) { + account, blockInfo, err := n.loadUserAccountHandlerByAddress(address, options) + if err != nil { + adaptedBlockInfo, isEmptyAccount := extractBlockInfoIfNewAccount(err) + if isEmptyAccount { + return accountInfo{ + accountResponse: api.AccountResponse{ + Address: address, + Balance: "0", + DeveloperReward: "0", + }, + block: adaptedBlockInfo}, err + } + } + + ownerAddress := "" + if len(account.GetOwnerAddress()) > 0 { + addressPubkeyConverter := n.coreComponents.AddressPubKeyConverter() + ownerAddress, err = addressPubkeyConverter.Encode(account.GetOwnerAddress()) + if err != nil { + return accountInfo{ + accountResponse: api.AccountResponse{}, + block: api.BlockInfo{}, + }, err + } + } + + return accountInfo{ + accountResponse: api.AccountResponse{ + Address: address, + Nonce: account.GetNonce(), + Balance: account.GetBalance().String(), + Username: string(account.GetUserName()), + CodeHash: account.GetCodeHash(), + RootHash: account.GetRootHash(), + CodeMetadata: account.GetCodeMetadata(), + DeveloperReward: account.GetDeveloperReward().String(), + OwnerAddress: ownerAddress, + }, block: blockInfo}, nil +} + // GetCode returns the code for the given code hash func (n *Node) GetCode(codeHash []byte, options api.AccountQueryOptions) ([]byte, api.BlockInfo) { return n.loadAccountCode(codeHash, options) diff --git a/node/node_test.go b/node/node_test.go index 71eba9f5467..f1740ada505 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -3447,7 +3447,7 @@ func TestNode_GetAccountAccountExistsShouldReturn(t *testing.T) { require.Equal(t, testscommon.TestAddressAlice, recovAccnt.OwnerAddress) } -func TestNode_GetAccountAccountWithKeysErrorShouldErr(t *testing.T) { +func TestNode_GetAccountAccountWithKeysErrorShouldFail(t *testing.T) { accnt := createAcc(testscommon.TestPubKeyBob) _ = accnt.AddToBalance(big.NewInt(1)) expectedErr := errors.New("expected error") @@ -3470,20 +3470,7 @@ func TestNode_GetAccountAccountWithKeysErrorShouldErr(t *testing.T) { }, } - coreComponents := getDefaultCoreComponents() - dataComponents := getDefaultDataComponents() - stateComponents := getDefaultStateComponents() - args := state.ArgsAccountsRepository{ - FinalStateAccountsWrapper: accDB, - CurrentStateAccountsWrapper: accDB, - HistoricalStateAccountsWrapper: accDB, - } - stateComponents.AccountsRepo, _ = state.NewAccountsRepository(args) - n, _ := node.NewNode( - node.WithCoreComponents(coreComponents), - node.WithDataComponents(dataComponents), - node.WithStateComponents(stateComponents), - ) + n := getNodeWithAccount(accDB) recovAccnt, blockInfo, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) @@ -3492,7 +3479,7 @@ func TestNode_GetAccountAccountWithKeysErrorShouldErr(t *testing.T) { require.Equal(t, api.BlockInfo{}, blockInfo) } -func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { +func TestNode_GetAccountAccountWithKeysShouldWork(t *testing.T) { t.Parallel() accnt := createAcc(testscommon.TestPubKeyBob) @@ -3533,6 +3520,23 @@ func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { }, } + n := getNodeWithAccount(accDB) + + recovAccnt, _, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) + + require.Nil(t, err) + require.Nil(t, recovAccnt.Pairs) + + recovAccnt, _, err = n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) + + require.Nil(t, err) + require.NotNil(t, recovAccnt.Pairs) + require.Equal(t, 2, len(recovAccnt.Pairs)) + require.Equal(t, hex.EncodeToString(v1), recovAccnt.Pairs[hex.EncodeToString(k1)]) + require.Equal(t, hex.EncodeToString(v2), recovAccnt.Pairs[hex.EncodeToString(k2)]) +} + +func getNodeWithAccount(accDB *stateMock.AccountsStub) *node.Node { coreComponents := getDefaultCoreComponents() dataComponents := getDefaultDataComponents() stateComponents := getDefaultStateComponents() @@ -3547,19 +3551,7 @@ func TestNode_GetAccountAccountWithKeysShouldReturn(t *testing.T) { node.WithDataComponents(dataComponents), node.WithStateComponents(stateComponents), ) - - recovAccnt, _, err := n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: false}, context.Background()) - - require.Nil(t, err) - require.Nil(t, recovAccnt.Pairs) - - recovAccnt, _, err = n.GetAccountWithKeys(testscommon.TestAddressBob, api.AccountQueryOptions{WithKeys: true}, context.Background()) - - require.Nil(t, err) - require.NotNil(t, recovAccnt.Pairs) - require.Equal(t, 2, len(recovAccnt.Pairs)) - require.Equal(t, hex.EncodeToString(v1), recovAccnt.Pairs[hex.EncodeToString(k1)]) - require.Equal(t, hex.EncodeToString(v2), recovAccnt.Pairs[hex.EncodeToString(k2)]) + return n } func TestNode_AppStatusHandlersShouldIncrement(t *testing.T) { From 3531379b72a2ac8e53fc39a39f03e1094d987fe4 Mon Sep 17 00:00:00 2001 From: radu chis Date: Tue, 23 Apr 2024 13:58:26 +0300 Subject: [PATCH 5/5] refactored and fixed test --- node/node.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/node/node.go b/node/node.go index 9cdf368c57e..992cba53768 100644 --- a/node/node.go +++ b/node/node.go @@ -1001,8 +1001,15 @@ func (n *Node) getAccountInfo(address string, options api.AccountQueryOptions) ( Balance: "0", DeveloperReward: "0", }, - block: adaptedBlockInfo}, err + block: adaptedBlockInfo, + account: account, + }, nil } + return accountInfo{ + accountResponse: api.AccountResponse{}, + block: api.BlockInfo{}, + account: nil, + }, err } ownerAddress := "" @@ -1013,6 +1020,7 @@ func (n *Node) getAccountInfo(address string, options api.AccountQueryOptions) ( return accountInfo{ accountResponse: api.AccountResponse{}, block: api.BlockInfo{}, + account: nil, }, err } } @@ -1028,7 +1036,10 @@ func (n *Node) getAccountInfo(address string, options api.AccountQueryOptions) ( CodeMetadata: account.GetCodeMetadata(), DeveloperReward: account.GetDeveloperReward().String(), OwnerAddress: ownerAddress, - }, block: blockInfo}, nil + }, + block: blockInfo, + account: account, + }, nil } // GetCode returns the code for the given code hash