From b53c465315b7ab8b3c0a292c5adc243abb01b43c Mon Sep 17 00:00:00 2001 From: Harsh Vinod Sawarkar Date: Mon, 15 Jul 2024 22:18:21 +0530 Subject: [PATCH] chore: Made StakingValidator static (#14159) Signed-off-by: Harsh Sawarkar Co-authored-by: Roger Barker Co-authored-by: Michael Heinrichs --- .../token/impl/api/TokenServiceApiImpl.java | 8 +--- .../impl/api/TokenServiceApiProvider.java | 5 +-- .../impl/handlers/CryptoCreateHandler.java | 2 +- .../impl/handlers/CryptoUpdateHandler.java | 2 +- .../impl/validators/StakingValidator.java | 10 ++--- .../test/api/TokenServiceApiImplTest.java | 43 ++++++++----------- .../handlers/CryptoDeleteHandlerTest.java | 3 +- 7 files changed, 28 insertions(+), 45 deletions(-) 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 328b4d7dbde9..d465e98a9222 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 @@ -67,7 +67,6 @@ public class TokenServiceApiImpl implements TokenServiceApi { private static final Key STANDIN_CONTRACT_KEY = Key.newBuilder().contractID(ContractID.newBuilder().contractNum(0)).build(); - private final StakingValidator stakingValidator; private final WritableAccountStore accountStore; private final AccountID fundingAccountID; private final AccountID stakingRewardAccountID; @@ -79,20 +78,17 @@ public class TokenServiceApiImpl implements TokenServiceApi { * Constructs a {@link TokenServiceApiImpl} * @param config the configuration * @param storeMetricsService the store metrics service - * @param stakingValidator the staking validator * @param writableStates the writable states * @param customFeeTest a predicate for determining if a transfer has custom fees */ public TokenServiceApiImpl( @NonNull final Configuration config, @NonNull final StoreMetricsService storeMetricsService, - @NonNull final StakingValidator stakingValidator, @NonNull final WritableStates writableStates, @NonNull final Predicate customFeeTest) { this.customFeeTest = customFeeTest; requireNonNull(config); this.accountStore = new WritableAccountStore(writableStates, config, storeMetricsService); - this.stakingValidator = requireNonNull(stakingValidator); // Determine whether staking is enabled stakingConfig = config.getConfigData(StakingConfig.class); @@ -125,7 +121,7 @@ public void assertValidStakingElectionForCreation( @Nullable final Long stakedNodeIdInOp, @NonNull final ReadableAccountStore accountStore, @NonNull final NetworkInfo networkInfo) { - stakingValidator.validateStakedIdForCreation( + StakingValidator.validateStakedIdForCreation( isStakingEnabled, hasDeclineRewardChange, stakedIdKind, @@ -144,7 +140,7 @@ public void assertValidStakingElectionForUpdate( @Nullable final Long stakedNodeIdInOp, @NonNull final ReadableAccountStore accountStore, @NonNull final NetworkInfo networkInfo) { - stakingValidator.validateStakedIdForUpdate( + StakingValidator.validateStakedIdForUpdate( isStakingEnabled, hasDeclineRewardChange, stakedIdKind, diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiProvider.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiProvider.java index 4a82880cdff1..5e23bb6835c2 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiProvider.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/api/TokenServiceApiProvider.java @@ -23,7 +23,6 @@ import com.hedera.node.app.service.token.impl.ReadableTokenRelationStoreImpl; import com.hedera.node.app.service.token.impl.ReadableTokenStoreImpl; import com.hedera.node.app.service.token.impl.handlers.transfer.CustomFeeAssessmentStep; -import com.hedera.node.app.service.token.impl.validators.StakingValidator; import com.hedera.node.app.spi.api.ServiceApiProvider; import com.hedera.node.app.spi.metrics.StoreMetricsService; import com.swirlds.config.api.Configuration; @@ -37,8 +36,6 @@ public enum TokenServiceApiProvider implements ServiceApiProvider { + return new TokenServiceApiImpl(configuration, storeMetricsService, writableStates, op -> { final var assessor = new CustomFeeAssessmentStep(op); try { final var result = assessor.assessFees( diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoCreateHandler.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoCreateHandler.java index d31bdaef11a4..5e5ec0d9a948 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoCreateHandler.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoCreateHandler.java @@ -374,7 +374,7 @@ private void validateSemantics( // Validate the staking information included in this account creation. if (op.hasStakedAccountId() || op.hasStakedNodeId()) { - stakingValidator.validateStakedIdForCreation( + StakingValidator.validateStakedIdForCreation( context.configuration().getConfigData(StakingConfig.class).isEnabled(), op.declineReward(), op.stakedId().kind().name(), diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoUpdateHandler.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoUpdateHandler.java index babcd6ac41f0..6c805bdb02f1 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoUpdateHandler.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoUpdateHandler.java @@ -312,7 +312,7 @@ private void validateFields( .validateAutoRenewPeriod(op.autoRenewPeriod().seconds()); } - stakingValidator.validateStakedIdForUpdate( + StakingValidator.validateStakedIdForUpdate( context.configuration().getConfigData(StakingConfig.class).isEnabled(), op.hasDeclineReward(), op.stakedId().kind().name(), diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/StakingValidator.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/StakingValidator.java index 8dfab79ab089..9160c7072f48 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/StakingValidator.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/validators/StakingValidator.java @@ -55,7 +55,7 @@ public StakingValidator() { * @param accountStore readable account store * @param networkInfo network info */ - public void validateStakedIdForCreation( + public static void validateStakedIdForCreation( final boolean isStakingEnabled, final boolean hasDeclineRewardChange, @NonNull final String stakedIdKind, @@ -89,7 +89,7 @@ public void validateStakedIdForCreation( * @param accountStore readable account store * @param networkInfo network info */ - public void validateStakedIdForUpdate( + public static void validateStakedIdForUpdate( final boolean isStakingEnabled, final boolean hasDeclineRewardChange, @NonNull final String stakedIdKind, @@ -121,7 +121,7 @@ public void validateStakedIdForUpdate( * @param stakedNodeIdInOp staked node id * @param accountStore readable account store */ - private void validateStakedId( + private static void validateStakedId( @NonNull final String stakedIdKind, @Nullable final AccountID stakedAccountIdInOp, @Nullable final Long stakedNodeIdInOp, @@ -146,7 +146,7 @@ private void validateStakedId( * @param stakedNodeId staked node id * @return true if staked id is a sentinel value */ - private boolean isValidStakingSentinel( + private static boolean isValidStakingSentinel( @NonNull String stakedIdKind, @Nullable AccountID stakedAccountId, @Nullable Long stakedNodeId) { if (stakedIdKind.equals("STAKED_ACCOUNT_ID")) { // current checking only account num since shard and realm are 0.0 @@ -158,7 +158,7 @@ private boolean isValidStakingSentinel( } } - private boolean isValidStakingIdForCreation( + private static boolean isValidStakingIdForCreation( final String stakedIdKind, final AccountID stakedAccountId, final Long stakedNodeId) { if (stakedIdKind.equals("STAKED_ACCOUNT_ID")) { // current checking only account num since shard and realm are 0.0 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 b86790d27207..b4b18adc29ee 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 @@ -26,7 +26,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; import com.hedera.hapi.node.base.AccountID; import com.hedera.hapi.node.base.ContractID; @@ -100,9 +101,6 @@ class TokenServiceApiImplTest { V0490TokenSchema.ALIASES_KEY, aliasesState)); private WritableAccountStore accountStore; - @Mock - private StakingValidator stakingValidator; - @Mock private NetworkInfo networkInfo; @@ -117,8 +115,7 @@ class TokenServiceApiImplTest { @BeforeEach void setUp() { accountStore = new WritableAccountStore(writableStates, DEFAULT_CONFIG, storeMetricsService); - subject = new TokenServiceApiImpl( - DEFAULT_CONFIG, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(DEFAULT_CONFIG, storeMetricsService, writableStates, customFeeTest); } @Test @@ -129,11 +126,14 @@ void delegatesToCustomFeeTest() { @Test void delegatesStakingValidationAsExpected() { - subject.assertValidStakingElectionForCreation( - true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); - - verify(stakingValidator) - .validateStakedIdForCreation(true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); + try (var mockedValidator = mockStatic(StakingValidator.class)) { + subject.assertValidStakingElectionForCreation( + true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo); + mockedValidator.verify( + () -> StakingValidator.validateStakedIdForCreation( + true, false, "STAKED_NODE_ID", null, 123L, accountStore, networkInfo), + times(1)); + } } @Test @@ -475,8 +475,7 @@ void withStakingRewards() { final var config = configBuilder.withValue("staking.isEnabled", true).getOrCreateConfig(); - subject = new TokenServiceApiImpl( - config, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(config, storeMetricsService, writableStates, customFeeTest); // When we charge network+service fees of 10 tinybars and a node fee of 2 tinybars subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb); @@ -506,8 +505,7 @@ void withoutStakingRewards() { final var config = configBuilder.withValue("staking.isEnabled", false).getOrCreateConfig(); - subject = new TokenServiceApiImpl( - config, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(config, storeMetricsService, writableStates, customFeeTest); // When we charge fees of 10 tinybars subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb); @@ -547,8 +545,7 @@ void missingFundingAccount() { .withValue("ledger.fundingAccount", unknownAccountId.accountNumOrThrow()) .getOrCreateConfig(); - subject = new TokenServiceApiImpl( - config, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(config, storeMetricsService, writableStates, customFeeTest); // When we try to charge a payer account that DOES exist, then we get an IllegalStateException assertThatThrownBy(() -> subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb)) @@ -565,8 +562,7 @@ void missingStakingRewardAccount() { .withValue("accounts.stakingRewardAccount", unknownAccountId.accountNumOrThrow()) .getOrCreateConfig(); - subject = new TokenServiceApiImpl( - config, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(config, storeMetricsService, writableStates, customFeeTest); // When we try to charge a payer account that DOES exist, then we get an IllegalStateException assertThatThrownBy(() -> subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb)) @@ -583,8 +579,7 @@ void missingNodeRewardAccount() { .withValue("accounts.nodeRewardAccount", unknownAccountId.accountNumOrThrow()) .getOrCreateConfig(); - subject = new TokenServiceApiImpl( - config, storeMetricsService, stakingValidator, writableStates, customFeeTest); + subject = new TokenServiceApiImpl(config, storeMetricsService, writableStates, customFeeTest); // When we try to charge a payer account that DOES exist, then we get an IllegalStateException assertThatThrownBy(() -> subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb)) @@ -598,11 +593,7 @@ void chargesRemainingBalanceIfInsufficient() { fees = new Fees(1000, 100, 0); // more than the 100 the user has subject = new TokenServiceApiImpl( - configBuilder.getOrCreateConfig(), - storeMetricsService, - stakingValidator, - writableStates, - customFeeTest); + configBuilder.getOrCreateConfig(), storeMetricsService, writableStates, customFeeTest); subject.chargeFees(EOA_ACCOUNT_ID, NODE_ACCOUNT_ID, fees, rb); final var payerAccount = requireNonNull(accountState.get(EOA_ACCOUNT_ID)); diff --git a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoDeleteHandlerTest.java b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoDeleteHandlerTest.java index d7a2522fa075..5d04bf33270f 100644 --- a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoDeleteHandlerTest.java +++ b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/CryptoDeleteHandlerTest.java @@ -422,8 +422,7 @@ private void givenTxnWith(AccountID deleteAccountId, AccountID transferAccountId final var txn = deleteAccountTransaction(deleteAccountId, transferAccountId); given(handleContext.body()).willReturn(txn); given(handleContext.expiryValidator()).willReturn(expiryValidator); - final var impl = new TokenServiceApiImpl( - configuration, storeMetricsService, stakingValidator, writableStates, op -> false); + final var impl = new TokenServiceApiImpl(configuration, storeMetricsService, writableStates, op -> false); given(storeFactory.serviceApi(TokenServiceApi.class)).willReturn(impl); } }