From a2db3b0bfe596c89a541e4f43b6a6719b8fc7bcc Mon Sep 17 00:00:00 2001 From: Michael Tinker Date: Wed, 17 Jul 2024 21:06:03 -0500 Subject: [PATCH] Add tests, fix typo in throttle advisor Signed-off-by: Michael Tinker --- .../node/app/spi/workflows/HandleContext.java | 2 +- .../SingleTransactionRecordBuilder.java | 10 +++- .../node/app/throttle/AppThrottleAdviser.java | 2 +- .../handle/DispatchHandleContext.java | 4 +- .../AssociateTokenRecipientsStep.java | 21 ++++--- .../junit/extensions/SpecEntityExtension.java | 4 +- .../bdd/spec/dsl/annotations/AccountSpec.java | 6 ++ .../bdd/spec/dsl/entities/SpecAccount.java | 4 +- .../UnlimitedAutoAssociationSuite.java | 58 +++++++++++++++++++ 9 files changed, 95 insertions(+), 16 deletions(-) diff --git a/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/HandleContext.java b/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/HandleContext.java index 0d23530f96b6..dafa5fdb3489 100644 --- a/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/HandleContext.java +++ b/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/HandleContext.java @@ -124,7 +124,7 @@ enum PrecedingTransactionCategory { * @param amount the amount to charge * @throws IllegalArgumentException if the payer cannot afford the fee */ - void chargePayerFee(long amount); + boolean tryToChargePayer(long amount); /** * Returns the current {@link Configuration} for the node. diff --git a/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/record/SingleTransactionRecordBuilder.java b/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/record/SingleTransactionRecordBuilder.java index 695f2935f947..805a721c5c11 100644 --- a/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/record/SingleTransactionRecordBuilder.java +++ b/hedera-node/hedera-app-spi/src/main/java/com/hedera/node/app/spi/workflows/record/SingleTransactionRecordBuilder.java @@ -16,7 +16,8 @@ package com.hedera.node.app.spi.workflows.record; -import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.USER; +import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.CHILD; +import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.PRECEDING; import com.hedera.hapi.node.base.ResponseCodeEnum; import com.hedera.hapi.node.base.Transaction; @@ -63,8 +64,13 @@ public interface SingleTransactionRecordBuilder { HandleContext.TransactionCategory category(); + /** + * Returns true if this transaction originated from inside another handler or workflow; and not + * a user transaction (or scheduled user transaction). + * @return true if this transaction is internal + */ default boolean isInternalDispatch() { - return !category().equals(USER); + return category() == CHILD || category() == PRECEDING; } /** diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/throttle/AppThrottleAdviser.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/throttle/AppThrottleAdviser.java index a3d80cfab9c3..69bb9b429a51 100644 --- a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/throttle/AppThrottleAdviser.java +++ b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/throttle/AppThrottleAdviser.java @@ -107,7 +107,7 @@ public boolean hasThrottleCapacityForChildTransactions() { } } if (isAllowed && numAutoAssociations > 0) { - isAllowed = networkUtilizationManager.shouldThrottleNOfUnscaled( + isAllowed = !networkUtilizationManager.shouldThrottleNOfUnscaled( numAutoAssociations, TOKEN_ASSOCIATE_TO_ACCOUNT, consensusNow); } if (!isAllowed) { diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/DispatchHandleContext.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/DispatchHandleContext.java index 7eb69a67ef27..99619edb1c84 100644 --- a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/DispatchHandleContext.java +++ b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/DispatchHandleContext.java @@ -192,8 +192,8 @@ public AccountID payer() { } @Override - public void chargePayerFee(final long amount) { - feeAccumulator.chargeNetworkFee(syntheticPayer, amount); + public boolean tryToChargePayer(final long amount) { + return feeAccumulator.chargeNetworkFee(syntheticPayer, amount); } @NonNull diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/AssociateTokenRecipientsStep.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/AssociateTokenRecipientsStep.java index f47db7e9c86a..5b5ea557e8ba 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/AssociateTokenRecipientsStep.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/AssociateTokenRecipientsStep.java @@ -50,6 +50,7 @@ import com.hedera.node.app.spi.workflows.ComputeDispatchFeesAsTopLevel; import com.hedera.node.app.spi.workflows.HandleContext; import com.hedera.node.app.spi.workflows.HandleException; +import com.hedera.node.app.spi.workflows.record.SingleTransactionRecordBuilder; import com.hedera.node.config.data.EntitiesConfig; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayList; @@ -194,14 +195,18 @@ private TokenAssociation validateAndBuildAutoAssociation( validateFalse(token.hasKycKey(), ACCOUNT_KYC_NOT_GRANTED_FOR_TOKEN); validateFalse(token.accountsFrozenByDefault(), ACCOUNT_FROZEN_FOR_TOKEN); - final var unlimitedAssociationsEnabled = - config.getConfigData(EntitiesConfig.class).unlimitedAutoAssociationsEnabled(); - if (unlimitedAssociationsEnabled) { - final var autoAssociationFee = associationFeeFor(context, PLACEHOLDER_SYNTHETIC_ASSOCIATION); - try { - context.chargePayerFee(autoAssociationFee); - } catch (IllegalArgumentException ignore) { - throw new HandleException(INSUFFICIENT_PAYER_BALANCE); + // We only charge auto-association fees inline if this is not an internal dispatch, since in that case the + // contract service will take the auto-association costs from the remaining EVM gas + if (!context.recordBuilders() + .getOrCreate(SingleTransactionRecordBuilder.class) + .isInternalDispatch()) { + final var unlimitedAssociationsEnabled = + config.getConfigData(EntitiesConfig.class).unlimitedAutoAssociationsEnabled(); + if (unlimitedAssociationsEnabled) { + final var autoAssociationFee = associationFeeFor(context, PLACEHOLDER_SYNTHETIC_ASSOCIATION); + if (!context.tryToChargePayer(autoAssociationFee)) { + throw new HandleException(INSUFFICIENT_PAYER_BALANCE); + } } } final var newRelation = autoAssociate(account, token, accountStore, tokenRelStore, config); diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/extensions/SpecEntityExtension.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/extensions/SpecEntityExtension.java index e05c2fcb5f98..19db1a856dd2 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/extensions/SpecEntityExtension.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/extensions/SpecEntityExtension.java @@ -155,7 +155,9 @@ private SpecAccount accountFrom(@Nullable final AccountSpec annotation, @NonNull .map(AccountSpec::name) .filter(n -> !n.isBlank()) .orElse(defaultName); - return new SpecAccount(name); + final var account = new SpecAccount(name); + account.builder().maxAutoAssociations(annotation == null ? 0 : annotation.maxAutoAssociations()); + return account; } private SpecKey keyFrom(@Nullable final KeySpec annotation, @NonNull final String defaultName) { diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/annotations/AccountSpec.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/annotations/AccountSpec.java index 422894143427..dd1559ef5b7d 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/annotations/AccountSpec.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/annotations/AccountSpec.java @@ -36,4 +36,10 @@ * @return the spec name of the account */ String name() default ""; + + /** + * If set, the maximum number of auto-associations to allow for the account. + * @return the maximum number of auto-associations + */ + int maxAutoAssociations() default 0; } diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/entities/SpecAccount.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/entities/SpecAccount.java index f513b04e370c..55f14d8b3721 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/entities/SpecAccount.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/dsl/entities/SpecAccount.java @@ -150,7 +150,9 @@ public void updateKeyFrom(@NonNull final Key key, @NonNull final HapiSpec spec) @Override protected Creation newCreation(@NonNull final HapiSpec spec) { final var model = builder.build(); - final var op = cryptoCreate(name).balance(ONE_HUNDRED_HBARS); + final var op = cryptoCreate(name) + .balance(ONE_HUNDRED_HBARS) + .maxAutomaticTokenAssociations(model.maxAutoAssociations()); return new Creation<>(op, model); } diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/airdrops/UnlimitedAutoAssociationSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/airdrops/UnlimitedAutoAssociationSuite.java index 0d0443b3aaae..f05d0c3822a0 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/airdrops/UnlimitedAutoAssociationSuite.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/crypto/airdrops/UnlimitedAutoAssociationSuite.java @@ -22,6 +22,7 @@ import static com.hedera.services.bdd.spec.HapiPropertySource.asSolidityAddress; import static com.hedera.services.bdd.spec.HapiSpec.hapiTest; import static com.hedera.services.bdd.spec.assertions.AccountInfoAsserts.accountWith; +import static com.hedera.services.bdd.spec.assertions.ContractInfoAsserts.contractWith; import static com.hedera.services.bdd.spec.keys.TrieSigMapGenerator.uniqueWithFullPrefixesFor; import static com.hedera.services.bdd.spec.queries.QueryVerbs.getAccountInfo; import static com.hedera.services.bdd.spec.queries.QueryVerbs.getAliasedAccountInfo; @@ -32,17 +33,21 @@ import static com.hedera.services.bdd.spec.transactions.TxnVerbs.cryptoTransfer; import static com.hedera.services.bdd.spec.transactions.TxnVerbs.mintToken; import static com.hedera.services.bdd.spec.transactions.TxnVerbs.tokenCreate; +import static com.hedera.services.bdd.spec.transactions.crypto.HapiCryptoTransfer.tinyBarsFromTo; import static com.hedera.services.bdd.spec.transactions.token.TokenMovement.moving; import static com.hedera.services.bdd.spec.transactions.token.TokenMovement.movingHbar; import static com.hedera.services.bdd.spec.transactions.token.TokenMovement.movingUnique; import static com.hedera.services.bdd.spec.utilops.CustomSpecAssert.allRunFor; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.newKeyNamed; +import static com.hedera.services.bdd.spec.utilops.UtilVerbs.sourcing; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.validateChargedUsdWithChild; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.withOpContext; +import static com.hedera.services.bdd.suites.HapiSuite.GENESIS; import static com.hedera.services.bdd.suites.HapiSuite.ONE_HBAR; import static com.hedera.services.bdd.suites.HapiSuite.ONE_HUNDRED_HBARS; import static com.hedera.services.bdd.suites.HapiSuite.ONE_MILLION_HBARS; import static com.hedera.services.bdd.suites.HapiSuite.SECP_256K1_SHAPE; +import static com.hedera.services.bdd.suites.HapiSuite.TINY_PARTS_PER_WHOLE; import static com.hedera.services.bdd.suites.contract.Utils.aaWith; import static com.hedera.services.bdd.suites.contract.Utils.accountId; import static com.hedera.services.bdd.suites.contract.Utils.ocWith; @@ -50,6 +55,7 @@ import static com.hedera.services.bdd.suites.crypto.CryptoApproveAllowanceSuite.NON_FUNGIBLE_TOKEN; import static com.hedera.services.bdd.suites.crypto.CryptoDeleteSuite.TREASURY; import static com.hedera.services.bdd.suites.token.TokenAssociationSpecs.MULTI_KEY; +import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INSUFFICIENT_PAYER_BALANCE; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS; import static com.hederahashgraph.api.proto.java.TokenType.FUNGIBLE_COMMON; import static com.hederahashgraph.api.proto.java.TokenType.NON_FUNGIBLE_UNIQUE; @@ -57,10 +63,17 @@ import com.google.protobuf.ByteString; import com.hedera.services.bdd.junit.HapiTest; import com.hedera.services.bdd.junit.HapiTestLifecycle; +import com.hedera.services.bdd.spec.dsl.annotations.AccountSpec; +import com.hedera.services.bdd.spec.dsl.annotations.ContractSpec; +import com.hedera.services.bdd.spec.dsl.annotations.NonFungibleTokenSpec; +import com.hedera.services.bdd.spec.dsl.entities.SpecAccount; +import com.hedera.services.bdd.spec.dsl.entities.SpecContract; +import com.hedera.services.bdd.spec.dsl.entities.SpecNonFungibleToken; import com.hederahashgraph.api.proto.java.TokenID; import com.hederahashgraph.api.proto.java.TokenTransferList; import com.hederahashgraph.api.proto.java.TokenType; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; import org.junit.jupiter.api.DisplayName; @@ -130,6 +143,51 @@ final Stream autoAssociateTokensHappyPath() { validateChargedUsdWithChild(transferNonFungible, 0.05 + 0.001, 0.1)); } + @HapiTest + @DisplayName("Auto-association with insufficient payer balance fails") + final Stream autoAssociationWithInsufficientPayerBalanceFails( + @NonFungibleTokenSpec(numPreMints = 2) SpecNonFungibleToken token) { + final var insufficientPayerBalance = new AtomicLong(); + return hapiTest( + token.getInfo(), + cryptoCreate("aReceiver").maxAutomaticTokenAssociations(1), + cryptoCreate("bReceiver").maxAutomaticTokenAssociations(1), + // Create a payer with just $0.09 balance, enough to afford just one auto-association + withOpContext((spec, opLog) -> insufficientPayerBalance.set( + spec.ratesProvider().toTbWithActiveRates(9 * TINY_PARTS_PER_WHOLE))), + sourcing(() -> cryptoCreate("payer").balance(insufficientPayerBalance.get())), + cryptoTransfer( + movingUnique(token.name(), 1L) + .between(token.treasury().name(), "aReceiver"), + movingUnique(token.name(), 2L) + .between(token.treasury().name(), "bReceiver")) + .payingWith("payer") + .hasKnownStatus(INSUFFICIENT_PAYER_BALANCE), + token.serialNo(1L).assertOwnerIs(token.treasury()), + token.serialNo(2L).assertOwnerIs(token.treasury())); + } + + @HapiTest + @DisplayName("auto-association through HTS system contract does not charge dispatch payer") + final Stream autoAssociationThroughSystemContractChangesGasCost( + @ContractSpec(contract = "HTSCalls", creationGas = 4_000_000) SpecContract htsCallsContract, + @NonFungibleTokenSpec(numPreMints = 1) SpecNonFungibleToken token, + @AccountSpec(maxAutoAssociations = 1) SpecAccount autoAssociated) { + return hapiTest( + token.treasury().authorizeContract(htsCallsContract), + cryptoTransfer(tinyBarsFromTo(GENESIS, htsCallsContract.name(), ONE_HUNDRED_HBARS)), + htsCallsContract + .getInfo() + .andAssert(query -> query.has(contractWith().balance(ONE_HUNDRED_HBARS))), + htsCallsContract + .call("transferNFTCall", token, token.treasury(), autoAssociated, 1L) + .andAssert(txn -> txn.via("autoAssociation").gas(1_000_000)), + getTxnRecord("autoAssociation").andAllChildRecords().logged(), + htsCallsContract + .getInfo() + .andAssert(query -> query.has(contractWith().balance(ONE_HUNDRED_HBARS)))); + } + @DisplayName("Hollow account creation has correct auto associations") @HapiTest final Stream createHollowAccountWithFtTransfer() {