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

Contract class balances method calls are typed as implicit any #4464

Open
MajorLift opened this issue Jun 26, 2024 · 2 comments
Open

Contract class balances method calls are typed as implicit any #4464

MajorLift opened this issue Jun 26, 2024 · 2 comments
Labels
bug Something isn't working team-wallet-framework

Comments

@MajorLift
Copy link
Contributor

MajorLift commented Jun 26, 2024

In the following, contract.balances and result are implicitly typed as any, because the Contract class doesn't have a balances method.

import { Contract } from '@ethersproject/contracts';
...
async getBalancesInSingleCall(
  ...
  ) {
    ...
    const contract = new Contract(
      contractAddress,
      abiSingleCallBalancesContract,
      provider,
    );
    const result = await contract.balances([selectedAddress], tokensToDetect);
    const nonZeroBalances: BalanceMap = {};
    /* istanbul ignore else */
    if (result.length > 0) {
      tokensToDetect.forEach((tokenAddress, index) => {
        const balance: BN = result[index];
        /* istanbul ignore else */
        if (String(balance) !== '0') {
          nonZeroBalances[tokenAddress] = balance;
        }
      });
    }
    return nonZeroBalances;
  }
}

https://github.com/MetaMask/core/blob/main/packages/assets-controllers/src/AssetsContractController.ts#L522-L554

All tests for getBalancesInSingleCall only check for toBeDefined() or strictEquals({}), with the exception of the following:

  • 'should track and use the currently selected chain ID and provider when getting balances in a single call'

The fact that this test passes at runtime suggests that either this bug is only an issue at the type level, or there is a fallback method being called instead.

@MajorLift MajorLift added bug Something isn't working team-wallet-framework labels Jun 26, 2024
@MajorLift MajorLift changed the title [assets-contract] Calls non-existent balances method on Contract class instance [assets-contract] Implicit any typing of Contract.balances method calls Jul 19, 2024
@MajorLift MajorLift changed the title [assets-contract] Implicit any typing of Contract.balances method calls Contract class balances method calls are typed as implicit any Jul 19, 2024
@desi
Copy link
Contributor

desi commented Sep 26, 2024

The reason this type isn't defined is because those methods of that object are defined dynamically based on the abi. Presumably the ethers contract class doesn't type the response based on the input. Its probably expected.

Edit (majorlift): We should ensure that any is not returned and propagated to the rest of the code base. We currently don't know whether any is being returned or propagated.

@desi
Copy link
Contributor

desi commented Sep 26, 2024

Probably should at least add a comment explaining why the balances method exists and how we know that it is there and can be called so it isn't confusing in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working team-wallet-framework
Projects
None yet
Development

No branches or pull requests

2 participants