fix: eth_accounts / accountsChanged behavior when wallet is locked (#405

) * fix: remove handleUnlockStateChanged * chore: update jest.config.js * lint * test: update jest.config.js * remove more instances of isUnlocked * update jest.config * test: add test case for isUnlocked in MetaMaskInpageProvider.test.ts --------- Co-authored-by: Alex <[email protected]>
MetaMask · Feb 3, 2025 · bda8d72 · bda8d72
1 parent d919ab6
commit bda8d72
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 63 deletions.
diff --git a/jest.config.js b/jest.config.js
@@ -45,10 +45,10 @@ const baseConfig = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 68.23,
-      functions: 68.69,
-      lines: 68.68,
-      statements: 68.71,
+      branches: 69.63,
+      functions: 71.42,
+      lines: 70.27,
+      statements: 70.4,
     },
   },
 

diff --git a/src/BaseProvider.ts b/src/BaseProvider.ts
@@ -52,7 +52,6 @@ export type RequestArguments = {
 export type BaseProviderState = {
   accounts: null | string[];
   isConnected: boolean;
-  isUnlocked: boolean;
   initialized: boolean;
   isPermanentlyDisconnected: boolean;
 };
@@ -77,7 +76,6 @@ export abstract class BaseProvider extends SafeEventEmitter {
   protected static _defaultState: BaseProviderState = {
     accounts: null,
     isConnected: false,
-    isUnlocked: false,
     initialized: false,
     isPermanentlyDisconnected: false,
   };
@@ -129,7 +127,6 @@ export abstract class BaseProvider extends SafeEventEmitter {
     this._handleConnect = this._handleConnect.bind(this);
     this._handleChainChanged = this._handleChainChanged.bind(this);
     this._handleDisconnect = this._handleDisconnect.bind(this);
-    this._handleUnlockStateChanged = this._handleUnlockStateChanged.bind(this);
     this._rpcRequest = this._rpcRequest.bind(this);
     this.request = this.request.bind(this);
 
@@ -236,7 +233,6 @@ export abstract class BaseProvider extends SafeEventEmitter {
    * @param initialState - The provider's initial state.
    * @param initialState.accounts - The user's accounts.
    * @param initialState.chainId - The chain ID.
-   * @param initialState.isUnlocked - Whether the user has unlocked MetaMask.
    * @param initialState.networkVersion - The network version.
    * @param initialState.isConnected - Whether the network is available.
    * @fires BaseProvider#_initialized - If `initialState` is defined.
@@ -245,7 +241,6 @@ export abstract class BaseProvider extends SafeEventEmitter {
   protected _initializeState(initialState?: {
     accounts: string[];
     chainId: string;
-    isUnlocked: boolean;
     networkVersion?: string;
     isConnected?: boolean;
   }) {
@@ -254,13 +249,11 @@ export abstract class BaseProvider extends SafeEventEmitter {
     }
 
     if (initialState) {
-      const { accounts, chainId, isUnlocked, networkVersion, isConnected } =
-        initialState;
+      const { accounts, chainId, networkVersion, isConnected } = initialState;
 
       // EIP-1193 connect
       this._handleConnect({ chainId, isConnected });
       this._handleChainChanged({ chainId, networkVersion, isConnected });
-      this._handleUnlockStateChanged({ accounts, isUnlocked });
       this._handleAccountsChanged(accounts);
     }
 
@@ -367,7 +360,6 @@ export abstract class BaseProvider extends SafeEventEmitter {
         this.#chainId = null;
         this._state.accounts = null;
         this.#selectedAddress = null;
-        this._state.isUnlocked = false;
         this._state.isPermanentlyDisconnected = true;
       }
 
@@ -472,33 +464,4 @@ export abstract class BaseProvider extends SafeEventEmitter {
       }
     }
   }
-
-  /**
-   * Upon receipt of a new isUnlocked state, sets relevant public state.
-   * Calls the accounts changed handler with the received accounts, or an empty
-   * array.
-   *
-   * Does nothing if the received value is equal to the existing value.
-   * There are no lock/unlock events.
-   *
-   * @param opts - Options bag.
-   * @param opts.accounts - The exposed accounts, if any.
-   * @param opts.isUnlocked - The latest isUnlocked value.
-   */
-  protected _handleUnlockStateChanged({
-    accounts,
-    isUnlocked,
-  }: { accounts?: string[]; isUnlocked?: boolean } = {}) {
-    if (typeof isUnlocked !== 'boolean') {
-      this._log.error(
-        'MetaMask: Received invalid isUnlocked parameter. Please report this bug.',
-      );
-      return;
-    }
-
-    if (isUnlocked !== this._state.isUnlocked) {
-      this._state.isUnlocked = isUnlocked;
-      this._handleAccountsChanged(accounts ?? []);
-    }
-  }
 }
diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts
@@ -50,7 +50,6 @@ async function getInitializedProvider({
   initialState: {
     accounts = [],
     chainId = '0x0',
-    isUnlocked = true,
     networkVersion = '0',
     isConnected = true,
   } = {},
@@ -81,7 +80,6 @@ async function getInitializedProvider({
           result: {
             accounts,
             chainId,
-            isUnlocked,
             networkVersion,
             isConnected,
           },
@@ -1114,7 +1112,6 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {
           return {
             accounts: ['0xabc'],
             chainId: '0x0',
-            isUnlocked: true,
             networkVersion: '0',
             isConnected: true,
           };
@@ -1221,4 +1218,25 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {
       expect(provider.selectedAddress).toBe('0xdeadbeef');
     });
   });
+
+  describe('_getExperimentalApi', () => {
+    let provider: any | MetaMaskInpageProvider;
+
+    beforeEach(async () => {
+      provider = (
+        await getInitializedProvider({
+          initialState: {
+            accounts: ['0xdeadbeef'],
+          },
+        })
+      ).provider;
+    });
+
+    describe('isUnlocked', () => {
+      it('should return negated value of `state.isPermanentlyDisconnected`', async () => {
+        const isUnlocked = await provider._getExperimentalApi().isUnlocked();
+        expect(isUnlocked).toBe(!provider._state.isPermanentlyDisconnected);
+      });
+    });
+  });
 });
diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts
@@ -409,14 +409,7 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
          *
          * @returns Promise resolving to true if MetaMask is currently unlocked.
          */
-        isUnlocked: async () => {
-          if (!this._state.initialized) {
-            await new Promise<void>((resolve) => {
-              this.on('_initialized', () => resolve());
-            });
-          }
-          return this._state.isUnlocked;
-        },
+        isUnlocked: async () => !this._state.isPermanentlyDisconnected,
 
         /**
          * Make a batch RPC request.

diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts
@@ -38,7 +38,6 @@ describe('StreamProvider', () => {
       const accounts = ['0xabc'];
       const chainId = '0x1';
       const networkVersion = '1';
-      const isUnlocked = true;
       const isConnected = true;
 
       const streamProvider = new StreamProvider(new MockConnectionStream());
@@ -49,7 +48,6 @@ describe('StreamProvider', () => {
           return {
             accounts,
             chainId,
-            isUnlocked,
             networkVersion,
             isConnected,
           };
@@ -71,7 +69,6 @@ describe('StreamProvider', () => {
       const accounts = ['0xabc'];
       const chainId = '0x1';
       const networkVersion = '1';
-      const isUnlocked = true;
       const isConnected = true;
 
       const streamProvider = new StreamProvider(new MockConnectionStream());
@@ -80,7 +77,6 @@ describe('StreamProvider', () => {
         return {
           accounts,
           chainId,
-          isUnlocked,
           networkVersion,
           isConnected,
         };
@@ -411,7 +407,6 @@ describe('StreamProvider', () => {
             return {
               accounts: [],
               chainId: '0x0',
-              isUnlocked: true,
               networkVersion: '0',
               isConnected: true,
             };
@@ -450,7 +445,6 @@ describe('StreamProvider', () => {
             return {
               accounts: [],
               chainId: '0x0',
-              isUnlocked: true,
               networkVersion: '0',
               isConnected: true,
             };

diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts
@@ -81,8 +81,6 @@ export abstract class AbstractStreamProvider extends BaseProvider {
       const { method, params } = payload;
       if (method === 'metamask_accountsChanged') {
         this._handleAccountsChanged(params);
-      } else if (method === 'metamask_unlockStateChanged') {
-        this._handleUnlockStateChanged(params);
       } else if (method === 'metamask_chainChanged') {
         this._handleChainChanged(params);
       } else if (EMITTED_NOTIFICATIONS.includes(method)) {

diff --git a/src/extension-provider/createExternalExtensionProvider.test.ts b/src/extension-provider/createExternalExtensionProvider.test.ts
@@ -43,7 +43,6 @@ async function getInitializedProvider({
   initialState: {
     accounts = [],
     chainId = '0x0',
-    isUnlocked = true,
     networkVersion = '0',
     isConnected = true,
   } = {},
@@ -72,7 +71,6 @@ async function getInitializedProvider({
           result: {
             accounts,
             chainId,
-            isUnlocked,
             networkVersion,
             isConnected,
           },