Skip to content

Commit

Permalink
chore: Made StakingValidator static (#14159)
Browse files Browse the repository at this point in the history
Signed-off-by: Harsh Sawarkar <[email protected]>
Co-authored-by: Roger Barker <[email protected]>
Co-authored-by: Michael Heinrichs <[email protected]>
  • Loading branch information
3 people authored Jul 15, 2024
1 parent d435632 commit b53c465
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<CryptoTransferTransactionBody> 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);
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,8 +36,6 @@ public enum TokenServiceApiProvider implements ServiceApiProvider<TokenServiceAp
/** The singleton instance. */
TOKEN_SERVICE_API_PROVIDER;

private final StakingValidator stakingValidator = new StakingValidator();

@Override
public String serviceName() {
return TokenService.NAME;
Expand All @@ -49,7 +46,7 @@ public TokenServiceApi newInstance(
@NonNull final Configuration configuration,
@NonNull final StoreMetricsService storeMetricsService,
@NonNull final WritableStates writableStates) {
return new TokenServiceApiImpl(configuration, storeMetricsService, stakingValidator, writableStates, op -> {
return new TokenServiceApiImpl(configuration, storeMetricsService, writableStates, op -> {
final var assessor = new CustomFeeAssessmentStep(op);
try {
final var result = assessor.assessFees(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,9 +101,6 @@ class TokenServiceApiImplTest {
V0490TokenSchema.ALIASES_KEY, aliasesState));
private WritableAccountStore accountStore;

@Mock
private StakingValidator stakingValidator;

@Mock
private NetworkInfo networkInfo;

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit b53c465

Please sign in to comment.