Implement queries for historical balances (state) #4371

andreibancioiu · 2022-08-12T12:38:39Z

Description of the reasoning behind the pull request (what feature was missing / how the problem was manifesting itself / what was the motive behind the refactoring)

Node API does not support historical balances lookup yet. That is, it does not support queries such as address/erd1alice?blockNonce=12345.
Rosetta API would greatly benefit from historical balances (e.g. improved robustness, improved debugging)

Proposed Changes

At the API controllers level (i.e., in addressGroup), allow one to provide extra URL parameters (query parameters) for account-related endpoints. These parameters are: blockNonce, blockHash, blockRootHash. They are mutually exclusive.
At a lower level, in the node object, handle the new query options and appropriately delegate the queries to the accountsRepository. The accountsRepository relies on a new component (see below) to resolve historical queries.
In addition to accountsDBApi, add a new implementation of AccountsAdapterAPI: accountsDBApiWithHistory. This implementation is able to provide historical state (balance etc.) for the queried account, by recreating the trie at a specified rootHash.
The re-creation of the trie at an older rootHash is possible thanks to: Enable snapshot even if pruning is disabled #4365.
Historical balances lookup require the following node configuration: AccountsStatePruningEnabled = false. The implementation is also subject to (affected by): NumActivePersisters = K.

Testing procedure

Run Rosetta's check:data validation flow, with historical balances enabled, on local testnet, then on devnet.
On Node API, perform a large number of account queries such as: /address/erd1alice?blockNonce=..., /address/erd1alice?blockHash=..., /address/erd1alice?blockRootHash=.... On the response, also check the blockInfo field. It should match the provided coordinates.
For the queries above, also provide bad parameters (e.g. missing blocks).
For sender = Alice, broadcast a transaction that transfers some value. Say this transaction is added in block N. Then, query address/erd1alice?blockNonce=(N-1), address/erd1alice?blockNonce=(N) and address/erd1alice?blockNonce=(N+1). Check output.
On internal testnets, make sure to set AccountsStatePruningEnabled = false and NumActivePersisters = K, where K should be at least 3 (3 is fine).

codecov-commenter · 2022-08-12T12:47:04Z

Codecov Report

Merging #4371 (5b41b3f) into rc/2022-july (311f2d1) will increase coverage by 0.04%.
The diff coverage is 87.16%.

@@               Coverage Diff                @@
##           rc/2022-july    #4371      +/-   ##
================================================
+ Coverage         73.75%   73.80%   +0.04%     
================================================
  Files               671      675       +4     
  Lines             86488    86769     +281     
================================================
+ Hits              63793    64039     +246     
- Misses            17919    17938      +19     
- Partials           4776     4792      +16

Impacted Files	Coverage Δ
api/groups/addressGroup.go	`71.53% <54.54%> (-0.91%)`	⬇️
factory/stateComponents.go	`54.79% <66.66%> (-0.14%)`	⬇️
node/node.go	`73.36% <66.66%> (-0.24%)`	⬇️
common/holders/rootHashHolder.go	`80.00% <80.00%> (ø)`
state/accountsRepository.go	`88.23% <80.95%> (-11.77%)`	⬇️
state/factory/accountsAdapterAPICreator.go	`80.00% <85.71%> (-20.00%)`	⬇️
api/groups/addressGroupOptions.go	`86.95% <86.95%> (ø)`
node/nodeLoadAccounts.go	`88.04% <87.50%> (-1.25%)`	⬇️
api/groups/urlParams.go	`93.10% <90.90%> (-6.90%)`	⬇️
state/accountsDBApiWithHistory.go	`94.79% <94.79%> (ø)`
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

…elrond-go into historical-balances-08-12

api/groups/addressGroupOptions.go

andreibancioiu · 2022-08-17T17:02:44Z

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/ElrondNetwork/concurrent-map v0.1.3
 	github.com/ElrondNetwork/covalent-indexer-go v1.0.6
 	github.com/ElrondNetwork/elastic-indexer-go v1.2.38
-	github.com/ElrondNetwork/elrond-go-core v1.1.18
+	github.com/ElrondNetwork/elrond-go-core v1.1.19-0.20220813193723-a7d8c4aeaa87


See multiversx/mx-chain-core-go#86.

node/nodeBlocks.go

andreibancioiu · 2022-08-17T17:05:53Z

node/nodeBlocks.go

+
+func (n *Node) getBlockRootHash(headerHash []byte, header data.HeaderHandler) []byte {
+	blockRootHash, err := n.processComponents.ScheduledTxsExecutionHandler().GetScheduledRootHashForHeader(headerHash)
+	if err != nil {


Unfortunately, we have to swallow this error.

Perhaps compare it with some well-known errors, then log.Warn if necessary?

it is ok like this. We either return the roothash (that we can find on disk) as the function states.
I think we have a similar approach in the block processors process/block/shardblock.go L1208 + L1250-L1253

iulianpascalau · 2022-08-18T14:45:03Z

api/groups/addressGroupOptions.go

+		numSpecifiedBlockCoordinates++
+	}
+
+	if numSpecifiedBlockCoordinates > 1 {


elegant implementation 👍

iulianpascalau · 2022-08-18T14:45:22Z

api/groups/urlParams.go

@@ -15,11 +17,44 @@ func parseBoolUrlParam(c *gin.Context, name string) (bool, error) {
 	return strconv.ParseBool(param)
 }

-func parseUintUrlParam(c *gin.Context, name string) (uint64, error) {
+func parseUint32UrlParam(c *gin.Context, name string) (core.OptionalUint32, error) {


iulianpascalau · 2022-08-18T14:55:14Z

api/groups/urlParams.go

+	return core.OptionalUint32{Value: uint32(value), HasValue: true}, nil
+}
+
+func parseUint64UrlParam(c *gin.Context, name string) (core.OptionalUint64, error) {


common/holders/getAccountStateOptions.go

state/accountsDBApi.go

state/accountsDBApiWithHistory.go

…elrond-go into historical-balances-08-12

…thBlockInfo().

iulianpascalau · 2022-08-19T20:36:57Z

node/nodeLoadAccounts.go

@@ -30,6 +31,13 @@ func (n *Node) loadUserAccountHandlerByPubKey(pubKey []byte, options api.Account

 	account, blockInfo, err := repository.GetAccountWithBlockInfo(pubKey, options)
 	if err != nil {
+		blockInfo, ok := extractBlockInfoIfErrAccountNotFoundAtBlock(err)


interesting approach for error handling/passing/extracting data out of it

iulianpascalau · 2022-08-19T20:39:21Z

state/accountsDBApiWithHistory.go

+	accountsDB.mutRecreateAndGet.RLock()
+	if !accountsDB.shouldRecreateTrieUnprotected(rootHash) {
+		// Re-creation is not necessary, but make sure no other routine re-creates it.
+		defer accountsDB.mutRecreateAndGet.RUnlock()


👍 yup, this should stay on defer

iulianpascalau · 2022-08-19T20:47:17Z

state/accountsDBApiWithHistory_test.go

+			for j := 0; j < numCallsPerRootHash; j++ {
+				wg.Add(1)
+
+				go func(rootHashAsInt int) {


wondering how these numbers numRootHashes = 128 and numCallsPerRootHash = 128 do not crash the race detector since it can handle a maximum 8192 go routines

gabi-vuls

System test passed.

Sketch historical accounts db API.

b62ea4f

andreibancioiu changed the base branch from master to rc/2022-july August 12, 2022 12:38

andreibancioiu self-assigned this Aug 12, 2022

Merge branch 'rc/2022-july' into historical-balances-08-12

bc3d648

andreibancioiu added 2 commits August 12, 2022 15:50

Adjust interface AccountsAdapterAPI (add parameter "options").

d1f9075

Implement accountsDBApiWithHistory. Define GetAccountsStateOptions.

056aeca

andreibancioiu added type:feature New feature or request area:node-api Node API interested:rosetta size:medium complexity:medium labels Aug 12, 2022

andreibancioiu and others added 8 commits August 12, 2022 17:43

Fix issue (detected by linter).

cf18d9c

Check / validate account query options.

f9b3e6c

transformAccountQueryOptions: infer rootHash from headerHash / nonce.

ae8a62f

Parse URL params. Handle optional params.

2057994

Merge branch 'rc/2022-july' into historical-balances-08-12

c763f5c

Merge account query options into the block info.

478d24f

Merge branch 'historical-balances-08-12' of github.com:ElrondNetwork/…

776f671

…elrond-go into historical-balances-08-12

Fix linter, fix error reporting.

dcf8d17

andreibancioiu mentioned this pull request Aug 14, 2022

Add support for historical balances multiversx/mx-chain-core-go#86

Merged

andreibancioiu and others added 9 commits August 14, 2022 23:06

Fix addBlockCoordinatesToAccountQueryOptions().

989e28a

Merge branch 'rc/2022-july' into historical-balances-08-12

25bf7e1

Added some unit tests (plus some refactoring).

26d59b8

Added some unit tests.

6a6d5cd

Merge branch 'rc/2022-july' into historical-balances-08-12

75420ac

Merge branch 'historical-balances-08-12' of github.com:ElrondNetwork/…

0924e59

…elrond-go into historical-balances-08-12

Add extra unit tests.

741f12d

Minor refactoring, better symmetry.

b4c62cd

Fix implementation, add extra tests (WIP).

95bbbc7

Add unit tests.

643be40

andreibancioiu marked this pull request as ready for review August 17, 2022 16:50

andreibancioiu commented Aug 17, 2022

View reviewed changes

api/groups/addressGroupOptions.go Outdated Show resolved Hide resolved

andreibancioiu commented Aug 17, 2022

View reviewed changes

node/nodeBlocks.go Show resolved Hide resolved

andreibancioiu commented Aug 17, 2022

View reviewed changes

andreibancioiu mentioned this pull request Aug 18, 2022

Historical balances lookup (initial implementation) multiversx/mx-chain-rosetta#29

Merged

iulianpascalau self-requested a review August 18, 2022 14:42

iulianpascalau reviewed Aug 18, 2022

View reviewed changes

andreibancioiu and others added 6 commits August 19, 2022 18:31

Fix after review (part 1).

97c70b4

Merge branch 'rc/2022-july' into historical-balances-08-12

d6f6576

Merge branch 'historical-balances-08-12' of github.com:ElrondNetwork/…

2072214

…elrond-go into historical-balances-08-12

Fix after review (part 2) - improve critical sections on GetAccountWi…

9c69abe

…thBlockInfo().

Fix linting warnings.

10fb969

Provide block info when account does not exist.

32fb586

andreibancioiu requested a review from iulianpascalau August 19, 2022 18:52

iulianpascalau approved these changes Aug 19, 2022

View reviewed changes

Merge branch 'rc/2022-july' into historical-balances-08-12

291fad2

gabi-vuls approved these changes Aug 21, 2022

View reviewed changes

Merge branch 'rc/2022-july' into historical-balances-08-12

5b41b3f

andreibancioiu mentioned this pull request Aug 21, 2022

For historical lookups, load block coordinates from older epochs, as well #4386

Merged

AdoAdoAdo self-requested a review August 22, 2022 07:21

AdoAdoAdo approved these changes Aug 22, 2022

View reviewed changes

Merge branch 'rc/2022-july' into historical-balances-08-12

0cc1714

andreibancioiu merged commit 4d64e3d into rc/2022-july Aug 22, 2022

andreibancioiu deleted the historical-balances-08-12 branch August 22, 2022 09:45

andreibancioiu mentioned this pull request Aug 25, 2022

Merge feat/historical into main multiversx/mx-chain-rosetta#33

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement queries for historical balances (state) #4371

Implement queries for historical balances (state) #4371

andreibancioiu commented Aug 12, 2022 •

edited

codecov-commenter commented Aug 12, 2022 •

edited

andreibancioiu Aug 17, 2022

andreibancioiu Aug 17, 2022

andreibancioiu Aug 17, 2022

iulianpascalau Aug 18, 2022

iulianpascalau Aug 18, 2022

iulianpascalau Aug 18, 2022

iulianpascalau Aug 18, 2022

iulianpascalau Aug 19, 2022

iulianpascalau Aug 19, 2022

iulianpascalau Aug 19, 2022

gabi-vuls left a comment

Implement queries for historical balances (state) #4371

Implement queries for historical balances (state) #4371

Conversation

andreibancioiu commented Aug 12, 2022 • edited

Description of the reasoning behind the pull request (what feature was missing / how the problem was manifesting itself / what was the motive behind the refactoring)

Proposed Changes

Testing procedure

codecov-commenter commented Aug 12, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

andreibancioiu commented Aug 12, 2022 •

edited

codecov-commenter commented Aug 12, 2022 •

edited