Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Cancun SELFDESTRUCT semantics incorrect when beneficiary is contract itself and it isn't deleted #17478

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,8 @@

package com.hedera.node.app.service.contract.impl.exec.operations;

import static com.hedera.node.app.service.contract.impl.exec.failure.CustomExceptionalHaltReason.CONTRACT_IS_TREASURY;
import static com.hedera.node.app.service.contract.impl.exec.failure.CustomExceptionalHaltReason.CONTRACT_STILL_OWNS_NFTS;
import static com.hedera.node.app.service.contract.impl.exec.failure.CustomExceptionalHaltReason.INVALID_SOLIDITY_ADDRESS;
import static com.hedera.node.app.service.contract.impl.exec.operations.CustomizedOpcodes.SELFDESTRUCT;
import static com.hedera.node.app.service.contract.impl.exec.utils.FrameUtils.isDelegateCall;
Expand All @@ -24,13 +26,16 @@
import static org.hyperledger.besu.evm.frame.ExceptionalHaltReason.INSUFFICIENT_GAS;

import com.hedera.node.app.service.contract.impl.exec.AddressChecks;
import com.hedera.node.app.service.contract.impl.state.AbstractProxyEvmAccount;
import com.hedera.node.app.service.contract.impl.state.ProxyWorldUpdater;
import com.hedera.node.app.service.contract.impl.state.ScheduleEvmAccount;
import com.hedera.node.app.service.contract.impl.state.TokenEvmAccount;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.util.Optional;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.evm.EVM;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.account.MutableAccount;
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
import org.hyperledger.besu.evm.frame.MessageFrame;
Expand Down Expand Up @@ -92,65 +97,135 @@
final var tbdAddress = frame.getRecipientAddress();
final var proxyWorldUpdater = (ProxyWorldUpdater) frame.getWorldUpdater();

// Now proceed with the self-destruct
// In EVM the SELFDESTRUCT operation never fails. It always sweeps ETH, and the contract
// is either deleted or not (per EIP-6780).
//
// In Hedera we have to allow for our semantics for transfers. Notably, we can't
// transfer hbar unless the signature requirements are met on the transaction, we can't
// burn hbar, and we don't allow deletion of a contract which is a token treasury.
// There's also a restriction due to our performance guarantees: We don't allow contracts
// holding native tokens to self destruct because all the tokens would have to be
// transferred in the current `handleTransaction` call and if there were too many tokens
// it would be too expensive (in CPU/memory/database resources) to transfer them all.
//
// If the beneficiary account is the contract itself then we have two cases:
// * If (per EIP-6780) the contract is _not_ going to be deleted: That's ok. SELFDESTRUCT
// is a noop. But if the contract _is_ going to be deleted and it has a balance of hbar
// or any token then SELFDESTRUCT will fail.
// * Otherwise, if the beneficiary account is _not_ the contract itself then we fail the
// SELFDESTRUCT if the contract owns any tokens.

final boolean contractCreatedInThisTransaction = frame.wasCreatedInTransaction(tbdAddress);
final boolean contractIsItsOwnBeneficiary = tbdAddress.equals(beneficiaryAddress);
final boolean contractIsToBeDeleted =
switch (eip6780Semantics) {
case NO -> true;
case YES -> contractCreatedInThisTransaction;
};

// inheritance == amount hbar to sweep
final var inheritance =
requireNonNull(proxyWorldUpdater.get(tbdAddress)).getBalance();
final var beneficiary = proxyWorldUpdater.get(beneficiaryAddress);
final var beneficiaryIsWarm =
frame.warmUpAddress(beneficiaryAddress) || gasCalculator().isPrecompile(beneficiaryAddress);
final var cost = gasCalculator().selfDestructOperationGasCost(beneficiary, inheritance)
final var costWithoutBeneficiary = gasCalculator().selfDestructOperationGasCost(null, Wei.ZERO);
final var costWithBeneficiary = gasCalculator().selfDestructOperationGasCost(beneficiary, inheritance)
+ (beneficiaryIsWarm ? 0L : gasCalculator().getColdAccountAccessCost());
if (frame.isStatic()) {
return new OperationResult(cost, ILLEGAL_STATE_CHANGE);
} else if (frame.getRemainingGas() < cost) {
return new OperationResult(cost, INSUFFICIENT_GAS);
}

// Enforce Hedera-specific checks on the beneficiary address
if (addressChecks.isSystemAccount(beneficiaryAddress)
|| !addressChecks.isPresent(beneficiaryAddress, frame)) {
return haltFor(null, 0, INVALID_SOLIDITY_ADDRESS);
}
// Initial checks for EVM suitability
if (frame.isStatic()) return resultFor(costWithBeneficiary, ILLEGAL_STATE_CHANGE);
if (frame.getRemainingGas() < costWithBeneficiary) return resultFor(costWithBeneficiary, INSUFFICIENT_GAS);

// Enforce Hedera-specific restrictions on account deletion
final var maybeHaltReason =
var maybeReasonToHalt = validateHederaRestrictionsOnBeneficiary(tbdAddress, beneficiaryAddress, frame);
if (maybeReasonToHalt.isPresent()) return resultFor(costWithoutBeneficiary, maybeReasonToHalt.get());

maybeReasonToHalt =
validateHederaRestrictionsOnContract(tbdAddress, beneficiaryAddress, frame, contractIsToBeDeleted);

Check warning on line 145 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java#L144-L145

Added lines #L144 - L145 were not covered by tests
if (maybeReasonToHalt.isPresent()) return resultFor(costWithoutBeneficiary, maybeReasonToHalt.get());

maybeReasonToHalt =

Check warning on line 148 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L148 was not covered by tests
proxyWorldUpdater.tryTrackingSelfDestructBeneficiary(tbdAddress, beneficiaryAddress, frame);
if (maybeHaltReason.isPresent()) {
return haltFor(null, 0, maybeHaltReason.get());
}
if (maybeReasonToHalt.isPresent()) return resultFor(costWithoutBeneficiary, maybeReasonToHalt.get());

// This will enforce the Hedera signing requirements (while treating any Key{contractID=tbdAddress}
// or Key{delegatable_contract_id=tbdAddress} keys on the beneficiary account as active); it could
// also fail if the beneficiary is a token address
final var maybeReasonToHalt = proxyWorldUpdater.tryTransfer(
// Sweeps the hbar from the contract being deleted, if Hedera signing requirements met (while treating any
// Key{contractID=tbdAddress} or Key{delegatable_contract_id=tbdAddress} keys on the beneficiary account as
// active); it could also fail if the beneficiary is a token address.
maybeReasonToHalt = proxyWorldUpdater.tryTransfer(

Check warning on line 155 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L155 was not covered by tests
tbdAddress, beneficiaryAddress, inheritance.toLong(), isDelegateCall(frame));
if (maybeReasonToHalt.isPresent()) {
return new OperationResult(cost, maybeReasonToHalt.get());
}
if (maybeReasonToHalt.isPresent()) return resultFor(costWithoutBeneficiary, maybeReasonToHalt.get());

// Tell the EVM to delete this contract if pre-Cancun, or, if post-Cancun, only in the
// same transaction it was created in
final boolean tellEVMToDoContractDestruct =
switch (eip6780Semantics) {
case NO -> true;
case YES -> frame.wasCreatedInTransaction(tbdAddress);
};
// From this point success is assured ...

// Frame tracks contracts to be deleted (for handling later)
if (contractIsToBeDeleted) frame.addSelfDestruct(tbdAddress);

if (tellEVMToDoContractDestruct) {
frame.addSelfDestruct(tbdAddress);
if (!contractIsItsOwnBeneficiary || contractIsToBeDeleted) {
proxyWorldUpdater.trackSelfDestructBeneficiary(tbdAddress, beneficiaryAddress, frame);

Check warning on line 165 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L165 was not covered by tests
// Frame tracks balance changes
frame.addRefund(beneficiaryAddress, inheritance);

Check warning on line 167 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L167 was not covered by tests
}

frame.addRefund(beneficiaryAddress, inheritance);
frame.setState(State.CODE_SUCCESS);
return new OperationResult(cost, null);
return resultFor(costWithBeneficiary, null);

Check warning on line 171 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L171 was not covered by tests

} catch (UnderflowException ignore) {
return UNDERFLOW_RESPONSE;
}
}

private OperationResult haltFor(
@Nullable final Account beneficiary, final long inheritance, @NonNull final ExceptionalHaltReason reason) {
final long cost = gasCalculator().selfDestructOperationGasCost(beneficiary, Wei.of(inheritance));
protected @NonNull Optional<ExceptionalHaltReason> validateHederaRestrictionsOnBeneficiary(
@NonNull final Address deleted, @NonNull final Address beneficiary, @NonNull final MessageFrame frame) {
requireNonNull(deleted);
requireNonNull(beneficiary);
requireNonNull(frame);

final var proxyWorldUpdater = (ProxyWorldUpdater) frame.getWorldUpdater();
final var beneficiaryAccount = proxyWorldUpdater.getAccount(beneficiary);

// Beneficiary must not be a system account, and ...
if (addressChecks.isSystemAccount(beneficiary)) return Optional.of(INVALID_SOLIDITY_ADDRESS);

// ... must be present in the frame, and ...
if (!addressChecks.isPresent(beneficiary, frame)) return Optional.of(INVALID_SOLIDITY_ADDRESS);

// must exist, and ...
if (beneficiaryAccount == null) return Optional.of(INVALID_SOLIDITY_ADDRESS);

// ... must not be a token or schedule.
if (beneficiaryAccount instanceof TokenEvmAccount || beneficiaryAccount instanceof ScheduleEvmAccount)
return Optional.of(INVALID_SOLIDITY_ADDRESS);

Check warning on line 198 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L198 was not covered by tests

return Optional.empty();

Check warning on line 200 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L200 was not covered by tests
}

protected @NonNull Optional<ExceptionalHaltReason> validateHederaRestrictionsOnContract(
@NonNull final Address deleted,
@NonNull final Address beneficiary,
@NonNull final MessageFrame frame,
final boolean contractIsToBeDeleted) {
requireNonNull(deleted);
requireNonNull(beneficiary);
requireNonNull(frame);

Check warning on line 210 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java#L208-L210

Added lines #L208 - L210 were not covered by tests

final var proxyWorldUpdater = (ProxyWorldUpdater) frame.getWorldUpdater();
final var deletedAccount = (AbstractProxyEvmAccount) requireNonNull(proxyWorldUpdater.get(deleted));

Check warning on line 213 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java#L212-L213

Added lines #L212 - L213 were not covered by tests

// (Contract) account being self-destructed must not be a token treasury
if (deletedAccount.numTreasuryTitles() > 0) return Optional.of(CONTRACT_IS_TREASURY);

// Can't sweep native tokens (fungible or non-fungible) from contract being self-destructed
if (contractIsToBeDeleted || !deleted.equals(beneficiary)) {
// Any other situation must sweep, but cannot do that if contract being destructed owns tokens
// N.B.: Response code name is misleading: Contract can't own fungible tokens either!
if (deletedAccount.numPositiveTokenBalances() > 0) return Optional.of(CONTRACT_STILL_OWNS_NFTS);
}

return Optional.empty();

Check warning on line 225 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/operations/CustomSelfDestructOperation.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L225 was not covered by tests
}

private @NonNull OperationResult resultFor(final long cost, @Nullable final ExceptionalHaltReason reason) {
return new OperationResult(cost, reason);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -193,6 +193,19 @@ Optional<ExceptionalHaltReason> tryTransfer(
Optional<ExceptionalHaltReason> tryTrackingSelfDestructBeneficiary(
@NonNull Address deleted, @NonNull Address beneficiary, MessageFrame frame);

/**
* Tracks the given deletion of an account with the designated beneficiary.
*
* @param deleted the address of the account being deleted, a contract
* @param beneficiary the address of the beneficiary of the deletion
* @param frame
*
* `Beneficiary` must not be a token or a schedule. Contract `deleted` must not be any token's
* treasury. Contract `deleted` must not own any tokens. These conditions are _not_ checked
* by this method.
*/
void trackSelfDestructBeneficiary(@NonNull Address deleted, @NonNull Address beneficiary, MessageFrame frame);

/**
* Given the HAPI operation initiating a top-level {@code CONTRACT_CREATION} message, sets up the
* {@link PendingCreation} a {@link ProxyWorldUpdater} can use to complete the creation of the new
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,22 @@
return Optional.empty();
}

/**
* {@inheritDoc}
*/
@Override
public void trackSelfDestructBeneficiary(
@NonNull final Address deleted, @NonNull final Address beneficiary, @NonNull final MessageFrame frame) {
requireNonNull(deleted);
requireNonNull(beneficiary);

Check warning on line 553 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java#L552-L553

Added lines #L552 - L553 were not covered by tests

final var beneficiaryAccount = getAccount(beneficiary);
final var deletedAccount = (AbstractProxyEvmAccount) requireNonNull(getAccount(deleted));

Check warning on line 556 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java#L555-L556

Added lines #L555 - L556 were not covered by tests

nativeOperations.trackSelfDestructBeneficiary(
deletedAccount.hederaId(), ((AbstractProxyEvmAccount) beneficiaryAccount).hederaId(), frame);
}

Check warning on line 560 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/DispatchingEvmFrameState.java#L558-L560

Added lines #L558 - L560 were not covered by tests

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -120,6 +120,20 @@ Optional<ExceptionalHaltReason> tryTransfer(
Optional<ExceptionalHaltReason> tryTrackingSelfDestructBeneficiary(
@NonNull Address deleted, @NonNull Address beneficiary, @NonNull MessageFrame frame);

/**
* Tracks the given deletion of an account with the designated beneficiary.
*
* @param deleted the address of the account being deleted, a contract
* @param beneficiary the address of the beneficiary of the deletion
* @param frame
*
* `Beneficiary` must not be a token or a schedule. Contract `deleted` must not be any token's
* treasury. Contract `deleted` must not own any tokens. These conditions are _not_ checked
* by this method.
*/
void trackSelfDestructBeneficiary(
@NonNull Address deleted, @NonNull Address beneficiary, @NonNull MessageFrame frame);

/**
* Returns the read-only account with the given address, or {@code null} if the account is missing,
* deleted, or expired; or if this get() used the account's "long zero" address and not is priority
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -315,6 +315,16 @@
return evmFrameState.tryTrackingSelfDestructBeneficiary(deleted, beneficiary, frame);
}

/**
* {@inheritDoc}
*/
@Override
public void trackSelfDestructBeneficiary(
@NonNull final Address deleted, @NonNull final Address beneficiary, @NonNull final MessageFrame frame) {
evmFrameState.trackSelfDestructBeneficiary(deleted, beneficiary, frame);
}

Check warning on line 325 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/ProxyWorldUpdater.java

View check run for this annotation

Codecov / codecov/patch

hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/state/ProxyWorldUpdater.java#L324-L325

Added lines #L324 - L325 were not covered by tests
;

/**
* {@inheritDoc}
*/
Expand Down
Loading