MetaMask / core

This monorepo is a collection of packages used across multiple MetaMask clients
MIT License
290 stars 187 forks source link

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

Open MajorLift opened 4 months ago

MajorLift commented 4 months ago

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:

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.

desi commented 1 month ago

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 commented 1 month ago

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.