From c118442db1361c7a2c347a226fdac81d8eaf0289 Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Wed, 8 Nov 2023 10:10:50 +0200 Subject: [PATCH 01/13] feat: add staking elections validation Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../impl/handlers/ContractUpdateHandler.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 23a5c06b469c..c52b2a9a4317 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -48,6 +48,7 @@ import com.hedera.node.app.spi.workflows.TransactionHandler; import com.hedera.node.config.data.EntitiesConfig; import com.hedera.node.config.data.LedgerConfig; +import com.hedera.node.config.data.StakingConfig; import com.hedera.node.config.data.TokensConfig; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Optional; @@ -102,7 +103,7 @@ public void handle(@NonNull final HandleContext context) throws HandleException final var accountStore = context.readableStore(ReadableAccountStore.class); final var toBeUpdated = accountStore.getContractById(target); - validateSemantics(toBeUpdated, context, op); + validateSemantics(toBeUpdated, context, op, accountStore); final var changed = update(toBeUpdated, context, op); context.serviceApi(TokenServiceApi.class).updateContract(changed); @@ -169,7 +170,11 @@ public Account update( return builder.build(); } - private void validateSemantics(Account contract, HandleContext context, ContractUpdateTransactionBody op) { + private void validateSemantics( + Account contract, + HandleContext context, + ContractUpdateTransactionBody op, + ReadableAccountStore accountStore) { validateTrue(contract != null, INVALID_CONTRACT_ID); if (op.hasAdminKey()) { @@ -199,6 +204,18 @@ private void validateSemantics(Account contract, HandleContext context, Contract op.hasAutoRenewPeriod() ? op.autoRenewPeriod().seconds() : NA, null); context.expiryValidator().resolveUpdateAttempt(currentMetadata, updateMeta); + + context.serviceApi(TokenServiceApi.class) + .assertValidStakingElection( + context.configuration() + .getConfigData(StakingConfig.class) + .isEnabled(), + contract.declineReward(), + contract.stakedId().kind().name(), + contract.stakedAccountId(), + contract.stakedNodeId(), + accountStore, + context.networkInfo()); } boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { From 8b553b45435808b0f83a39a7fdeff9676aabd57c Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Wed, 8 Nov 2023 10:54:49 +0200 Subject: [PATCH 02/13] fix: calling the update staking election validation instead of the create one Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../impl/handlers/ContractUpdateHandler.java | 28 +++++++++---------- .../impl/infra/HevmTransactionFactory.java | 2 +- .../infra/HevmTransactionFactoryTest.java | 2 +- .../token/impl/api/TokenServiceApiImpl.java | 21 +++++++++++++- .../test/api/TokenServiceApiImplTest.java | 3 +- .../service/token/api/TokenServiceApi.java | 24 ++++++++++++++-- 6 files changed, 60 insertions(+), 20 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index c52b2a9a4317..7fd654dc88ed 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -171,10 +171,10 @@ public Account update( } private void validateSemantics( - Account contract, - HandleContext context, - ContractUpdateTransactionBody op, - ReadableAccountStore accountStore) { + Account contract, + HandleContext context, + ContractUpdateTransactionBody op, + ReadableAccountStore accountStore) { validateTrue(contract != null, INVALID_CONTRACT_ID); if (op.hasAdminKey()) { @@ -206,16 +206,16 @@ private void validateSemantics( context.expiryValidator().resolveUpdateAttempt(currentMetadata, updateMeta); context.serviceApi(TokenServiceApi.class) - .assertValidStakingElection( - context.configuration() - .getConfigData(StakingConfig.class) - .isEnabled(), - contract.declineReward(), - contract.stakedId().kind().name(), - contract.stakedAccountId(), - contract.stakedNodeId(), - accountStore, - context.networkInfo()); + .assertValidStakingElectionForUpdate( + context.configuration() + .getConfigData(StakingConfig.class) + .isEnabled(), + contract.declineReward(), + contract.stakedId().kind().name(), + contract.stakedAccountId(), + contract.stakedNodeId(), + accountStore, + context.networkInfo()); } boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/infra/HevmTransactionFactory.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/infra/HevmTransactionFactory.java index 94bb0244b031..3aeb7fa3ffee 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/infra/HevmTransactionFactory.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/infra/HevmTransactionFactory.java @@ -268,7 +268,7 @@ private void assertValidCreation(@NonNull final ContractCreateTransactionBody bo REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); final var usesNonDefaultProxyId = body.hasProxyAccountID() && !AccountID.DEFAULT.equals(body.proxyAccountID()); validateFalse(usesNonDefaultProxyId, PROXY_ACCOUNT_ID_FIELD_IS_DEPRECATED); - tokenServiceApi.assertValidStakingElection( + tokenServiceApi.assertValidStakingElectionForCreation( stakingConfig.isEnabled(), body.declineReward(), body.stakedId().kind().name(), diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/infra/HevmTransactionFactoryTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/infra/HevmTransactionFactoryTest.java index 0ccdb4809110..d5e0392d8875 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/infra/HevmTransactionFactoryTest.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/infra/HevmTransactionFactoryTest.java @@ -281,7 +281,7 @@ void fromHapiCreationDoesNotPermitNonDefaultProxyField() { void fromHapiCreationValidatesStaking() { doThrow(new HandleException(INVALID_STAKING_ID)) .when(tokenServiceApi) - .assertValidStakingElection( + .assertValidStakingElectionForCreation( DEFAULT_STAKING_CONFIG.isEnabled(), false, "STAKED_NODE_ID", diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java index 9991d2f10422..a92cd66dae8a 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiImpl.java @@ -106,7 +106,7 @@ public TokenServiceApiImpl( } @Override - public void assertValidStakingElection( + public void assertValidStakingElectionForCreation( final boolean isStakingEnabled, final boolean hasDeclineRewardChange, @NonNull final String stakedIdKind, @@ -124,6 +124,25 @@ public void assertValidStakingElection( networkInfo); } + @Override + public void assertValidStakingElectionForUpdate( + final boolean isStakingEnabled, + final boolean hasDeclineRewardChange, + @NonNull final String stakedIdKind, + @Nullable final AccountID stakedAccountIdInOp, + @Nullable final Long stakedNodeIdInOp, + @NonNull final ReadableAccountStore accountStore, + @NonNull final NetworkInfo networkInfo) { + stakingValidator.validateStakedIdForUpdate( + isStakingEnabled, + hasDeclineRewardChange, + stakedIdKind, + stakedAccountIdInOp, + stakedNodeIdInOp, + accountStore, + networkInfo); + } + /** * {@inheritDoc} */ diff --git a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/api/TokenServiceApiImplTest.java b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/api/TokenServiceApiImplTest.java index 30759249c0b8..0c737e94a782 100644 --- a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/api/TokenServiceApiImplTest.java +++ b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/api/TokenServiceApiImplTest.java @@ -120,7 +120,8 @@ void delegatesToCustomFeeTest() { @Test void delegatesStakingValidationAsExpected() { - subject.assertValidStakingElection(true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); + subject.assertValidStakingElectionForCreation( + true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); verify(stakingValidator) .validateStakedIdForCreation(true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); diff --git a/hedera-node/hedera-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java b/hedera-node/hedera-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java index 9a16e194c875..e3c08f8fa6a0 100644 --- a/hedera-node/hedera-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java +++ b/hedera-node/hedera-token-service/src/main/java/com/hedera/node/app/service/token/api/TokenServiceApi.java @@ -61,7 +61,7 @@ void deleteAndTransfer( @NonNull DeleteCapableTransactionRecordBuilder recordBuilder); /** - * Validates the given staking election relative to the given account store, network info, and staking config. + * Validates the creation of a given staking election relative to the given account store, network info, and staking config. * * @param isStakingEnabled if staking is enabled * @param hasDeclineRewardChange if the transaction body has decline reward field to be updated @@ -71,7 +71,27 @@ void deleteAndTransfer( * @param accountStore readable account store * @throws HandleException if the staking election is invalid */ - void assertValidStakingElection( + void assertValidStakingElectionForCreation( + boolean isStakingEnabled, + boolean hasDeclineRewardChange, + @NonNull String stakedIdKind, + @Nullable AccountID stakedAccountIdInOp, + @Nullable Long stakedNodeIdInOp, + @NonNull ReadableAccountStore accountStore, + @NonNull NetworkInfo networkInfo); + + /** + * Validates the update of a given staking election relative to the given account store, network info, and staking config. + * + * @param isStakingEnabled if staking is enabled + * @param hasDeclineRewardChange if the transaction body has decline reward field to be updated + * @param stakedIdKind staked id kind (account or node) + * @param stakedAccountIdInOp staked account id + * @param stakedNodeIdInOp staked node id + * @param accountStore readable account store + * @throws HandleException if the staking election is invalid + */ + void assertValidStakingElectionForUpdate( boolean isStakingEnabled, boolean hasDeclineRewardChange, @NonNull String stakedIdKind, From 5ad7a7992d84bc3d34b539e57211133dc5f84142 Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:52:55 +0200 Subject: [PATCH 03/13] feature: add auto associations validation Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../contract/impl/handlers/ContractUpdateHandler.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 7fd654dc88ed..4d8144978eb5 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -23,6 +23,7 @@ import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_AUTORENEW_ACCOUNT; import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_CONTRACT_ID; import static com.hedera.hapi.node.base.ResponseCodeEnum.MODIFYING_IMMUTABLE_CONTRACT; +import static com.hedera.hapi.node.base.ResponseCodeEnum.NOT_SUPPORTED; import static com.hedera.hapi.node.base.ResponseCodeEnum.REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT; import static com.hedera.node.app.service.token.api.AccountSummariesApi.SENTINEL_ACCOUNT_ID; import static com.hedera.node.app.spi.HapiUtils.EMPTY_KEY_LIST; @@ -46,6 +47,7 @@ import com.hedera.node.app.spi.workflows.PreCheckException; import com.hedera.node.app.spi.workflows.PreHandleContext; import com.hedera.node.app.spi.workflows.TransactionHandler; +import com.hedera.node.config.data.ContractsConfig; import com.hedera.node.config.data.EntitiesConfig; import com.hedera.node.config.data.LedgerConfig; import com.hedera.node.config.data.StakingConfig; @@ -154,6 +156,7 @@ public Account update( final var ledgerConfig = context.configuration().getConfigData(LedgerConfig.class); final var entitiesConfig = context.configuration().getConfigData(EntitiesConfig.class); final var tokensConfig = context.configuration().getConfigData(TokensConfig.class); + final var contractsConfig = context.configuration().getConfigData(ContractsConfig.class); validateFalse( op.maxAutomaticTokenAssociations() > ledgerConfig.maxAutoAssociations(), @@ -166,7 +169,10 @@ public Account update( REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); builder.maxAutoAssociations(op.maxAutomaticTokenAssociations()); + + validateTrue(contractsConfig.allowAutoAssociations(), NOT_SUPPORTED); } + return builder.build(); } From 60160e682052cdb18ac9883928b4776b7c5ad5b6 Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Wed, 8 Nov 2023 13:16:55 +0200 Subject: [PATCH 04/13] feature: update onlyAffectsExpiry check Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../contract/impl/handlers/ContractUpdateHandler.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 4d8144978eb5..22eb38991aed 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -226,10 +226,11 @@ private void validateSemantics( boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { return !(op.hasProxyAccountID() - || op.hasFileID() - || affectsMemo(op) - || op.hasAutoRenewPeriod() - || op.hasAdminKey()); + || op.hasFileID() + || affectsMemo(op) + || op.hasAutoRenewPeriod() + || op.hasAdminKey()) + || op.hasMaxAutomaticTokenAssociations(); } boolean affectsMemo(ContractUpdateTransactionBody op) { From d754d9076ef7d02f7585e0dcdfcaf73761ccfe99 Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Wed, 8 Nov 2023 14:49:48 +0200 Subject: [PATCH 05/13] feature: add new validation and enable a test Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../impl/handlers/ContractUpdateHandler.java | 21 ++++++++++++------- .../contract/hapi/ContractUpdateSuite.java | 1 + 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 22eb38991aed..1b6a7562738c 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -35,12 +35,11 @@ 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.Key.KeyOneOfType; +import com.hedera.hapi.node.base.Key; import com.hedera.hapi.node.contract.ContractUpdateTransactionBody; import com.hedera.hapi.node.state.token.Account; import com.hedera.node.app.service.token.ReadableAccountStore; import com.hedera.node.app.service.token.api.TokenServiceApi; -import com.hedera.node.app.spi.key.KeyUtils; import com.hedera.node.app.spi.validation.ExpiryMeta; import com.hedera.node.app.spi.workflows.HandleContext; import com.hedera.node.app.spi.workflows.HandleException; @@ -183,11 +182,8 @@ private void validateSemantics( ReadableAccountStore accountStore) { validateTrue(contract != null, INVALID_CONTRACT_ID); - if (op.hasAdminKey()) { - boolean keyNotSentinel = !EMPTY_KEY_LIST.equals(op.adminKey()); - boolean keyIsUnset = op.adminKey().key().kind() == KeyOneOfType.UNSET; - boolean keyIsNotValid = !KeyUtils.isValid(op.adminKey()); - validateFalse(keyNotSentinel && (keyIsUnset || keyIsNotValid), INVALID_ADMIN_KEY); + if (op.hasAdminKey() && processAdminKey(op)) { + throw new HandleException(INVALID_ADMIN_KEY); } if (op.hasExpirationTime()) { @@ -224,6 +220,17 @@ private void validateSemantics( context.networkInfo()); } + private boolean processAdminKey(ContractUpdateTransactionBody op) { + if (EMPTY_KEY_LIST.equals(op.adminKey())) { + return false; + } + return keyIfAcceptable(op.adminKey()); + } + + private boolean keyIfAcceptable(Key candidate) { + return candidate == null || candidate.contractID() != null; + } + boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { return !(op.hasProxyAccountID() || op.hasFileID() diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java index b5e218530e0b..b53a46175273 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java @@ -323,6 +323,7 @@ private HapiSpec canMakeContractImmutableWithEmptyKeyList() { .then(contractUpdate(CONTRACT).newKey(NEW_ADMIN_KEY).hasKnownStatus(MODIFYING_IMMUTABLE_CONTRACT)); } + @HapiTest private HapiSpec givenAdminKeyMustBeValid() { final var contract = "BalanceLookup"; return defaultHapiSpec("GivenAdminKeyMustBeValid") From 2fa659bbbfc9a9640971474a8a2841d9a9bca6ed Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Thu, 9 Nov 2023 06:02:07 +0200 Subject: [PATCH 06/13] fix: incorrect validation Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../service/contract/impl/handlers/ContractUpdateHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 1b6a7562738c..6165b092caa4 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -40,6 +40,7 @@ import com.hedera.hapi.node.state.token.Account; import com.hedera.node.app.service.token.ReadableAccountStore; import com.hedera.node.app.service.token.api.TokenServiceApi; +import com.hedera.node.app.spi.key.KeyUtils; import com.hedera.node.app.spi.validation.ExpiryMeta; import com.hedera.node.app.spi.workflows.HandleContext; import com.hedera.node.app.spi.workflows.HandleException; @@ -228,7 +229,8 @@ private boolean processAdminKey(ContractUpdateTransactionBody op) { } private boolean keyIfAcceptable(Key candidate) { - return candidate == null || candidate.contractID() != null; + boolean keyIsNotValid = !KeyUtils.isValid(candidate); + return keyIsNotValid || candidate.contractID() != null; } boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { From 32e40487a52fa56ad654fb3f4b499ca84a91b1ee Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Fri, 10 Nov 2023 17:23:57 +0200 Subject: [PATCH 07/13] test: initial unit tests Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../handlers/ContractUpdateHandlerTest.java | 259 ++++++++++++++++++ 1 file changed, 259 insertions(+) create mode 100644 hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java new file mode 100644 index 000000000000..c17be93e488d --- /dev/null +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java @@ -0,0 +1,259 @@ +/* + * Copyright (C) 2023 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.node.app.service.contract.impl.test.handlers; + +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_ADMIN_KEY; +import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_AUTORENEW_ACCOUNT; +import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_CONTRACT_ID; +import static com.hedera.hapi.node.base.ResponseCodeEnum.MODIFYING_IMMUTABLE_CONTRACT; +import static com.hedera.node.app.service.contract.impl.test.TestHelpers.assertFailsWith; +import static com.hedera.node.app.spi.fixtures.Assertions.assertThrowsPreCheck; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.when; + +import com.hedera.hapi.node.base.ContractID; +import com.hedera.hapi.node.base.Key; +import com.hedera.hapi.node.base.Timestamp; +import com.hedera.hapi.node.base.TransactionID; +import com.hedera.hapi.node.contract.ContractUpdateTransactionBody; +import com.hedera.hapi.node.state.token.Account; +import com.hedera.hapi.node.transaction.TransactionBody; +import com.hedera.node.app.service.contract.impl.handlers.ContractUpdateHandler; +import com.hedera.node.app.service.token.ReadableAccountStore; +import com.hedera.node.app.spi.fixtures.workflows.FakePreHandleContext; +import com.hedera.node.app.spi.validation.AttributeValidator; +import com.hedera.node.app.spi.workflows.HandleContext; +import com.hedera.node.app.spi.workflows.HandleException; +import com.hedera.node.app.spi.workflows.PreCheckException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class ContractUpdateHandlerTest extends ContractHandlerTestBase { + + final TransactionID transactionID = TransactionID.newBuilder() + .accountID(payer) + .transactionValidStart(consensusTimestamp) + .build(); + + @Mock + private HandleContext context; + + @Mock + private Account contract; + + @Mock + private AttributeValidator attributeValidator; + + private ContractUpdateHandler subject; + + @BeforeEach + public void setUp() { + subject = new ContractUpdateHandler(); + } + + @Test + void sigRequiredWithoutKeyFails() throws PreCheckException { + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder()) + .transactionID(transactionID) + .build(); + final var context = new FakePreHandleContext(accountStore, txn); + + assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_CONTRACT_ID); + } + + @Test + void invalidAutoRenewAccountIdFails() throws PreCheckException { + when(accountStore.getContractById(targetContract)).thenReturn(payerAccount); + + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance( + ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .autoRenewAccountId(asAccount("0.0.11111")) // invalid account + ) + .transactionID(transactionID) + .build(); + final var context = new FakePreHandleContext(accountStore, txn); + + assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_AUTORENEW_ACCOUNT); + } + + @Test + void handleWithNullContextFails() { + final HandleContext context = null; + assertThrows(NullPointerException.class, () -> subject.handle(context)); + } + + @Test + void handleWithNullContractUpdateTransactionBodyFails() { + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance((ContractUpdateTransactionBody) null) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertThrows(NullPointerException.class, () -> subject.handle(context)); + } + + @Test + void handleWithNoContractIdFails() { + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance( + ContractUpdateTransactionBody.newBuilder().contractID((ContractID) null)) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertThrows(NullPointerException.class, () -> subject.handle(context)); + } + + @Test + void handleWithNonExistingContractIdFails() { + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance( + ContractUpdateTransactionBody.newBuilder().contractID(targetContract)) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertFailsWith(INVALID_CONTRACT_ID, () -> subject.handle(context)); + } + + @Test + void handleWithInvalidKeyFails() { + when(accountStore.getContractById(targetContract)).thenReturn(contract); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(Key.newBuilder())) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertFailsWith(INVALID_ADMIN_KEY, () -> subject.handle(context)); + } + + @Test + void handleWithInvalidContractIdKeyFails() { + when(accountStore.getContractById(targetContract)).thenReturn(contract); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(Key.newBuilder().contractID(ContractID.DEFAULT))) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertFailsWith(INVALID_ADMIN_KEY, () -> subject.handle(context)); + } + + @Test + void handleWithAValidContractIdKeyFails() { + when(accountStore.getContractById(targetContract)).thenReturn(contract); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(Key.newBuilder() + .contractID(ContractID.newBuilder().contractNum(100)))) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + + assertFailsWith(INVALID_ADMIN_KEY, () -> subject.handle(context)); + } + + @Test + void handleWithInvalidExpirationTimeAndExpiredAndPendingRemovalTrueFails() { + final var expirationTime = 1L; + + when(accountStore.getContractById(targetContract)).thenReturn(contract); + doReturn(attributeValidator).when(context).attributeValidator(); + doThrow(HandleException.class).when(attributeValidator).validateExpiry(expirationTime); + when(contract.expiredAndPendingRemoval()).thenReturn(true); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .expirationTime(Timestamp.newBuilder().seconds(expirationTime))) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(CONTRACT_EXPIRED_AND_PENDING_REMOVAL, () -> subject.handle(context)); + } + + @Test + void handleModifyImmutableContract() { + when(accountStore.getContractById(targetContract)).thenReturn(contract); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey)) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(MODIFYING_IMMUTABLE_CONTRACT, () -> subject.handle(context)); + } + + @Test + void handleWithExpirationTimeLesserThenExpirationSecondsFails() { + final var expirationTime = 1L; + + doReturn(attributeValidator).when(context).attributeValidator(); + when(accountStore.getContractById(targetContract)).thenReturn(contract); + when(contract.key()).thenReturn(Key.newBuilder().build()); + when(contract.expirationSecond()).thenReturn(expirationTime + 1); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo") + .expirationTime(Timestamp.newBuilder().seconds(expirationTime))) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(EXPIRATION_REDUCTION_NOT_ALLOWED, () -> subject.handle(context)); + } +} From ff73aba0b2332959e3c3e3d38cba27a4957fc54b Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 10:53:10 +0200 Subject: [PATCH 08/13] refactor: move validations from update to validateSemantics Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../impl/handlers/ContractUpdateHandler.java | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 6165b092caa4..84c2e9ac9770 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -153,26 +153,8 @@ public Account update( builder.autoRenewAccountId(op.autoRenewAccountId()); } if (op.hasMaxAutomaticTokenAssociations()) { - final var ledgerConfig = context.configuration().getConfigData(LedgerConfig.class); - final var entitiesConfig = context.configuration().getConfigData(EntitiesConfig.class); - final var tokensConfig = context.configuration().getConfigData(TokensConfig.class); - final var contractsConfig = context.configuration().getConfigData(ContractsConfig.class); - - validateFalse( - op.maxAutomaticTokenAssociations() > ledgerConfig.maxAutoAssociations(), - REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); - - final long newMax = op.maxAutomaticTokenAssociations(); - validateFalse(newMax < contract.maxAutoAssociations(), EXISTING_AUTOMATIC_ASSOCIATIONS_EXCEED_GIVEN_LIMIT); - validateFalse( - entitiesConfig.limitTokenAssociations() && newMax > tokensConfig.maxPerAccount(), - REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); - builder.maxAutoAssociations(op.maxAutomaticTokenAssociations()); - - validateTrue(contractsConfig.allowAutoAssociations(), NOT_SUPPORTED); } - return builder.build(); } @@ -199,6 +181,25 @@ private void validateSemantics( validateFalse(!onlyAffectsExpiry(op) && !isMutable(contract), MODIFYING_IMMUTABLE_CONTRACT); validateFalse(reducesExpiry(op, contract.expirationSecond()), EXPIRATION_REDUCTION_NOT_ALLOWED); + if (op.hasMaxAutomaticTokenAssociations()) { + final var ledgerConfig = context.configuration().getConfigData(LedgerConfig.class); + final var entitiesConfig = context.configuration().getConfigData(EntitiesConfig.class); + final var tokensConfig = context.configuration().getConfigData(TokensConfig.class); + final var contractsConfig = context.configuration().getConfigData(ContractsConfig.class); + + validateFalse( + op.maxAutomaticTokenAssociations() > ledgerConfig.maxAutoAssociations(), + REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); + + final long newMax = op.maxAutomaticTokenAssociations(); + validateFalse(newMax < contract.maxAutoAssociations(), EXISTING_AUTOMATIC_ASSOCIATIONS_EXCEED_GIVEN_LIMIT); + validateFalse( + entitiesConfig.limitTokenAssociations() && newMax > tokensConfig.maxPerAccount(), + REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); + + validateTrue(contractsConfig.allowAutoAssociations(), NOT_SUPPORTED); + } + // validate expiry metadata final var currentMetadata = new ExpiryMeta(contract.expirationSecond(), contract.autoRenewSeconds(), contract.autoRenewAccountId()); From 0445e52445325d762fffdf8dbbf3dabcb1b9b28c Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 11:02:36 +0200 Subject: [PATCH 09/13] refactor: reorder methods Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../impl/handlers/ContractUpdateHandler.java | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 84c2e9ac9770..0f6c6b3512ca 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -111,53 +111,6 @@ public void handle(@NonNull final HandleContext context) throws HandleException context.serviceApi(TokenServiceApi.class).updateContract(changed); } - public Account update( - @NonNull final Account contract, - @NonNull final HandleContext context, - @NonNull final ContractUpdateTransactionBody op) { - final var builder = contract.copyBuilder(); - if (op.hasAdminKey()) { - if (EMPTY_KEY_LIST.equals(op.adminKey())) { - builder.key(contract.key()); - } else { - builder.key(op.adminKey()); - } - } - if (op.hasExpirationTime()) { - if (contract.expiredAndPendingRemoval()) { - builder.expiredAndPendingRemoval(false); - } - builder.expirationSecond(op.expirationTime().seconds()); - } - if (op.hasAutoRenewPeriod()) { - builder.autoRenewSeconds(op.autoRenewPeriod().seconds()); - } - if (affectsMemo(op)) { - final var newMemo = op.hasMemoWrapper() ? op.memoWrapper() : op.memo(); - context.attributeValidator().validateMemo(newMemo); - builder.memo(newMemo); - } - if (op.hasStakedAccountId()) { - if (SENTINEL_ACCOUNT_ID.equals(op.stakedAccountId())) { - builder.stakedAccountId((AccountID) null); - } else { - builder.stakedAccountId(op.stakedAccountId()); - } - } else if (op.hasStakedNodeId()) { - builder.stakedNodeId(op.stakedNodeId()); - } - if (op.hasDeclineReward()) { - builder.declineReward(op.declineReward()); - } - if (op.hasAutoRenewAccountId()) { - builder.autoRenewAccountId(op.autoRenewAccountId()); - } - if (op.hasMaxAutomaticTokenAssociations()) { - builder.maxAutoAssociations(op.maxAutomaticTokenAssociations()); - } - return builder.build(); - } - private void validateSemantics( Account contract, HandleContext context, @@ -234,7 +187,7 @@ private boolean keyIfAcceptable(Key candidate) { return keyIsNotValid || candidate.contractID() != null; } - boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { + private boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { return !(op.hasProxyAccountID() || op.hasFileID() || affectsMemo(op) @@ -243,11 +196,11 @@ boolean onlyAffectsExpiry(ContractUpdateTransactionBody op) { || op.hasMaxAutomaticTokenAssociations(); } - boolean affectsMemo(ContractUpdateTransactionBody op) { + private boolean affectsMemo(ContractUpdateTransactionBody op) { return op.hasMemoWrapper() || (op.memo() != null && op.memo().length() > 0); } - boolean isMutable(final Account contract) { + private boolean isMutable(final Account contract) { return Optional.ofNullable(contract.key()) .map(key -> !key.hasContractID()) .orElse(false); @@ -256,4 +209,51 @@ boolean isMutable(final Account contract) { private boolean reducesExpiry(ContractUpdateTransactionBody op, long curExpiry) { return op.hasExpirationTime() && op.expirationTime().seconds() < curExpiry; } + + public Account update( + @NonNull final Account contract, + @NonNull final HandleContext context, + @NonNull final ContractUpdateTransactionBody op) { + final var builder = contract.copyBuilder(); + if (op.hasAdminKey()) { + if (EMPTY_KEY_LIST.equals(op.adminKey())) { + builder.key(contract.key()); + } else { + builder.key(op.adminKey()); + } + } + if (op.hasExpirationTime()) { + if (contract.expiredAndPendingRemoval()) { + builder.expiredAndPendingRemoval(false); + } + builder.expirationSecond(op.expirationTime().seconds()); + } + if (op.hasAutoRenewPeriod()) { + builder.autoRenewSeconds(op.autoRenewPeriod().seconds()); + } + if (affectsMemo(op)) { + final var newMemo = op.hasMemoWrapper() ? op.memoWrapper() : op.memo(); + context.attributeValidator().validateMemo(newMemo); + builder.memo(newMemo); + } + if (op.hasStakedAccountId()) { + if (SENTINEL_ACCOUNT_ID.equals(op.stakedAccountId())) { + builder.stakedAccountId((AccountID) null); + } else { + builder.stakedAccountId(op.stakedAccountId()); + } + } else if (op.hasStakedNodeId()) { + builder.stakedNodeId(op.stakedNodeId()); + } + if (op.hasDeclineReward()) { + builder.declineReward(op.declineReward()); + } + if (op.hasAutoRenewAccountId()) { + builder.autoRenewAccountId(op.autoRenewAccountId()); + } + if (op.hasMaxAutomaticTokenAssociations()) { + builder.maxAutoAssociations(op.maxAutomaticTokenAssociations()); + } + return builder.build(); + } } From d7492e036074c22edf3666b867fc4ccbc49d255a Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 11:34:32 +0200 Subject: [PATCH 10/13] refactor: reuse variable Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../contract/impl/handlers/ContractUpdateHandler.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java index 0f6c6b3512ca..fd586d353b2c 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.java @@ -140,11 +140,12 @@ private void validateSemantics( final var tokensConfig = context.configuration().getConfigData(TokensConfig.class); final var contractsConfig = context.configuration().getConfigData(ContractsConfig.class); + final long newMax = op.maxAutomaticTokenAssociations(); + validateFalse( - op.maxAutomaticTokenAssociations() > ledgerConfig.maxAutoAssociations(), + newMax > ledgerConfig.maxAutoAssociations(), REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT); - final long newMax = op.maxAutomaticTokenAssociations(); validateFalse(newMax < contract.maxAutoAssociations(), EXISTING_AUTOMATIC_ASSOCIATIONS_EXCEED_GIVEN_LIMIT); validateFalse( entitiesConfig.limitTokenAssociations() && newMax > tokensConfig.maxPerAccount(), From 363c2320dd25220edf7a3044b93033a046679dca Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 11:35:11 +0200 Subject: [PATCH 11/13] test: finish unit tests Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../handlers/ContractUpdateHandlerTest.java | 373 +++++++++++++++++- 1 file changed, 372 insertions(+), 1 deletion(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java index c17be93e488d..396618f4978d 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java @@ -17,33 +17,61 @@ package com.hedera.node.app.service.contract.impl.test.handlers; import static com.hedera.hapi.node.base.ResponseCodeEnum.CONTRACT_EXPIRED_AND_PENDING_REMOVAL; +import static com.hedera.hapi.node.base.ResponseCodeEnum.EXISTING_AUTOMATIC_ASSOCIATIONS_EXCEED_GIVEN_LIMIT; import static com.hedera.hapi.node.base.ResponseCodeEnum.EXPIRATION_REDUCTION_NOT_ALLOWED; import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ADMIN_KEY; import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_AUTORENEW_ACCOUNT; import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_CONTRACT_ID; import static com.hedera.hapi.node.base.ResponseCodeEnum.MODIFYING_IMMUTABLE_CONTRACT; +import static com.hedera.hapi.node.base.ResponseCodeEnum.NOT_SUPPORTED; +import static com.hedera.hapi.node.base.ResponseCodeEnum.REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT; import static com.hedera.node.app.service.contract.impl.test.TestHelpers.assertFailsWith; +import static com.hedera.node.app.service.token.api.AccountSummariesApi.SENTINEL_ACCOUNT_ID; +import static com.hedera.node.app.spi.HapiUtils.EMPTY_KEY_LIST; import static com.hedera.node.app.spi.fixtures.Assertions.assertThrowsPreCheck; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.hedera.hapi.node.base.AccountID; import com.hedera.hapi.node.base.ContractID; +import com.hedera.hapi.node.base.Duration; import com.hedera.hapi.node.base.Key; import com.hedera.hapi.node.base.Timestamp; import com.hedera.hapi.node.base.TransactionID; import com.hedera.hapi.node.contract.ContractUpdateTransactionBody; import com.hedera.hapi.node.state.token.Account; +import com.hedera.hapi.node.state.token.Account.Builder; +import com.hedera.hapi.node.state.token.Account.StakedIdOneOfType; import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.service.contract.impl.handlers.ContractUpdateHandler; import com.hedera.node.app.service.token.ReadableAccountStore; +import com.hedera.node.app.service.token.api.TokenServiceApi; import com.hedera.node.app.spi.fixtures.workflows.FakePreHandleContext; import com.hedera.node.app.spi.validation.AttributeValidator; +import com.hedera.node.app.spi.validation.ExpiryValidator; import com.hedera.node.app.spi.workflows.HandleContext; import com.hedera.node.app.spi.workflows.HandleException; import com.hedera.node.app.spi.workflows.PreCheckException; +import com.hedera.node.config.data.ContractsConfig; +import com.hedera.node.config.data.EntitiesConfig; +import com.hedera.node.config.data.LedgerConfig; +import com.hedera.node.config.data.StakingConfig; +import com.hedera.node.config.data.TokensConfig; +import com.hedera.pbj.runtime.OneOf; +import com.hedera.test.utils.KeyUtils; +import com.swirlds.config.api.Configuration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -53,7 +81,7 @@ @ExtendWith(MockitoExtension.class) class ContractUpdateHandlerTest extends ContractHandlerTestBase { - final TransactionID transactionID = TransactionID.newBuilder() + private final TransactionID transactionID = TransactionID.newBuilder() .accountID(payer) .transactionValidStart(consensusTimestamp) .build(); @@ -67,6 +95,30 @@ class ContractUpdateHandlerTest extends ContractHandlerTestBase { @Mock private AttributeValidator attributeValidator; + @Mock + private ExpiryValidator expiryValidator; + + @Mock + private TokenServiceApi tokenServiceApi; + + @Mock + private Configuration configuration; + + @Mock + private StakingConfig stakingConfig; + + @Mock + private LedgerConfig ledgerConfig; + + @Mock + private EntitiesConfig entitiesConfig; + + @Mock + private TokensConfig tokensConfig; + + @Mock + private ContractsConfig contractsConfig; + private ContractUpdateHandler subject; @BeforeEach @@ -256,4 +308,323 @@ void handleWithExpirationTimeLesserThenExpirationSecondsFails() { assertFailsWith(EXPIRATION_REDUCTION_NOT_ALLOWED, () -> subject.handle(context)); } + + @Test + void maxAutomaticTokenAssociationsBiggerThenAllowedFails() { + final var maxAutomaticTokenAssociations = 10; + + when(configuration.getConfigData(LedgerConfig.class)).thenReturn(ledgerConfig); + when(ledgerConfig.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations - 1); + when(context.configuration()).thenReturn(configuration); + + when(accountStore.getContractById(targetContract)).thenReturn(contract); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo") + .maxAutomaticTokenAssociations(maxAutomaticTokenAssociations)) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT, () -> subject.handle(context)); + } + + @Test + void maxAutomaticTokenAssociationsSmallerThenContractLimitFails() { + final var maxAutomaticTokenAssociations = 10; + + when(configuration.getConfigData(LedgerConfig.class)).thenReturn(ledgerConfig); + when(ledgerConfig.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations + 1); + when(context.configuration()).thenReturn(configuration); + + when(accountStore.getContractById(targetContract)).thenReturn(contract); + when(contract.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations + 1); + + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo") + .maxAutomaticTokenAssociations(maxAutomaticTokenAssociations)) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(EXISTING_AUTOMATIC_ASSOCIATIONS_EXCEED_GIVEN_LIMIT, () -> subject.handle(context)); + } + + @Test + void maxAutomaticTokenAssociationsBiggerThenMaxConfigFails() { + final var maxAutomaticTokenAssociations = 10; + + when(configuration.getConfigData(LedgerConfig.class)).thenReturn(ledgerConfig); + when(configuration.getConfigData(EntitiesConfig.class)).thenReturn(entitiesConfig); + when(configuration.getConfigData(TokensConfig.class)).thenReturn(tokensConfig); + when(ledgerConfig.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations + 1); + when(entitiesConfig.limitTokenAssociations()).thenReturn(true); + when(tokensConfig.maxPerAccount()).thenReturn(maxAutomaticTokenAssociations - 1); + when(context.configuration()).thenReturn(configuration); + + when(contract.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations - 1); + when(accountStore.getContractById(targetContract)).thenReturn(contract); + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo") + .maxAutomaticTokenAssociations(maxAutomaticTokenAssociations)) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(REQUESTED_NUM_AUTOMATIC_ASSOCIATIONS_EXCEEDS_ASSOCIATION_LIMIT, () -> subject.handle(context)); + } + + @Test + void maxAutomaticTokenAssociationsWhenItIsNotAllowedFails() { + final var maxAutomaticTokenAssociations = 10; + + when(configuration.getConfigData(LedgerConfig.class)).thenReturn(ledgerConfig); + when(configuration.getConfigData(EntitiesConfig.class)).thenReturn(entitiesConfig); + when(configuration.getConfigData(TokensConfig.class)).thenReturn(tokensConfig); + when(configuration.getConfigData(ContractsConfig.class)).thenReturn(contractsConfig); + when(ledgerConfig.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations + 1); + when(entitiesConfig.limitTokenAssociations()).thenReturn(true); + when(tokensConfig.maxPerAccount()).thenReturn(maxAutomaticTokenAssociations + 1); + when(contractsConfig.allowAutoAssociations()).thenReturn(false); + when(context.configuration()).thenReturn(configuration); + + when(contract.maxAutoAssociations()).thenReturn(maxAutomaticTokenAssociations - 1); + when(accountStore.getContractById(targetContract)).thenReturn(contract); + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo") + .maxAutomaticTokenAssociations(maxAutomaticTokenAssociations)) + .transactionID(transactionID) + .build(); + + when(context.body()).thenReturn(txn); + + assertFailsWith(NOT_SUPPORTED, () -> subject.handle(context)); + } + + @Test + void verifyTheCorrectOutsideValidatorsAndUpdateContractAPIAreCalled() { + doReturn(attributeValidator).when(context).attributeValidator(); + when(accountStore.getContractById(targetContract)).thenReturn(contract); + when(contract.key()).thenReturn(Key.newBuilder().build()); + when(contract.stakedId()).thenReturn(new OneOf<>(StakedIdOneOfType.STAKED_ACCOUNT_ID, null)); + when(context.expiryValidator()).thenReturn(expiryValidator); + when(context.serviceApi(TokenServiceApi.class)).thenReturn(tokenServiceApi); + given(context.readableStore(ReadableAccountStore.class)).willReturn(accountStore); + final var txn = TransactionBody.newBuilder() + .contractUpdateInstance(ContractUpdateTransactionBody.newBuilder() + .contractID(targetContract) + .adminKey(adminKey) + .memo("memo")) + .transactionID(transactionID) + .build(); + when(context.body()).thenReturn(txn); + when(context.configuration()).thenReturn(configuration); + when(configuration.getConfigData(StakingConfig.class)).thenReturn(stakingConfig); + when(stakingConfig.isEnabled()).thenReturn(true); + when(contract.copyBuilder()).thenReturn(mock(Builder.class)); + + subject.handle(context); + + verify(expiryValidator, times(1)).resolveUpdateAttempt(any(), any()); + verify(tokenServiceApi, times(1)) + .assertValidStakingElectionForUpdate(anyBoolean(), anyBoolean(), any(), any(), any(), any(), any()); + verify(tokenServiceApi, times(1)).updateContract(any()); + } + + @Test + void adminKeyUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .adminKey(KeyUtils.A_COMPLEX_KEY) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.adminKey(), updatedContract.key()); + } + + @Test + void adminKeyNotUpdatedWhenKeyIsEmpty() { + final var contract = Account.newBuilder().key(KeyUtils.A_COMPLEX_KEY).build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .adminKey(EMPTY_KEY_LIST) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(contract.key(), updatedContract.key()); + } + + @Test + void expirationTimeUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .expirationTime(Timestamp.newBuilder().seconds(10)) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.expirationTime().seconds(), updatedContract.expirationSecond()); + assertFalse(updatedContract.expiredAndPendingRemoval()); + } + + @Test + void autoRenewSecondsUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .autoRenewPeriod(Duration.newBuilder().seconds(10)) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.autoRenewPeriod().seconds(), updatedContract.autoRenewSeconds()); + } + + @Test + void memoUpdatedPassingMemoField() { + when(context.attributeValidator()).thenReturn(attributeValidator); + + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder().memo("memo").build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.memo(), updatedContract.memo()); + verify(attributeValidator, times(1)).validateMemo(op.memo()); + } + + @Test + void memoUpdatedPassingMemoWrapperField() { + when(context.attributeValidator()).thenReturn(attributeValidator); + + final var contract = Account.newBuilder().build(); + final var op = + ContractUpdateTransactionBody.newBuilder().memoWrapper("memo").build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.memoWrapper(), updatedContract.memo()); + verify(attributeValidator, times(1)).validateMemo(op.memoWrapper()); + } + + @Test + void stakedAccountIdUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .stakedAccountId(AccountID.newBuilder().accountNum(1)) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.stakedAccountId(), updatedContract.stakedAccountId()); + } + + @Test + void stakedAccountIdWithSentinelAccountID() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .stakedAccountId(SENTINEL_ACCOUNT_ID) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertNull(updatedContract.stakedAccountId()); + } + + @Test + void stakedNodeIdUpdated() { + final var contract = Account.newBuilder().build(); + final var op = + ContractUpdateTransactionBody.newBuilder().stakedNodeId(10).build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.stakedNodeId(), updatedContract.stakedNodeId()); + } + + @Test + void declineRewardUpdated() { + final var contract = Account.newBuilder().build(); + final var op = + ContractUpdateTransactionBody.newBuilder().declineReward(true).build(); + + final var updatedContract = subject.update(contract, context, op); + + assertTrue(updatedContract.declineReward()); + } + + @Test + void autoRenewAccountIdUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .autoRenewAccountId(AccountID.newBuilder().accountNum(10)) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.autoRenewAccountId(), updatedContract.autoRenewAccountId()); + } + + @Test + void maxAutomaticTokenAssociationsUpdated() { + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .maxAutomaticTokenAssociations(10) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.maxAutomaticTokenAssociations(), updatedContract.maxAutoAssociations()); + } + + @Test + void updateAllFields() { + when(context.attributeValidator()).thenReturn(attributeValidator); + + final var contract = Account.newBuilder().build(); + final var op = ContractUpdateTransactionBody.newBuilder() + .adminKey(KeyUtils.A_COMPLEX_KEY) + .expirationTime(Timestamp.newBuilder().seconds(10)) + .autoRenewPeriod(Duration.newBuilder().seconds(10)) + .memo("memo") + .stakedAccountId(AccountID.newBuilder().accountNum(1)) + .stakedNodeId(10) + .declineReward(true) + .autoRenewAccountId(AccountID.newBuilder().accountNum(10)) + .maxAutomaticTokenAssociations(10) + .build(); + + final var updatedContract = subject.update(contract, context, op); + + assertEquals(op.adminKey(), updatedContract.key()); + assertEquals(op.expirationTime().seconds(), updatedContract.expirationSecond()); + assertFalse(updatedContract.expiredAndPendingRemoval()); + assertEquals(op.autoRenewPeriod().seconds(), updatedContract.autoRenewSeconds()); + assertEquals(op.memo(), updatedContract.memo()); + assertEquals(op.stakedAccountId(), updatedContract.stakedAccountId()); + assertEquals(op.stakedNodeId(), updatedContract.stakedNodeId()); + assertTrue(updatedContract.declineReward()); + assertEquals(op.autoRenewAccountId(), updatedContract.autoRenewAccountId()); + assertEquals(op.maxAutomaticTokenAssociations(), updatedContract.maxAutoAssociations()); + verify(attributeValidator, times(1)).validateMemo(op.memo()); + } } From 27eedc2b5468f463f77719bb3871e5c7dfb429c0 Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 13:25:52 +0200 Subject: [PATCH 12/13] test: fix compile error Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../contract/impl/test/handlers/ContractUpdateHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java index 396618f4978d..08fd05990f71 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/handlers/ContractUpdateHandlerTest.java @@ -444,7 +444,7 @@ void verifyTheCorrectOutsideValidatorsAndUpdateContractAPIAreCalled() { subject.handle(context); - verify(expiryValidator, times(1)).resolveUpdateAttempt(any(), any()); + verify(expiryValidator, times(1)).resolveUpdateAttempt(any(), any(), anyBoolean()); verify(tokenServiceApi, times(1)) .assertValidStakingElectionForUpdate(anyBoolean(), anyBoolean(), any(), any(), any(), any(), any()); verify(tokenServiceApi, times(1)).updateContract(any()); From ed4d17b7217f636bc9214cb6c41fd85ad5be360d Mon Sep 17 00:00:00 2001 From: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> Date: Mon, 13 Nov 2023 13:26:03 +0200 Subject: [PATCH 13/13] test: enable passing test Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com> --- .../services/bdd/suites/contract/hapi/ContractUpdateSuite.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java index b53a46175273..ce3eb9566ea8 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractUpdateSuite.java @@ -151,6 +151,7 @@ private HapiSpec updateStakingFieldsWorks() { } // https://github.com/hashgraph/hedera-services/issues/2877 + @HapiTest private HapiSpec eip1014AddressAlwaysHasPriority() { final var contract = "VariousCreate2Calls"; final var creationTxn = "creationTxn";