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

Suppress recoverable error for inactive evm addresses #8536

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -44,6 +44,7 @@
import com.hederahashgraph.api.proto.java.ContractID;
import com.hederahashgraph.api.proto.java.ResponseCodeEnum;
import com.hederahashgraph.api.proto.java.TransactionBody;
import com.hederahashgraph.api.proto.java.TransactionRecord;
import jakarta.inject.Named;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -88,15 +89,17 @@ public void process(@NonNull RecordItem recordItem, Transaction transaction) {

// contractResult
var transactionHandler = transactionHandlerFactory.get(TransactionType.of(transaction.getType()));

var throwRecoverableError = shouldThrowRecoverableError(functionResult, recordItem, transactionRecord);
steven-sheehy marked this conversation as resolved.
Show resolved Hide resolved
// in pre-compile case transaction is not a contract type and entityId will be of a different type
var contractId = (contractCallOrCreate
? Optional.ofNullable(transaction.getEntityId())
: entityIdService.lookup(functionResult.getContractID()))
: entityIdService.lookup(functionResult.getContractID(), throwRecoverableError))
.orElse(EntityId.EMPTY);
var isRecoverableError = EntityId.isEmpty(contractId)
&& !contractCallOrCreate
&& !ContractID.getDefaultInstance().equals(functionResult.getContractID());
var isRecoverableError =
!(recordItem.isSuccessful() && functionResult.getContractID().hasEvmAddress())
steven-sheehy marked this conversation as resolved.
Show resolved Hide resolved
&& EntityId.isEmpty(contractId)
&& !contractCallOrCreate
&& (!ContractID.getDefaultInstance().equals(functionResult.getContractID()));
if (isRecoverableError) {
Utility.handleRecoverableError(
"Invalid contract id for contract result at {}", recordItem.getConsensusTimestamp());
Expand Down Expand Up @@ -149,6 +152,13 @@ private boolean isContractCreateOrCall(TransactionBody transactionBody) {
return transactionBody.hasContractCall() || transactionBody.hasContractCreateInstance();
}

private boolean shouldThrowRecoverableError(
ContractFunctionResult functionResult, RecordItem recordItem, TransactionRecord transactionRecord) {
return !(recordItem.isSuccessful()
&& (transactionRecord.getReceipt().getContractID().hasEvmAddress()
|| functionResult.getContractID().hasEvmAddress()));
}

private void processContractAction(ContractAction action, int index, RecordItem recordItem) {
long consensusTimestamp = recordItem.getConsensusTimestamp();
var contractAction = new com.hedera.mirror.common.domain.contract.ContractAction();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ public interface EntityIdService {
*/
Optional<EntityId> lookup(ContractID contractId);

/**
* Converts a protobuf ContractID to an EntityID, resolving any EVM addresses that may be present.
*
* @param contractId The protobuf contract ID
* @param throwRecoverableError If true, will throw a recoverable error if an EVM address cannot be found.
* @return An optional of the converted EntityId if it can be resolved, or EntityId.EMPTY if none can be resolved.
*/
Optional<EntityId> lookup(ContractID contractId, boolean throwRecoverableError);

/**
* Specialized form of lookup(ContractID) that returns the first contract ID parameter that resolves to a non-empty
* EntityId.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public Optional<EntityId> lookup(AccountID... accountIds) {

@Override
public Optional<EntityId> lookup(ContractID contractId) {
return lookup(contractId, true);
}

@Override
public Optional<EntityId> lookup(ContractID contractId, boolean throwRecoverableError) {
if (contractId == null || contractId.equals(ContractID.getDefaultInstance())) {
return EMPTY;
}
Expand All @@ -95,7 +100,10 @@ public Optional<EntityId> lookup(ContractID contractId) {
case EVM_ADDRESS -> cacheLookup(
contractId.getEvmAddress(),
() -> findByEvmAddress(
toBytes(contractId.getEvmAddress()), contractId.getShardNum(), contractId.getRealmNum()));
toBytes(contractId.getEvmAddress()),
contractId.getShardNum(),
contractId.getRealmNum(),
throwRecoverableError));
default -> {
Utility.handleRecoverableError("Invalid ContractID: {}", contractId);
yield Optional.empty();
Expand Down Expand Up @@ -158,12 +166,17 @@ public void notify(Entity entity) {
}

private Optional<EntityId> findByEvmAddress(byte[] evmAddress, long shardNum, long realmNum) {
return findByEvmAddress(evmAddress, shardNum, realmNum, true);
}

private Optional<EntityId> findByEvmAddress(
byte[] evmAddress, long shardNum, long realmNum, boolean throwRecoverableError) {
var id = Optional.ofNullable(DomainUtils.fromEvmAddress(evmAddress))
// Verify shard and realm match when assuming evmAddress is in the 'shard.realm.num' form
.filter(e -> e.getShard() == shardNum && e.getRealm() == realmNum)
.or(() -> entityRepository.findByEvmAddress(evmAddress).map(EntityId::of));

if (id.isEmpty()) {
if (id.isEmpty() && throwRecoverableError) {
Utility.handleRecoverableError("Entity not found for EVM address {}", Hex.encodeHexString(evmAddress));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.protobuf.ByteString;
import com.hedera.mirror.common.domain.DomainBuilder;
import com.hedera.mirror.common.domain.contract.ContractTransaction;
import com.hedera.mirror.common.domain.entity.EntityId;
Expand Down Expand Up @@ -91,6 +92,14 @@ private static Stream<Arguments> provideEntities() {
.record(x -> x.setContractCallResult(builder.contractFunctionResult()))
.build();

var contractIdWithEvm = ContractID.newBuilder()
.setEvmAddress(ByteString.copyFromUtf8("1234"))
.build();
Function<RecordItemBuilder, RecordItem> withInactiveEvm =
(RecordItemBuilder builder) -> builder.tokenMint(TokenType.FUNGIBLE_COMMON)
.record(x -> x.setContractCallResult(builder.contractFunctionResult(contractIdWithEvm)))
.build();

Function<RecordItemBuilder, RecordItem> contractCreate =
(RecordItemBuilder builder) -> builder.contractCreate().build();

Expand All @@ -101,7 +110,9 @@ private static Stream<Arguments> provideEntities() {
Arguments.of(withDefaultContractId, EntityId.EMPTY, false),
Arguments.of(contractCreate, EntityId.EMPTY, false),
Arguments.of(contractCreate, null, false),
Arguments.of(contractCreate, EntityId.of(0, 0, 5), false));
Arguments.of(contractCreate, EntityId.of(0, 0, 5), false),
Arguments.of(withInactiveEvm, null, false),
Arguments.of(withInactiveEvm, EntityId.EMPTY, false));
steven-sheehy marked this conversation as resolved.
Show resolved Hide resolved
}

@BeforeEach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@
import com.hederahashgraph.api.proto.java.ContractID;
import lombok.RequiredArgsConstructor;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.boot.test.system.CapturedOutput;
import org.springframework.boot.test.system.OutputCaptureExtension;

@RequiredArgsConstructor
@ExtendWith(OutputCaptureExtension.class)
class EntityIdServiceImplTest extends ImporterIntegrationTest {

// in the form 'shard.realm.num'
Expand All @@ -45,6 +50,8 @@ class EntityIdServiceImplTest extends ImporterIntegrationTest {
0, 0, 0, 0, 0, 0, 0, 100, // num
};

private static final String RECOVERABLE_ERROR_LOG_PREFIX = "Recoverable error. ";

private final EntityRepository entityRepository;
private final EntityIdService entityIdService;

Expand Down Expand Up @@ -213,13 +220,31 @@ void lookupContractEvmAddressSpecific() {
}

@Test
void lookupContractEvmAddressNoMatch() {
void lookupContractEvmAddressNoMatch(CapturedOutput output) {
Entity contract = domainBuilder
.entity()
.customize(e -> e.alias(null).type(CONTRACT))
.get();
var contractId = getProtoContractId(contract);
assertThat(entityIdService.lookup(contractId)).isEmpty();
assertThat(output.getAll()).containsIgnoringCase(RECOVERABLE_ERROR_LOG_PREFIX);
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
void lookupContractEvmAddressRecoverableError(boolean throwRecoverableError, CapturedOutput output) {
Entity contract = domainBuilder
.entity()
.customize(e -> e.alias(null).type(CONTRACT))
.get();
var contractId = getProtoContractId(contract);

assertThat(entityIdService.lookup(contractId, throwRecoverableError)).isEmpty();
if (throwRecoverableError) {
assertThat(output.getAll()).containsIgnoringCase(RECOVERABLE_ERROR_LOG_PREFIX);
} else {
assertThat(output.getAll()).doesNotContainIgnoringCase(RECOVERABLE_ERROR_LOG_PREFIX);
}
}

@Test
Expand Down
Loading