Skip to content

Commit

Permalink
Fix EVM address lookup corner case (#7088)
Browse files Browse the repository at this point in the history 
* Bump acceptance test max retries to workaround testnet throttle
* Fix entity ID lookup when create and transfer to EVM address occur in the same record file

Signed-off-by: Steven Sheehy <[email protected]>
  • Loading branch information
@steven-sheehy
steven-sheehy authored Oct 17, 2023
1 parent dd0f3e2 commit 9507636
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 32 deletions.
34 changes: 20 additions & 14 deletions ...-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/EntityIdServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,24 +109,30 @@ public void notify(Entity entity) {

EntityId entityId = entity.toEntityId();
EntityType type = entity.getType();
GeneratedMessageV3.Builder<?> builder;

switch (type) {
case ACCOUNT -> builder = AccountID.newBuilder()
.setShardNum(entityId.getShard())
.setRealmNum(entityId.getRealm())
.setAlias(alias);
case CONTRACT -> builder = ContractID.newBuilder()
.setShardNum(entityId.getShard())
.setRealmNum(entityId.getRealm())
.setEvmAddress(alias);
default -> {
Utility.handleRecoverableError("Invalid Entity: {} entity can't have alias", type);
return;
case ACCOUNT -> {
var builder = AccountID.newBuilder()
.setShardNum(entityId.getShard())
.setRealmNum(entityId.getRealm())
.setAlias(alias);
cache.put(builder.build(), entityId);

// Accounts can have an alias and an EVM address so warm the cache with both
if (entity.getAlias() != null && entity.getEvmAddress() != null) {
builder.setAlias(DomainUtils.fromBytes(entity.getEvmAddress()));
cache.put(builder.build(), entityId);
}
}
case CONTRACT -> {
var builder = ContractID.newBuilder()
.setShardNum(entityId.getShard())
.setRealmNum(entityId.getRealm())
.setEvmAddress(alias);
cache.put(builder.build(), entityId);
}
default -> Utility.handleRecoverableError("Invalid Entity: {} entity can't have alias", type);
}

cache.put(builder.build(), entityId);
}

private EntityId load(AccountID accountId) {
Expand Down
3 changes: 1 addition & 2 deletions hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/util/Utility.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@
public class Utility {

public static final Instant MAX_INSTANT_LONG = Instant.ofEpochSecond(0, Long.MAX_VALUE);

public static final String HALT_ON_ERROR_PROPERTY = "HEDERA_MIRROR_IMPORTER_PARSER_HALTONERROR";
static final String RECOVERABLE_ERROR = "Recoverable error. ";
static final String HALT_ON_ERROR_PROPERTY = "HEDERA_MIRROR_IMPORTER_PARSER_HALTONERROR";
static final String HALT_ON_ERROR_DEFAULT = "false";
private static final int ECDSA_SECP256K1_COMPRESSED_KEY_LENGTH = 33;

Expand Down
23 changes: 15 additions & 8 deletions ...ror-importer/src/test/java/com/hedera/mirror/importer/domain/EntityIdServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,16 +298,23 @@ void lookupContractsEntityIdNotFound() {
}

@ParameterizedTest
@CsvSource(value = {"false", ","})
void storeAccount(Boolean deleted) {
@CsvSource(
nullValues = "null",
value = {"false", "null"})
void notifyAccount(Boolean deleted) {
Entity account =
domainBuilder.entity().customize(e -> e.deleted(deleted)).get();
var alias = getProtoAccountId(account);
var evmAddress = alias.toBuilder()
.setAlias(DomainUtils.fromBytes(account.getEvmAddress()))
.build();
entityIdService.notify(account);
assertThat(entityIdService.lookup(getProtoAccountId(account))).hasValue(account.toEntityId());
assertThat(entityIdService.lookup(alias)).hasValue(account.toEntityId());
assertThat(entityIdService.lookup(evmAddress)).hasValue(account.toEntityId());
}

@Test
void storeAccountDeleted() {
void notifyAccountDeleted() {
Entity account = domainBuilder.entity().customize(e -> e.deleted(true)).get();
entityIdService.notify(account);
var accountId = getProtoContractId(account);
Expand All @@ -316,7 +323,7 @@ void storeAccountDeleted() {

@ParameterizedTest
@CsvSource(value = {"false", ","})
void storeContract(Boolean deleted) {
void notifyContract(Boolean deleted) {
Entity contract = domainBuilder
.entity()
.customize(c -> c.alias(null).deleted(deleted).type(CONTRACT))
Expand All @@ -326,7 +333,7 @@ void storeContract(Boolean deleted) {
}

@Test
void storeContractDeleted() {
void notifyContractDeleted() {
Entity contract = domainBuilder
.entity()
.customize(c -> c.alias(null).deleted(true).type(CONTRACT))
Expand All @@ -337,12 +344,12 @@ void storeContractDeleted() {
}

@Test
void storeNull() {
void notifyNull() {
assertDoesNotThrow(() -> entityIdService.notify(null));
}

@Test
void unknownEntityType() {
void notifyUnknown() {
Entity contract = domainBuilder
.entity()
.customize(
Expand Down
17 changes: 12 additions & 5 deletions ...a/com/hedera/mirror/importer/parser/record/entity/EntityRecordItemListenerCryptoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.hedera.mirror.importer.TestUtils.toEntityTransactions;
import static com.hedera.mirror.importer.config.CacheConfiguration.CACHE_ALIAS;
import static com.hedera.mirror.importer.parser.domain.RecordItemBuilder.STAKING_REWARD_ACCOUNT;
import static com.hedera.mirror.importer.util.Utility.HALT_ON_ERROR_PROPERTY;
import static com.hedera.mirror.importer.util.UtilityTest.ALIAS_ECDSA_SECP256K1;
import static com.hedera.mirror.importer.util.UtilityTest.EVM_ADDRESS;
import static java.lang.String.format;
Expand Down Expand Up @@ -1434,6 +1435,8 @@ void cryptoTransferUpdatesAllowanceAmount() {

@Test
void cryptoTransferWithAlias() {
var haltOnError = System.getProperty(HALT_ON_ERROR_PROPERTY, "false");
System.setProperty(HALT_ON_ERROR_PROPERTY, "true");
entityProperties.getPersist().setCryptoTransferAmounts(true);
entityProperties.getPersist().setItemizedTransfers(true);
Entity entity = domainBuilder.entity().persist();
Expand All @@ -1452,13 +1455,16 @@ void cryptoTransferWithAlias() {
ResponseCodeEnum.SUCCESS.getNumber());

var transfer1 = accountAliasAmount(ALIAS_KEY, 1003).build();
var transfer2 =
accountAliasAmount(ByteString.copyFrom(entity.getAlias()), 1004).build();
var transfer2 = accountAliasAmount(DomainUtils.fromBytes(entity.getAlias()), 1004)
.build();
var transfer3 = accountAliasAmount(DomainUtils.fromBytes(entity.getEvmAddress()), 1005)
.build();
// Crypto transfer to both existing alias and newly created alias accounts
Transaction transaction = buildTransaction(builder -> builder.getCryptoTransferBuilder()
.getTransfersBuilder()
.addAccountAmounts(transfer1)
.addAccountAmounts(transfer2));
.addAccountAmounts(transfer2)
.addAccountAmounts(transfer3));
TransactionBody transactionBody = getTransactionBody(transaction);
TransactionRecord recordTransfer = transactionRecordSuccess(
transactionBody, builder -> groupCryptoTransfersByAccountId(builder, List.of()));
Expand Down Expand Up @@ -1488,9 +1494,10 @@ void cryptoTransferWithAlias() {
.extracting(com.hedera.mirror.common.domain.transaction.Transaction::getItemizedTransfer)
.asList()
.map(transfer ->
((ItemizedTransfer) transfer).getEntityId().getId())
((ItemizedTransfer) transfer).getEntityId().getNum())
.asList()
.contains(newAccount.getAccountNum(), entity.getNum()));
.containsExactly(newAccount.getAccountNum(), entity.getNum(), entity.getNum()));
System.setProperty(HALT_ON_ERROR_PROPERTY, haltOnError);
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions hedera-mirror-test/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ include:
| `hedera.mirror.test.acceptance.mirrorNodeAddress` | testnet.mirrornode.hedera.com:443 | The mirror node gRPC server endpoint including IP address and port. |
| `hedera.mirror.test.acceptance.network` | TESTNET | Which Hedera network to use. Can be either `MAINNET`, `PREVIEWNET`, `TESTNET` or `OTHER`. |
| `hedera.mirror.test.acceptance.nodes` | [] | A map of custom consensus node with the key being the account ID and the value its endpoint. |
| `hedera.mirror.test.acceptance.operatorBalance` | 70000000000 | The amount of tinybars to fund the operator. Applicable only when `createOperatorAccount` is `true`. |
| `hedera.mirror.test.acceptance.operatorBalance` | 80000000000 | The amount of tinybars to fund the operator. Applicable only when `createOperatorAccount` is `true`. |
| `hedera.mirror.test.acceptance.operatorId` | 0.0.2 | Operator account ID used to pay for transactions. |
| `hedera.mirror.test.acceptance.operatorKey` | Genesis key | Operator ED25519 or ECDSA private key used to sign transactions in hex encoded DER format. |
| `hedera.mirror.test.acceptance.rest.baseUrl` | https://testnet.mirrornode.hedera.com/api/v1 | The URL to the mirror node REST API. |
Expand All @@ -62,7 +62,7 @@ include:
| `hedera.mirror.test.acceptance.rest.retryableExceptions` | [java.lang.Exception] | A list of exception classes that will be considered for retry. |
| `hedera.mirror.test.acceptance.retrieveAddressBook` | true | Whether to download the address book from the mirror node and use those nodes to publish transactions. |
| `hedera.mirror.test.acceptance.sdk.grpcDeadline` | 10s | The maximum amount of time to wait for a grpc call to complete. |
| `hedera.mirror.test.acceptance.sdk.maxAttempts` | 100 | The maximum number of times the sdk should try to submit a transaction to the network. |
| `hedera.mirror.test.acceptance.sdk.maxAttempts` | 1000 | The maximum number of times the sdk should try to submit a transaction to the network. |
| `hedera.mirror.test.acceptance.sdk.maxNodesPerTransaction` | 2147483647 | The maximum number of nodes that a transaction can be submitted to. |
| `hedera.mirror.test.acceptance.startupTimeout` | 60m | How long the startup probe should wait for the network as a whole to be healthy before failing the tests. |
| `hedera.mirror.test.acceptance.web3.baseUrl` | Inherits `rest.baseUrl` | The endpoint associated with the web3 API. |
Expand Down
2 changes: 1 addition & 1 deletion ...mirror-test/src/test/java/com/hedera/mirror/test/e2e/acceptance/config/SdkProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class SdkProperties {
private Duration grpcDeadline = Duration.ofSeconds(10L);

@Min(1)
private int maxAttempts = 100;
private int maxAttempts = 1000;

@Min(1)
private int maxNodesPerTransaction = Integer.MAX_VALUE;
Expand Down

0 comments on commit 9507636

Please sign in to comment.