Skip to content

Commit 87f11e9

Browse files
feat: Disable hook execution in certain calls (#21732)
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
1 parent 51cf857 commit 87f11e9

File tree

69 files changed

+3252
-322
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+3252
-322
lines changed

hapi/hedera-protobuf-java-api/src/main/proto/services/basic_types.proto

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -480,14 +480,10 @@ message HookEntityId {
480480
*/
481481
message HookCall {
482482
oneof id {
483-
/**
484-
* The full id of the hook to call, when the owning entity is not forced by the call site.
485-
*/
486-
HookId full_hook_id = 1;
487483
/**
488484
* The numeric id of the hook to call, when the owning entity is forced by the call site.
489485
*/
490-
int64 hook_id = 2;
486+
int64 hook_id = 1;
491487
}
492488

493489
/**

hapi/hedera-protobuf-java-api/src/main/proto/services/response_code.proto

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,4 +1903,10 @@ enum ResponseCodeEnum {
19031903
* consensus node.
19041904
*/
19051905
ACCOUNT_IS_LINKED_TO_A_NODE = 524;
1906+
1907+
/**
1908+
* Hooks are not supported to be used in Batch transactions and Scheduled transactions.
1909+
* They are only supported in a top level CryptoTransfer transaction.
1910+
*/
1911+
HOOKS_EXECUTIONS_REQUIRE_TOP_LEVEL_CRYPTO_TRANSFER = 525;
19061912
}

hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/contracts/HookUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public static Bytes leftPad32(@NonNull final com.hedera.pbj.runtime.io.buffer.By
7676
* @param op the crypto transfer operation
7777
* @return true if the crypto transfer operation has any hooks set in any of the account amounts or nft transfers
7878
*/
79-
public static boolean hasHooks(final @NonNull CryptoTransferTransactionBody op) {
79+
public static boolean hasHookExecutions(final @NonNull CryptoTransferTransactionBody op) {
8080
for (final AccountAmount aa : op.transfersOrElse(TransferList.DEFAULT).accountAmounts()) {
8181
if (aa.hasPreTxAllowanceHook() || aa.hasPrePostTxAllowanceHook()) {
8282
return true;

hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleCreateHandler.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package com.hedera.node.app.service.schedule.impl.handlers;
33

44
import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_ID_DOES_NOT_EXIST;
5+
import static com.hedera.hapi.node.base.ResponseCodeEnum.HOOKS_EXECUTIONS_REQUIRE_TOP_LEVEL_CRYPTO_TRANSFER;
56
import static com.hedera.hapi.node.base.ResponseCodeEnum.IDENTICAL_SCHEDULE_ALREADY_CREATED;
67
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ADMIN_KEY;
78
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION;
@@ -40,6 +41,7 @@
4041
import com.hedera.hapi.node.state.throttles.ThrottleUsageSnapshots;
4142
import com.hedera.node.app.hapi.fees.usage.SigUsage;
4243
import com.hedera.node.app.hapi.fees.usage.schedule.ScheduleOpsUsage;
44+
import com.hedera.node.app.hapi.utils.contracts.HookUtils;
4345
import com.hedera.node.app.hapi.utils.fee.SigValueObj;
4446
import com.hedera.node.app.service.entityid.EntityIdFactory;
4547
import com.hedera.node.app.service.schedule.ScheduleStreamBuilder;
@@ -104,6 +106,12 @@ public void pureChecks(@NonNull final PureChecksContext context) throws PreCheck
104106
validateTruePreCheck(op.hasScheduledTransactionBody(), INVALID_TRANSACTION);
105107
// (FUTURE) Add a dedicated response code for an op waiting for an unspecified expiration time
106108
validateFalsePreCheck(op.waitForExpiry() && !op.hasExpirationTime(), MISSING_EXPIRY_TIME);
109+
if (op.scheduledTransactionBodyOrThrow().hasCryptoTransfer()) {
110+
validateFalsePreCheck(
111+
HookUtils.hasHookExecutions(
112+
op.scheduledTransactionBodyOrThrow().cryptoTransferOrThrow()),
113+
HOOKS_EXECUTIONS_REQUIRE_TOP_LEVEL_CRYPTO_TRANSFER);
114+
}
107115
}
108116

109117
@Override

hedera-node/hedera-schedule-service-impl/src/test/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleCreateHandlerTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,32 @@
22
package com.hedera.node.app.service.schedule.impl.handlers;
33

44
import static com.hedera.hapi.node.base.ResponseCodeEnum.ACCOUNT_ID_DOES_NOT_EXIST;
5+
import static com.hedera.hapi.node.base.ResponseCodeEnum.HOOKS_EXECUTIONS_REQUIRE_TOP_LEVEL_CRYPTO_TRANSFER;
56
import static com.hedera.hapi.node.base.ResponseCodeEnum.IDENTICAL_SCHEDULE_ALREADY_CREATED;
67
import static com.hedera.hapi.node.base.ResponseCodeEnum.MAX_ENTITIES_IN_PRICE_REGIME_HAVE_BEEN_CREATED;
78
import static com.hedera.hapi.node.base.ResponseCodeEnum.SCHEDULED_TRANSACTION_NOT_IN_WHITELIST;
9+
import static com.hedera.node.app.spi.fixtures.workflows.ExceptionConditions.responseCode;
10+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
811
import static org.assertj.core.api.BDDAssertions.assertThat;
912
import static org.mockito.ArgumentMatchers.any;
1013
import static org.mockito.ArgumentMatchers.anyInt;
1114
import static org.mockito.BDDMockito.given;
1215
import static org.mockito.Mockito.mock;
1316

17+
import com.hedera.hapi.node.base.AccountAmount;
1418
import com.hedera.hapi.node.base.AccountID;
1519
import com.hedera.hapi.node.base.HederaFunctionality;
20+
import com.hedera.hapi.node.base.HookCall;
1621
import com.hedera.hapi.node.base.Key;
1722
import com.hedera.hapi.node.base.ScheduleID;
1823
import com.hedera.hapi.node.base.Timestamp;
24+
import com.hedera.hapi.node.base.TransferList;
1925
import com.hedera.hapi.node.scheduled.SchedulableTransactionBody;
2026
import com.hedera.hapi.node.scheduled.SchedulableTransactionBody.DataOneOfType;
27+
import com.hedera.hapi.node.scheduled.ScheduleCreateTransactionBody;
2128
import com.hedera.hapi.node.state.schedule.Schedule;
2229
import com.hedera.hapi.node.state.throttles.ThrottleUsageSnapshots;
30+
import com.hedera.hapi.node.token.CryptoTransferTransactionBody;
2331
import com.hedera.hapi.node.transaction.TransactionBody;
2432
import com.hedera.node.app.hapi.utils.keys.KeyComparator;
2533
import com.hedera.node.app.service.entityid.EntityNumGenerator;
@@ -32,6 +40,7 @@
3240
import com.hedera.node.app.spi.workflows.HandleException;
3341
import com.hedera.node.app.spi.workflows.PreCheckException;
3442
import com.hedera.node.app.spi.workflows.PreHandleContext;
43+
import com.hedera.node.app.spi.workflows.PureChecksContext;
3544
import com.hedera.node.app.workflows.prehandle.PreHandleContextImpl;
3645
import java.security.InvalidKeyException;
3746
import java.time.InstantSource;
@@ -51,6 +60,9 @@ class ScheduleCreateHandlerTest extends ScheduleHandlerTestBase {
5160
@Mock
5261
private ScheduleFeeCharging feeCharging;
5362

63+
@Mock
64+
private PureChecksContext pureChecksContext;
65+
5466
private ScheduleCreateHandler subject;
5567
private PreHandleContext realPreContext;
5668

@@ -60,6 +72,33 @@ void setUp() throws PreCheckException, InvalidKeyException {
6072
setUpBase();
6173
}
6274

75+
@Test
76+
void schedulingHookExecutionFails() {
77+
final TransactionBody createTransaction = TransactionBody.newBuilder()
78+
.scheduleCreate(ScheduleCreateTransactionBody.newBuilder()
79+
.memo("test memo")
80+
.scheduledTransactionBody(SchedulableTransactionBody.newBuilder()
81+
.cryptoTransfer(CryptoTransferTransactionBody.newBuilder()
82+
.transfers(TransferList.newBuilder()
83+
.accountAmounts(
84+
AccountAmount.newBuilder()
85+
.accountID(payer)
86+
.preTxAllowanceHook(HookCall.DEFAULT)
87+
.amount(1L)
88+
.build(),
89+
AccountAmount.newBuilder()
90+
.accountID(scheduler)
91+
.amount(-1L)
92+
.build()))
93+
.build()))
94+
.build())
95+
.build();
96+
given(pureChecksContext.body()).willReturn(createTransaction);
97+
assertThatThrownBy(() -> subject.pureChecks(pureChecksContext))
98+
.isInstanceOf(PreCheckException.class)
99+
.has(responseCode(HOOKS_EXECUTIONS_REQUIRE_TOP_LEVEL_CRYPTO_TRANSFER));
100+
}
101+
63102
@Test
64103
void preHandleVanilla() throws PreCheckException {
65104
realPreContext = new PreHandleContextImpl(

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/ContextTransactionProcessor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ private HevmTransactionCreationResult safeCreateHevmTransaction() {
261261
try {
262262
final var hevmTransaction = hevmTransactionFactory.fromHapiTransaction(context.body(), context.payer());
263263
validatePayloadLength(hevmTransaction);
264-
return new HevmTransactionCreationResult(hevmTransaction, hevmTransaction.isHookDispatch());
264+
return new HevmTransactionCreationResult(hevmTransaction, hevmTransaction.hookOwnerAddress() != null);
265265
} catch (HandleException e) {
266266
final var evmTxn = hevmTransactionFactory.fromContractTxException(context.body(), e);
267267
// Return a HederaEvmTransaction that represents the error in order to charge fees to the sender

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/TransactionProcessor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ private HederaEvmTransactionResult processTransactionWithParties(
126126
@NonNull final OpsDurationCounter opsDurationCounter,
127127
@NonNull final InvolvedParties parties) {
128128
// If it is hook dispatch, skip gas charging because gas is pre-paid in cryptoTransfer already
129-
final var gasCharges = transaction.isHookDispatch()
129+
final var gasCharges = transaction.hookOwnerAddress() != null
130130
? GasCharges.NONE
131131
: gasCharging.chargeForGas(parties.sender(), parties.relayer(), context, updater, transaction);
132132
final var initialFrame = frameBuilder.buildInitialFrameWith(
@@ -148,7 +148,7 @@ private HederaEvmTransactionResult processTransactionWithParties(
148148
// Maybe refund some of the charged fees before committing if not a hook dispatch
149149
// Note that for hook dispatch, gas is charged during cryptoTransfer and will not be refunded once
150150
// hook is executed
151-
if (!transaction.isHookDispatch()) {
151+
if (transaction.hookOwnerAddress() == null) {
152152
gasCharging.maybeRefundGiven(
153153
transaction.unusedGas(result.gasUsed()),
154154
gasCharges.relayerAllowanceUsed(),

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/AbstractCustomCreateOperation.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.hyperledger.besu.evm.frame.ExceptionalHaltReason.INSUFFICIENT_GAS;
66
import static org.hyperledger.besu.evm.internal.Words.clampedToLong;
77

8+
import com.hedera.node.app.service.contract.impl.exec.utils.FrameUtils;
89
import com.hedera.node.app.service.contract.impl.state.ProxyWorldUpdater;
910
import edu.umd.cs.findbugs.annotations.NonNull;
1011
import edu.umd.cs.findbugs.annotations.Nullable;
@@ -90,6 +91,9 @@ protected AbstractCustomCreateOperation(
9091

9192
@Override
9293
public OperationResult execute(@NonNull final MessageFrame frame, @NonNull final EVM evm) {
94+
if (FrameUtils.isHookExecution(frame)) {
95+
return new OperationResult(0, ExceptionalHaltReason.INVALID_OPERATION);
96+
}
9397
if (!isEnabled(frame)) {
9498
return INVALID_RESPONSE;
9599
}

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomCallCodeOperation.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
import com.hedera.node.app.service.contract.impl.exec.AddressChecks;
55
import com.hedera.node.app.service.contract.impl.exec.FeatureFlags;
66
import com.hedera.node.app.service.contract.impl.exec.processors.CustomMessageCallProcessor;
7+
import com.hedera.node.app.service.contract.impl.exec.utils.FrameUtils;
78
import edu.umd.cs.findbugs.annotations.NonNull;
89
import java.util.Objects;
910
import org.hyperledger.besu.datatypes.Address;
1011
import org.hyperledger.besu.evm.EVM;
12+
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
1113
import org.hyperledger.besu.evm.frame.MessageFrame;
1214
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
1315
import org.hyperledger.besu.evm.operation.CallCodeOperation;
@@ -58,6 +60,10 @@ public OperationResult executeUnchecked(@NonNull MessageFrame frame, @NonNull EV
5860

5961
@Override
6062
public OperationResult execute(@NonNull final MessageFrame frame, @NonNull final EVM evm) {
63+
// Call code operations originating from a hook execution are not allowed
64+
if (FrameUtils.isHookExecution(frame)) {
65+
return new OperationResult(0, ExceptionalHaltReason.INVALID_OPERATION);
66+
}
6167
return BasicCustomCallOperation.super.executeChecked(frame, evm);
6268
}
6369
}

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomCallOperation.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package com.hedera.node.app.service.contract.impl.exec.operations;
33

44
import static com.hedera.node.app.service.contract.impl.exec.failure.CustomExceptionalHaltReason.INVALID_SOLIDITY_ADDRESS;
5+
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.HtsSystemContract.HTS_HOOKS_CONTRACT_ADDRESS;
56
import static com.hedera.node.app.service.contract.impl.exec.utils.FrameUtils.contractRequired;
67
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.isLongZero;
78

@@ -77,6 +78,15 @@ public OperationResult execute(@NonNull final MessageFrame frame, @NonNull final
7778
}
7879
}
7980

81+
@Override
82+
public Address sender(final MessageFrame frame) {
83+
if (frame.getRecipientAddress().equals(HTS_HOOKS_CONTRACT_ADDRESS)) {
84+
// If the sender is the HTS hooks contract, we want to use the owner of the hook as the sender
85+
return FrameUtils.hookOwnerAddress(frame);
86+
}
87+
return super.sender(frame);
88+
}
89+
8090
private boolean mustBePresent(@NonNull final MessageFrame frame, @NonNull final Address toAddress) {
8191
// This call will create the "to" address, so it doesn't need to be present
8292
if (impliesLazyCreation(frame, toAddress) && featureFlags.isImplicitCreationEnabled()) {

0 commit comments

Comments
 (0)