metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard
verified publisher icon MetaMask

fix: ensure inpage provider retries any pending requests sent before BackgroundBridge fully initialized

Open jiexi opened this issue 6 months ago • 10 comments

Description

Fixes a race condition in Android builds that made it highly likely for the inpage provider to end up in an "uninitialized" state when loading a BrowserTab for the first time after fresh app start. This was a result of the provider's metamask_getProviderState request used to initialize the provider state never resolving if the request was sent before BackgroundBridge had fully initialized its own EIP-1193 JSON-RPC engine pipeline. The provider would end up in a semi-functional state where subsequent requests could continue to be made, but chainChanged and accountsChanged events would not get emitted to the dapp despite the wallet sending and the inpage provider receiving both metamask_chainChanged and metamask_accountsChanged events.

This does not seem to affect iOS although the bug should also exist there.

The solution was to copy the retry triggering logic that already exists in Extension.

It is questionable, both in Extension and Mobile, whether this needs to be triggered by metamask_chainChanged or if the background can send METAMASK_EXTENSION_CONNECT_CAN_RETRY directly after JSON-RPC pipelines are initialized. I've decided to simply mimic Extension for sake of parity and consistency for future maintainers.

Related issues

Fixes: https://github.com/MetaMask/metamask-mobile/issues/15664

Manual testing steps

Unsure, but you may need many many accounts

  1. Restart the Android app (i.e. swipe it away and launch it again)
  2. Open the in app browser
  3. Go to any webpage
  4. In your desktop chrome, visit chrome://inspect/#devices
  5. Open the debugger for the webview you opened in the mobile app browser
  6. In console, enter window.ethereum._state
  7. initialized property should be true

Optionally, try the repro steps in the linked bug ticket which should no longer show the incorrect first account in the confirmation after you have switched the selected account from your first account to the second account.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

jiexi avatar Jun 12 '25 23:06 jiexi

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

github-actions[bot] avatar Jun 12 '25 23:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 11772062ebc60cb6e835ba0a78968a8106cbdc9f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/02f9e07a-f21b-4b2c-8546-d1fbe4923323

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 13 '25 20:06 github-actions[bot]

Hi Team,

Using the QA builds from here the fix works for both iOS and Android:

https://github.com/user-attachments/assets/16fdfa2c-3e18-4f74-b5bf-08e0e6866d19

https://github.com/user-attachments/assets/c6e9ab34-d7fc-4fae-a25e-ec032490e372

Issues not related to the bug, but observed while testing this PR:

  • Base network shows as Private Network in the network picker if Current network is selected
  • Swap estimation error in the confirmation request
  • Unable to add space when trying to import SRP and typing it (for both iOS and Android)

https://github.com/user-attachments/assets/3f4dc0d4-2c83-4a23-a06b-60e499a8c8f6

Unik0rnMaggie avatar Jun 16 '25 13:06 Unik0rnMaggie

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: bed6d7be7b67bc7689083740ac0a87eb9cb5f0d7 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d4de4371-38b2-402a-99f7-104ebedc01da

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 17 '25 15:06 github-actions[bot]

Do you think this can be a bug with remote connections as well? (WC and SDK).

Based on the code i see for SDK, it seems this bug may also exist with SDK, but i'm not entirely sure. In the case that SDK is being used by a dapp in the in-app browser, the fix in this PR would also cover the SDK (after we loosen the remoteConn guard).

jiexi avatar Jun 17 '25 16:06 jiexi

I can't replicate the bug in QA and Release build:

release Android build https://app.bitrise.io/app/be69d4368ee7e86d/installable-artifacts/2ae5c97335fd122e

https://github.com/user-attachments/assets/3d8aa25c-b002-4c35-9066-c4f20243c9a7

https://github.com/user-attachments/assets/07c71621-63bb-4d4c-852c-829eee5d8f40

sleepytanya avatar Jun 17 '25 16:06 sleepytanya

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e9c93e9087ec5e861218d6f547c57b015acf1acd Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/64317009-217f-4862-a4c6-2184a0e68fc6

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 17 '25 19:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: dd80f3bbcfc43e0410939d7bdf649e923aa1488b Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e9793fa1-dba8-4798-a61a-570b1f47c022

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 17 '25 21:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ed27bb8998f83a51cd5ca51964d9d4399fa7d90f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/fe675ba4-cfd8-411e-8fe4-4a6663945f25

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 17 '25 23:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 613ef72540b020d4a75dbfd94af40bb2a5ae163d Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b7446a8f-92b3-4a2b-8bcd-ed3c241b3949

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 18 '25 15:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4229b131bd8d687e1a6f8f29d432a2d0e7f47853 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a906713f-f492-4c9d-a12d-a4b9bf2c18f7

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 18 '25 18:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 6b24706cd53d344035a549bef36377e2094883fb Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8438e314-0ccd-472a-9efa-e701e0a71ed2

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 18 '25 19:06 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 6230e2fecf85c3240dbf1d300c899b87036ffc8f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f059d135-008d-41af-bf14-6c5aa5192452

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Jun 19 '25 15:06 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c3572ff9dba0db319e1d48cebc225c722ad8b612 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/52d8e96f-32e2-4971-b828-6aae93300255

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Jun 19 '25 16:06 github-actions[bot]

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 70b2c1bbbc186182e83a92fca5f75c2fb2849476 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/76376b3a-8ee0-46de-a146-9a320c7d8d7f

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Jun 19 '25 16:06 github-actions[bot]

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
50.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sonarqubecloud[bot] avatar Jun 19 '25 17:06 sonarqubecloud[bot]