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: correct owner/spender priority addresses #10312

Merged
merged 10 commits into from
Dec 8, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,6 @@ private void dispatchAndMarkCreation(
(bodyToExternalize == null)
? SUPPRESSING_EXTERNALIZED_RECORD_CUSTOMIZER
: contractBodyCustomizerFor(bodyToExternalize));

final var contractId = ContractID.newBuilder().contractNum(number).build();
// add additional create record fields
recordBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_FULL_PREFIX_SIGNATURE_FOR_PRECOMPILE;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SIGNATURE;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.asEvmAddress;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.asHeadlongAddress;
import static java.util.Objects.requireNonNull;

import com.esaulpaugh.headlong.abi.Address;
import com.esaulpaugh.headlong.abi.TupleType;
import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.ContractID;
Expand All @@ -42,6 +45,7 @@ private ReturnTypes() {
// When no value is set for AccountID, ContractID or TokenId the return value is set to 0.
public static final AccountID ZERO_ACCOUNT_ID =
AccountID.newBuilder().accountNum(0).build();
public static final Address ZERO_ADDRESS = asHeadlongAddress(asEvmAddress(0L));
public static final ContractID ZERO_CONTRACT_ID =
ContractID.newBuilder().contractNum(0).build();
public static final TokenID ZERO_TOKEN_ID = TokenID.newBuilder().tokenNum(0).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,26 @@
package com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts;

import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_ACCOUNT_ID;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_ADDRESS;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_CONTRACT_ID;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_FIXED_FEE;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_FRACTION;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_TOKEN_ID;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.headlongAddressOf;
import static java.util.Objects.requireNonNull;

import com.esaulpaugh.headlong.abi.Address;
import com.esaulpaugh.headlong.abi.Tuple;
import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.Key;
import com.hedera.hapi.node.base.Timestamp;
import com.hedera.hapi.node.state.token.Account;
import com.hedera.hapi.node.state.token.Nft;
import com.hedera.hapi.node.state.token.Token;
import com.hedera.hapi.node.transaction.CustomFee;
import com.hedera.hapi.node.transaction.FixedFee;
import com.hedera.hapi.node.transaction.FractionalFee;
import com.hedera.hapi.node.transaction.RoyaltyFee;
import com.hedera.node.app.service.contract.impl.exec.scope.HederaNativeOperations;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.hederahashgraph.api.proto.java.TokenSupplyType;
import edu.umd.cs.findbugs.annotations.NonNull;
Expand Down Expand Up @@ -62,7 +65,7 @@ public static Tuple expiryTupleFor(@NonNull final Token token) {
return Tuple.of(
token.expirationSecond(),
headlongAddressOf(token.autoRenewAccountIdOrElse(ZERO_ACCOUNT_ID)),
(token.autoRenewSeconds() > 0 ? token.autoRenewSeconds() : 0));
tinker-michaelj marked this conversation as resolved.
Show resolved Hide resolved
token.autoRenewSeconds());
}

/**
Expand Down Expand Up @@ -233,17 +236,30 @@ public static Tuple nftTokenInfoTupleFor(
@NonNull final Nft nft,
final long serialNumber,
@NonNull final String ledgerId,
@NonNull final Account ownerAccount) {
requireNonNull(ownerAccount);
final var nftMetaData = nft.metadata() != null ? nft.metadata().toByteArray() : Bytes.EMPTY.toByteArray();
@NonNull final HederaNativeOperations nativeOperations) {
requireNonNull(nft);
requireNonNull(token);
requireNonNull(ledgerId);
requireNonNull(nativeOperations);

final var nftMetaData = nft.metadata() != null ? nft.metadata().toByteArray() : Bytes.EMPTY.toByteArray();
return Tuple.of(
tokenInfoTupleFor(token, ledgerId),
serialNumber,
headlongAddressOf(ownerAccount),
// The odd construct allowing a token to not have a treasury account set is to accommodate
// Token.DEFAULT being passed into this method, which a few HtsCall implementations do
priorityAddressOf(nft.ownerIdOrElse(token.treasuryAccountIdOrElse(ZERO_ACCOUNT_ID)), nativeOperations),
tinker-michaelj marked this conversation as resolved.
Show resolved Hide resolved
nft.mintTimeOrElse(new Timestamp(0, 0)).seconds(),
nftMetaData,
headlongAddressOf(nft.spenderIdOrElse(ZERO_ACCOUNT_ID)));
priorityAddressOf(nft.spenderIdOrElse(ZERO_ACCOUNT_ID), nativeOperations));
}

private static Address priorityAddressOf(
tinker-michaelj marked this conversation as resolved.
Show resolved Hide resolved
@NonNull final AccountID accountId, @NonNull final HederaNativeOperations nativeOperations) {
requireNonNull(accountId);
return (ZERO_ACCOUNT_ID == accountId)
tinker-michaelj marked this conversation as resolved.
Show resolved Hide resolved
? ZERO_ADDRESS
: headlongAddressOf(requireNonNull(nativeOperations.getAccount(accountId.accountNumOrThrow())));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ReturnTypes.ZERO_TOKEN_ID;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.TokenTupleUtils.nftTokenInfoTupleFor;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.nfttokeninfo.NftTokenInfoTranslator.NON_FUNGIBLE_TOKEN_INFO;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.ownerof.OwnerOfCall.getOwnerAccountNum;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.ResponseCodeEnum;
Expand Down Expand Up @@ -89,15 +88,11 @@ public NftTokenInfoCall(
}

final var nonNullNft = nft != null ? nft : Nft.DEFAULT;
final var ownerAccount = nativeOperations().getAccount(getOwnerAccountNum(nonNullNft, token));
final var ledgerConfig = configuration.getConfigData(LedgerConfig.class);
final var ledgerId = Bytes.wrap(ledgerConfig.id().toByteArray()).toString();
final var nftTokenInfo = nftTokenInfoTupleFor(token, nonNullNft, serialNumber, ledgerId, nativeOperations());
return successResult(
NON_FUNGIBLE_TOKEN_INFO
.getOutputs()
.encodeElements(
status.protoOrdinal(),
nftTokenInfoTupleFor(token, nonNullNft, serialNumber, ledgerId, ownerAccount)),
NON_FUNGIBLE_TOKEN_INFO.getOutputs().encodeElements(status.protoOrdinal(), nftTokenInfo),
gasRequirement);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public OwnerOfCall(
}
}

public static Long getOwnerAccountNum(Nft nft, Token token) {
private long getOwnerAccountNum(@NonNull final Nft nft, @NonNull final Token token) {
final var explicitId = nft.ownerIdOrElse(AccountID.DEFAULT);
if (explicitId.accountNumOrElse(TREASURY_OWNER_NUM) == TREASURY_OWNER_NUM) {
return token.treasuryAccountIdOrThrow().accountNumOrThrow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public Response findResponse(@NonNull final QueryContext context, @NonNull final
requireNonNull(context);
requireNonNull(header);

var component = provider.get().create(context, Instant.now(), CONTRACT_CALL_LOCAL);
final var component = provider.get().create(context, Instant.now(), CONTRACT_CALL_LOCAL);
final var outcome = component.contextQueryProcessor().call();

final var responseHeader = outcome.isSuccess()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@

package com.hedera.node.app.service.contract.impl.handlers;

import static com.hedera.hapi.node.base.ResponseCodeEnum.CONTRACT_DELETED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_CONTRACT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSFER_ACCOUNT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.MODIFYING_IMMUTABLE_CONTRACT;
import static com.hedera.hapi.node.base.ResponseCodeEnum.OBTAINER_DOES_NOT_EXIST;
import static com.hedera.hapi.node.base.ResponseCodeEnum.OBTAINER_REQUIRED;
import static com.hedera.hapi.node.base.ResponseCodeEnum.OBTAINER_SAME_CONTRACT_ID;
import static com.hedera.hapi.node.base.ResponseCodeEnum.PERMANENT_REMOVAL_REQUIRES_SYSTEM_INITIATION;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.asNumericContractId;
import static com.hedera.node.app.spi.validation.Validations.mustExist;
import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
import static com.hedera.node.app.spi.workflows.PreCheckException.validateFalsePreCheck;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -86,11 +89,13 @@ public void handle(@NonNull final HandleContext context) throws HandleException
validateTrue(op.hasTransferAccountID() || op.hasTransferContractID(), OBTAINER_REQUIRED);
final var accountStore = context.readableStore(ReadableAccountStore.class);
final var toBeDeleted = requireNonNull(accountStore.getContractById(op.contractIDOrThrow()));
validateFalse(toBeDeleted.deleted(), CONTRACT_DELETED);
final var obtainer = getObtainer(accountStore, op);
validateTrue(obtainer != null, OBTAINER_DOES_NOT_EXIST);
if (obtainer.deleted()) {
throw new HandleException(obtainer.smartContract() ? INVALID_CONTRACT_ID : OBTAINER_DOES_NOT_EXIST);
}
validateFalse(toBeDeleted.accountIdOrThrow().equals(obtainer.accountIdOrThrow()), OBTAINER_SAME_CONTRACT_ID);
final var recordBuilder = context.recordBuilder(ContractDeleteRecordBuilder.class);
final var deletedId = toBeDeleted.accountIdOrThrow();
context.serviceApi(TokenServiceApi.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,13 @@
final var op = context.body().contractUpdateInstanceOrThrow();

if (isAdminSigRequired(op)) {
context.requireKeyOrThrow(op.contractIDOrElse(ContractID.DEFAULT), INVALID_CONTRACT_ID);
final var accountStore = context.createStore(ReadableAccountStore.class);
final var targetId = op.contractIDOrElse(ContractID.DEFAULT);
final var maybeContract = accountStore.getContractById(targetId);
if (maybeContract != null && maybeContract.keyOrThrow().key().kind() == Key.KeyOneOfType.CONTRACT_ID) {
throw new PreCheckException(MODIFYING_IMMUTABLE_CONTRACT);

Check warning on line 82 in hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractUpdateHandler.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/handlers/ContractUpdateHandler.java#L82

Added line #L82 was not covered by tests
}
context.requireKeyOrThrow(targetId, INVALID_CONTRACT_ID);
}
if (hasCryptoAdminKey(op)) {
context.requireKey(op.adminKeyOrThrow());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,21 @@
import static com.hedera.hapi.node.base.ResponseCodeEnum.INSUFFICIENT_GAS;
import static com.hedera.hapi.node.base.ResponseCodeEnum.MAX_GAS_LIMIT_EXCEEDED;
import static com.hedera.node.app.service.contract.impl.hevm.HederaEvmTransaction.NOT_APPLICABLE;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.EVM_ADDRESS_LENGTH_AS_LONG;
import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
import static java.util.Objects.requireNonNull;
import static org.apache.tuweni.bytes.Bytes.EMPTY;

import com.hedera.hapi.node.base.AccountID;
import com.hedera.hapi.node.base.ContractID;
import com.hedera.hapi.node.contract.ContractCallLocalQuery;
import com.hedera.hapi.node.transaction.Query;
import com.hedera.node.app.service.contract.impl.annotations.QueryScope;
import com.hedera.node.app.service.contract.impl.hevm.HederaEvmTransaction;
import com.hedera.node.app.service.token.ReadableAccountStore;
import com.hedera.node.app.spi.workflows.QueryContext;
import com.hedera.node.config.data.ContractsConfig;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.Objects;
import javax.inject.Inject;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;

Expand All @@ -44,14 +47,16 @@ public class HevmStaticTransactionFactory {
private static final long INTRINSIC_GAS_LOWER_BOUND = 21_000L;
private final ContractsConfig contractsConfig;
private final GasCalculator gasCalculator;
private final QueryContext context;
private final AccountID payerId;

@Inject
public HevmStaticTransactionFactory(
@NonNull final QueryContext context, @NonNull final GasCalculator gasCalculator) {
this.contractsConfig = Objects.requireNonNull(context).configuration().getConfigData(ContractsConfig.class);
this.gasCalculator = gasCalculator;
this.payerId = Objects.requireNonNull(context.payer());
this.context = requireNonNull(context);
this.contractsConfig = context.configuration().getConfigData(ContractsConfig.class);
this.gasCalculator = requireNonNull(gasCalculator);
this.payerId = requireNonNull(context.payer());
}

/**
Expand All @@ -65,18 +70,15 @@ public HederaEvmTransaction fromHapiQuery(@NonNull final Query query) {
final var op = query.contractCallLocalOrThrow();
assertValidCall(op);
final var senderId = op.hasSenderId() ? op.senderIdOrThrow() : payerId;
var targetId = op.contractIDOrThrow();
// For mono-service fidelity, allow calls using 0.0.X id even to contracts with a priority EVM address
final var maybeContract =
context.createStore(ReadableAccountStore.class).getContractById(targetId);
if (maybeContract != null && maybeContract.alias().length() == EVM_ADDRESS_LENGTH_AS_LONG) {
tinker-michaelj marked this conversation as resolved.
Show resolved Hide resolved
targetId = ContractID.newBuilder().evmAddress(maybeContract.alias()).build();
}
return new HederaEvmTransaction(
senderId,
null,
op.contractIDOrThrow(),
NOT_APPLICABLE,
op.functionParameters(),
null,
0L,
op.gas(),
1L,
0L,
null);
senderId, null, targetId, NOT_APPLICABLE, op.functionParameters(), null, 0L, op.gas(), 1L, 0L, null);
}

private void assertValidCall(@NonNull final ContractCallLocalQuery body) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,11 @@ public static long asExactLongValueOrZero(@NonNull final BigInteger value) {

/**
* Given a {@link AccountID}, returns its address as a headlong address.
* @param accountID
* @return
* @param accountID the account id
* @return the headlong address
*/
public static com.esaulpaugh.headlong.abi.Address headlongAddressOf(@NonNull final AccountID accountID) {
requireNonNull(accountID);
if (accountID.account().kind() == AccountID.AccountOneOfType.UNSET) {
return asHeadlongAddress(Bytes.EMPTY.toArray());
}

final var integralAddress = accountID.hasAccountNum()
? asEvmAddress(accountID.accountNum())
: accountID.alias().toByteArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,6 @@ public class TestHelpers {
public static final AccountID OPERATOR_ACCOUNT_ID =
AccountID.newBuilder().accountNum(7777777L).build();

public static final Account A_NEW_ACCOUNT =
Account.newBuilder().accountId(A_NEW_ACCOUNT_ID).build();
public static final Account B_NEW_ACCOUNT =
Account.newBuilder().accountId(B_NEW_ACCOUNT_ID).build();

public static final Nft TREASURY_OWNED_NFT = Nft.newBuilder()
.metadata(Bytes.wrap("Unsold"))
.nftId(NftID.newBuilder().tokenId(NON_FUNGIBLE_TOKEN_ID).serialNumber(NFT_SERIAL_NO))
Expand Down
Loading
Loading