Skip to content

Commit

Permalink
fix: handle multicall revert in TokenBalancesController (#5083)
Browse files Browse the repository at this point in the history
## Explanation

When fetching erc20 token balances via the multicall3 contract, there's
a case where the call can revert. Even though we pass
`requireSuccess=false` to `tryAggregate`, this does not catch all
errors. Specifically, trying to execute code on a target address that is
not a smart contract.

There were some cases where users had tokens added with address
`0x0000000000000000000000000000000000000000` which caused such a revert,
preventing balances fetches from other tokens.

In addition to migrating out of state tokens we can detect as invalid,
the fix here is to fallback to parallel `balanceOf` calls if the
multicall request reverts.

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/assets-controllers`

- **FIXED**: `TokenBalancesController` was fixed to fetch erc20 token
balances even if there's an invalid token in state whose address does
not point to a smart contract.


## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
bergeron authored Dec 19, 2024
1 parent 945349b commit 9e83960
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 3 deletions.
46 changes: 46 additions & 0 deletions packages/assets-controllers/src/multicall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,50 @@ describe('multicall', () => {
]);
});
});

describe('error handling of reverts', () => {
const call = {
contract: new Contract(
'0x0000000000000000000000000000000000000001',
abiERC20,
provider,
),
functionSignature: 'balanceOf(address)',
arguments: ['0x0000000000000000000000000000000000000000'],
};

it('should fall back to parallel calls when multicall reverts', async () => {
jest.spyOn(provider, 'call').mockImplementationOnce(() => {
const error = { code: 'CALL_EXCEPTION' };
return Promise.reject(error);
});

jest
.spyOn(provider, 'call')
.mockImplementationOnce(() =>
Promise.resolve(defaultAbiCoder.encode(['uint256'], [1])),
);

const results = await multicallOrFallback([call], '0x1', provider);

expect(results).toMatchObject([
{
success: true,
// eslint-disable-next-line @typescript-eslint/naming-convention
value: { _hex: '0x01' },
},
]);
});

it('should throw rpc errors other than revert', async () => {
const error = { code: 'network error' };
jest.spyOn(provider, 'call').mockImplementationOnce(() => {
return Promise.reject(error);
});

await expect(
multicallOrFallback([call], '0x1', provider),
).rejects.toMatchObject(error);
});
});
});
26 changes: 23 additions & 3 deletions packages/assets-controllers/src/multicall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,27 @@ export const multicallOrFallback = async (
}

const multicallAddress = MULTICALL_CONTRACT_BY_CHAINID[chainId];
return await (multicallAddress
? multicall(calls, multicallAddress, provider, maxCallsPerMulticall)
: fallback(calls, maxCallsParallel));
if (multicallAddress) {
try {
return await multicall(
calls,
multicallAddress,
provider,
maxCallsPerMulticall,
);
} catch (error: unknown) {
// Fallback only on revert
// https://docs.ethers.org/v5/troubleshooting/errors/#help-CALL_EXCEPTION
if (
!error ||
typeof error !== 'object' ||
!('code' in error) ||
error.code !== 'CALL_EXCEPTION'
) {
throw error;
}
}
}

return await fallback(calls, maxCallsParallel);
};

0 comments on commit 9e83960

Please sign in to comment.