Alias fee schedule support #2774

anighanta · 2022-01-19T03:18:39Z

Description:

Related issue(s):

addresses #2691

Notes for reviewer:

Checklist

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

…ure scneraios in sigRequirements Signed-off-by: anighanta <[email protected]>

Signed-off-by: anighanta <[email protected]>

Neeharika-Sompalli · 2022-01-19T03:28:33Z

hedera-node/src/main/java/com/hedera/services/sigs/order/CodeOrderResultFactory.java

@@ -120,8 +127,15 @@
 return MISSING_FEE_COLLECTOR_RESULT;
 }

+ @Override
+ public SigningOrderResult<ResponseCodeEnum> forMissingTokenTreasury() {
+ return MISSING_TOKEN_TRASURY;


typo TREASURY

Neeharika-Sompalli · 2022-01-19T03:33:00Z

hedera-node/src/main/java/com/hedera/services/store/tokens/HederaTokenStore.java

@@ -584,14 +585,26 @@ public ResponseCodeEnum update(final TokenUpdateTransactionBody changes, final l

 private AliasLookup checkNewAutoRenewAccount(final TokenUpdateTransactionBody changes) {
 if (changes.hasAutoRenewAccount()) {
- return lookUpAccountId(changes.getAutoRenewAccount(), INVALID_AUTORENEW_ACCOUNT);
+ final var autoRenewLookUp = lookUpAccountId(changes.getAutoRenewAccount(), INVALID_ALIAS_KEY);


should return INVALID_AUTORENEW_ACCOUNT

I made it into 2 parts. first we look up the alias key. If found and its not a usable accountId we return INVALID_AUTORENEW_ACCOUNT but if that alias is not found we return INVALID_ALIAS_KEY. I open to change it though.

Neeharika-Sompalli · 2022-01-19T03:35:09Z

hedera-node/src/main/java/com/hedera/services/store/tokens/HederaTokenStore.java

+ if (treasuryLookUp.response() != OK) {
+ return treasuryLookUp;
+ } else {
+ final var validity = usableOrElse(treasuryLookUp.resolvedId(), INVALID_TREASURY_ACCOUNT_FOR_TOKEN);


can we do usableOrElse inside lookUpAccountId?

that is what we did previously.

Neeharika-Sompalli · 2022-01-19T03:35:53Z

hedera-node/src/main/java/com/hedera/services/txns/consensus/SubmitMessageTransitionLogic.java

+ /* I dont think we need these checks as it is handled just aftet these checks */
+// if (chunkAccountIDLookup.response() != OK) {
+// transactionContext.setStatus(chunkAccountIDLookup.response());
+// }
+// if (transactionIDLookup.response() != OK) {
+// transactionContext.setStatus(transactionIDLookup.response());
+// }


why are we removing these ?

I was actually not sure why we didn't return after the err response for each.

Neeharika-Sompalli · 2022-01-19T03:36:22Z

hedera-node/src/main/java/com/hedera/services/txns/consensus/TopicUpdateTransitionLogic.java

@@ -204,7 +204,7 @@ private boolean canApplyNewAutoRenewAccount(ConsensusUpdateTopicTransactionBody
 return true;
 }

- final var result = ledger.lookUpAccountId(op.getAutoRenewAccount(), INVALID_AUTORENEW_ACCOUNT);
+ final var result = ledger.lookUpAccountId(op.getAutoRenewAccount());


Need to give INVALID_AUTORENEW_ACCOUNT as response

Again, that will be handled just below this on line 222

Neeharika-Sompalli · 2022-01-19T03:38:50Z

hedera-node/src/main/java/com/hedera/services/txns/crypto/CryptoDeleteTransitionLogic.java

@@ -87,6 +86,12 @@ public void doStateTransition() {
 txnCtx.setStatus(result.response());
 return;
 }
+ var validity = ledger.usableOrElse(result.resolvedId(), INVALID_ACCOUNT_ID);


may be we should give result.response() instead of INVALID_ACCOUNT_ID . Same in all places

here result.response() will be OK right ?

Neeharika-Sompalli · 2022-01-19T03:38:59Z

hedera-node/src/main/java/com/hedera/services/txns/crypto/CryptoDeleteTransitionLogic.java

-
- if (ledger.isDetached(id) || ledger.isDetached(beneficiary)) {
- txnCtx.setStatus(ACCOUNT_EXPIRED_AND_PENDING_REMOVAL);
+ validity = ledger.usableOrElse(result.resolvedId(), INVALID_ACCOUNT_ID);


Neeharika-Sompalli · 2022-01-19T03:39:10Z

hedera-node/src/main/java/com/hedera/services/txns/crypto/CryptoUpdateTransitionLogic.java

@@ -171,6 +171,7 @@ private ResponseCodeEnum sanityCheck(AccountID target, HederaAccountCustomizer c
 if (result.response() != OK) {
 return result.response();
 }
+ return ledger.usableOrElse(result.resolvedId(), INVALID_ACCOUNT_ID);


here as well

Neeharika-Sompalli · 2022-01-19T03:39:25Z

hedera-node/src/main/java/com/hedera/services/txns/token/process/Creation.java

+// final var customFeeList = op.getCustomFeesList();
+// for (var fee : customFeeList) {
+// if (fee.hasFeeCollectorAccountId()) {
+// final var feeCollector = fee.getFeeCollectorAccountId();
+// final var feeCollectorNum = accountStore.getAccountNumFromAlias(feeCollector.getAlias(), feeCollector.getAccountNum());
+// final var feeCollectorId = new Id(feeCollector.getShardNum(), feeCollector.getRealmNum(), feeCollectorNum);
+// accountStore.loadAccountOrFailWith(feeCollectorId, INVALID_ACCOUNT_ID);
+// }
+// }


can these be removed?

yup, can be removed. The feeCollector validation is actually already done in sigRequirements.

Neeharika-Sompalli · 2022-01-19T03:39:44Z

hedera-node/src/test/java/com/hedera/services/ledger/HederaLedgerTest.java

+// given(validator.isAfterConsensusSecond(expiry)).willReturn(false);
+// given(accountsLedger.exists(testId)).willReturn(true);
+// given(accountsLedger.get(testId, IS_DELETED)).willReturn(false);
+// given(accountsLedger.get(testId, IS_SMART_CONTRACT)).willReturn(false);
+// given(accountsLedger.get(testId, BALANCE)).willReturn(balance);
+// given(accountsLedger.get(testId, EXPIRY)).willReturn(expiry);
+//
+// assertEquals(ACCOUNT_EXPIRED_AND_PENDING_REMOVAL, subject.usableOrElse(testId, ACCOUNT_DELETED));


This needs fixing. Will uncomment and fix

Signed-off-by: anighanta <[email protected]>

codecov · 2022-01-19T16:02:29Z

Codecov Report

Merging #2774 (8bbf474) into 02691-M-ExtendAliasSupport (4bd9c10) will increase coverage by 0.02%.
The diff coverage is 98.86%.

❗ Current head 8bbf474 differs from pull request most recent head 08aa26b. Consider uploading reports for the commit 08aa26b to get more accurate results

@@                       Coverage Diff                        @@
##             02691-M-ExtendAliasSupport    #2774      +/-   ##
================================================================
+ Coverage                         95.67%   95.70%   +0.02%     
- Complexity                         9676     9679       +3     
================================================================
  Files                               700      700              
  Lines                             26570    26569       -1     
  Branches                           2738     2733       -5     
================================================================
+ Hits                              25422    25428       +6     
+ Misses                              601      598       -3     
+ Partials                            547      543       -4

Impacted Files	Coverage Δ
.../hedera/services/store/schedule/ScheduleStore.java	`100.00% <ø> (ø)`
...a/com/hedera/services/store/tokens/TokenStore.java	`80.76% <ø> (ø)`
...ces/txns/token/TokenDissociateTransitionLogic.java	`100.00% <ø> (ø)`
...s/txns/consensus/SubmitMessageTransitionLogic.java	`90.19% <90.90%> (+6.58%)`	⬆️
.../java/com/hedera/services/ledger/HederaLedger.java	`94.84% <100.00%> (+0.51%)`	⬆️
...dera/services/queries/meta/GetTxnRecordAnswer.java	`100.00% <100.00%> (ø)`
...era/services/queries/validation/QueryFeeCheck.java	`98.80% <100.00%> (ø)`
...ra/services/sigs/order/CodeOrderResultFactory.java	`100.00% <100.00%> (ø)`
...om/hedera/services/sigs/order/SigRequirements.java	`93.64% <100.00%> (-0.38%)`	⬇️
...services/state/logic/AwareNodeDiligenceScreen.java	`100.00% <100.00%> (+3.44%)`	⬆️
... and 22 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 4bd9c10...08aa26b. Read the comment docs.

Signed-off-by: anighanta <[email protected]>

Neeharika-Sompalli · 2022-01-19T19:50:23Z

hedera-node/src/main/java/com/hedera/services/queries/crypto/GetAccountBalanceAnswer.java

@@ -120,7 +120,7 @@ private ResponseCodeEnum validityOf(
 } else if (op.hasAccountID()) {
 final var result = aliasManager.lookUpAccountID(op.getAccountID());
 if (result.response() != OK) {
- return result.response();
+ return INVALID_ACCOUNT_ID;


should this be result.response()

Signed-off-by: anighanta <[email protected]>

anighanta added 5 commits January 18, 2022 16:24

add alias support to feeSchedules and added some more meaningful fail…

e2352a1

…ure scneraios in sigRequirements Signed-off-by: anighanta <[email protected]>

cleanUp of alias lookUp validation

eed99ba

Signed-off-by: anighanta <[email protected]>

fix fialing eet specs and address sonar code smells

9dd921f

Signed-off-by: anighanta <[email protected]>

fix unit test

9f2be60

Signed-off-by: anighanta <[email protected]>

fix fialing EET specs in CI

a5f8542

Signed-off-by: anighanta <[email protected]>

anighanta requested a review from Neeharika-Sompalli January 19, 2022 03:18

Neeharika-Sompalli reviewed Jan 19, 2022

View reviewed changes

address review comments

2a99c9d

Signed-off-by: anighanta <[email protected]>

anighanta added 5 commits January 19, 2022 21:39

merge master

55d94a1

Signed-off-by: anighanta <[email protected]>

resolve sonar code smells

23a826d

Signed-off-by: anighanta <[email protected]>

address review comments

dfd97af

Signed-off-by: anighanta <[email protected]>

address review comments

62c81f1

Signed-off-by: anighanta <[email protected]>

address review comments

8bbf474

Signed-off-by: anighanta <[email protected]>

Neeharika-Sompalli approved these changes Jan 19, 2022

View reviewed changes

anighanta added 2 commits January 20, 2022 01:34

remove commented out code

d6cc40f

Signed-off-by: anighanta <[email protected]>

Merge branch '02691-M-ExtendAliasSupport' into alias-FeeScheduleSupport

08aa26b

anighanta merged commit 25e4d6b into 02691-M-ExtendAliasSupport Jan 19, 2022

anighanta deleted the alias-FeeScheduleSupport branch January 19, 2022 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alias fee schedule support #2774

Alias fee schedule support #2774

anighanta commented Jan 19, 2022 •

edited

Loading

Neeharika-Sompalli Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

Neeharika-Sompalli Jan 19, 2022

anighanta Jan 19, 2022

codecov bot commented Jan 19, 2022 •

edited

Loading

Neeharika-Sompalli Jan 19, 2022

Alias fee schedule support #2774

Alias fee schedule support #2774

Conversation

anighanta commented Jan 19, 2022 • 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

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

codecov bot commented Jan 19, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

anighanta commented Jan 19, 2022 •

edited

Loading

codecov bot commented Jan 19, 2022 •

edited

Loading