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.

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

Implement pureChecks for file handler classes #9921

Merged
merged 5 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.hedera.hapi.node.base.SubType;
import com.hedera.hapi.node.file.FileAppendTransactionBody;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.service.file.ReadableFileStore;
import com.hedera.node.app.service.file.impl.WritableFileStore;
import com.hedera.node.app.service.file.impl.WritableUpgradeFileStore;
Expand Down Expand Up @@ -65,6 +66,22 @@ public FileAppendHandler() {
// Exists for injection
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final FileAppendTransactionBody transactionBody = txn.fileAppendOrThrow();

if (transactionBody.fileID() == null) {
throw new PreCheckException(INVALID_FILE_ID);
}
if (transactionBody.contents() == null || transactionBody.contents().length() == 0) {
throw new PreCheckException(FILE_CONTENT_EMPTY);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -79,8 +96,8 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx

final var transactionBody = context.body().fileAppendOrThrow();
final var fileStore = context.createStore(ReadableFileStore.class);
final var transactionFileId = requireNonNull(transactionBody.fileID());
preValidate(transactionFileId, fileStore, context, false);
final var transactionFileId = transactionBody.fileIDOrThrow();
preValidate(transactionFileId, fileStore, context);

var file = fileStore.getFileLeaf(transactionFileId);
validateAndAddRequiredKeys(file, null, context);
Expand All @@ -94,11 +111,11 @@ public void handle(@NonNull final HandleContext handleContext) throws HandleExce
final var target = fileAppend.fileID();
final var data = fileAppend.contents();
final var fileServiceConfig = handleContext.configuration().getConfigData(FilesConfig.class);
if (data == null || data.length() <= 0) {
if (data == null || data.length() <= 0) { // should never happen, this is checked in pureChecks
logger.debug("FileAppend: No data to append");
}

if (target == null) {
if (target == null) { // should never happen, this is checked in pureChecks
throw new HandleException(INVALID_FILE_ID);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.hapi.node.base.KeyList;
import com.hedera.hapi.node.base.SubType;
import com.hedera.hapi.node.file.FileCreateTransactionBody;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.hapi.fees.usage.file.FileOpsUsage;
import com.hedera.node.app.service.file.impl.WritableFileStore;
import com.hedera.node.app.service.file.impl.records.CreateFileRecordBuilder;
Expand Down Expand Up @@ -60,6 +62,19 @@ public FileCreateHandler(final FileOpsUsage fileOpsUsage) {
this.fileOpsUsage = fileOpsUsage;
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final FileCreateTransactionBody transactionBody = txn.fileCreateOrThrow();

if (!transactionBody.hasExpirationTime()) {
throw new PreCheckException(INVALID_EXPIRATION_TIME);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -75,10 +90,6 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
final var transactionBody = context.body().fileCreateOrThrow();

validateAndAddRequiredKeys(null, transactionBody.keys(), context);

if (!transactionBody.hasExpirationTime()) {
throw new PreCheckException(INVALID_EXPIRATION_TIME);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@

import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.hapi.node.base.SubType;
import com.hedera.hapi.node.file.FileDeleteTransactionBody;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder;
import com.hedera.node.app.service.file.ReadableFileStore;
import com.hedera.node.app.service.file.impl.WritableFileStore;
Expand Down Expand Up @@ -58,6 +60,19 @@ public FileDeleteHandler(final FileFeeBuilder usageEstimator) {
this.usageEstimator = usageEstimator;
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final FileDeleteTransactionBody transactionBody = txn.fileDeleteOrThrow();

if (transactionBody.fileID() == null) {
throw new PreCheckException(INVALID_FILE_ID);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -74,7 +89,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
final var transactionBody = context.body().fileDeleteOrThrow();
final var fileStore = context.createStore(ReadableFileStore.class);
final var transactionFileId = requireNonNull(transactionBody.fileID());
preValidate(transactionFileId, fileStore, context, true);
preValidate(transactionFileId, fileStore, context);

var file = fileStore.getFileLeaf(transactionFileId);
validateAndAddRequiredKeysForDelete(file, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.hedera.hapi.node.base.SubType;
import com.hedera.hapi.node.base.TimestampSeconds;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder;
import com.hedera.node.app.service.file.ReadableFileStore;
import com.hedera.node.app.service.file.impl.WritableFileStore;
Expand Down Expand Up @@ -55,6 +56,19 @@ public FileSystemDeleteHandler(final FileFeeBuilder usageEstimator) {
this.usageEstimator = usageEstimator;
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final var transactionBody = txn.systemDeleteOrThrow();

if (transactionBody.fileID() == null) {
throw new PreCheckException(INVALID_FILE_ID);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -71,7 +85,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
final var transactionBody = context.body().systemDeleteOrThrow();
final var fileStore = context.createStore(ReadableFileStore.class);
final var transactionFileId = requireNonNull(transactionBody.fileID());
preValidate(transactionFileId, fileStore, context, true);
preValidate(transactionFileId, fileStore, context);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.hedera.hapi.node.base.HederaFunctionality;
import com.hedera.hapi.node.base.SubType;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder;
import com.hedera.node.app.service.file.ReadableFileStore;
import com.hedera.node.app.service.file.impl.WritableFileStore;
Expand Down Expand Up @@ -54,6 +55,19 @@ public FileSystemUndeleteHandler(final FileFeeBuilder usageEstimator) {
this.usageEstimator = usageEstimator;
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final var transactionBody = txn.systemUndeleteOrThrow();

if (transactionBody.fileID() == null) {
throw new PreCheckException(INVALID_FILE_ID);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -70,7 +84,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
final var transactionBody = context.body().systemUndeleteOrThrow();
final var fileStore = context.createStore(ReadableFileStore.class);
final var transactionFileId = requireNonNull(transactionBody.fileID());
preValidate(transactionFileId, fileStore, context, true);
preValidate(transactionFileId, fileStore, context);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.hedera.hapi.node.base.Timestamp;
import com.hedera.hapi.node.file.FileUpdateTransactionBody;
import com.hedera.hapi.node.state.file.File;
import com.hedera.hapi.node.transaction.TransactionBody;
import com.hedera.node.app.hapi.fees.usage.file.FileOpsUsage;
import com.hedera.node.app.service.file.ReadableFileStore;
import com.hedera.node.app.service.file.impl.WritableFileStore;
Expand Down Expand Up @@ -69,6 +70,19 @@ public FileUpdateHandler(final FileOpsUsage fileOpsUsage) {
this.fileOpsUsage = fileOpsUsage;
}

/**
* Performs checks independent of state or context
* @param txn the transaction to check
*/
@Override
public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
final var transactionBody = txn.fileUpdateOrThrow();

if (transactionBody.fileID() == null) {
throw new PreCheckException(INVALID_FILE_ID);
}
}

/**
* This method is called during the pre-handle workflow.
*
Expand All @@ -84,7 +98,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx
final var transactionBody = context.body().fileUpdateOrThrow();
final var fileStore = context.createStore(ReadableFileStore.class);
final var transactionFileId = requireNonNull(transactionBody.fileID());
preValidate(transactionFileId, fileStore, context, false);
preValidate(transactionFileId, fileStore, context);

var file = fileStore.getFileLeaf(transactionFileId);
if (wantsToMutateNonExpiryField(transactionBody)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,20 @@ public static void validateContent(@NonNull byte[] content, @NonNull FilesConfig
}

/**
* The function validates that the fileId is non-null, not a reserved system Id, and matches a file in the store.
* The function validates that the fileId is not a reserved system Id and that it matches a file in the store.
*
* @param fileId the file id to validate and to fetch the metadata
* @param fileStore the file store to fetch the metadata of specified file id
* @param context the prehandle context for the transaction
* @throws PreCheckException if the file id is invalid or the file does not exist
*/
public static void preValidate(
@Nullable final FileID fileId,
@NonNull final FileID fileId,
@NonNull final ReadableFileStore fileStore,
@NonNull final PreHandleContext context,
boolean isDelete)
@NonNull final PreHandleContext context)
throws PreCheckException {
requireNonNull(context);

if (fileId == null) {
kimbor marked this conversation as resolved.
Show resolved Hide resolved
throw new PreCheckException(INVALID_FILE_ID);
}
requireNonNull(fileId);

final var fileConfig = context.configuration().getConfigData(FilesConfig.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,19 @@ void createWithEmptyKeys() throws PreCheckException {
}

@Test
@DisplayName("no expatriation time is added")
@DisplayName("no expiration time is added")
void createAddsDifferentSubmitKey() throws PreCheckException {
// given:
final var payerKey = mockPayerLookup();
final var keys = anotherKeys;

// when:
final var context = new FakePreHandleContext(accountStore, newCreateTxn(keys, 0));
final var txn = newCreateTxn(keys, 0);
final var context = new FakePreHandleContext(accountStore, txn);

// then:
assertThat(context.payerKey()).isEqualTo(payerKey);
assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_EXPIRATION_TIME);
assertThrowsPreCheck(() -> subject.pureChecks(txn), INVALID_EXPIRATION_TIME);
}

@Test
Expand Down