Merge branch 'develop' into 09567-fix-token-update-expiry-e2e

hashgraph · Nov 8, 2023 · de2caa0 · de2caa0
2 parents 94d965f + 527b92a
commit de2caa0
Show file tree

Hide file tree

Showing 23 changed files with 214 additions and 109 deletions.
diff --git a/...node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/validation/ExpiryValidator.java b/...node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/validation/ExpiryValidator.java
@@ -16,6 +16,7 @@
 
 package com.hedera.node.app.spi.validation;
 
+import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hedera.hapi.node.base.ResponseCodeEnum;
 import com.hedera.node.app.spi.workflows.HandleException;
 import com.hedera.node.app.spi.workflows.TransactionHandler;
@@ -31,16 +32,18 @@ public interface ExpiryValidator {
      * Validates the expiry metadata for an attempt to create an entity.
      * @param entityCanSelfFundRenewal whether the entity can self-fund its own auto-renewal
      * @param creationMetadata the expiry metadata for the attempted creation
-     * @param isForTopicCreation if the expiry metadata is for topic creation.
+     * @param functionality the HederaFunctionality that is being attempted
      *                           This is only needed until differential testing is completed to comply with
-     *                           mono-service respoinse codes.
+     *                           mono-service response codes.
      *
      * @return the expiry metadata that will result from the creation
      * @throws HandleException if the metadata is invalid
      */
     @NonNull
     ExpiryMeta resolveCreationAttempt(
-            boolean entityCanSelfFundRenewal, @NonNull ExpiryMeta creationMetadata, final boolean isForTopicCreation);
+            boolean entityCanSelfFundRenewal,
+            @NonNull final ExpiryMeta creationMetadata,
+            @NonNull final HederaFunctionality functionality);
 
     /**
      * Validates the expiry metadata for an attempt to update an entity, and returns the
@@ -49,11 +52,13 @@ ExpiryMeta resolveCreationAttempt(
      *
      * @param currentMetadata the current expiry metadata for the entity
      * @param updateMetadata the expiry metadata for the attempted update
+     * @param isForTokenUpdate if the expiry metadata is for token update
      * @return the expiry metadata that will result from the update
      * @throws HandleException if the metadata is invalid
      */
     @NonNull
-    ExpiryMeta resolveUpdateAttempt(@NonNull ExpiryMeta currentMetadata, @NonNull ExpiryMeta updateMetadata);
+    ExpiryMeta resolveUpdateAttempt(
+            @NonNull ExpiryMeta currentMetadata, @NonNull ExpiryMeta updateMetadata, boolean isForTokenUpdate);
 
     /**
      * Gets the expiration status of an entity based on the {@link EntityType}.

diff --git a/...src/main/java/com/hedera/node/app/workflows/handle/validation/AttributeValidatorImpl.java b/...src/main/java/com/hedera/node/app/workflows/handle/validation/AttributeValidatorImpl.java
@@ -16,6 +16,7 @@
 
 package com.hedera.node.app.workflows.handle.validation;
 
+import static com.hedera.hapi.node.base.ResponseCodeEnum.AUTORENEW_DURATION_NOT_IN_RANGE;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.BAD_ENCODING;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_EXPIRATION_TIME;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
@@ -100,10 +101,11 @@ public void validateExpiry(long expiry) {
     @Override
     public void validateAutoRenewPeriod(long autoRenewPeriod) {
         final var ledgerConfig = context.configuration().getConfigData(LedgerConfig.class);
+        validateTrue(autoRenewPeriod > 0, INVALID_RENEWAL_PERIOD);
         validateTrue(
                 autoRenewPeriod >= ledgerConfig.autoRenewPeriodMinDuration()
                         && autoRenewPeriod <= ledgerConfig.autoRenewPeriodMaxDuration(),
-                INVALID_RENEWAL_PERIOD);
+                AUTORENEW_DURATION_NOT_IN_RANGE);
     }
 
     private void validateKeyAtLevel(@NonNull final Key key, final int level) {

diff --git a/...pp/src/main/java/com/hedera/node/app/workflows/handle/validation/ExpiryValidatorImpl.java b/...pp/src/main/java/com/hedera/node/app/workflows/handle/validation/ExpiryValidatorImpl.java
@@ -17,16 +17,19 @@
 package com.hedera.node.app.workflows.handle.validation;
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_EXPIRED_AND_PENDING_REMOVAL;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.AUTORENEW_DURATION_NOT_IN_RANGE;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.CONTRACT_EXPIRED_AND_PENDING_REMOVAL;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.EXPIRATION_REDUCTION_NOT_ALLOWED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_AUTORENEW_ACCOUNT;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
 import static com.hedera.node.app.service.mono.fees.calculation.FeeCalcUtils.clampedAdd;
 import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
 import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
 import static java.util.Objects.requireNonNull;
 
 import com.hedera.hapi.node.base.AccountID;
+import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hedera.hapi.node.base.ResponseCodeEnum;
 import com.hedera.node.app.service.evm.exceptions.InvalidTransactionException;
 import com.hedera.node.app.service.mono.pbj.PbjConverter;
@@ -62,7 +65,7 @@ public ExpiryValidatorImpl(@NonNull final HandleContext context) {
     public ExpiryMeta resolveCreationAttempt(
             final boolean entityCanSelfFundRenewal,
             @NonNull final ExpiryMeta creationMeta,
-            final boolean isForTopicCreate) {
+            @NonNull final HederaFunctionality functionality) {
         if (creationMeta.hasAutoRenewAccountId()) {
             validateAutoRenewAccount(creationMeta.autoRenewAccountId());
         }
@@ -74,33 +77,52 @@ public ExpiryMeta resolveCreationAttempt(
         if (hasCompleteAutoRenewSpec(entityCanSelfFundRenewal, creationMeta)) {
             effectiveExpiry = clampedAdd(context.consensusNow().getEpochSecond(), creationMeta.autoRenewPeriod());
         }
-        // In mono-service this check is done first for topic creation.
         // To maintain same behaviour for differential testing this condition is needed.
         // FUTURE: This condition should be removed after differential testing is done
-        if (isForTopicCreate) {
+        if (functionality == HederaFunctionality.CONSENSUS_CREATE_TOPIC) {
+            // In mono-service this check is done first for topic creation.
             context.attributeValidator().validateExpiry(effectiveExpiry);
             // Even if the effective expiry is valid, we still also require any explicit auto-renew period to be valid
-            if (creationMeta.hasAutoRenewPeriod()) {
-                context.attributeValidator().validateAutoRenewPeriod(creationMeta.autoRenewPeriod());
+            validateAutoRenew(creationMeta);
+        } else if (functionality == HederaFunctionality.TOKEN_CREATE) {
+            // In mono-service, INVALID_RENEWAL_PERIOD is thrown for token creation and update
+            // if the autoRenewPeriod is not in range.
+            // It would be more correct to throw AUTO_RENEW_DURATION_NOT_IN_RANGE like the other services do,
+            // but this would break differential testing. So for now, we replicate the mono-service behaviour.
+            try {
+                validateAutoRenew(creationMeta);
+            } catch (HandleException e) {
+                if (e.getStatus() == AUTORENEW_DURATION_NOT_IN_RANGE) {
+                    throw new HandleException(INVALID_RENEWAL_PERIOD);
+                } else {
+                    throw e;
+                }
             }
+            context.attributeValidator().validateExpiry(effectiveExpiry);
         } else {
             // Even if the effective expiry is valid, we still also require any explicit auto-renew period to be valid
-            if (creationMeta.hasAutoRenewPeriod()) {
-                context.attributeValidator().validateAutoRenewPeriod(creationMeta.autoRenewPeriod());
-            }
+            validateAutoRenew(creationMeta);
             context.attributeValidator().validateExpiry(effectiveExpiry);
         }
 
         return new ExpiryMeta(effectiveExpiry, creationMeta.autoRenewPeriod(), creationMeta.autoRenewAccountId());
     }
 
+    private void validateAutoRenew(final ExpiryMeta creationMeta) {
+        if (creationMeta.hasAutoRenewPeriod()) {
+            context.attributeValidator().validateAutoRenewPeriod(creationMeta.autoRenewPeriod());
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
     @NonNull
     @Override
     public ExpiryMeta resolveUpdateAttempt(
-            @NonNull final ExpiryMeta currentMeta, @NonNull final ExpiryMeta updateMeta) {
+            @NonNull final ExpiryMeta currentMeta,
+            @NonNull final ExpiryMeta updateMeta,
+            final boolean isForTokenUpdate) {
         if (updateMeta.hasAutoRenewAccountId()) {
             validateAutoRenewAccount(updateMeta.autoRenewAccountId());
         }
@@ -114,8 +136,21 @@ public ExpiryMeta resolveUpdateAttempt(
 
         var resolvedAutoRenewPeriod = currentMeta.autoRenewPeriod();
         if (updateMeta.hasAutoRenewPeriod()) {
-            context.attributeValidator().validateAutoRenewPeriod(updateMeta.autoRenewPeriod());
-            resolvedAutoRenewPeriod = updateMeta.autoRenewPeriod();
+            try {
+                context.attributeValidator().validateAutoRenewPeriod(updateMeta.autoRenewPeriod());
+                resolvedAutoRenewPeriod = updateMeta.autoRenewPeriod();
+            } catch (HandleException e) {
+                // In mono-service, INVALID_RENEWAL_PERIOD is thrown for token creation and update
+                // if the autoRenewPeriod is not in range.
+                // It would be more correct to throw AUTO_RENEW_DURATION_NOT_IN_RANGE like the other services do,
+                // but this would break differential testing. So for now, we replicate the mono-service behaviour.
+                // FUTURE: This condition should be removed after differential testing is done
+                if (isForTokenUpdate && e.getStatus() == AUTORENEW_DURATION_NOT_IN_RANGE) {
+                    throw new HandleException(INVALID_RENEWAL_PERIOD);
+                } else {
+                    throw e;
+                }
+            }
         }
 
         var resolvedAutoRenewAccountId = currentMeta.autoRenewAccountId();

diff --git a/...ain/java/com/hedera/node/app/workflows/handle/validation/StandardizedExpiryValidator.java b/...ain/java/com/hedera/node/app/workflows/handle/validation/StandardizedExpiryValidator.java
@@ -17,14 +17,17 @@
 package com.hedera.node.app.workflows.handle.validation;
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_EXPIRED_AND_PENDING_REMOVAL;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.AUTORENEW_DURATION_NOT_IN_RANGE;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.CONTRACT_EXPIRED_AND_PENDING_REMOVAL;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.EXPIRATION_REDUCTION_NOT_ALLOWED;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_AUTORENEW_ACCOUNT;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.OK;
 import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
 import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
 
 import com.hedera.hapi.node.base.AccountID;
+import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hedera.hapi.node.base.ResponseCodeEnum;
 import com.hedera.node.app.service.mono.config.HederaNumbers;
 import com.hedera.node.app.service.mono.store.models.Id;
@@ -78,7 +81,7 @@ public StandardizedExpiryValidator(
     public ExpiryMeta resolveCreationAttempt(
             final boolean entityCanSelfFundRenewal,
             @NonNull final ExpiryMeta creationMeta,
-            final boolean isForTopicCreate) {
+            @NonNull final HederaFunctionality functionality) {
         if (creationMeta.hasAutoRenewAccountId()) {
             validateAutoRenewAccount(creationMeta.autoRenewAccountId());
         }
@@ -94,13 +97,30 @@ public ExpiryMeta resolveCreationAttempt(
         // In mono-service this check is done first for topic creation.
         // To maintain same behaviour for differential testing this condition is needed.
         // FUTURE: This condition should be removed after differential testing is done
-        if (isForTopicCreate) {
+        if (functionality == HederaFunctionality.CONSENSUS_CREATE_TOPIC) {
             attributeValidator.validateExpiry(effectiveExpiry);
 
             // Even if the effective expiry is valid, we still also require any explicit auto-renew period to be valid
             if (creationMeta.hasAutoRenewPeriod()) {
                 attributeValidator.validateAutoRenewPeriod(creationMeta.autoRenewPeriod());
             }
+        } else if (functionality == HederaFunctionality.TOKEN_CREATE) {
+            // In mono-service, INVALID_RENEWAL_PERIOD is thrown for token creation and update
+            // if the autoRenewPeriod is not in range.
+            // It would be more correct to throw AUTO_RENEW_DURATION_NOT_IN_RANGE like the other services do,
+            // but this would break differential testing. So for now, we replicate the mono-service behaviour.
+            try {
+                if (creationMeta.hasAutoRenewPeriod()) {
+                    attributeValidator.validateAutoRenewPeriod(creationMeta.autoRenewPeriod());
+                }
+            } catch (HandleException e) {
+                if (e.getStatus() == AUTORENEW_DURATION_NOT_IN_RANGE) {
+                    throw new HandleException(INVALID_RENEWAL_PERIOD);
+                } else {
+                    throw e;
+                }
+            }
+            attributeValidator.validateExpiry(effectiveExpiry);
         } else {
             // Even if the effective expiry is valid, we still also require any explicit auto-renew period to be valid
             if (creationMeta.hasAutoRenewPeriod()) {
@@ -117,7 +137,9 @@ public ExpiryMeta resolveCreationAttempt(
     @NonNull
     @Override
     public ExpiryMeta resolveUpdateAttempt(
-            @NonNull final ExpiryMeta currentMeta, @NonNull final ExpiryMeta updateMeta) {
+            @NonNull final ExpiryMeta currentMeta,
+            @NonNull final ExpiryMeta updateMeta,
+            final boolean isForTokenUpdate) {
         if (updateMeta.hasAutoRenewAccountId()) {
             validateAutoRenewAccount(updateMeta.autoRenewAccountId());
         }
@@ -131,8 +153,21 @@ public ExpiryMeta resolveUpdateAttempt(
 
         var resolvedAutoRenewPeriod = currentMeta.autoRenewPeriod();
         if (updateMeta.hasAutoRenewPeriod()) {
-            attributeValidator.validateAutoRenewPeriod(updateMeta.autoRenewPeriod());
-            resolvedAutoRenewPeriod = updateMeta.autoRenewPeriod();
+            try {
+                attributeValidator.validateAutoRenewPeriod(updateMeta.autoRenewPeriod());
+                resolvedAutoRenewPeriod = updateMeta.autoRenewPeriod();
+            } catch (HandleException e) {
+                // In mono-service, INVALID_RENEWAL_PERIOD is thrown for token creation and update
+                // if the autoRenewPeriod is not in range.
+                // It would be more correct to throw AUTO_RENEW_DURATION_NOT_IN_RANGE like the other services do,
+                // but this would break differential testing. So for now, we replicate the mono-service behaviour.
+                // FUTURE: This condition should be removed after differential testing is done
+                if (isForTokenUpdate && e.getStatus() == AUTORENEW_DURATION_NOT_IN_RANGE) {
+                    throw new HandleException(INVALID_RENEWAL_PERIOD);
+                } else {
+                    throw e;
+                }
+            }
         }
 
         var resolvedAutoRenewId = currentMeta.autoRenewAccountId();

diff --git a/...test/java/com/hedera/node/app/workflows/handle/validation/AttributeValidatorImplTest.java b/...test/java/com/hedera/node/app/workflows/handle/validation/AttributeValidatorImplTest.java
@@ -16,9 +16,9 @@
 
 package com.hedera.node.app.workflows.handle.validation;
 
+import static com.hedera.hapi.node.base.ResponseCodeEnum.AUTORENEW_DURATION_NOT_IN_RANGE;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.BAD_ENCODING;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_EXPIRATION_TIME;
-import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_RENEWAL_PERIOD;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ZERO_BYTE_IN_STRING;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.MEMO_TOO_LONG;
 import static com.hedera.node.app.spi.fixtures.workflows.ExceptionConditions.responseCode;
@@ -117,7 +117,7 @@ void rejectsBriefAutoRenewPeriod() {
 
         assertThatThrownBy(() -> subject.validateAutoRenewPeriod(55L))
                 .isInstanceOf(HandleException.class)
-                .has(responseCode(INVALID_RENEWAL_PERIOD));
+                .has(responseCode(AUTORENEW_DURATION_NOT_IN_RANGE));
     }
 
     @Test
@@ -130,7 +130,7 @@ void rejectsOverLongAutoRenewPeriod() {
 
         assertThatThrownBy(() -> subject.validateAutoRenewPeriod(10_001L))
                 .isInstanceOf(HandleException.class)
-                .has(responseCode(INVALID_RENEWAL_PERIOD));
+                .has(responseCode(AUTORENEW_DURATION_NOT_IN_RANGE));
     }
 
     @Test