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

Jump to bottom

Move token revoke verification method to preHandle #6318

Merged
merged 3 commits into from May 2, 2023
+51 −48
Merged

Move token revoke verification method to preHandle #6318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
654dbc0
Move token revoke verification method to preHandle
mhess-swl Apr 28, 2023
2deb60b
Remove redundant test params
mhess-swl Apr 28, 2023
bfbd19f
Keep pureChecks method name
mhess-swl May 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 10 additions & 7 deletions ...ava/com/hedera/node/app/service/token/impl/handlers/TokenRevokeKycFromAccountHandler.java
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.token.impl.handlers;

import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TOKEN_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_HAS_NO_KYC_KEY;
import static java.util.Objects.requireNonNull;

Expand All @@ -27,7 +28,6 @@
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.service.token.ReadableTokenStore;
import com.hedera.node.app.service.token.impl.WritableTokenRelationStore;
import com.hedera.node.app.spi.workflows.HandleException;
import com.hedera.node.app.spi.workflows.PreCheckException;
import com.hedera.node.app.spi.workflows.PreHandleContext;
import com.hedera.node.app.spi.workflows.TransactionHandler;
Expand Down Expand Up @@ -57,9 +57,11 @@ public TokenRevokeKycFromAccountHandler() {
public void preHandle(@NonNull final PreHandleContext context) throws PreCheckException {
requireNonNull(context);
final var op = context.body().tokenRevokeKycOrThrow();
pureChecks(op);

final var tokenStore = context.createStore(ReadableTokenStore.class);
final var tokenMeta = tokenStore.getTokenMeta(op.tokenOrElse(TokenID.DEFAULT));
if (tokenMeta == null) throw new PreCheckException(ResponseCodeEnum.INVALID_TOKEN_ID);
if (tokenMeta == null) throw new PreCheckException(INVALID_TOKEN_ID);
if (tokenMeta.hasKycKey()) {
context.requireKey(tokenMeta.kycKey());
} else {
Expand All @@ -79,8 +81,6 @@ public void handle(@NonNull TransactionBody txn, @NonNull WritableTokenRelationS
requireNonNull(tokenRelStore);

final var op = txn.tokenRevokeKycOrThrow();
pureChecks(op);

final var tokenId = op.tokenOrThrow().tokenNum();
final var accountId = op.accountOrElse(AccountID.DEFAULT).accountNumOrThrow();
final var tokenRel = tokenRelStore.getForModify(tokenId, accountId);
Expand All @@ -90,13 +90,16 @@ public void handle(@NonNull TransactionBody txn, @NonNull WritableTokenRelationS
tokenRelStore.put(tokenRelBuilder.build());
}

private void pureChecks(TokenRevokeKycTransactionBody op) {
/**
* Performs checks independent of state or context
*/
private void pureChecks(TokenRevokeKycTransactionBody op) throws PreCheckException {
if (!op.hasToken()) {
throw new HandleException(ResponseCodeEnum.INVALID_TOKEN_ID);
throw new PreCheckException(INVALID_TOKEN_ID);
}

if (!op.hasAccount()) {
throw new HandleException(ResponseCodeEnum.INVALID_ACCOUNT_ID);
throw new PreCheckException(ResponseCodeEnum.INVALID_ACCOUNT_ID);
}
}
}
82 changes: 41 additions & 41 deletions ...edera/node/app/service/token/impl/test/handlers/TokenRevokeKycFromAccountHandlerTest.java
View file
Open in desktop
Expand Up @@ -19,22 +19,24 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TOKEN_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_HAS_NO_KYC_KEY;
import static com.hedera.node.app.service.mono.pbj.PbjConverter.protoToPbj;
import static com.hedera.node.app.service.token.impl.test.handlers.AdapterUtils.txnFrom;
import static com.hedera.node.app.service.token.impl.test.util.MetaAssertion.basicContextAssertions;
import static com.hedera.node.app.spi.fixtures.Assertions.assertThrowsPreCheck;
import static com.hedera.test.factories.scenarios.TokenKycRevokeScenarios.REVOKE_FOR_TOKEN_WITHOUT_KYC;
import static com.hedera.test.factories.scenarios.TokenKycRevokeScenarios.REVOKE_WITH_INVALID_TOKEN;
import static com.hedera.test.factories.scenarios.TokenKycRevokeScenarios.REVOKE_WITH_MISSING_TXN_BODY;
import static com.hedera.test.factories.scenarios.TokenKycRevokeScenarios.VALID_REVOKE_WITH_EXTANT_TOKEN;
import static com.hedera.test.factories.scenarios.TxnHandlingScenario.MISC_ACCOUNT;
import static com.hedera.test.factories.scenarios.TxnHandlingScenario.TOKEN_KYC_KT;
import static com.hedera.test.factories.txns.SignedTxnFactory.DEFAULT_PAYER_KT;
import static com.hedera.test.utils.IdUtils.asAccount;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.BDDMockito.given;
Expand All @@ -44,6 +46,7 @@

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.TokenID;
import com.hedera.hapi.node.base.TransactionID;
import com.hedera.hapi.node.state.token.TokenRelation;
import com.hedera.hapi.node.token.TokenRevokeKycTransactionBody;
import com.hedera.hapi.node.transaction.TransactionBody;
Expand All @@ -53,17 +56,17 @@
import com.hedera.node.app.service.token.impl.handlers.TokenRevokeKycFromAccountHandler;
import com.hedera.node.app.service.token.impl.test.util.SigReqAdapterUtils;
import com.hedera.node.app.spi.fixtures.workflows.FakePreHandleContext;
import com.hedera.node.app.spi.workflows.HandleException;
import com.hedera.node.app.spi.workflows.PreCheckException;
import java.util.NoSuchElementException;
import java.util.Optional;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

class TokenRevokeKycFromAccountHandlerTest {

private static final AccountID PBJ_PAYER_ID = protoToPbj(asAccount("0.0.3"), AccountID.class);
private static final TokenID TOKEN_10 = TokenID.newBuilder().tokenNum(10).build();
private static final AccountID ACCOUNT_100 =
AccountID.newBuilder().accountNum(100).build();
Expand Down Expand Up @@ -99,10 +102,39 @@ void tokenRevokeMissingTxnBody() throws PreCheckException {
final var txn = txnFrom(REVOKE_WITH_MISSING_TXN_BODY);

final var context = new FakePreHandleContext(accountStore, txn);
context.registerStore(ReadableTokenStore.class, tokenStore);
assertThrows(NullPointerException.class, () -> subject.preHandle(context));
}

@Test
@DisplayName("When op token ID is null, tokenOrThrow throws an exception")
void nullTokenIdThrowsException() throws PreCheckException {
final var txn = TransactionBody.newBuilder()
.transactionID(TransactionID.newBuilder().accountID(PBJ_PAYER_ID))
.tokenRevokeKyc(TokenRevokeKycTransactionBody.newBuilder()
.token((TokenID) null)
.account(AccountID.newBuilder().accountNum(MISC_ACCOUNT.getAccountNum()))
.build())
.build();

final var context = new FakePreHandleContext(accountStore, txn);
assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_TOKEN_ID);
}

@Test
@DisplayName("When op account ID is null, accountOrThrow throws an exception")
void nullAccountIdThrowsException() throws PreCheckException {
final var txn = TransactionBody.newBuilder()
.transactionID(TransactionID.newBuilder().accountID(PBJ_PAYER_ID))
.tokenRevokeKyc(TokenRevokeKycTransactionBody.newBuilder()
.token(TOKEN_10)
.account((AccountID) null)
.build())
.build();

final var context = new FakePreHandleContext(accountStore, txn);
assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_ACCOUNT_ID);
}

@Test
void tokenRevokeKycWithInvalidToken() throws PreCheckException {
final var txn = txnFrom(REVOKE_WITH_INVALID_TOKEN);
Expand Down Expand Up @@ -153,40 +185,12 @@ void nullTokenRevokeKycThrowsException() {
assertThatThrownBy(() -> subject.handle(txnBody, tokenRelStore)).isInstanceOf(NullPointerException.class);
}

@Test
@DisplayName("When op token ID is null, tokenOrThrow throws an exception")
void nullTokenIdThrowsException() {
final var txnBody = newTxnBody(false, true);

try {
subject.handle(txnBody, tokenRelStore);
} catch (HandleException result) {
Assertions.assertThat(result.getStatus()).isEqualTo(INVALID_TOKEN_ID);
} catch (Exception result) {
fail("Expected HandleException, got " + result.getClass().getSimpleName());
}
}

@Test
@DisplayName("When op account ID is null, accountOrThrow throws an exception")
void nullAccountIdThrowsException() {
final var txnBody = newTxnBody(true, false);

try {
subject.handle(txnBody, tokenRelStore);
} catch (HandleException result) {
Assertions.assertThat(result.getStatus()).isEqualTo(INVALID_ACCOUNT_ID);
} catch (Exception result) {
fail("Expected HandleException, got " + result.getClass().getSimpleName());
}
}

@Test
@DisplayName("When getForModify returns empty, should not put or commit")
void emptyGetForModifyShouldNotPersist() {
given(tokenRelStore.getForModify(anyLong(), anyLong())).willReturn(Optional.empty());

final var txnBody = newTxnBody(true, true);
final var txnBody = newTxnBody();
assertThatThrownBy(() -> subject.handle(txnBody, tokenRelStore)).isInstanceOf(NoSuchElementException.class);

verify(tokenRelStore, never()).put(any(TokenRelation.class));
Expand All @@ -203,7 +207,7 @@ void kycRevokedAndPersisted() {
given(tokenRelStore.getForModify(TOKEN_10.tokenNum(), ACCOUNT_100.accountNumOrThrow()))
.willReturn(Optional.of(stateTokenRel));

final var txnBody = newTxnBody(true, true);
final var txnBody = newTxnBody();
subject.handle(txnBody, tokenRelStore);

verify(tokenRelStore)
Expand All @@ -216,14 +220,10 @@ private TokenRelation.Builder newTokenRelationBuilder() {
.accountNumber(ACCOUNT_100.accountNumOrThrow());
}

private TransactionBody newTxnBody(final boolean tokenPresent, final boolean accountPresent) {
private TransactionBody newTxnBody() {
TokenRevokeKycTransactionBody.Builder builder = TokenRevokeKycTransactionBody.newBuilder();
if (tokenPresent) {
builder.token(TOKEN_10);
}
if (accountPresent) {
builder.account(ACCOUNT_100);
}
builder.token(TOKEN_10);
builder.account(ACCOUNT_100);
return TransactionBody.newBuilder()
.tokenRevokeKyc(builder.build())
.memo(this.getClass().getName() + System.currentTimeMillis())
Expand Down