fix: Cherry-pick: add pureChecks to ingest workflow (#12549)

netopyr · web-flow · commit 6a62b45620f7 · 2024-04-08T17:22:36.000+02:00
Signed-off-by: Michael Heinrichs &lt;netopyr@users.noreply.github.com&gt;
diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/grpc/impl/MethodBase.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/grpc/impl/MethodBase.java
@@ -140,7 +140,7 @@ public void invoke(
             callsHandledSpeedometer.cycle();
         } catch (final Exception e) {
             // Track the number of times we failed to handle a call
-            logger.error("Possibly CATASTROPHIC failure while handling a call and running the ingest workflow", e);
+            logger.error("Possibly CATASTROPHIC failure while handling a GRPC message", e);
             callsFailedCounter.increment();
             responseObserver.onError(e);
         }
diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/ingest/IngestChecker.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/ingest/IngestChecker.java
@@ -217,6 +217,9 @@ public TransactionInfo runAllChecks(
             throw new PreCheckException(BUSY);
         }
 
+        // 4a. Run pure checks
+        dispatcher.dispatchPureChecks(txBody);
+
         // 5. Get payer account
         final var storeFactory = new ReadableStoreFactory(state);
         final var payer = solvencyPreCheck.getPayerAccount(storeFactory, txInfo.payerID());
diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/ingest/IngestWorkflowImpl.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/ingest/IngestWorkflowImpl.java
@@ -20,6 +20,7 @@
 
 import com.hedera.hapi.node.base.ResponseCodeEnum;
 import com.hedera.hapi.node.transaction.TransactionResponse;
+import com.hedera.node.app.spi.workflows.HandleException;
 import com.hedera.node.app.spi.workflows.InsufficientBalanceException;
 import com.hedera.node.app.spi.workflows.PreCheckException;
 import com.hedera.node.app.state.HederaState;
@@ -33,9 +34,14 @@
 import java.io.UncheckedIOException;
 import java.util.function.Supplier;
 import javax.inject.Inject;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 /** Implementation of {@link IngestWorkflow} */
 public final class IngestWorkflowImpl implements IngestWorkflow {
+
+    private static final Logger logger = LogManager.getLogger(IngestWorkflowImpl.class);
+
     private final Supplier<AutoCloseableWrapper<HederaState>> stateAccessor;
     private final TransactionChecker transactionChecker;
     private final IngestChecker ingestChecker;
@@ -92,6 +98,13 @@ public void submitTransaction(@NonNull final Bytes requestBuffer, @NonNull final
             result = e.responseCode();
         } catch (final PreCheckException e) {
             result = e.responseCode();
+        } catch (final HandleException e) {
+            // Conceptually, this should never happen, because we should use PreCheckException only during pre-checks
+            // But we catch it here to play it safe
+            result = e.getStatus();
+        } catch (final Exception e) {
+            logger.error("Possibly CATASTROPHIC failure while running the ingest workflow", e);
+            result = ResponseCodeEnum.FAIL_INVALID;
         }
 
         // 8. Return PreCheck code and estimated fee
diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/query/QueryWorkflowImpl.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/query/QueryWorkflowImpl.java
@@ -19,6 +19,7 @@
 import static com.hedera.hapi.node.base.HederaFunctionality.GET_ACCOUNT_DETAILS;
 import static com.hedera.hapi.node.base.HederaFunctionality.NETWORK_GET_EXECUTION_TIME;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.BUSY;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.FAIL_INVALID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.NOT_SUPPORTED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.PAYER_ACCOUNT_NOT_FOUND;
@@ -45,9 +46,11 @@
 import com.hedera.node.app.spi.authorization.Authorizer;
 import com.hedera.node.app.spi.fees.ExchangeRateInfo;
 import com.hedera.node.app.spi.records.RecordCache;
+import com.hedera.node.app.spi.workflows.HandleException;
 import com.hedera.node.app.spi.workflows.InsufficientBalanceException;
 import com.hedera.node.app.spi.workflows.PreCheckException;
 import com.hedera.node.app.spi.workflows.QueryContext;
+import com.hedera.node.app.spi.workflows.QueryHandler;
 import com.hedera.node.app.state.HederaState;
 import com.hedera.node.app.throttle.SynchronizedThrottleAccumulator;
 import com.hedera.node.app.workflows.dispatcher.ReadableStoreFactory;
@@ -275,11 +278,16 @@ public void handleQuery(@NonNull final Bytes requestBuffer, @NonNull final Buffe
                     response = handler.findResponse(context, header);
                 }
             } catch (InsufficientBalanceException e) {
-                final var header = createResponseHeader(responseType, e.responseCode(), e.getEstimatedFee());
-                response = handler.createEmptyResponse(header);
+                response = createErrorResponse(handler, responseType, e.responseCode(), e.getEstimatedFee());
             } catch (PreCheckException e) {
-                final var header = createResponseHeader(responseType, e.responseCode(), 0L);
-                response = handler.createEmptyResponse(header);
+                response = createErrorResponse(handler, responseType, e.responseCode(), 0L);
+            } catch (HandleException e) {
+                // Conceptually, this should never happen, because we should use PreCheckException only for queries
+                // But we catch it here to play it safe
+                response = createErrorResponse(handler, responseType, e.getStatus(), 0L);
+            } catch (Exception e) {
+                logger.error("Unexpected exception while handling a query", e);
+                response = createErrorResponse(handler, responseType, FAIL_INVALID, 0L);
             }
         } else {
             response = DEFAULT_UNSUPPORTED_RESPONSE;
@@ -311,6 +319,15 @@ private Query parseQuery(Bytes requestBuffer) {
         }
     }
 
+    private static Response createErrorResponse(
+            @NonNull final QueryHandler handler,
+            @NonNull final ResponseType responseType,
+            @NonNull final ResponseCodeEnum responseCode,
+            final long fee) {
+        final var header = createResponseHeader(responseType, responseCode, fee);
+        return handler.createEmptyResponse(header);
+    }
+
     private static ResponseHeader createResponseHeader(
             @NonNull final ResponseType type, @NonNull final ResponseCodeEnum responseCode, final long fee) {
         return ResponseHeader.newBuilder()
diff --git a/hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/ingest/IngestWorkflowImplTest.java b/hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/ingest/IngestWorkflowImplTest.java
@@ -17,6 +17,7 @@
 package com.hedera.node.app.workflows.ingest;
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.BUSY;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.FAIL_INVALID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SIGNATURE;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION_BODY;
@@ -250,14 +251,18 @@ void onsetFailsWithPreCheckException(ResponseCodeEnum failureReason) throws PreC
 
         @Test
         @DisplayName("If some random exception is thrown from TransactionChecker, the exception is bubbled up")
-        void randomException() throws PreCheckException {
+        void randomException() throws PreCheckException, ParseException {
             // Given a WorkflowOnset that will throw a RuntimeException
             when(transactionChecker.parse(any())).thenThrow(new RuntimeException("parseAndCheck exception"));
 
-            // When the transaction is submitted, then the exception is bubbled up
-            assertThatThrownBy(() -> workflow.submitTransaction(requestBuffer, responseBuffer))
-                    .isInstanceOf(RuntimeException.class)
-                    .hasMessageContaining("parseAndCheck exception");
+            // When the transaction is submitted
+            workflow.submitTransaction(requestBuffer, responseBuffer);
+
+            // Then the response will indicate the platform rejected the transaction
+            final TransactionResponse response = parseResponse(responseBuffer);
+            assertThat(response.nodeTransactionPrecheckCode()).isEqualTo(FAIL_INVALID);
+            // And the cost will be zero
+            assertThat(response.cost()).isZero();
             // And the transaction is not submitted to the platform
             verify(submissionManager, never()).submit(any(), any());
         }
@@ -296,15 +301,19 @@ void testIngestChecksFail(ResponseCodeEnum failureReason) throws PreCheckExcepti
 
         @Test
         @DisplayName("If some random exception is thrown from IngestChecker, the exception is bubbled up")
-        void randomException() throws PreCheckException {
+        void randomException() throws PreCheckException, ParseException {
             // Given a ThrottleAccumulator that will throw a RuntimeException
             when(ingestChecker.runAllChecks(state, transaction, configuration))
                     .thenThrow(new RuntimeException("runAllChecks exception"));
 
-            // When the transaction is submitted, then the exception is bubbled up
-            assertThatThrownBy(() -> workflow.submitTransaction(requestBuffer, responseBuffer))
-                    .isInstanceOf(RuntimeException.class)
-                    .hasMessageContaining("runAllChecks exception");
+            // When the transaction is submitted
+            workflow.submitTransaction(requestBuffer, responseBuffer);
+
+            // Then the response will indicate the platform rejected the transaction
+            final TransactionResponse response = parseResponse(responseBuffer);
+            assertThat(response.nodeTransactionPrecheckCode()).isEqualTo(FAIL_INVALID);
+            // And the cost will be zero
+            assertThat(response.cost()).isZero();
             // And the transaction is not submitted to the platform
             verify(submissionManager, never()).submit(any(), any());
         }
@@ -334,16 +343,20 @@ void testSubmitFails() throws PreCheckException, ParseException {
 
         @Test
         @DisplayName("If some random exception is thrown from submitting to the platform, the exception is bubbled up")
-        void randomException() throws PreCheckException {
+        void randomException() throws PreCheckException, ParseException {
             // Given a SubmissionManager that will throw a RuntimeException from submit
             doThrow(new RuntimeException("submit exception"))
                     .when(submissionManager)
                     .submit(any(), any());
 
-            // When the transaction is submitted, then the exception is bubbled up
-            assertThatThrownBy(() -> workflow.submitTransaction(requestBuffer, responseBuffer))
-                    .isInstanceOf(RuntimeException.class)
-                    .hasMessageContaining("submit exception");
+            // When the transaction is submitted
+            workflow.submitTransaction(requestBuffer, responseBuffer);
+
+            // Then the response will indicate the platform rejected the transaction
+            final TransactionResponse response = parseResponse(responseBuffer);
+            assertThat(response.nodeTransactionPrecheckCode()).isEqualTo(FAIL_INVALID);
+            // And the cost will be zero
+            assertThat(response.cost()).isZero();
         }
     }
 
diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/CryptoDeleteSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/CryptoDeleteSuite.java
@@ -209,7 +209,7 @@ final HapiSpec mustSpecifyTargetId() {
                 .then(cryptoDelete("0.0.0")
                         .sansTargetId()
                         .signedBy(DEFAULT_PAYER)
-                        .hasKnownStatus(ACCOUNT_ID_DOES_NOT_EXIST));
+                        .hasPrecheck(ACCOUNT_ID_DOES_NOT_EXIST));
     }
 
     @HapiTest
diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/token/TokenTransactSpecs.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/token/TokenTransactSpecs.java
@@ -674,7 +674,7 @@ public HapiSpec missingEntitiesRejected() {
                         cryptoTransfer(moving(100_000_000_000_000L, SENTINEL_ACCOUNT)
                                         .between(DEFAULT_PAYER, FUNDING))
                                 .signedBy(DEFAULT_PAYER)
-                                .hasKnownStatus(INVALID_TOKEN_ID));
+                                .hasPrecheck(INVALID_TOKEN_ID));
     }
 
     @HapiTest

Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ public void invoke(`
`140`	`140`	`callsHandledSpeedometer.cycle();`
`141`	`141`	`} catch (final Exception e) {`
`142`	`142`	`// Track the number of times we failed to handle a call`
`143`		`- logger.error("Possibly CATASTROPHIC failure while handling a call and running the ingest workflow", e);`
	`143`	`+ logger.error("Possibly CATASTROPHIC failure while handling a GRPC message", e);`
`144`	`144`	`callsFailedCounter.increment();`
`145`	`145`	`responseObserver.onError(e);`
`146`	`146`	`}`
Original file line number	Diff line number	Diff line change
`@@ -217,6 +217,9 @@ public TransactionInfo runAllChecks(`
`217`	`217`	`throw new PreCheckException(BUSY);`
`218`	`218`	`}`
`219`	`219`
	`220`	`+ // 4a. Run pure checks`
	`221`	`+ dispatcher.dispatchPureChecks(txBody);`
	`222`	`+`
`220`	`223`	`// 5. Get payer account`
`221`	`224`	`final var storeFactory = new ReadableStoreFactory(state);`
`222`	`225`	`final var payer = solvencyPreCheck.getPayerAccount(storeFactory, txInfo.payerID());`
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ final HapiSpec mustSpecifyTargetId() {`
`209`	`209`	`.then(cryptoDelete("0.0.0")`
`210`	`210`	`.sansTargetId()`
`211`	`211`	`.signedBy(DEFAULT_PAYER)`
`212`		`- .hasKnownStatus(ACCOUNT_ID_DOES_NOT_EXIST));`
	`212`	`+ .hasPrecheck(ACCOUNT_ID_DOES_NOT_EXIST));`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`@HapiTest`
Original file line number	Diff line number	Diff line change
`@@ -674,7 +674,7 @@ public HapiSpec missingEntitiesRejected() {`
`674`	`674`	`cryptoTransfer(moving(100_000_000_000_000L, SENTINEL_ACCOUNT)`
`675`	`675`	`.between(DEFAULT_PAYER, FUNDING))`
`676`	`676`	`.signedBy(DEFAULT_PAYER)`
`677`		`- .hasKnownStatus(INVALID_TOKEN_ID));`
	`677`	`+ .hasPrecheck(INVALID_TOKEN_ID));`
`678`	`678`	`}`
`679`	`679`
`680`	`680`	`@HapiTest`