-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Workaround the missing disappearing token transfer issue #3289
Workaround the missing disappearing token transfer issue #3289
Conversation
…e after the token is deleted Signed-off-by: Xin Li <xin.li@hedera.com>
- name: Get Genesis Account Balances | ||
run: ./scripts/validation/get-genesis-balance.sh testnet | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied the logic to check:data
from block 1 so no longer needs the genesis balance JSON file
Codecov Report
@@ 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 Continue to review full report at Codecov.
|
), '[]') 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` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get the latest state of the token associations for an account X <= consensus timestamp T for tokens created after genesis timestamp
for _, txn := range transactions { | ||
if txn.Type == transactionTypeTokenDissociate && IsTransactionResultSuccessful(int32(txn.Result)) { | ||
hasSuccessTokenDissociate = true | ||
break | ||
} | ||
} | ||
if !hasSuccessTokenDissociate { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't even run the SQL query if there's no successful token dissociate.
"historical_balance_enabled": true, | ||
"inactive_discrepancy_search_disabled": false, | ||
"reconciliation_disabled": false, | ||
"start_index": 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this config, check:data
no longer needs the bootstrap genesis balances JSON file, nor the made up genesis block.
tb.account_id in (:account_ids) and | ||
balance <> 0 | ||
EOF | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there won't be token balances at genesis with the changes in go code
Signed-off-by: Xin Li <xin.li@hedera.com>
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>
3f36a7b
to
16d729d
Compare
…ervisord Signed-off-by: Xin Li <xin.li@hedera.com>
Signed-off-by: Xin Li <xin.li@hedera.com>
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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make the crypto transfers in a consistent order
) (*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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed since now AccountRepository.RetrieveBalanceAtBlock also handles the ever owned tokens and returns 0 amount for those tokens in a single call
listen_addresses = '*' | ||
log_checkpoints = on | ||
max_connections = 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with exec
, the supervisord process can receive signals, so we can do docker stop -t 90 <container_id>
to gracefully shutdown the services
[supervisorctl] | ||
serverurl=unix:///tmp/supervisor.sock | ||
|
||
[rpcinterface:supervisor] | ||
supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
complete the config to enable supervisorctl which works similarly as systemctl
@@ -17,5 +23,4 @@ autorestart=true | |||
redirect_stderr=true | |||
stdout_logfile=/dev/fd/1 | |||
stdout_logfile_maxbytes=0 | |||
stopsignal=SIGTERM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIGTERM is the default stopsignal supervisord sends
Signed-off-by: Xin Li <xin.li@hedera.com>
Signed-off-by: Xin Li <xin.li@hedera.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, some q's and suggestions on 1st
Still digging into test side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Great work on this
Signed-off-by: Xin Li <xin.li@hedera.com>
Signed-off-by: Xin Li <xin.li@hedera.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Xin Li <xin.li@hedera.com>
SonarCloud Quality Gate failed. 0 Bugs No Coverage information |
- 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 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 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 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>
Description:
rosetta-cli
check:data configuration to sync from index 1supervisorctl
supervisord
process doesn't get SIGTERM sent bydocker stop
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