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: Cherry pick (0.56): isAuthorizedRaw bug fix #16405

Merged
Merged
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
Expand Up @@ -148,7 +148,7 @@ public PricedResult execute(@NonNull final MessageFrame frame) {
// Key must match signature type
if (key.isPresent()) {
if (!switch (signatureType) {
case EC -> false;
case EC -> key.get().hasEcdsa384();
case ED -> key.get().hasEd25519();
case INVALID -> false;
}) return bail.apply(INVALID_SIGNATURE_TYPE_MISMATCHING_KEY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@
package com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.isvalidalias;

import static com.hedera.hapi.node.base.ResponseCodeEnum.SUCCESS;
import static com.hedera.node.app.service.contract.impl.exec.scope.HederaNativeOperations.MISSING_ENTITY_NUMBER;
import static com.hedera.node.app.service.contract.impl.exec.scope.HederaNativeOperations.NON_CANONICAL_REFERENCE_NUMBER;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.FullResult.successResult;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.common.Call.PricedResult.gasOnly;
import static com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.isvalidalias.IsValidAliasTranslator.IS_VALID_ALIAS;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.accountNumberForEvmReference;
import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.isLongZero;
import static java.util.Objects.requireNonNull;

import com.esaulpaugh.headlong.abi.Address;
import com.hedera.node.app.service.contract.impl.exec.scope.HederaNativeOperations;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.FullResult;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.common.AbstractCall;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.HasCallAttempt;
Expand All @@ -49,13 +52,50 @@ public IsValidAliasCall(@NonNull final HasCallAttempt attempt, @NonNull final Ad
@Override
public @NonNull PricedResult execute() {

// It is OK if given address is long zero whether it does or doesn't have an EVM alias (which is
// why NON_CANONICAL_REFERENCE_NUMBER is an acceptable result).
final long accountNum = accountNumberForEvmReference(address, nativeOperations());
boolean isValidAlias = accountNum >= 0 || accountNum == NON_CANONICAL_REFERENCE_NUMBER;
// It is OK if given address is long zero whether it does or doesn't have an EVM alias

final var aliasKind = getAliasKindForAddressWithAccount(address, nativeOperations());
final boolean isValidAlias = aliasKind != AliasKind.EVM_ADDRESS_WITH_NO_ASSOCIATED_ACCOUNT;
return gasOnly(fullResultsFor(isValidAlias), SUCCESS, true);
}

// FUTURE: Consider moving `AliasKind` and `getAliasKindForAddressWithAccount` to
// `com.hedera.node.app.service.token.AliasUtils`

public enum AliasKind {
ACCOUNT_NUM_ALIAS_OF_ACCOUNT_WITH_EVM_ALIAS,
ACCOUNT_NUM_ALIAS_OF_ACCOUNT_WITHOUT_EVM_ALIAS,
EVM_ALIAS_OF_ACCOUNT,
EVM_ADDRESS_WITH_NO_ASSOCIATED_ACCOUNT
};

/**
* Determine what kind of alias the {@link Address} is, w.r.t. current accounts known to the system
* @param address An EVM address
* @param nativeOperations Provides access to state (for Hedera accounts)
* @return an {@link AliasKind} describing the kind of alias this address is
*/
public static @NonNull AliasKind getAliasKindForAddressWithAccount(
@NonNull final Address address, @NonNull final HederaNativeOperations nativeOperations) {
final boolean isAccountNumAlias /*aka long-zero*/ = isLongZero(address);
final long accountNum = accountNumberForEvmReference(address, nativeOperations);

if (accountNum == MISSING_ENTITY_NUMBER) {
// No account for this alias (accountNumberForEvmReference)
// Key alias given as protobuf but not EC key
// Not an EVM address (wrong size)
return AliasKind.EVM_ADDRESS_WITH_NO_ASSOCIATED_ACCOUNT;
} else if (accountNum == NON_CANONICAL_REFERENCE_NUMBER) {
// Account doesn't have an EVM address alias
return AliasKind.ACCOUNT_NUM_ALIAS_OF_ACCOUNT_WITHOUT_EVM_ALIAS;
} else {
// valid account with EVM address alias (possibly hollow?)
return isAccountNumAlias
? AliasKind.ACCOUNT_NUM_ALIAS_OF_ACCOUNT_WITH_EVM_ALIAS
: AliasKind.EVM_ALIAS_OF_ACCOUNT;
}
}

private @NonNull FullResult fullResultsFor(final boolean result) {
return successResult(IS_VALID_ALIAS.getOutputs().encodeElements(result), gasCalculator.viewGasRequirement());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,13 +400,23 @@ public static long numberOfLongZero(@NonNull final Address address) {
/**
* Given an EVM address, returns whether it is long-zero.
*
* @param address the EVM address
* @param address the EVM address (as a BESU {@link org.hyperledger.besu.datatypes.Address})
* @return whether it is long-zero
*/
public static boolean isLongZero(@NonNull final Address address) {
return isLongZeroAddress(address.toArrayUnsafe());
}

/**
* Given an EVM address, returns whether it is long-zero.
*
* @param address the EVM address (as a headlong {@link com.esaulpaugh.headlong.abi.Address})
* @return whether it is long-zero
*/
public static boolean isLongZero(@NonNull final com.esaulpaugh.headlong.abi.Address address) {
return isLongZeroAddress(explicitFromHeadlong(address));
}

/**
* Converts an EVM address to a PBJ {@link com.hedera.pbj.runtime.io.buffer.Bytes} alias.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,26 @@
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.revertOutputFor;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.signature;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;

import com.esaulpaugh.headlong.abi.Address;
import com.hedera.node.app.service.contract.impl.exec.gas.CustomGasCalculator;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.HasCallAttempt;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.isauthorizedraw.IsAuthorizedRawCall;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.has.isauthorizedraw.IsAuthorizedRawCall.SignatureType;
import com.hedera.node.app.service.contract.impl.test.exec.systemcontracts.common.CallTestBase;
import edu.umd.cs.findbugs.annotations.NonNull;
import org.hyperledger.besu.evm.frame.MessageFrame.State;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;

class IsAuthorizedRawCallTest extends CallTestBase {
Expand All @@ -50,7 +54,7 @@ class IsAuthorizedRawCallTest extends CallTestBase {
@Mock
private HasCallAttempt attempt;

private CustomGasCalculator customGasCalculator = new CustomGasCalculator();
private final CustomGasCalculator customGasCalculator = new CustomGasCalculator();

@BeforeEach
void setup() {
Expand Down Expand Up @@ -83,6 +87,19 @@ void revertsWhenEcdsaIsNotEvmAddress() {
assertEquals(revertOutputFor(INVALID_ACCOUNT_ID), result.getOutput());
}

@Test
void notValidAccountIfNegative() {
final var result = getSubject(mock(Address.class)).isValidAccount(-25L, mock(SignatureType.class));
assertFalse(result);
}

@ParameterizedTest
@ValueSource(longs = {0L, 1L, 10L, 100L, 1_000_000_000_000L, Long.MAX_VALUE})
void anyNonNegativeAccountValidIfED(final long account) {
final var result = getSubject(mock(Address.class)).isValidAccount(account, SignatureType.ED);
assertTrue(result);
}

@ParameterizedTest
@CsvSource({"0,27", "1,28", "27,27", "28,28", "45,27", "46,28", "18,"})
void reverseVTest(final byte fromV, final Byte expectedV) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@

import com.hedera.services.bdd.spec.HapiSpec;
import com.hedera.services.bdd.spec.HapiSpecOperation;
import com.hedera.services.bdd.spec.SpecOperation;
import com.hederahashgraph.api.proto.java.Transaction;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;

public abstract class UtilOp extends HapiSpecOperation {
@Override
Expand All @@ -30,4 +35,37 @@ public UtilOp logged() {
verboseLoggingOn = true;
return this;
}

/**
* Given an array of mixed
* <ul>
* <li>{@link SpecOperation},</li>
* <li>{@link SpecOperation}[],</li>
* <li>{@link Collection}&lt;{@link SpecOperation}></li>
* </ul>
* flatten it into an {@link SpecOperation}[] with all the elements in the order iterated. (Only
* flattens top-level.)
*/
@SuppressWarnings("rawtypes")
public static @NonNull SpecOperation[] flatten(@NonNull final Object... sos) {
final var ops = new ArrayList<SpecOperation>();
for (final var o : sos) {
switch (o) {
case SpecOperation so -> ops.add(so);
case SpecOperation[] aso -> Collections.addAll(ops, aso);
case Collection co -> {
for (final var e : co) {
if (e instanceof SpecOperation so) {
ops.add(so);
}
// and ... silently ignore anything in the collection that's _not_ a `SpecOperation` ...
}
}
default -> {
/* silently ignore anything in the top-level array that's _not_ what we expect ... */
}
}
}
return ops.toArray(new SpecOperation[0]);
}
}
Loading
Loading