Check for deleted accounts in crypto allowances

hedera-node/hedera-network-admin-service-impl/src/main/java/com/hedera/node/app/service/networkadmin/impl/handlers/NetworkGetAccountDetailsHandler.java

@@ -16,11 +16,13 @@
 
 package com.hedera.node.app.service.networkadmin.impl.handlers;
 
+import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_DELETED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.FAIL_INVALID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
 import static com.hedera.hapi.node.base.ResponseType.COST_ANSWER;
 import static com.hedera.node.app.spi.validation.Validations.mustExist;
+import static com.hedera.node.app.spi.workflows.PreCheckException.validateTruePreCheck;
 import static java.util.Objects.requireNonNull;
 
 import com.hedera.hapi.node.base.AccountID;
@@ -96,8 +98,9 @@ public void validate(@NonNull final QueryContext context) throws PreCheckExcepti
 
         // The account must exist for that transaction ID
         final var accountStore = context.createStore(ReadableAccountStore.class);
-        final var accountMetadata = accountStore.getAccountById(op.accountIdOrThrow());
-        mustExist(accountMetadata, INVALID_ACCOUNT_ID);
+        final var account = accountStore.getAccountById(op.accountIdOrThrow());
+        mustExist(account, INVALID_ACCOUNT_ID);
+        validateTruePreCheck(!account.deleted(), ACCOUNT_DELETED);
     }
 
     @Override

hedera-node/hedera-network-admin-service-impl/src/test/java/com/hedera/node/app/service/networkadmin/impl/test/handlers/NetworkAdminHandlerTestBase.java

@@ -141,7 +141,7 @@ public class NetworkAdminHandlerTestBase {
     private DeduplicationCache dedupeCache;
 
     @Mock
-    WorkingStateAccessor wsa;
+    private WorkingStateAccessor wsa;
 
     @Mock
     private ConfigProvider props;

hedera-node/hedera-network-admin-service-impl/src/test/java/com/hedera/node/app/service/networkadmin/impl/test/handlers/NetworkGetAccountDetailsHandlerTest.java

@@ -16,6 +16,7 @@
 
 package com.hedera.node.app.service.networkadmin.impl.test.handlers;
 
+import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_DELETED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID;
 import static com.hedera.node.app.spi.fixtures.Assertions.assertThrowsPreCheck;
 import static org.assertj.core.api.Assertions.assertThatCode;
@@ -134,6 +135,21 @@ void validatesQueryWhenNoAccount() throws Throwable {
         assertThrowsPreCheck(() -> networkGetAccountDetailsHandler.validate(context), INVALID_ACCOUNT_ID);
     }
 
+    @Test
+    void validatesQueryWhenDeletedAccount() {
+        givenValidAccount(true, Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
+        refreshStoresWithEntitiesOnlyInReadable();
+        final var data = GetAccountDetailsQuery.newBuilder()
+                .header(QueryHeader.newBuilder().build())
+                .accountId(accountId)
+                .build();
+        final var query = Query.newBuilder().accountDetails(data).build();
+        given(context.query()).willReturn(query);
+        given(context.createStore(ReadableAccountStore.class)).willReturn(readableAccountStore);
+
+        assertThrowsPreCheck(() -> networkGetAccountDetailsHandler.validate(context), ACCOUNT_DELETED);
+    }
+
     @Test
     void getsResponseIfFailedResponse() {
         final var responseHeader = ResponseHeader.newBuilder()

hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoDeleteAllowanceHandler.java

@@ -35,6 +35,7 @@
 import com.hedera.node.app.service.token.impl.WritableNftStore;
 import com.hedera.node.app.service.token.impl.WritableTokenStore;
 import com.hedera.node.app.service.token.impl.validators.DeleteAllowanceValidator;
+import com.hedera.node.app.spi.validation.ExpiryValidator;
 import com.hedera.node.app.spi.workflows.*;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.util.List;
@@ -61,9 +62,11 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
         final var txn = context.body();
         pureChecks(txn);
         final var op = txn.cryptoDeleteAllowanceOrThrow();
-        // Every owner whose allowances are being removed should sign, if the owner is not payer
+        // Every owner whose allowances are being removed should sign (or the payer, if there is no owner)
         for (final var allowance : op.nftAllowancesOrElse(emptyList())) {
-            context.requireKeyOrThrow(allowance.ownerOrElse(AccountID.DEFAULT), INVALID_ALLOWANCE_OWNER_ID);
+            if (allowance.hasOwner()) {
+                context.requireKeyOrThrow(allowance.ownerOrThrow(), INVALID_ALLOWANCE_OWNER_ID);
+            }
         }
     }
 
@@ -121,7 +124,7 @@ private void deleteAllowance(
         final var nftStore = context.writableStore(WritableNftStore.class);
         final var tokenStore = context.writableStore(WritableTokenStore.class);
 
-        deleteNftSerials(nftAllowances, payer, accountStore, tokenStore, nftStore);
+        deleteNftSerials(nftAllowances, payer, accountStore, tokenStore, nftStore, context.expiryValidator());
     }
 
     /**
@@ -138,15 +141,17 @@ private void deleteNftSerials(
             final Account payerAccount,
             final WritableAccountStore accountStore,
             final WritableTokenStore tokenStore,
-            final WritableNftStore nftStore) {
+            final WritableNftStore nftStore,
+            @NonNull final ExpiryValidator expiryValidator)
+            throws HandleException {
         if (nftAllowances.isEmpty()) {
             return;
         }
         for (final var allowance : nftAllowances) {
             final var serialNums = allowance.serialNumbers();
             final var tokenId = allowance.tokenIdOrElse(TokenID.DEFAULT);
             // If owner is not provided in allowance, consider payer as owner
-            final var owner = getEffectiveOwner(allowance.owner(), payerAccount, accountStore);
+            final var owner = getEffectiveOwner(allowance.owner(), payerAccount, accountStore, expiryValidator);
             final var token = tokenStore.get(tokenId);
             for (final var serial : serialNums) {
                 final var nft = nftStore.get(tokenId, serial);

hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/util/TokenHandlerHelper.java

@@ -75,24 +75,28 @@ private TokenHandlerHelper() {
      * @param accountId the ID of the account to get
      * @param accountStore the {@link ReadableTokenStore} to use for account retrieval
      * @param expiryValidator the {@link ExpiryValidator} to determine if the account is expired
+     * @param errorIfNotFound the {@link ResponseCodeEnum} to use if the account is not found
+     * @param errorIfDeleted the {@link ResponseCodeEnum} to use if the account is deleted
      * @throws HandleException if any of the account conditions are not met
      */
     @NonNull
     public static Account getIfUsable(
             @NonNull final AccountID accountId,
             @NonNull final ReadableAccountStore accountStore,
             @NonNull final ExpiryValidator expiryValidator,
-            @NonNull final ResponseCodeEnum errorIfNotUsable) {
+            @NonNull final ResponseCodeEnum errorIfNotFound,
+            @NonNull final ResponseCodeEnum errorIfDeleted) {
         requireNonNull(accountId);
         requireNonNull(accountStore);
         requireNonNull(expiryValidator);
-        requireNonNull(errorIfNotUsable);
+        requireNonNull(errorIfNotFound);
+        requireNonNull(errorIfDeleted);
 
         final var acct = accountStore.getAccountById(accountId);
-        validateTrue(acct != null, errorIfNotUsable);
+        validateTrue(acct != null, errorIfNotFound);
         final var isContract = acct.smartContract();
 
-        validateFalse(acct.deleted(), isContract ? CONTRACT_DELETED : ACCOUNT_DELETED);
+        validateFalse(acct.deleted(), isContract ? CONTRACT_DELETED : errorIfDeleted);
         final var type = isContract ? EntityType.CONTRACT : EntityType.ACCOUNT;
 
         final var expiryStatus =
@@ -102,6 +106,18 @@ public static Account getIfUsable(
         return acct;
     }
 
+    /**
+     * Override of {@link #getIfUsable(AccountID, ReadableAccountStore, ExpiryValidator,
+     * ResponseCodeEnum, ResponseCodeEnum)} above, with a default `ACCOUNT_DELETED` error code
+     */
+    public static Account getIfUsable(
+            @NonNull final AccountID accountId,
+            @NonNull final ReadableAccountStore accountStore,
+            @NonNull final ExpiryValidator expiryValidator,
+            @NonNull final ResponseCodeEnum errorIfNotFound) {
+        return getIfUsable(accountId, accountStore, expiryValidator, errorIfNotFound, ACCOUNT_DELETED);
+    }
+
     /**
      * Returns the token if it exists and is usable. A {@link HandleException} is thrown if the token is invalid
      *

hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/AllowanceValidator.java

@@ -30,6 +30,8 @@
 import com.hedera.hapi.node.token.NftAllowance;
 import com.hedera.node.app.service.token.ReadableAccountStore;
 import com.hedera.node.app.service.token.ReadableNftStore;
+import com.hedera.node.app.service.token.impl.util.TokenHandlerHelper;
+import com.hedera.node.app.spi.validation.ExpiryValidator;
 import com.hedera.node.config.data.HederaConfig;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
@@ -139,14 +141,14 @@ public static boolean isValidOwner(
     public static Account getEffectiveOwner(
             @Nullable final AccountID owner,
             @NonNull final Account payer,
-            @NonNull final ReadableAccountStore accountStore) {
+            @NonNull final ReadableAccountStore accountStore,
+            @NonNull final ExpiryValidator expiryValidator) {
         if (owner == null || owner.accountNumOrElse(0L) == 0L || owner.equals(payer.accountId())) {
             return payer;
         } else {
             // If owner is in modifications get the modified account from state
-            final var ownerAccount = accountStore.getAccountById(owner);
-            validateTrue(ownerAccount != null, INVALID_ALLOWANCE_OWNER_ID);
-            return ownerAccount;
+            return TokenHandlerHelper.getIfUsable(
+                    owner, accountStore, expiryValidator, INVALID_ALLOWANCE_OWNER_ID, INVALID_ALLOWANCE_OWNER_ID);
         }
     }
 }

hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/ApproveAllowanceValidator.java

@@ -45,6 +45,7 @@
 import com.hedera.node.app.service.token.ReadableNftStore;
 import com.hedera.node.app.service.token.ReadableTokenRelationStore;
 import com.hedera.node.app.service.token.ReadableTokenStore;
+import com.hedera.node.app.spi.validation.ExpiryValidator;
 import com.hedera.node.app.spi.workflows.HandleContext;
 import com.hedera.node.config.data.HederaConfig;
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -84,9 +85,12 @@ public void validate(
         // validate total count of allowances
         validateAllowanceCount(cryptoAllowances, tokenAllowances, nftAllowances, hederaConfig);
         // validate all allowances
-        validateCryptoAllowances(cryptoAllowances, payerAccount, accountStore);
-        validateFungibleTokenAllowances(tokenAllowances, payerAccount, accountStore, tokenStore, tokenRelStore);
-        validateNftAllowances(nftAllowances, payerAccount, accountStore, tokenStore, tokenRelStore, nftStore);
+        final var expiryValidator = context.expiryValidator();
+        validateCryptoAllowances(cryptoAllowances, payerAccount, accountStore, expiryValidator);
+        validateFungibleTokenAllowances(
+                tokenAllowances, payerAccount, accountStore, tokenStore, tokenRelStore, expiryValidator);
+        validateNftAllowances(
+                nftAllowances, payerAccount, accountStore, tokenStore, tokenRelStore, nftStore, expiryValidator);
     }
 
     /**
@@ -98,14 +102,15 @@ public void validate(
     void validateCryptoAllowances(
             @NonNull final List<CryptoAllowance> cryptoAllowances,
             @NonNull final Account payer,
-            @NonNull final ReadableAccountStore accountStore) {
+            @NonNull final ReadableAccountStore accountStore,
+            @NonNull final ExpiryValidator expiryValidator) {
         for (final var allowance : cryptoAllowances) {
             final var owner = allowance.ownerOrElse(AccountID.DEFAULT);
             final var spender = allowance.spenderOrElse(AccountID.DEFAULT);
 
             // check if owner specified in allowances exists.
             // If not set, owner will be treated as payer for the transaction
-            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore);
+            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
             // validate spender account
             final var spenderAccount = accountStore.getAccountById(spender);
             validateTrue(spenderAccount != null, INVALID_ALLOWANCE_SPENDER_ID);
@@ -119,7 +124,8 @@ private void validateFungibleTokenAllowances(
             @NonNull final Account payer,
             final ReadableAccountStore accountStore,
             final ReadableTokenStore tokenStore,
-            final ReadableTokenRelationStore tokenRelStore) {
+            final ReadableTokenRelationStore tokenRelStore,
+            @NonNull final ExpiryValidator expiryValidator) {
         for (final var allowance : tokenAllowances) {
             final var owner = allowance.owner();
             final var spender = allowance.spenderOrThrow();
@@ -129,7 +135,7 @@ private void validateFungibleTokenAllowances(
 
             // check if owner specified in allowances exists.
             // If not set, owner will be treated as payer for the transaction
-            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore);
+            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
             // validate spender account
             final var spenderAccount = accountStore.getAccountById(spender);
             validateTrue(spenderAccount != null, INVALID_ALLOWANCE_SPENDER_ID);
@@ -159,7 +165,8 @@ private void validateNftAllowances(
             final ReadableAccountStore accountStore,
             final ReadableTokenStore tokenStore,
             final ReadableTokenRelationStore tokenRelStore,
-            final ReadableNftStore uniqueTokenStore) {
+            final ReadableNftStore uniqueTokenStore,
+            @NonNull final ExpiryValidator expiryValidator) {
         for (final var allowance : nftAllowancesList) {
             final var owner = allowance.owner();
             final var spender = allowance.spenderOrThrow();
@@ -170,7 +177,7 @@ private void validateNftAllowances(
             validateTrue(token != null, INVALID_TOKEN_ID);
             validateFalse(token.tokenType() == TokenType.FUNGIBLE_COMMON, FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES);
 
-            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore);
+            final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
             validateTokenBasics(effectiveOwner, spender, token, tokenRelStore);
 
             // If a spender has been given approveForAll privileges, then it has the same privileges as owner of NFT.

hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/DeleteAllowanceValidator.java

@@ -18,9 +18,10 @@
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.EMPTY_ALLOWANCES;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TOKEN_ID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.NOT_SUPPORTED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_NOT_ASSOCIATED_TO_ACCOUNT;
-import static com.hedera.node.app.service.token.impl.util.TokenHandlerHelper.getIfUsable;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_WAS_DELETED;
 import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
 import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
 
@@ -35,10 +36,10 @@
 import com.hedera.node.app.service.token.ReadableNftStore;
 import com.hedera.node.app.service.token.ReadableTokenRelationStore;
 import com.hedera.node.app.service.token.ReadableTokenStore;
+import com.hedera.node.app.spi.validation.ExpiryValidator;
 import com.hedera.node.app.spi.workflows.HandleContext;
 import com.hedera.node.config.data.HederaConfig;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import java.util.HashSet;
 import java.util.List;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -73,7 +74,14 @@ public void validate(
 
         validateAllowancesCount(nftAllowances, hederaConfig);
 
-        validateNftDeleteAllowances(nftAllowances, payerAccount, accountStore, tokenStore, tokenRelStore, nftStore);
+        validateNftDeleteAllowances(
+                nftAllowances,
+                payerAccount,
+                accountStore,
+                tokenStore,
+                tokenRelStore,
+                nftStore,
+                handleContext.expiryValidator());
     }
 
     /**
@@ -91,7 +99,8 @@ private void validateNftDeleteAllowances(
             final ReadableAccountStore accountStore,
             final ReadableTokenStore tokenStore,
             final ReadableTokenRelationStore tokenRelStore,
-            final ReadableNftStore nftStore) {
+            final ReadableNftStore nftStore,
+            @NonNull final ExpiryValidator expiryValidator) {
         if (nftAllowances.isEmpty()) {
             return;
         }
@@ -100,10 +109,13 @@ private void validateNftDeleteAllowances(
             final var tokenId = allowance.tokenIdOrElse(TokenID.DEFAULT);
             final var serialNums = allowance.serialNumbers();
 
-            final Token token = getIfUsable(allowance.tokenIdOrElse(TokenID.DEFAULT), tokenStore);
+            // Paused tokens are OK here, so we only check for existence and deletion
+            final Token token = tokenStore.get(allowance.tokenIdOrElse(TokenID.DEFAULT));
+            validateTrue(token != null, INVALID_TOKEN_ID);
+            validateTrue(!token.deleted(), TOKEN_WAS_DELETED);
             validateFalse(token.tokenType().equals(TokenType.FUNGIBLE_COMMON), FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES);
 
-            final var effectiveOwner = getEffectiveOwner(ownerId, payerAccount, accountStore);
+            final var effectiveOwner = getEffectiveOwner(ownerId, payerAccount, accountStore, expiryValidator);
 
             final var relation = tokenRelStore.get(effectiveOwner.accountId(), token.tokenId());
             validateTrue(relation != null, TOKEN_NOT_ASSOCIATED_TO_ACCOUNT);
@@ -135,11 +147,8 @@ private void validateAllowancesCount(
      */
     public static int aggregateNftDeleteAllowances(final List<NftRemoveAllowance> nftAllowances) {
         var count = 0;
-        final var serialsSet = new HashSet<Long>();
         for (final var allowance : nftAllowances) {
-            serialsSet.addAll(allowance.serialNumbers());
-            count += serialsSet.size();
-            serialsSet.clear();
+            count += allowance.serialNumbers().size();
         }
         return count;
     }

hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoApproveAllowanceHandlerTest.java

@@ -22,6 +22,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.BDDMockito.given;
 
 import com.hedera.hapi.node.base.AccountID;
@@ -41,6 +44,7 @@
 import com.hedera.node.app.service.token.impl.test.handlers.util.CryptoTokenHandlerTestBase;
 import com.hedera.node.app.service.token.impl.validators.ApproveAllowanceValidator;
 import com.hedera.node.app.spi.fixtures.workflows.FakePreHandleContext;
+import com.hedera.node.app.spi.validation.ExpiryValidator;
 import com.hedera.node.app.spi.workflows.HandleContext;
 import com.hedera.node.app.spi.workflows.HandleException;
 import com.hedera.node.app.spi.workflows.PreCheckException;
@@ -58,6 +62,9 @@ class CryptoApproveAllowanceHandlerTest extends CryptoTokenHandlerTestBase {
     @Mock(strictness = Strictness.LENIENT)
     private HandleContext handleContext;
 
+    @Mock(strictness = Strictness.LENIENT)
+    private ExpiryValidator expiryValidator;
+
     private CryptoApproveAllowanceHandler subject;
 
     @BeforeEach
@@ -66,6 +73,8 @@ public void setUp() {
         refreshWritableStores();
         final var validator = new ApproveAllowanceValidator();
         givenStoresAndConfig(handleContext);
+        given(handleContext.expiryValidator()).willReturn(expiryValidator);
+        given(expiryValidator.expirationStatus(any(), anyBoolean(), anyLong())).willReturn(OK);
 
         subject = new CryptoApproveAllowanceHandler(validator);
     }