Skip to content

Commit

Permalink
Fixed code to enable additional 14 HapiTests in CryptoApproveAllowanc…
Browse files Browse the repository at this point in the history 
…eSuite (hashgraph#9032)

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
  • Loading branch information
@iwsimon
iwsimon authored Oct 5, 2023
1 parent bcd162d commit a6b0d4a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 30 deletions.
4 changes: 2 additions & 2 deletions hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/SolvencyPreCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ public Account getPayerAccount(@NonNull final ReadableStoreFactory storeFactory,
final var account = accountStore.getAccountById(accountID);

if (account == null) {
throw new PreCheckException(ResponseCodeEnum.INVALID_ACCOUNT_ID);
throw new PreCheckException(ResponseCodeEnum.PAYER_ACCOUNT_NOT_FOUND);
}

if (account.deleted()) {
throw new PreCheckException(ResponseCodeEnum.PAYER_ACCOUNT_DELETED);
}

if (account.smartContract()) {
throw new PreCheckException(ResponseCodeEnum.INVALID_ACCOUNT_ID);
throw new PreCheckException(ResponseCodeEnum.PAYER_ACCOUNT_NOT_FOUND);
}

return account;
Expand Down
4 changes: 2 additions & 2 deletions hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/SolvencyPreCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void testGetPayerAccountSuccess() {
void testGetUnknownPayerAccountFails() {
assertThatThrownBy(() -> subject.getPayerAccount(storeFactory, BOB.accountID()))
.isInstanceOf(PreCheckException.class)
.has(responseCode(ResponseCodeEnum.INVALID_ACCOUNT_ID));
.has(responseCode(ResponseCodeEnum.PAYER_ACCOUNT_NOT_FOUND));
}

@Test
Expand All @@ -163,7 +163,7 @@ void testGetSmartContractPayerAccountFails() {
// then
assertThatThrownBy(() -> subject.getPayerAccount(storeFactory, ALICE.accountID()))
.isInstanceOf(PreCheckException.class)
.has(responseCode(ResponseCodeEnum.INVALID_ACCOUNT_ID));
.has(responseCode(ResponseCodeEnum.PAYER_ACCOUNT_NOT_FOUND));
}
}

Expand Down
30 changes: 22 additions & 8 deletions ...ain/java/com/hedera/node/app/service/token/impl/validators/ApproveAllowanceValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ void validateCryptoAllowances(
final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
// validate spender account
final var spenderAccount = accountStore.getAccountById(spender);
validateTrue(spenderAccount != null, INVALID_ALLOWANCE_SPENDER_ID);

validateSpender(allowance.amount(), spenderAccount);
validateTrue(allowance.amount() >= 0, NEGATIVE_ALLOWANCE_AMOUNT);
validateFalse(spender.equals(effectiveOwner.accountId()), SPENDER_ACCOUNT_SAME_AS_OWNER);
}
Expand All @@ -138,14 +139,14 @@ private void validateFungibleTokenAllowances(
final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
// validate spender account
final var spenderAccount = accountStore.getAccountById(spender);
validateTrue(spenderAccount != null, INVALID_ALLOWANCE_SPENDER_ID);
validateTrue(token.tokenType() == TokenType.FUNGIBLE_COMMON, NFT_IN_FUNGIBLE_TOKEN_ALLOWANCES);
validateTrue(TokenType.FUNGIBLE_COMMON.equals(token.tokenType()), NFT_IN_FUNGIBLE_TOKEN_ALLOWANCES);

// validate token amount
final var amount = allowance.amount();
validateSpender(amount, spenderAccount);
validateTrue(amount >= 0, NEGATIVE_ALLOWANCE_AMOUNT);
validateFalse(
token.supplyType() == TokenSupplyType.FINITE && amount > token.maxSupply(),
TokenSupplyType.FINITE.equals(token.supplyType()) && amount > token.maxSupply(),
AMOUNT_EXCEEDS_TOKEN_MAX_SUPPLY);
// validate
validateTokenBasics(effectiveOwner, spender, token, tokenRelStore);
Expand Down Expand Up @@ -175,7 +176,10 @@ private void validateNftAllowances(

final var token = tokenStore.get(tokenId);
validateTrue(token != null, INVALID_TOKEN_ID);
validateFalse(token.tokenType() == TokenType.FUNGIBLE_COMMON, FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES);
validateFalse(TokenType.FUNGIBLE_COMMON.equals(token.tokenType()), FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES);

final var spenderAccount = accountStore.getAccountById(spender);
validateNFTSpender(serialNums, spenderAccount);

final var effectiveOwner = getEffectiveOwner(owner, payer, accountStore, expiryValidator);
validateTokenBasics(effectiveOwner, spender, token, tokenRelStore);
Expand All @@ -186,12 +190,11 @@ private void validateNftAllowances(
&& allowance.delegatingSpenderOrThrow().accountNumOrThrow() != 0) {
if (allowance.hasApprovedForAll()) {
validateFalse(
Boolean.TRUE.equals(allowance.approvedForAll()),
DELEGATING_SPENDER_CANNOT_GRANT_APPROVE_FOR_ALL);
allowance.approvedForAll().booleanValue(), DELEGATING_SPENDER_CANNOT_GRANT_APPROVE_FOR_ALL);
}
final var approveForAllKey = AccountApprovalForAllAllowance.newBuilder()
.tokenId(tokenId)
.spenderId(spender)
.spenderId(allowance.delegatingSpender())
.build();
validateTrue(
effectiveOwner
Expand Down Expand Up @@ -231,4 +234,15 @@ private void validateTokenBasics(
final var relation = tokenRelStore.get(ownerId, tokenId);
validateTrue(relation != null, TOKEN_NOT_ASSOCIATED_TO_ACCOUNT);
}

private void validateSpender(long amount, Account spenderAccount) {
validateTrue(
amount == 0 || (spenderAccount != null && !spenderAccount.deleted()), INVALID_ALLOWANCE_SPENDER_ID);
}

private void validateNFTSpender(List<Long> serialNumbers, Account spenderAccount) {
validateTrue(
serialNumbers.isEmpty() || (spenderAccount != null && !spenderAccount.deleted()),
INVALID_ALLOWANCE_SPENDER_ID);
}
}
4 changes: 3 additions & 1 deletion ...com/hedera/node/app/service/token/impl/test/validators/ApproveAllowanceValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,9 @@ void canGrantNftSerialAllowanceIfDelegatingSpenderHasNoApproveForAllAllowance()
.delegatingSpender(delegatingSpenderId)
.build()));

assertThatNoException().isThrownBy(() -> subject.validate(handleContext, account, readableAccountStore));
assertThatThrownBy(() -> subject.validate(handleContext, account, readableAccountStore))
.isInstanceOf(HandleException.class)
.has(responseCode(DELEGATING_SPENDER_DOES_NOT_HAVE_APPROVE_FOR_ALL));
}

@Test
Expand Down
32 changes: 16 additions & 16 deletions ...ents/src/main/java/com/hedera/services/bdd/suites/crypto/CryptoApproveAllowanceSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public List<HapiSpec> getSpecsInSuite() {
transferringMissingNftViaApprovalFailsWithInvalidNftId());
}

// @HapiTest
@HapiTest
private HapiSpec cannotPayForAnyTransactionWithContractAccount() {
final var cryptoAdminKey = "cryptoAdminKey";
final var contractNum = new AtomicLong();
Expand Down Expand Up @@ -216,7 +216,7 @@ private HapiSpec transferringMissingNftViaApprovalFailsWithInvalidNftId() {
.hasKnownStatus(INVALID_NFT_ID));
}

// @HapiTest
@HapiTest
private HapiSpec canDeleteAllowanceFromDeletedSpender() {
return defaultHapiSpec("canDeleteAllowanceFromDeletedSpender")
.given(
Expand Down Expand Up @@ -313,7 +313,7 @@ private HapiSpec canDeleteAllowanceFromDeletedSpender() {
.then();
}

// @HapiTest
// @HapiTest INSUFFICIENT_TX_FEE, expecting OK
private HapiSpec duplicateKeysAndSerialsInSameTxnDoesntThrow() {
return defaultHapiSpec("duplicateKeysAndSerialsInSameTxnDoesntThrow")
.given(
Expand Down Expand Up @@ -399,7 +399,7 @@ private HapiSpec duplicateKeysAndSerialsInSameTxnDoesntThrow() {
getTokenNftInfo(NON_FUNGIBLE_TOKEN, 3L).hasSpenderID(SPENDER));
}

// @HapiTest
// @HapiTest Expected DELEGATING_SPENDER_CANNOT_GRANT_APPROVE_FOR_ALL, was INVALID_SIGNATURE!
private HapiSpec approveForAllSpenderCanDelegateOnNFT() {
final String delegatingSpender = "delegatingSpender";
final String newSpender = "newSpender";
Expand Down Expand Up @@ -459,7 +459,7 @@ private HapiSpec approveForAllSpenderCanDelegateOnNFT() {
getTokenNftInfo(NON_FUNGIBLE_TOKEN, 1L).hasSpenderID(newSpender));
}

// @HapiTest
@HapiTest
private HapiSpec canGrantFungibleAllowancesWithTreasuryOwner() {
return defaultHapiSpec("canGrantFungibleAllowancesWithTreasuryOwner")
.given(
Expand Down Expand Up @@ -538,7 +538,7 @@ private HapiSpec canGrantNftAllowancesWithTreasuryOwner() {
.logged());
}

// @HapiTest
@HapiTest
private HapiSpec invalidOwnerFails() {
return defaultHapiSpec("invalidOwnerFails")
.given(
Expand Down Expand Up @@ -603,7 +603,7 @@ private HapiSpec invalidOwnerFails() {
.hasAnswerOnlyPrecheck(ACCOUNT_DELETED));
}

// @HapiTest
@HapiTest
private HapiSpec invalidSpenderFails() {
return defaultHapiSpec("invalidSpenderFails")
.given(
Expand Down Expand Up @@ -814,7 +814,7 @@ private HapiSpec canHaveMultipleOwners() {
.cryptoAllowancesContaining(SPENDER, 2 * ONE_HBAR)));
}

// @HapiTest
@HapiTest
private HapiSpec feesAsExpected() {
return defaultHapiSpec("feesAsExpected")
.given(
Expand Down Expand Up @@ -985,7 +985,7 @@ private HapiSpec serialsInAscendingOrder() {
.nftApprovedAllowancesContaining(NON_FUNGIBLE_TOKEN, SPENDER)));
}

// @HapiTest
@HapiTest
private HapiSpec succeedsWhenTokenPausedFrozenKycRevoked() {
return defaultHapiSpec("succeedsWhenTokenPausedFrozenKycRevoked")
.given(
Expand Down Expand Up @@ -1076,7 +1076,7 @@ private HapiSpec succeedsWhenTokenPausedFrozenKycRevoked() {
.tokenAllowancesCount(4)));
}

// @HapiTest
@HapiTest
private HapiSpec tokenExceedsMaxSupplyFails() {
return defaultHapiSpec("tokenExceedsMaxSupplyFails")
.given(
Expand Down Expand Up @@ -1152,7 +1152,7 @@ private HapiSpec validatesSerialNums() {
.then();
}

// @HapiTest
@HapiTest
private HapiSpec invalidTokenTypeFails() {
return defaultHapiSpec("invalidTokenTypeFails")
.given(
Expand Down Expand Up @@ -1210,7 +1210,7 @@ private HapiSpec emptyAllowancesRejected() {
.then();
}

// @HapiTest
@HapiTest
private HapiSpec tokenNotAssociatedToAccountFails() {
return defaultHapiSpec("tokenNotAssociatedToAccountFails")
.given(
Expand Down Expand Up @@ -1261,7 +1261,7 @@ private HapiSpec tokenNotAssociatedToAccountFails() {
.tokenAllowancesCount(0)));
}

// @HapiTest
@HapiTest
private HapiSpec negativeAmountFailsForFungible() {
return defaultHapiSpec("negativeAmountFailsForFungible")
.given(
Expand Down Expand Up @@ -1316,7 +1316,7 @@ private HapiSpec negativeAmountFailsForFungible() {
.tokenAllowancesCount(0)));
}

// @HapiTest
@HapiTest
private HapiSpec happyPathWorks() {
return defaultHapiSpec("happyPathWorks")
.given(
Expand Down Expand Up @@ -1382,7 +1382,7 @@ private HapiSpec happyPathWorks() {
getTokenNftInfo(NON_FUNGIBLE_TOKEN, 1L).hasSpenderID(SPENDER));
}

// @HapiTest
@HapiTest
private HapiSpec duplicateEntriesGetsReplacedWithDifferentTxn() {
return defaultHapiSpec("duplicateEntriesGetsReplacedWithDifferentTxn")
.given(
Expand Down Expand Up @@ -1593,7 +1593,7 @@ private HapiSpec approveForAllDoesNotSetExplicitNFTSpender() {
getTokenNftInfo(NON_FUNGIBLE_TOKEN, 1L).hasNoSpender().logged());
}

// @HapiTest
// @HapiTest HapiScheduleCreate not working yet
private HapiSpec scheduledCryptoApproveAllowanceWorks() {
return defaultHapiSpec("ScheduledCryptoApproveAllowanceWorks")
.given(
Expand Down
2 changes: 1 addition & 1 deletion hedera-node/test-clients/src/main/resource/spec-default.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ num.opFinisher.threads=8
persistentEntities.dir.path=
persistentEntities.updateCreatedManifests=true
spec.autoScheduledTxns=
spec.streamlinedIngestChecks=INVALID_FILE_ID,ENTITY_NOT_ALLOWED_TO_DELETE,AUTHORIZATION_FAILED,INVALID_PRNG_RANGE,INVALID_STAKING_ID,NOT_SUPPORTED,TOKEN_ID_REPEATED_IN_TOKEN_LIST,ALIAS_ALREADY_ASSIGNED,INVALID_ALIAS_KEY,KEY_REQUIRED,BAD_ENCODING,AUTORENEW_DURATION_NOT_IN_RANGE,INVALID_ZERO_BYTE_IN_STRING,INVALID_ADMIN_KEY,ACCOUNT_DELETED,BUSY,INSUFFICIENT_PAYER_BALANCE,INSUFFICIENT_TX_FEE,INVALID_ACCOUNT_ID,INVALID_NODE_ACCOUNT,INVALID_SIGNATURE,INVALID_TRANSACTION,INVALID_TRANSACTION_BODY,INVALID_TRANSACTION_DURATION,INVALID_TRANSACTION_ID,INVALID_TRANSACTION_START,KEY_PREFIX_MISMATCH,MEMO_TOO_LONG,PAYER_ACCOUNT_NOT_FOUND,PLATFORM_NOT_ACTIVE,TRANSACTION_EXPIRED,TRANSACTION_HAS_UNKNOWN_FIELDS,TRANSACTION_ID_FIELD_NOT_ALLOWED,TRANSACTION_OVERSIZE,TRANSFER_ACCOUNT_SAME_AS_DELETE_ACCOUNT,EMPTY_ALLOWANCES,REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT,TOKEN_HAS_NO_FREEZE_KEY,TOKEN_HAS_NO_SUPPLY_KEY,INVALID_TOKEN_INITIAL_SUPPLY,INVALID_TOKEN_DECIMALS,INVALID_TOKEN_MAX_SUPPLY,ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS,TRANSFERS_NOT_ZERO_SUM_FOR_TOKEN,INVALID_ACCOUNT_AMOUNTS,TOKEN_NAME_TOO_LONG,TOKEN_SYMBOL_TOO_LONG,INVALID_TOKEN_NFT_SERIAL_NUMBER,PERMANENT_REMOVAL_REQUIRES_SYSTEM_INITIATION,MISSING_TOKEN_SYMBOL,MISSING_TOKEN_NAME,INVALID_EXPIRATION_TIME,EMPTY_TOKEN_TRANSFER_ACCOUNT_AMOUNTS,INVALID_ALLOWANCE_OWNER_ID,FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES,TOKEN_NOT_ASSOCIATED_TO_ACCOUNT,MAX_ALLOWANCES_EXCEEDED,INVALID_ALLOWANCE_SPENDER_ID
spec.streamlinedIngestChecks=INVALID_FILE_ID,ENTITY_NOT_ALLOWED_TO_DELETE,AUTHORIZATION_FAILED,INVALID_PRNG_RANGE,INVALID_STAKING_ID,NOT_SUPPORTED,TOKEN_ID_REPEATED_IN_TOKEN_LIST,ALIAS_ALREADY_ASSIGNED,INVALID_ALIAS_KEY,KEY_REQUIRED,BAD_ENCODING,AUTORENEW_DURATION_NOT_IN_RANGE,INVALID_ZERO_BYTE_IN_STRING,INVALID_ADMIN_KEY,ACCOUNT_DELETED,BUSY,INSUFFICIENT_PAYER_BALANCE,INSUFFICIENT_TX_FEE,INVALID_ACCOUNT_ID,INVALID_NODE_ACCOUNT,INVALID_SIGNATURE,INVALID_TRANSACTION,INVALID_TRANSACTION_BODY,INVALID_TRANSACTION_DURATION,INVALID_TRANSACTION_ID,INVALID_TRANSACTION_START,KEY_PREFIX_MISMATCH,MEMO_TOO_LONG,PAYER_ACCOUNT_NOT_FOUND,PLATFORM_NOT_ACTIVE,TRANSACTION_EXPIRED,TRANSACTION_HAS_UNKNOWN_FIELDS,TRANSACTION_ID_FIELD_NOT_ALLOWED,TRANSACTION_OVERSIZE,TRANSFER_ACCOUNT_SAME_AS_DELETE_ACCOUNT,EMPTY_ALLOWANCES,REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT,TOKEN_HAS_NO_FREEZE_KEY,TOKEN_HAS_NO_SUPPLY_KEY,INVALID_TOKEN_INITIAL_SUPPLY,INVALID_TOKEN_DECIMALS,INVALID_TOKEN_MAX_SUPPLY,ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS,TRANSFERS_NOT_ZERO_SUM_FOR_TOKEN,INVALID_ACCOUNT_AMOUNTS,TOKEN_NAME_TOO_LONG,TOKEN_SYMBOL_TOO_LONG,INVALID_TOKEN_NFT_SERIAL_NUMBER,PERMANENT_REMOVAL_REQUIRES_SYSTEM_INITIATION,MISSING_TOKEN_SYMBOL,MISSING_TOKEN_NAME,INVALID_EXPIRATION_TIME,EMPTY_TOKEN_TRANSFER_ACCOUNT_AMOUNTS,INVALID_ALLOWANCE_OWNER_ID,FUNGIBLE_TOKEN_IN_NFT_ALLOWANCES,TOKEN_NOT_ASSOCIATED_TO_ACCOUNT,MAX_ALLOWANCES_EXCEEDED,INVALID_ALLOWANCE_SPENDER_ID,AMOUNT_EXCEEDS_TOKEN_MAX_SUPPLY,NFT_IN_FUNGIBLE_TOKEN_ALLOWANCES,NEGATIVE_ALLOWANCE_AMOUNT,DELEGATING_SPENDER_DOES_NOT_HAVE_APPROVE_FOR_ALL,DELEGATING_SPENDER_CANNOT_GRANT_APPROVE_FOR_ALL
status.deferredResolves.doAsync=true
status.preResolve.pause.ms=0
status.wait.sleep.ms=500
Expand Down

0 comments on commit a6b0d4a

Please sign in to comment.