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.

Sign up for GitHub

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

feat: Enhance purechecks for CryptoGetAccountBalanceHandler #12839

Merged
merged 13 commits into from
Apr 24, 2024
Merged

feat: Enhance purechecks for CryptoGetAccountBalanceHandler #12839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7b110f4
Account Balance Handler validations
thomas-swirlds-labs Apr 16, 2024
cf831f3
refactor validations, add unit tests
thomas-swirlds-labs Apr 16, 2024
ebbf382
small change validation for shard, realm ids
thomas-swirlds-labs Apr 19, 2024
48c1462
Merge branch 'release/0.49' into 12715-enhance-pureChecks-for-CryptoG…
thomas-swirlds-labs Apr 19, 2024
9b67706
pr review feedback
thomas-swirlds-labs Apr 22, 2024
299105f
Merge remote-tracking branch 'origin/12715-enhance-pureChecks-for-Cry…
thomas-swirlds-labs Apr 22, 2024
6a7dd91
Merge branch 'release/0.49' into 12715-enhance-pureChecks-for-CryptoG…
thomas-swirlds-labs Apr 22, 2024
77fa91e
remove redundant check
thomas-swirlds-labs Apr 23, 2024
40de7b3
Merge branch 'release/0.49' into 12715-enhance-pureChecks-for-CryptoG…
thomas-swirlds-labs Apr 23, 2024
997001c
fix failing hapi test for evmAlias
thomas-swirlds-labs Apr 23, 2024
be1d3e8
Merge remote-tracking branch 'origin/12715-enhance-pureChecks-for-Cry…
thomas-swirlds-labs Apr 23, 2024
9e7e68f
Merge branch 'release/0.49' into 12715-enhance-pureChecks-for-CryptoG…
thomas-swirlds-labs Apr 23, 2024
9e319ae
Merge branch 'release/0.49' into 12715-enhance-pureChecks-for-CryptoG…
thomas-swirlds-labs Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions .../java/com/hedera/node/app/service/token/impl/handlers/CryptoGetAccountBalanceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_CONTRACT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
import static com.hedera.node.app.spi.validation.Validations.mustExist;
import static com.hedera.node.app.spi.validation.Validations.validateAccountID;
import static com.hedera.node.app.spi.workflows.PreCheckException.validateFalsePreCheck;
import static com.hedera.node.app.spi.workflows.PreCheckException.validateTruePreCheck;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.ContractID;
import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.hapi.node.base.QueryHeader;
import com.hedera.hapi.node.base.ResponseHeader;
Expand Down Expand Up @@ -83,18 +87,40 @@ public void validate(@NonNull final QueryContext context) throws PreCheckExcepti
final var accountStore = context.createStore(ReadableAccountStore.class);
final CryptoGetAccountBalanceQuery op = query.cryptogetAccountBalanceOrThrow();
if (op.hasAccountID()) {
final var account = accountStore.getAliasedAccountById(requireNonNull(op.accountID()));
validateFalsePreCheck(account == null, INVALID_ACCOUNT_ID);
validateFalsePreCheck(account.deleted(), ACCOUNT_DELETED);
validateAccountId(op, accountStore);
} else if (op.hasContractID()) {
final var contract = accountStore.getContractById(requireNonNull(op.contractID()));
validateFalsePreCheck(contract == null || !contract.smartContract(), INVALID_CONTRACT_ID);
validateFalsePreCheck(contract.deleted(), CONTRACT_DELETED);
validateContractId(op, accountStore);
} else {
throw new PreCheckException(INVALID_ACCOUNT_ID);
}
}

private void validateContractId(CryptoGetAccountBalanceQuery op, ReadableAccountStore accountStore)
throws PreCheckException {
mustExist(op.contractID(), INVALID_CONTRACT_ID);
final ContractID contractId = (ContractID) op.balanceSource().value();
validateTruePreCheck(contractId.shardNum() == 0, INVALID_CONTRACT_ID);
validateTruePreCheck(contractId.realmNum() == 0, INVALID_CONTRACT_ID);
validateTruePreCheck(
(contractId.hasContractNum() && contractId.contractNumOrThrow() >= 0) || contractId.hasEvmAddress(),
INVALID_CONTRACT_ID);
final var contract = accountStore.getContractById(requireNonNull(op.contractID()));
validateFalsePreCheck(contract == null, INVALID_CONTRACT_ID);
validateTruePreCheck(contract.smartContract(), INVALID_CONTRACT_ID);
validateFalsePreCheck(contract.deleted(), CONTRACT_DELETED);
}

private void validateAccountId(CryptoGetAccountBalanceQuery op, ReadableAccountStore accountStore)
mhess-swl marked this conversation as resolved.
Show resolved Hide resolved
throws PreCheckException {
AccountID accountId = (AccountID) op.balanceSource().value();
validateTruePreCheck(accountId.shardNum() == 0, INVALID_ACCOUNT_ID);
validateTruePreCheck(accountId.realmNum() == 0, INVALID_ACCOUNT_ID);
validateAccountID(accountId, INVALID_ACCOUNT_ID);
final var account = accountStore.getAliasedAccountById(requireNonNull(op.accountID()));
validateFalsePreCheck(account == null, INVALID_ACCOUNT_ID);
validateFalsePreCheck(account.deleted(), ACCOUNT_DELETED);
}

@Override
public Response findResponse(@NonNull final QueryContext context, @NonNull final ResponseHeader header) {
requireNonNull(context);
Expand Down
28 changes: 28 additions & 0 deletions .../hedera/node/app/service/token/impl/test/handlers/CryptoGetAccountBalanceHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,25 @@ void validatesQueryIfInvalidAccount() throws Throwable {
.has(responseCode(ResponseCodeEnum.INVALID_ACCOUNT_ID));
}

@Test
@DisplayName("Account Id with valid header is needed during validate")
void validatesQueryIfInvalidAccountHeader() throws Throwable {
final var state =
MapReadableKVState.<AccountID, Account>builder(ACCOUNTS).build();
given(readableStates.<AccountID, Account>get(ACCOUNTS)).willReturn(state);
final var store = new ReadableAccountStoreImpl(readableStates);
final AccountID invalidRealmAccountId =
AccountID.newBuilder().accountNum(5).realmNum(-1L).build();

final var query = createGetAccountBalanceQueryWithInvalidHeader(invalidRealmAccountId.accountNumOrThrow());
when(context.query()).thenReturn(query);
when(context.createStore(ReadableAccountStore.class)).thenReturn(store);

assertThatThrownBy(() -> subject.validate(context))
.isInstanceOf(PreCheckException.class)
.has(responseCode(ResponseCodeEnum.INVALID_ACCOUNT_ID));
}
mhess-swl marked this conversation as resolved.
Show resolved Hide resolved

@Test
@DisplayName("Contract Id is needed during validate")
void validatesQueryIfInvalidContract() throws Throwable {
Expand Down Expand Up @@ -450,6 +469,15 @@ private Query createGetAccountBalanceQuery(final long accountId) {
return Query.newBuilder().cryptogetAccountBalance(data).build();
}

private Query createGetAccountBalanceQueryWithInvalidHeader(final long accountId) {
final var data = CryptoGetAccountBalanceQuery.newBuilder()
.accountID(AccountID.newBuilder().accountNum(accountId).build())
.header((QueryHeader) null)
.build();

return Query.newBuilder().cryptogetAccountBalance(data).build();
}

private Query createGetAccountBalanceQueryWithContract(final long contractId) {
final var data = CryptoGetAccountBalanceQuery.newBuilder()
.contractID(ContractID.newBuilder().contractNum(contractId).build())
Expand Down
Loading