Workaround the missing disappearing token transfer issue #3289

xin-hedera · 2022-02-11T21:03:57Z

Description:

Add workaround for the missing disappearing token transfer issue
Improve nft balance tracking performance
Only show token related data for tokens created after genesis timestmap
Update rosetta-cli check:data configuration to sync from index 1
Remove genesis token balance retrieval logic in get-genesis-balance.sh
Update rosetta github workflow to not generate genesis balance JSON file
Fix bug that importer start date can't be changed
Enable supervisorctl
Fix issue that the supervisord process doesn't get SIGTERM sent by docker stop
Set stopwaitsecs for importer and postgres for graceful shutdown
Update postgres config for normal operation

Related issue(s):

Fixes #

Notes for reviewer:

Note the failing check:construction in our workflow is due to the STARTDATE env in supervisord.conf set to epoch and can't be overridden, so importer always syncs from testnet genesis.

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

…e after the token is deleted Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera · 2022-02-11T21:04:51Z

.github/workflows/rosetta-api.yml

-      - name: Get Genesis Account Balances
-        run: ./scripts/validation/get-genesis-balance.sh testnet
-


applied the logic to check:data from block 1 so no longer needs the genesis balance JSON file

codecov · 2022-02-11T21:05:55Z

Codecov Report

Merging #3289 (0105c06) into main (70a9cb4) will decrease coverage by 19.60%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##               main    #3289       +/-   ##
=============================================
- Coverage     88.17%   68.56%   -19.61%     
+ Complexity     2508      845     -1663     
=============================================
  Files           502      161      -341     
  Lines         14143     2761    -11382     
  Branches       1306      159     -1147     
=============================================
- Hits          12470     1893    -10577     
+ Misses         1365      804      -561     
+ Partials        308       64      -244

Impacted Files	Coverage Δ
hedera-mirror-rosetta/app/persistence/account.go
hedera-mirror-rosetta/app/persistence/block.go
...ra-mirror-rosetta/app/persistence/domain/entity.go
...dera-mirror-rosetta/app/persistence/transaction.go
...era-mirror-rosetta/app/services/account_service.go
errors/dbError.js
...a/mirror/grpc/listener/NotifyingTopicListener.java
...mporter/downloader/CommonDownloaderProperties.java
model/customFee.js
viewmodel/nftTransferViewModel.js
... and 333 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70a9cb4...0105c06. Read the comment docs.

xin-hedera · 2022-02-11T21:05:58Z

hedera-mirror-rosetta/app/persistence/account.go

+                              ), '[]') as token_values,
+                              (
+                                select coalesce(json_agg(json_build_object(
+                                  'associated', associated,
+                                  'decimals', decimals,
+                                  'token_id', token_id,
+                                  'type',  type
+                                ) order by token_id), '[]')
+                                from (
+                                  select distinct on (t.token_id) ta.associated, t.decimals, t.token_id, t.type
+                                  from token_account ta
+                                  join token t on t.token_id = ta.token_id
+                                  join genesis on t.created_timestamp > genesis.timestamp
+                                  where account_id = @account_id and ta.modified_timestamp <= @end
+                                  order by t.token_id, ta.modified_timestamp desc
+                                ) as associations
+                              ) as token_associations`


get the latest state of the token associations for an account X <= consensus timestamp T for tokens created after genesis timestamp

xin-hedera · 2022-02-11T21:07:38Z

hedera-mirror-rosetta/app/persistence/transaction.go

+	for _, txn := range transactions {
+		if txn.Type == transactionTypeTokenDissociate && IsTransactionResultSuccessful(int32(txn.Result)) {
+			hasSuccessTokenDissociate = true
+			break
+		}
+	}
+	if !hasSuccessTokenDissociate {
+		return nil
+	}


don't even run the SQL query if there's no successful token dissociate.

xin-hedera · 2022-02-11T21:09:13Z

hedera-mirror-rosetta/scripts/validation/demo/validation.json

+    "historical_balance_enabled": true,
+    "inactive_discrepancy_search_disabled": false,
+    "reconciliation_disabled": false,
+    "start_index": 1,


with this config, check:data no longer needs the bootstrap genesis balances JSON file, nor the made up genesis block.

xin-hedera · 2022-02-11T21:09:45Z

hedera-mirror-rosetta/scripts/validation/get-genesis-balance.sh

-  tb.account_id in (:account_ids) and
-  balance <> 0
-EOF
-)


there won't be token balances at genesis with the changes in go code

hedera-mirror-rosetta/app/persistence/transaction.go

Signed-off-by: Xin Li <xin.li@hedera.com>

…ostgres parrams Signed-off-by: Xin Li <xin.li@hedera.com>

Signed-off-by: Xin Li <xin.li@hedera.com>

…ervisord Signed-off-by: Xin Li <xin.li@hedera.com>

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera · 2022-02-16T15:58:58Z

hedera-mirror-rosetta/app/persistence/transaction.go

+	accountIds := make([]int64, 0)
 	for _, transfer := range cryptoTransfers {
-		key := transfer.AccountId.EncodedId
-		cryptoTransferMap[key] = hbarTransfer{
+		accountId := transfer.AccountId.EncodedId
+		if _, ok := cryptoTransferMap[accountId]; !ok {
+			accountIds = append(accountIds, accountId)
+		}
+		cryptoTransferMap[accountId] = hbarTransfer{
 			AccountId: transfer.AccountId,
-			Amount:    transfer.Amount + cryptoTransferMap[key].Amount,
+			Amount:    transfer.Amount + cryptoTransferMap[accountId].Amount,
 		}
 	}

 	adjusted := make([]hbarTransfer, 0, len(cryptoTransfers))
-	for key, aggregated := range cryptoTransferMap {
-		amount := aggregated.Amount - nonFeeTransferMap[key]
+	for _, accountId := range accountIds {
+		aggregated := cryptoTransferMap[accountId]
+		amount := aggregated.Amount - nonFeeTransferMap[accountId]


make the crypto transfers in a consistent order

xin-hedera · 2022-02-16T16:01:14Z

hedera-mirror-rosetta/app/services/account_service.go

 ) (*rTypes.AccountCoinsResponse, *rTypes.Error) {
 	return nil, errors.ErrNotImplemented
 }

-// getAdditionalTokenBalances get the additional token balances with 0 amount for tokens the account has ever owned by


no longer needed since now AccountRepository.RetrieveBalanceAtBlock also handles the ever owned tokens and returns 0 amount for those tokens in a single call

xin-hedera · 2022-02-16T16:02:30Z

hedera-mirror-rosetta/build/postgresql.conf

 listen_addresses = '*'
+log_checkpoints = on
+max_connections = 200


the default max connections in rosetta config is 100, the default of pg server is 100. under load, quite some queries can fail since the db server refuses to open more connections for rosetta server as certain ratio of the server's max connections are reserved for superuser

xin-hedera · 2022-02-16T16:05:11Z

hedera-mirror-rosetta/build/run_supervisord.sh

@@ -3,12 +3,12 @@ set -eo pipefail

 function run_offline_mode() {
  echo "Running in offline mode"
-  supervisord --configuration /app/supervisord-offline.conf
+  exec supervisord --configuration /app/supervisord-offline.conf


with exec, the supervisord process can receive signals, so we can do docker stop -t 90 <container_id> to gracefully shutdown the services

xin-hedera · 2022-02-16T16:06:03Z

hedera-mirror-rosetta/build/supervisord-offline.conf

+[supervisorctl]
+serverurl=unix:///tmp/supervisor.sock
+
+[rpcinterface:supervisor]
+supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface
+


complete the config to enable supervisorctl which works similarly as systemctl

xin-hedera · 2022-02-16T16:06:21Z

hedera-mirror-rosetta/build/supervisord-offline.conf

@@ -17,5 +23,4 @@ autorestart=true
 redirect_stderr=true
 stdout_logfile=/dev/fd/1
 stdout_logfile_maxbytes=0
-stopsignal=SIGTERM


SIGTERM is the default stopsignal supervisord sends

hedera-mirror-rosetta/app/persistence/domain/entity_test.go

hedera-mirror-rosetta/build/supervisord.conf

Signed-off-by: Xin Li <xin.li@hedera.com>

Nana-EC

Nice work, some q's and suggestions on 1st
Still digging into test side

hedera-mirror-rosetta/app/persistence/common.go

hedera-mirror-rosetta/app/persistence/domain/entity.go

docs/rosetta/README.md

hedera-mirror-rosetta/app/persistence/account.go

hedera-mirror-rosetta/app/persistence/transaction.go

steven-sheehy

LGTM! Great work on this

Signed-off-by: Xin Li <xin.li@hedera.com>

Nana-EC

LGTM

Signed-off-by: Xin Li <xin.li@hedera.com>

sonarcloud · 2022-02-16T19:04:47Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
9.2% Duplication

- Add workaround for the missing disappearing token transfer issue - Improve nft balance tracking performance - Only show token related data for tokens created after genesis timestmap - Update rosetta-cli check:data configuration to sync from index 1 - Remove genesis token balance retrieval logic in get-genesis-balance.sh - Update rosetta github workflow to not generate genesis balance JSON file - Fix bug that importer start date can't be changed - Enable supervisorctl - Fix issue that the supervisord process doesn't get SIGTERM sent by docker stop - Set stopwaitsecs for importer and postgres for graceful shutdown - Update postgres config for normal operation Signed-off-by: Xin Li <xin.li@hedera.com>

- Add workaround for the missing disappearing token transfer issue - Improve nft balance tracking performance - Only show token related data for tokens created after genesis timestmap - Update rosetta-cli check:data configuration to sync from index 1 - Remove genesis token balance retrieval logic in get-genesis-balance.sh - Update rosetta github workflow to not generate genesis balance JSON file - Fix bug that importer start date can't be changed - Enable supervisorctl - Fix issue that the supervisord process doesn't get SIGTERM sent by docker stop - Set stopwaitsecs for importer and postgres for graceful shutdown - Update postgres config for normal operation Signed-off-by: Xin Li <xin.li@hedera.com> Signed-off-by: Matheus DallRosa <matheus.dallrosa@swirlds.com>

add logic to create token transfer / nft transfer for token dissociat…

f50c5ed

…e after the token is deleted Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera self-assigned this Feb 11, 2022

xin-hedera added rosetta Area: Rosetta API P1 labels Feb 11, 2022

xin-hedera commented Feb 11, 2022

View reviewed changes

steven-sheehy added this to the 0.51.0 milestone Feb 11, 2022

xin-hedera commented Feb 11, 2022

View reviewed changes

xin-hedera requested a review from a team February 11, 2022 21:10

steven-sheehy reviewed Feb 14, 2022

View reviewed changes

hedera-mirror-rosetta/app/persistence/transaction.go Outdated Show resolved Hide resolved

hedera-mirror-rosetta/app/persistence/transaction.go Outdated Show resolved Hide resolved

steven-sheehy added the bug Type: Something isn't working label Feb 14, 2022

xin-hedera added 4 commits February 15, 2022 12:14

test cases, address review feedback

5d18ddb

Signed-off-by: Xin Li <xin.li@hedera.com>

change nft balance tracking to not show serial numbers

77f012a

Signed-off-by: Xin Li <xin.li@hedera.com>

remove STARTDATE env from supervisord.conf, update normal operation p…

eb0e074

…ostgres parrams Signed-off-by: Xin Li <xin.li@hedera.com>

graceful shutdown, enable supervisorctl, increase pg max_connections

16d729d

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera force-pushed the missing-disappearing-token-transfer-workaround branch from 3f36a7b to 16d729d Compare February 16, 2022 02:49

xin-hedera added 2 commits February 15, 2022 21:13

exec supervisord to replace bash, so signals are properly sent to sup…

ac7e802

…ervisord Signed-off-by: Xin Li <xin.li@hedera.com>

fix doc, cleanup

b24959e

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera marked this pull request as ready for review February 16, 2022 15:48

xin-hedera commented Feb 16, 2022

View reviewed changes

steven-sheehy reviewed Feb 16, 2022

View reviewed changes

hedera-mirror-rosetta/app/persistence/domain/entity_test.go Outdated Show resolved Hide resolved

hedera-mirror-rosetta/build/supervisord.conf Show resolved Hide resolved

address review feedback and clean up

8ec85a7

Signed-off-by: Xin Li <xin.li@hedera.com>

add mainnet validation.json and README

ae26cb1

Signed-off-by: Xin Li <xin.li@hedera.com>

Nana-EC requested changes Feb 16, 2022

View reviewed changes

steven-sheehy previously approved these changes Feb 16, 2022

View reviewed changes

address review feedback

8b22968

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera dismissed steven-sheehy’s stale review via 8b22968 February 16, 2022 18:01

xin-hedera requested review from Nana-EC and steven-sheehy February 16, 2022 18:07

steven-sheehy previously approved these changes Feb 16, 2022

View reviewed changes

handle historical self nft transfer

bd03546

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera dismissed steven-sheehy’s stale review via bd03546 February 16, 2022 18:32

xin-hedera requested a review from steven-sheehy February 16, 2022 18:32

Nana-EC previously approved these changes Feb 16, 2022

View reviewed changes

fix supervisord.conf, no such user "dummy"

0105c06

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera dismissed Nana-EC’s stale review via 0105c06 February 16, 2022 18:59

steven-sheehy approved these changes Feb 16, 2022

View reviewed changes

Nana-EC approved these changes Feb 16, 2022

View reviewed changes

xin-hedera merged commit 903df34 into main Feb 16, 2022

xin-hedera deleted the missing-disappearing-token-transfer-workaround branch February 16, 2022 19:27

steven-sheehy linked an issue Feb 16, 2022 that may be closed by this pull request

Rosetta reconciliation performance #3297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workaround the missing disappearing token transfer issue #3289

Workaround the missing disappearing token transfer issue #3289

xin-hedera commented Feb 11, 2022 •

edited

xin-hedera Feb 11, 2022

codecov bot commented Feb 11, 2022 •

edited

xin-hedera Feb 11, 2022 •

edited

xin-hedera Feb 11, 2022

xin-hedera Feb 11, 2022

xin-hedera Feb 11, 2022

xin-hedera Feb 16, 2022

xin-hedera Feb 16, 2022

xin-hedera Feb 16, 2022 •

edited

xin-hedera Feb 16, 2022

xin-hedera Feb 16, 2022

xin-hedera Feb 16, 2022

Nana-EC left a comment

steven-sheehy left a comment

Nana-EC left a comment

sonarcloud bot commented Feb 16, 2022

		- name: Get Genesis Account Balances
		run: ./scripts/validation/get-genesis-balance.sh testnet

Workaround the missing disappearing token transfer issue #3289

Workaround the missing disappearing token transfer issue #3289

Conversation

xin-hedera commented Feb 11, 2022 • edited

Choose a reason for hiding this comment

codecov bot commented Feb 11, 2022 • edited

Codecov Report

xin-hedera Feb 11, 2022 • edited

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

xin-hedera Feb 16, 2022 • edited

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

Nana-EC left a comment

Choose a reason for hiding this comment

steven-sheehy left a comment

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Feb 16, 2022

xin-hedera commented Feb 11, 2022 •

edited

codecov bot commented Feb 11, 2022 •

edited

xin-hedera Feb 11, 2022 •

edited

xin-hedera Feb 16, 2022 •

edited