HIP-1056 Account BlockItem to RecordItem transformers #10249

HarshSawarkar · 2025-01-27T19:17:56Z

Description:

Added tranformers for CryptoAddLiveHash, CryptoDeleteAllowance, CryptoDeleteLiveHash

Related issue(s):

Fixes #10164

Notes for reviewer:

Checklist

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

…oDeleteLiveHash Signed-off-by: Harsh Sawarkar <[email protected]>

...va/com/hedera/mirror/importer/downloader/block/transformer/CryptoAddLiveHashTransformer.java

...om/hedera/mirror/importer/downloader/block/transformer/CryptoDeleteAllowanceTransformer.java

...com/hedera/mirror/importer/downloader/block/transformer/CryptoDeleteLiveHashTransformer.java

edwin-greene · 2025-01-28T21:25:28Z

Thanks for contributing. There are tests that need to be added, you can find example test cases in BlockFileTransformerTest

Signed-off-by: Harsh Sawarkar <[email protected]>

HarshSawarkar · 2025-01-29T18:41:27Z

Thanks for contributing. There are tests that need to be added, you can find example test cases in BlockFileTransformerTest

@edwin-greene Sure! I will update the tests for all the account transformers. In the meantime, could you please review the transformer classes and suggest any necessary changes?

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Signed-off-by: Harsh Sawarkar <[email protected]>

xin-hedera · 2025-01-30T18:16:25Z

...in/java/com/hedera/mirror/importer/downloader/block/transformer/CryptoCreateTransformer.java

+            for (var stateChange : stateChanges.getStateChangesList()) {
+                if (stateChange.getStateId() == STATE_ID_ACCOUNTS.getNumber() && stateChange.hasMapUpdate()) {
+                    var value = stateChange.getMapUpdate().getValue();
+                    if (value.hasAccountIdValue()) {


The key issue here is any account with state changes will be here, with exact same state id, change case, key case, and value case.

It's very common for any transaction having at least the following account map update in state changes

node account id

fee collector account id, 0.0.98

staking reward account 0.0.800

node reward account 0.0.801

There isn't a way for us to confidently identify the new account. Using the key in the transaction body can work in most cases however the network allows many accounts to use the same key so it's not a 100% reliable method.

We may have to ask for a change to add the new account id to CreateAccountOutput

@edwin-greene Please chase this down with the relevant teams

I agree that this is a miss in the protobuf definitions.
Suggested resolution:

CreateAccountOutput should be added to TransactionOutput

The created AccountID should be added to CreateAccountOutput

The Account ID newly assigned for any account that is created (via CryptoCreate, EVM CREATE2, or similar) should be recorded in the CreateAccountOutput.

To be complete.
This is, I believe, specific to account. In most cases the state changes (which immediately follow the triggering transaction) are more than sufficient to identify the created entity.

Unfortunately, as noted above, Account state changes follow every transaction, and it is unusually difficult to separate the create from other account updates; particularly if an account is created as a result of an EVM transaction.

xin-hedera · 2025-01-30T18:22:03Z

...in/java/com/hedera/mirror/importer/downloader/block/transformer/CryptoDeleteTransformer.java

+    @Override
+    protected void updateTransactionRecord(BlockItem blockItem, TransactionRecord.Builder transactionRecordBuilder) {
+        if (blockItem.transactionResult().getStatus() != SUCCESS) {
+            return;
+        }
+
+        for (var stateChanges : blockItem.stateChanges()) {
+            for (var stateChange : stateChanges.getStateChangesList()) {
+                if (stateChange.getStateId() == STATE_ID_ACCOUNTS.getNumber() && stateChange.hasMapUpdate()) {
+                    var value = stateChange.getMapUpdate().getValue();
+                    if (value.hasAccountValue()) {
+                        var accountDelete = value.getAccountValue();
+                        if (accountDelete.getDeleted()) {
+                            transactionRecordBuilder.getReceiptBuilder().setAccountID(accountDelete.getAccountId());
+                            return;
+                        }
+                    }
+                }
+            }
+        }
+    }


receipt.account_id shouldn't be set for crypto delete transaction

Suggested change

@Override

protected void updateTransactionRecord(BlockItem blockItem, TransactionRecord.Builder transactionRecordBuilder) {

if (blockItem.transactionResult().getStatus() != SUCCESS) {

return;

}

for (var stateChanges : blockItem.stateChanges()) {

for (var stateChange : stateChanges.getStateChangesList()) {

if (stateChange.getStateId() == STATE_ID_ACCOUNTS.getNumber() && stateChange.hasMapUpdate()) {

var value = stateChange.getMapUpdate().getValue();

if (value.hasAccountValue()) {

var accountDelete = value.getAccountValue();

if (accountDelete.getDeleted()) {

transactionRecordBuilder.getReceiptBuilder().setAccountID(accountDelete.getAccountId());

return;

}

}

}

}

}

}

xin-hedera · 2025-01-30T18:27:26Z

...in/java/com/hedera/mirror/importer/downloader/block/transformer/CryptoUpdateTransformer.java

+                if (stateChange.getStateId() == STATE_ID_ACCOUNTS.getNumber() && stateChange.hasMapUpdate()) {
+                    var value = stateChange.getMapUpdate().getValue();
+                    if (value.hasAccountIdValue()) {
+                        transactionRecordBuilder.getReceiptBuilder().setAccountID(value.getAccountIdValue());
+                    }
+                }


weird enough, receipt.account_id for crypto update transaction is set in record stream files.

again, the logic shares the same problem in the comment for crypto create transaction.

Think we can choose not to set it at all, or set it to the value from crypto update transaction body

Temporarily removing the updateTransactionRecord method until we identify a better solution.

Signed-off-by: Harsh Sawarkar <[email protected]>

codecov · 2025-01-31T00:30:59Z

Codecov Report

Attention: Patch coverage is 46.66667% with 16 lines in your changes missing coverage. Please review.

Project coverage is 92.28%. Comparing base (d61c0e4) to head (2df4bfe).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...der/block/transformer/CryptoCreateTransformer.java	11.11%	16 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #10249      +/-   ##
============================================
- Coverage     92.33%   92.28%   -0.06%     
- Complexity     7841     7855      +14     
============================================
  Files           954      961       +7     
  Lines         32952    32984      +32     
  Branches       4158     4163       +5     
============================================
+ Hits          30425    30438      +13     
- Misses         1548     1565      +17     
- Partials        979      981       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Signed-off-by: Harsh Sawarkar <[email protected]>

...va/com/hedera/mirror/importer/downloader/block/transformer/CryptoAddLiveHashTransformer.java

...m/hedera/mirror/importer/downloader/block/transformer/CryptoApproveAllowanceTransformer.java

steven-sheehy · 2025-01-31T17:10:04Z

...in/java/com/hedera/mirror/importer/downloader/block/transformer/CryptoCreateTransformer.java

+
+    @Override
+    protected void updateTransactionRecord(BlockItem blockItem, TransactionRecord.Builder transactionRecordBuilder) {
+        if (blockItem.transactionResult().getStatus() != SUCCESS) {


We'll want to update this with the new blockItem.successful() method from #10248 once it's merged.

I will update it once the PR is merged.

steven-sheehy · 2025-01-31T17:10:42Z

...in/java/com/hedera/mirror/importer/downloader/block/transformer/CryptoCreateTransformer.java

+            for (var stateChange : stateChanges.getStateChangesList()) {
+                if (stateChange.getStateId() == STATE_ID_ACCOUNTS.getNumber() && stateChange.hasMapUpdate()) {
+                    var value = stateChange.getMapUpdate().getValue();
+                    if (value.hasAccountIdValue()) {


@edwin-greene Please chase this down with the relevant teams

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Signed-off-by: Harsh Sawarkar <[email protected]>

Added tranformers for CryptoAddLiveHash, CryptoDeleteAllowance, Crypt…

cce3dbd

…oDeleteLiveHash Signed-off-by: Harsh Sawarkar <[email protected]>

steven-sheehy changed the title ~~Chore:Account BlockItem to RecordItem transformers~~ HIP-1056 Account BlockItem to RecordItem transformers Jan 27, 2025

steven-sheehy assigned HarshSawarkar Jan 27, 2025

steven-sheehy added the enhancement Type: New feature label Jan 27, 2025

steven-sheehy added this to the 0.123.0 milestone Jan 27, 2025

steven-sheehy reviewed Jan 27, 2025

View reviewed changes

HarshSawarkar added 2 commits January 30, 2025 00:02

Added tranformer for Account crypto

311084d

Signed-off-by: Harsh Sawarkar <[email protected]>

Added tranformer for Account crypto

9094268

Signed-off-by: Harsh Sawarkar <[email protected]>

Merge branch 'main' of https://github.com/hashgraph/hedera-mirror-node …

88b9dde

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

HarshSawarkar requested a review from steven-sheehy January 30, 2025 08:19

HarshSawarkar added 2 commits January 30, 2025 19:46

Merge branch 'main' of https://github.com/hashgraph/hedera-mirror-node …

cab6e84

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Modified the transformer

c69c1b7

Signed-off-by: Harsh Sawarkar <[email protected]>

xin-hedera requested changes Jan 30, 2025

View reviewed changes

Updated the requested changes.

2df4bfe

Signed-off-by: Harsh Sawarkar <[email protected]>

HarshSawarkar added 2 commits January 31, 2025 13:20

Merge branch 'main' of https://github.com/hashgraph/hedera-mirror-node …

c15fc81

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Added the tests for the transformers classes

ccae223

Signed-off-by: Harsh Sawarkar <[email protected]>

HarshSawarkar marked this pull request as ready for review January 31, 2025 15:18

HarshSawarkar requested a review from a team as a code owner January 31, 2025 15:18

HarshSawarkar requested a review from xin-hedera January 31, 2025 15:18

steven-sheehy requested changes Jan 31, 2025

View reviewed changes

HarshSawarkar added 2 commits January 31, 2025 22:47

Merge branch 'main' of https://github.com/hashgraph/hedera-mirror-node …

2ab0eeb

…into chore-HIP-1056-Account-BlockItem-to-RecordItem-transformers

Deleted the no-op transformers.

4505520

Signed-off-by: Harsh Sawarkar <[email protected]>

HarshSawarkar requested a review from steven-sheehy January 31, 2025 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HIP-1056 Account BlockItem to RecordItem transformers #10249

HIP-1056 Account BlockItem to RecordItem transformers #10249

HarshSawarkar commented Jan 27, 2025 •

edited by steven-sheehy

Loading

edwin-greene commented Jan 28, 2025

HarshSawarkar commented Jan 29, 2025 •

edited

Loading

xin-hedera Jan 30, 2025

steven-sheehy Jan 31, 2025

jsync-swirlds Jan 31, 2025

jsync-swirlds Jan 31, 2025

xin-hedera Jan 30, 2025

HarshSawarkar Jan 30, 2025

xin-hedera Jan 30, 2025

HarshSawarkar Jan 30, 2025

codecov bot commented Jan 31, 2025 •

edited

Loading

steven-sheehy Jan 31, 2025

HarshSawarkar Jan 31, 2025

steven-sheehy Jan 31, 2025

HIP-1056 Account BlockItem to RecordItem transformers #10249

Are you sure you want to change the base?

HIP-1056 Account BlockItem to RecordItem transformers #10249

Conversation

HarshSawarkar commented Jan 27, 2025 • edited by steven-sheehy Loading

edwin-greene commented Jan 28, 2025

HarshSawarkar commented Jan 29, 2025 • edited Loading

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

codecov bot commented Jan 31, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HarshSawarkar commented Jan 27, 2025 •

edited by steven-sheehy

Loading

HarshSawarkar commented Jan 29, 2025 •

edited

Loading

codecov bot commented Jan 31, 2025 •

edited

Loading