diff --git a/hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/forensics/OrderedComparison.java b/hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/forensics/OrderedComparison.java index e792bc427dde..3f20d9a3182b 100644 --- a/hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/forensics/OrderedComparison.java +++ b/hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/forensics/OrderedComparison.java @@ -170,7 +170,8 @@ static List diff( "Additional modular record found at " + secondEntries.get(i).consensusTime() + " for transactionID : " + secondEntries.get(i).txnRecord().getTransactionID() + " transBody : " - + secondEntries.get(i).body())); + + secondEntries.get(i).body() + + "\n -> \n" + secondEntries.get(i).txnRecord())); continue; } final var secondEntry = entryWithMatchableRecord(secondEntries, i, firstEntry); diff --git a/hedera-node/hedera-mono-service/src/main/java/com/hedera/node/app/service/mono/txns/crypto/CryptoApproveAllowanceTransitionLogic.java b/hedera-node/hedera-mono-service/src/main/java/com/hedera/node/app/service/mono/txns/crypto/CryptoApproveAllowanceTransitionLogic.java index 8a6b0c5c06a6..2e524d11d685 100644 --- a/hedera-node/hedera-mono-service/src/main/java/com/hedera/node/app/service/mono/txns/crypto/CryptoApproveAllowanceTransitionLogic.java +++ b/hedera-node/hedera-mono-service/src/main/java/com/hedera/node/app/service/mono/txns/crypto/CryptoApproveAllowanceTransitionLogic.java @@ -18,6 +18,7 @@ import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS; +import com.hedera.node.app.service.evm.exceptions.InvalidTransactionException; import com.hedera.node.app.service.mono.context.TransactionContext; import com.hedera.node.app.service.mono.context.primitives.StateView; import com.hedera.node.app.service.mono.store.AccountStore; @@ -82,13 +83,16 @@ public Function semanticCheck() { private ResponseCodeEnum validate(TransactionBody cryptoAllowanceTxn) { final AccountID payer = cryptoAllowanceTxn.getTransactionID().getAccountID(); final var op = cryptoAllowanceTxn.getCryptoApproveAllowance(); - final var payerAccount = accountStore.loadAccount(Id.fromGrpcAccount(payer)); - - return allowanceChecks.allowancesValidation( - op.getCryptoAllowancesList(), - op.getTokenAllowancesList(), - op.getNftAllowancesList(), - payerAccount, - workingView); + try { + final var payerAccount = accountStore.loadAccount(Id.fromGrpcAccount(payer)); + return allowanceChecks.allowancesValidation( + op.getCryptoAllowancesList(), + op.getTokenAllowancesList(), + op.getNftAllowancesList(), + payerAccount, + workingView); + } catch (InvalidTransactionException e) { + return e.getResponseCode(); + } } } diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoTransferHandler.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoTransferHandler.java index 421ee422c6a5..d441d4c888e4 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoTransferHandler.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/CryptoTransferHandler.java @@ -226,7 +226,7 @@ public void handle(@NonNull final HandleContext context) throws HandleException new TransferContextImpl(context, enforceMonoServiceRestrictionsOnAutoCreationCustomFeePayments); // (TEMPORARY) Remove this after diff testing - transferContext.validateHbarAllowances(); + transferContext.validateTopLevelAllowances(); transferContext.validateAccountIds(); // Replace all aliases in the transaction body with its account ids diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/NFTOwnersChangeStep.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/NFTOwnersChangeStep.java index 299378efdc5b..c425f3cfe7bd 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/NFTOwnersChangeStep.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/NFTOwnersChangeStep.java @@ -201,6 +201,5 @@ private void updateOwnership( tokenRelStore.put(senderRelCopy.balance(fromTokenRelBalance - 1).build()); tokenRelStore.put(receiverRelCopy.balance(toTokenRelBalance + 1).build()); nftStore.put(nftCopy.build()); - // TODO: make sure finalize is capturing all these in transfer list } } diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContext.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContext.java index fb9f1c6a276e..527c9f6b20d4 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContext.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContext.java @@ -111,11 +111,11 @@ public interface TransferContext { boolean isEnforceMonoServiceRestrictionsOnAutoCreationCustomFeePayments(); /** - * Validates hbar allowances for the top-level operation in this transfer context. + * Validates allowances for the top-level operation in this transfer context. * *

(FUTURE) Remove this, only needed for diff testing and has no logical priority. */ - void validateHbarAllowances(); + void validateTopLevelAllowances(); /** * Validates account IDs for the top-level operation in this transfer context. diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContextImpl.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContextImpl.java index 4d1902f452a3..5dac38d05e23 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContextImpl.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/transfer/TransferContextImpl.java @@ -24,14 +24,18 @@ import static com.hedera.hapi.node.base.ResponseCodeEnum.SPENDER_DOES_NOT_HAVE_ALLOWANCE; import static com.hedera.node.app.service.mono.utils.EntityIdUtils.EVM_ADDRESS_SIZE; import static com.hedera.node.app.service.token.AliasUtils.isSerializedProtoKey; +import static com.hedera.node.app.service.token.impl.handlers.transfer.NFTOwnersChangeStep.validateSpenderHasAllowance; import static com.hedera.node.app.spi.workflows.HandleException.validateTrue; import static java.util.Collections.emptyList; import com.hedera.hapi.node.base.AccountID; +import com.hedera.hapi.node.base.NftID; import com.hedera.hapi.node.base.TokenAssociation; +import com.hedera.hapi.node.base.TokenID; import com.hedera.hapi.node.base.TransferList; import com.hedera.hapi.node.state.token.Account; import com.hedera.hapi.node.transaction.AssessedCustomFee; +import com.hedera.node.app.service.token.ReadableNftStore; import com.hedera.node.app.service.token.impl.WritableAccountStore; import com.hedera.node.app.spi.workflows.HandleContext; import com.hedera.node.app.spi.workflows.HandleException; @@ -177,7 +181,7 @@ public boolean isEnforceMonoServiceRestrictionsOnAutoCreationCustomFeePayments() } @Override - public void validateHbarAllowances() { + public void validateTopLevelAllowances() { final var topLevelPayer = context.payer(); final var op = context.body().cryptoTransferOrThrow(); for (final var aa : op.transfersOrElse(TransferList.DEFAULT).accountAmountsOrElse(emptyList())) { @@ -186,6 +190,21 @@ public void validateHbarAllowances() { accountStore.get(aa.accountIDOrElse(AccountID.DEFAULT)), topLevelPayer, aa.amount()); } } + final var nftStore = context.readableStore(ReadableNftStore.class); + for (final var tokenTransfers : op.tokenTransfersOrElse(emptyList())) { + final var tokenId = tokenTransfers.tokenOrElse(TokenID.DEFAULT); + for (final var oc : tokenTransfers.nftTransfersOrElse(emptyList())) { + if (oc.isApproval()) { + final var maybeOwner = accountStore.get(oc.senderAccountIDOrElse(AccountID.DEFAULT)); + if (maybeOwner != null) { + final var maybeNft = nftStore.get(new NftID(tokenId, oc.serialNumber())); + if (maybeNft != null) { + validateSpenderHasAllowance(maybeOwner, topLevelPayer, tokenId, maybeNft); + } + } + } + } + } } @Override diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/consensus/TopicCreateSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/consensus/TopicCreateSuite.java index fda45af7acd0..e02d53c2d735 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/consensus/TopicCreateSuite.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/consensus/TopicCreateSuite.java @@ -166,8 +166,6 @@ final HapiSpec signingRequirementsEnforced() { .payingWith("payer") .signedBy("wrongKey") .hasPrecheck(INVALID_SIGNATURE), - // In hedera-app, we'll allow contracts with admin keys to be auto-renew accounts - createTopic("withContractAutoRenew").autoRenewAccountId(contractWithAdminKey), // But contracts without admin keys will get INVALID_SIGNATURE (can't sign!) createTopic("NotToBe") .autoRenewAccountId(PAY_RECEIVABLE_CONTRACT) @@ -197,7 +195,11 @@ final HapiSpec signingRequirementsEnforced() { .autoRenewAccountId("autoRenewAccount") /* SigMap missing signature from adminKey. */ .signedBy("payer", "autoRenewAccount") - .hasKnownStatus(INVALID_SIGNATURE)) + .hasKnownStatus(INVALID_SIGNATURE), + // In hedera-app, we'll allow contracts with admin keys to be auto-renew accounts + createTopic("withContractAutoRenew") + .adminKeyName("adminKey") + .autoRenewAccountId(contractWithAdminKey)) .then( createTopic("noAdminKeyNoAutoRenewAccount"), getTopicInfo("noAdminKeyNoAutoRenewAccount") @@ -213,6 +215,10 @@ final HapiSpec signingRequirementsEnforced() { getTopicInfo("explicitAdminKeyExplicitAutoRenewAccount") .hasAdminKey("adminKey") .hasAutoRenewAccount("autoRenewAccount") + .logged(), + getTopicInfo("withContractAutoRenew") + .hasAdminKey("adminKey") + .hasAutoRenewAccount(contractWithAdminKey) .logged()); } diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/precompile/LazyCreateThroughPrecompileSuite.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/precompile/LazyCreateThroughPrecompileSuite.java index c5ce7d0a2647..9b1988508eee 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/precompile/LazyCreateThroughPrecompileSuite.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/precompile/LazyCreateThroughPrecompileSuite.java @@ -200,17 +200,23 @@ HapiSpec resourceLimitExceededRevertsAllRecords() { IntStream.range(0, n) .mapToObj(i -> ByteString.copyFromUtf8(ONE_TIME + i)) .toList())) - .when(sourcing(() -> contractCall( - AUTO_CREATION_MODES, - "createSeveralDirectly", - headlongFromHexed(nftMirrorAddr.get()), - nCopiesOfSender(n, mirrorAddrWith(civilianId.get())), - nNonMirrorAddressFrom(n, civilianId.get() + 3_050_000), - LongStream.iterate(1L, l -> l + 1).limit(n).toArray()) - .via(creationAttempt) - .gas(GAS_TO_OFFER) - .alsoSigningWithFullPrefix(CIVILIAN) - .hasKnownStatusFrom(MAX_CHILD_RECORDS_EXCEEDED, CONTRACT_REVERT_EXECUTED))) + .when( + cryptoApproveAllowance() + .payingWith(CIVILIAN) + .addNftAllowance(CIVILIAN, nft, AUTO_CREATION_MODES, true, List.of()), + sourcing(() -> contractCall( + AUTO_CREATION_MODES, + "createSeveralDirectly", + headlongFromHexed(nftMirrorAddr.get()), + nCopiesOfSender(n, mirrorAddrWith(civilianId.get())), + nNonMirrorAddressFrom(n, civilianId.get() + 3_050_000), + LongStream.iterate(1L, l -> l + 1) + .limit(n) + .toArray()) + .via(creationAttempt) + .gas(GAS_TO_OFFER) + .alsoSigningWithFullPrefix(CIVILIAN) + .hasKnownStatusFrom(MAX_CHILD_RECORDS_EXCEEDED, CONTRACT_REVERT_EXECUTED))) .then( // mono-service did not do an "orderly shutdown" of the EVM transaction when it hit // a resource limit exception like MAX_CHILD_RECORDS_EXCEEDED, instead throwing an @@ -257,17 +263,21 @@ HapiSpec autoCreationFailsWithMirrorAddress() { .exposingCreatedIdTo( idLit -> nftMirrorAddr.set(asHexedSolidityAddress(asToken(idLit)))), mintToken(nft, List.of(ByteString.copyFromUtf8(ONE_TIME)))) - .when(sourcing(() -> contractCall( - AUTO_CREATION_MODES, - CREATE_DIRECTLY, - headlongFromHexed(nftMirrorAddr.get()), - mirrorAddrWith(civilianId.get()), - mirrorAddrWith(civilianId.get() + 1_000_001), - 1L, - false) - .via(creationAttempt) - .gas(GAS_TO_OFFER) - .hasKnownStatus(CONTRACT_REVERT_EXECUTED))) + .when( + cryptoApproveAllowance() + .payingWith(CIVILIAN) + .addNftAllowance(CIVILIAN, nft, AUTO_CREATION_MODES, true, List.of()), + sourcing(() -> contractCall( + AUTO_CREATION_MODES, + CREATE_DIRECTLY, + headlongFromHexed(nftMirrorAddr.get()), + mirrorAddrWith(civilianId.get()), + mirrorAddrWith(civilianId.get() + 1_000_001), + 1L, + false) + .via(creationAttempt) + .gas(GAS_TO_OFFER) + .hasKnownStatus(CONTRACT_REVERT_EXECUTED))) .then(childRecordsCheck( creationAttempt, CONTRACT_REVERT_EXECUTED, recordWith().status(INVALID_ALIAS_KEY))); }