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

AccountReadableKVState must return pbj account with AccountNum accountId #9800

Closed
Tracked by #8828
kselveliev opened this issue Nov 22, 2024 · 0 comments · Fixed by #9801
Closed
Tracked by #8828

AccountReadableKVState must return pbj account with AccountNum accountId #9800

kselveliev opened this issue Nov 22, 2024 · 0 comments · Fixed by #9801
Assignees
Labels
enhancement Type: New feature web3 Area: Web3 API
Milestone

Comments

@kselveliev
Copy link
Contributor

Problem

When running tests with transaction executors flow.
Reusable services can be checking

  1. for Contract id ->
        final var contract = accountStore.getContractById(body.contractIDOrThrow());
        if (contract != null) {
		        // Throws exception if accountId is of returned with type alias 
            final var contractNum = contract.accountIdOrThrow().accountNumOrThrow();
            final var mayNotExist = featureFlags.isAllowCallsToNonContractAccountsEnabled(contractsConfig, contractNum);
            validateTrue(mayNotExist || !contract.deleted(), CONTRACT_DELETED);
        }

Throws exception when contractId has its accountId with type AccountOneOfType.Alias
2. Checks for account id

protected Account getAliasedAccountLeaf(@NonNull final AccountID id) {
     // The Account ID may be aliased, in which case we need to convert it to a number-based account ID first.
     requireNonNull(id);
     final var accountId = lookupAliasedAccountId(id);
     return accountId == null ? null : accountState.get(accountId);
 }

Tries to find the aliased account first and then returns it in a form of AccountOneOfType.AccountNum (expects the accountState to return it in account num -> this is our AccountReadableKVState)

Currently our AccountReadableKVState uses toAccountId method that returns it with type ALIAS
if the entity has evm address or alias.

While we observed from the above cases that we want to return the account with AccountOneOfType.AccountNum always instead of alias.

Otherwise we will be getting around 200 integration tests failing with exception thrown while handling dispatch java.lang.IllegalArgumentException: No account has id AccountID[Alias]

Solution

In AccountReadableKVState when returning the account model.
We should set it AccountId to an AccountNum type.

Alternatives

No response

@kselveliev kselveliev added enhancement Type: New feature web3 Area: Web3 API labels Nov 22, 2024
@kselveliev kselveliev self-assigned this Nov 22, 2024
@steven-sheehy steven-sheehy added this to the 0.119.0 milestone Nov 22, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Mirror Node Nov 22, 2024
@steven-sheehy steven-sheehy moved this from 📋 Backlog to 👀 In review in Mirror Node Nov 22, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Mirror Node Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type: New feature web3 Area: Web3 API
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants