core icon indicating copy to clipboard operation
core copied to clipboard

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

Open MajorLift opened this issue 1 year ago • 2 comments

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 avatar Jun 26 '24 13:06 MajorLift

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 avatar Sep 26 '24 14:09 desi

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.

desi avatar Sep 26 '24 14:09 desi