fix token detection fallback

[bergeron]

Explanation

Even when account API detection succeeded, it was doing an additional, redundant RPC based detection on the same chains.

References

Changelog

• @metamask/assets-controllers: Fixes an issue where the token detection was unnecessarily falling back to an RPC approach, causing redundant detections.

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

[bergeron]

This seems like it was just adding all the account API chains, so it was doing an additional, redundant RPC based detection even when the account api just succeeded for those chains.

The above condition is not sufficient to short circuit, when you have a mix of chains that do and do not support account api.

If we want to keep an rpc based fallback if the account API fails, we could instead make the condition something like:

if (apiResult?.result === 'failed') {
  this.#addChainsToRpcDetection(...)
}

[bergeron]

Let me know if either of these fixes makes sense to you @salimtb

[salimtb]

yes, i got the idea and the fix makes sense to me. Just to confirm, given that the unit test is currently failing, with this fix, do we revert to the RPC method if the call to the account API fails or not called ?

[bergeron]

I updated it to keep the RPC fallback if the account API request fails.

[bergeron]

This used to return failed if the api response was empty. But that's a valid response if it's an empty account with no tokens. Now it returns failed if the request is not http 200