Skip to content
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

HTS: Add support for token transactions to Importer #1089

Merged
merged 28 commits into from
Oct 6, 2020
Merged

Conversation

Nana-EC
Copy link
Collaborator

@Nana-EC Nana-EC commented Sep 30, 2020

Detailed description:
To support the new Token HAPI, we should enhance our importer to import the new token transactions.

  • Add a TransactionHandler for each new transaction type
  • Add new repository and domain classes for Token, TokenAccount and TokenTransfer
  • Update EntityListener and its concrete implementations for the new entities with OnToken(), OnTokenAccount() and OnTokenTransfer()
  • Update EntityTypeEnum and EntityId with new TokenID
  • Update EntityRecordItemListener to parse the new transaction types and create appropriate domain objects for persistence
  • Update TransactionHandlerFactory to return appropriate Token transactionHandler on appropriate transactions type
  • Update TransactionTypeEnum with Token transaction types

Which issue(s) this PR fixes:
Fixes #1051

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

@Nana-EC Nana-EC added enhancement Type: New feature P1 database Area: Database labels Sep 30, 2020
@Nana-EC Nana-EC added this to the Mirror 0.20.0 milestone Sep 30, 2020
@Nana-EC Nana-EC requested a review from a team September 30, 2020 00:02
@Nana-EC Nana-EC self-assigned this Sep 30, 2020
@steven-sheehy steven-sheehy changed the title Add HTS Transaction Logic to Importer HTS: Add support for token transactions to Importer Sep 30, 2020
@steven-sheehy steven-sheehy added parser Area: File parsing transactions File: Related to processing of transaction record stream file P2 and removed database Area: Database P1 labels Sep 30, 2020
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@06b6d6a). Click here to learn what that means.
The diff coverage is 89.70%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1089   +/-   ##
=========================================
  Coverage          ?   87.71%           
  Complexity        ?      234           
=========================================
  Files             ?      219           
  Lines             ?     5233           
  Branches          ?      575           
=========================================
  Hits              ?     4590           
  Misses            ?      448           
  Partials          ?      195           
Impacted Files Coverage Δ Complexity Δ
...va/com/hedera/mirror/importer/domain/EntityId.java 94.87% <ø> (ø) 0.00 <0.00> (?)
...om/hedera/mirror/importer/domain/TokenBalance.java 83.33% <ø> (ø) 0.00 <0.00> (?)
...sactionhandler/TokenCreateTransactionsHandler.java 75.00% <75.00%> (ø) 0.00 <0.00> (?)
...sactionhandler/TokenUpdateTransactionsHandler.java 75.00% <75.00%> (ø) 0.00 <0.00> (?)
.../java/com/hedera/mirror/importer/util/Utility.java 78.68% <77.77%> (ø) 0.00 <0.00> (?)
.../mirror/importer/domain/TokenFreezeStatusEnum.java 83.33% <83.33%> (ø) 0.00 <0.00> (?)
...era/mirror/importer/domain/TokenKycStatusEnum.java 83.33% <83.33%> (ø) 0.00 <0.00> (?)
...m/hedera/mirror/importer/domain/TokenTransfer.java 86.66% <86.66%> (ø) 0.00 <0.00> (?)
...parser/record/entity/EntityRecordItemListener.java 92.19% <87.60%> (ø) 0.00 <0.00> (?)
...om/hedera/mirror/importer/domain/TokenAccount.java 88.23% <88.23%> (ø) 0.00 <0.00> (?)
... and 18 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 06b6d6a...6c61ddd. Read the comment docs.

@Nana-EC Nana-EC marked this pull request as ready for review September 30, 2020 21:44
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review still in progress, but submitting what I have so you can address.

@@ -239,6 +255,21 @@ public void onLiveHash(LiveHash liveHash) throws ImporterException {
liveHashes.add(liveHash);
}

@Override
public void onToken(Token token) throws ImporterException {
tokenRepository.save(token);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two repository calls will definitely slow things down. I haven't had time to verify if multiple calls to save will be internally batched. This might be fine for now but we'll need to revisit this next sprint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This is best path to get the ball rolling but we definitely need to scale.

Challenge is each update needs to ensure it has the correct information from current state before updating.
e.g. A token gets created with a freeze and Kyc settings to default to FROZEN and REVOKED.
An account then Associates itself with the token, it will be frozen and revoked.
Say accounts it gets Unfrozen and then soon after gets GrantedKyc.
We need to make sure the persistence for the KycGrant picks the fact that it was unfrozen and doesn't accidentally revert the freeze state change.
That's why for now I'm just saving immediately, just like Entity updates do.

Currently using Spring cache so this should give us some breathing room to explore the right solution for Accounts and Tokens

@Nana-EC Nana-EC requested a review from steven-sheehy October 5, 2020 21:45
Nana-EC added 10 commits October 6, 2020 14:09
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Nana-EC added 16 commits October 6, 2020 14:09
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
@Nana-EC Nana-EC requested a review from steven-sheehy October 6, 2020 20:07
Signed-off-by: Nana-EC <56320167+Nana-EC@users.noreply.github.com>
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Nana-EC Nana-EC merged commit 53237aa into master Oct 6, 2020
@Nana-EC Nana-EC deleted the hts-importer branch October 6, 2020 20:50
@steven-sheehy steven-sheehy added the hts Hedera Token Service label Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type: New feature hts Hedera Token Service P2 parser Area: File parsing transactions File: Related to processing of transaction record stream file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTS: Import token transactions
2 participants