Auto-unsubscribe on disconnection (#50)

* Auto-unsubscribe on disconnection

* Use subscriptions for all in popup

* Cleanup state setters

* Plural for address(es)

packages/extension-dapp/src/index.ts

@@ -109,7 +109,7 @@ export async function web3Accounts (): Promise<Array<InjectedAccountWithMeta>> {
 
   const addresses = accounts.map(({ address }) => address);
 
-  console.log(`web3Accounts: Found ${accounts.length} addresses ${addresses.join(', ')}`);
+  console.log(`web3Accounts: Found ${accounts.length} address${accounts.length !== 1 ? 'es' : ''}: ${addresses.join(', ')}`);
 
   return accounts;
 }

packages/extension-ui/src/Popup/index.tsx

@@ -10,7 +10,7 @@ import { Route, Switch } from 'react-router';
 
 import { Loading } from '../components';
 import { AccountContext, ActionContext, AuthorizeContext, SigningContext } from '../components/contexts';
-import { getAccounts, getAuthRequests, getSignRequests } from '../messaging';
+import { subscribeAccounts, subscribeAuthorize, subscribeSigning } from '../messaging';
 import Accounts from './Accounts';
 import Authorize from './Authorize';
 import Create from './Create';
@@ -30,23 +30,17 @@ export default function Popup (props: Props) {
   const onAction = (to?: string): void => {
     setWelcomeDone(window.localStorage.getItem('welcome_read') === 'ok');
 
-    // loads all accounts & requests (this is passed through to children to trigger changes)
-    Promise
-      .all([getAccounts(), getAuthRequests(), getSignRequests()])
-      .then(([accounts, authRequests, signRequests]) => {
-        setAccounts(accounts);
-        setAuthRequests(authRequests);
-        setSignRequests(signRequests);
-      })
-      .catch(console.error);
-
     if (to) {
       window.location.hash = to;
     }
   };
 
   useEffect((): void => {
-    // with an empty state, load the accounts & requests
+    Promise.all([
+      subscribeAccounts(setAccounts),
+      subscribeAuthorize(setAuthRequests),
+      subscribeSigning(setSignRequests)
+    ]).catch(console.error);
     onAction();
   }, []);
 

packages/extension-ui/src/messaging.ts

@@ -9,11 +9,14 @@ import { KeypairType } from '@polkadot/util-crypto/types';
 import extension from 'extensionizer';
 import { PORT_POPUP } from '@polkadot/extension/defaults';
 
+type Handler = {
+  resolve: (data: any) => void,
+  reject: (error: Error) => void,
+  subscriber?: (data: any) => void
+};
+
 type Handlers = {
-  [index: string]: {
-    resolve: (data: any) => void,
-    reject: (error: Error) => void
-  }
+  [index: string]: Handler
 };
 
 const port = extension.runtime.connect({ name: PORT_POPUP });
@@ -29,20 +32,24 @@ port.onMessage.addListener((data) => {
     return;
   }
 
-  delete handlers[data.id];
+  if (!handler.subscriber) {
+    delete handlers[data.id];
+  }
 
-  if (data.error) {
+  if (data.subscription) {
+    (handler.subscriber as Function)(data.subscription);
+  } else if (data.error) {
     handler.reject(new Error(data.error));
   } else {
     handler.resolve(data.response);
   }
 });
 
-function sendMessage (message: MessageTypes, request: any = {}): Promise<any> {
+function sendMessage (message: MessageTypes, request: any = {}, subscriber?: (data: any) => void): Promise<any> {
   return new Promise((resolve, reject) => {
     const id = `${Date.now()}.${++idCounter}`;
 
-    handlers[id] = { resolve, reject };
+    handlers[id] = { resolve, reject, subscriber };
 
     port.postMessage({ id, message, request });
   });
@@ -92,6 +99,18 @@ export async function createSeed (length?: number, type?: KeypairType): Promise<
   return sendMessage('seed.create', { length, type });
 }
 
+export async function subscribeAccounts (cb: (accounts: Array<KeyringJson>) => void): Promise<boolean> {
+  return sendMessage('accounts.subscribe', {}, cb);
+}
+
+export async function subscribeAuthorize (cb: (accounts: Array<AuthorizeRequest>) => void): Promise<boolean> {
+  return sendMessage('authorize.subscribe', {}, cb);
+}
+
+export async function subscribeSigning (cb: (accounts: Array<SigningRequest>) => void): Promise<boolean> {
+  return sendMessage('signing.subscribe', {}, cb);
+}
+
 export async function validateSeed (seed: string, type?: KeypairType): Promise<{ address: string, seed: string }> {
   return sendMessage('seed.validate', { seed, type });
 }

packages/extension/src/background/handlers/Extension.ts

@@ -2,6 +2,7 @@
 // This software may be modified and distributed under the terms
 // of the Apache-2.0 license. See the LICENSE file for details.
 
+import { SubjectInfo } from '@polkadot/ui-keyring/observable/types';
 import { KeyringJson } from '@polkadot/ui-keyring/types';
 import { AuthorizeRequest, MessageTypes, MessageAccountCreate, MessageAccountEdit, MessageAuthorizeApprove, MessageAuthorizeReject, MessageExtrinsicSignApprove, MessageExtrinsicSignCancel, MessageSeedCreate, MessageSeedCreate$Response, MessageSeedValidate, MessageSeedValidate$Response, MessageAccountForget, SigningRequest } from '../types';
 
@@ -12,10 +13,15 @@ import { assert, u8aToHex } from '@polkadot/util';
 
 import RawPayload from '../RawPayload';
 import State from './State';
+import { createSubscription, unsubscribe } from './subscriptions';
 
 const SEED_DEFAULT_LENGTH = 12;
 const SEED_LENGTHS = [12, 24];
 
+function transformAccounts (accounts: SubjectInfo): Array<KeyringJson> {
+  return Object.values(accounts).map(({ json }) => json);
+}
+
 export default class Extension {
   private _state: State;
 
@@ -46,9 +52,22 @@ export default class Extension {
   }
 
   private accountsList (): Array<KeyringJson> {
-    return Object
-      .values(accountsObservable.subject.getValue())
-      .map(({ json }) => json);
+    return transformAccounts(accountsObservable.subject.getValue());
+  }
+
+  // FIXME This looks very much like what we have in Tabs
+  private accountsSubscribe (id: string, port: chrome.runtime.Port): boolean {
+    const cb = createSubscription(id, port);
+    const subscription = accountsObservable.subject.subscribe((accounts: SubjectInfo) =>
+      cb(transformAccounts(accounts))
+    );
+
+    port.onDisconnect.addListener(() => {
+      unsubscribe(id);
+      subscription.unsubscribe();
+    });
+
+    return true;
   }
 
   private authorizeApprove ({ id }: MessageAuthorizeApprove): boolean {
@@ -79,6 +98,21 @@ export default class Extension {
     return this._state.allAuthRequests;
   }
 
+  // FIXME This looks very much like what we have in accounts
+  private authorizeSubscribe (id: string, port: chrome.runtime.Port): boolean {
+    const cb = createSubscription(id, port);
+    const subscription = this._state.authSubject.subscribe((requests: Array<AuthorizeRequest>) =>
+      cb(requests)
+    );
+
+    port.onDisconnect.addListener(() => {
+      unsubscribe(id);
+      subscription.unsubscribe();
+    });
+
+    return true;
+  }
+
   private seedCreate ({ length = SEED_DEFAULT_LENGTH, type }: MessageSeedCreate): MessageSeedCreate$Response {
     const seed = mnemonicGenerate(length);
 
@@ -143,7 +177,22 @@ export default class Extension {
     return this._state.allSignRequests;
   }
 
-  async handle (type: MessageTypes, request: any): Promise<any> {
+  // FIXME This looks very much like what we have in authorization
+  private signingSubscribe (id: string, port: chrome.runtime.Port): boolean {
+    const cb = createSubscription(id, port);
+    const subscription = this._state.signSubject.subscribe((requests: Array<SigningRequest>) =>
+      cb(requests)
+    );
+
+    port.onDisconnect.addListener(() => {
+      unsubscribe(id);
+      subscription.unsubscribe();
+    });
+
+    return true;
+  }
+
+  async handle (id: string, type: MessageTypes, request: any, port: chrome.runtime.Port): Promise<any> {
     switch (type) {
       case 'authorize.approve':
         return this.authorizeApprove(request);
@@ -154,6 +203,9 @@ export default class Extension {
       case 'authorize.requests':
         return this.authorizeRequests();
 
+      case 'authorize.subscribe':
+        return this.authorizeSubscribe(id, port);
+
       case 'accounts.create':
         return this.accountsCreate(request);
 
@@ -166,6 +218,9 @@ export default class Extension {
       case 'accounts.list':
         return this.accountsList();
 
+      case 'accounts.subscribe':
+        return this.accountsSubscribe(id, port);
+
       case 'seed.create':
         return this.seedCreate(request);
 
@@ -181,6 +236,9 @@ export default class Extension {
       case 'signing.requests':
         return this.signingRequests();
 
+      case 'signing.subscribe':
+        return this.signingSubscribe(id, port);
+
       default:
         throw new Error(`Unable to handle message of type ${type}`);
     }

packages/extension/src/background/handlers/State.ts

@@ -5,6 +5,7 @@
 import { AuthorizeRequest, MessageAuthorize, MessageExtrinsicSign, MessageExtrinsicSign$Response, SigningRequest } from '../types';
 
 import extension from 'extensionizer';
+import { BehaviorSubject } from 'rxjs';
 import { assert } from '@polkadot/util';
 
 type AuthRequest = {
@@ -45,6 +46,8 @@ export default class State {
   private _authUrls: AuthUrls = {};
   private _authRequests: { [index: string]: AuthRequest } = {};
   private _signRequests: { [index: string]: SignRequest } = {};
+  readonly authSubject: BehaviorSubject<Array<AuthorizeRequest>> = new BehaviorSubject([] as Array<AuthorizeRequest>);
+  readonly signSubject: BehaviorSubject<Array<SigningRequest>> = new BehaviorSubject([] as Array<SigningRequest>);
 
   get hasAuthRequests (): boolean {
     return this.numAuthRequests === 0;
@@ -88,7 +91,7 @@ export default class State {
       };
 
       delete this._authRequests[id];
-      this.updateIcon();
+      this.updateIconAuth();
 
       fn(result);
     };
@@ -97,7 +100,7 @@ export default class State {
   private signComplete = (id: string, fn: Function) => {
     return (result: MessageExtrinsicSign$Response | Error): void => {
       delete this._signRequests[id];
-      this.updateIcon();
+      this.updateIconSign();
 
       fn(result);
     };
@@ -123,6 +126,16 @@ export default class State {
     extension.browserAction.setBadgeText({ text });
   }
 
+  private updateIconAuth (): void {
+    this.authSubject.next(this.allAuthRequests);
+    this.updateIcon();
+  }
+
+  private updateIconSign (): void {
+    this.signSubject.next(this.allSignRequests);
+    this.updateIcon();
+  }
+
   async authorizeUrl (url: string, request: MessageAuthorize): Promise<boolean> {
     const idStr = this.stripUrl(url);
 
@@ -144,7 +157,7 @@ export default class State {
         url
       };
 
-      this.updateIcon();
+      this.updateIconAuth();
     });
   }
 
@@ -177,7 +190,7 @@ export default class State {
         url
       };
 
-      this.updateIcon();
+      this.updateIconSign();
     });
   }
 }

packages/extension/src/background/handlers/Tabs.ts

@@ -2,15 +2,15 @@
 // This software may be modified and distributed under the terms
 // of the Apache-2.0 license. See the LICENSE file for details.
 
-import { Unsubcall } from '@polkadot/extension-dapp/types';
 import { SubjectInfo } from '@polkadot/ui-keyring/observable/types';
 import { MessageTypes, MessageAuthorize, MessageExtrinsicSign, MessageExtrinsicSign$Response } from '../types';
 
 import keyring from '@polkadot/ui-keyring';
 import accountsObservable from '@polkadot/ui-keyring/observable/accounts';
-import { assert, isFunction } from '@polkadot/util';
+import { assert } from '@polkadot/util';
 
 import State from './State';
+import { createSubscription, unsubscribe } from './subscriptions';
 
 type Accounts = Array<{ address: string, name?: string }>;
 
@@ -35,18 +35,19 @@ export default class Tabs {
     return transformAccounts(accountsObservable.subject.getValue());
   }
 
-  private accountsSubscribe (url: string, cb?: (accounts: Accounts) => void): Unsubcall {
-    if (!isFunction(cb)) {
-      throw new Error('Expected accountsSubscribe to be passed a subscriber');
-    }
-
+  // FIXME This looks very much like what we have in Extension
+  private accountsSubscribe (url: string, id: string, port: chrome.runtime.Port): boolean {
+    const cb = createSubscription(id, port);
     const subscription = accountsObservable.subject.subscribe((accounts: SubjectInfo) =>
       cb(transformAccounts(accounts))
     );
 
-    return (): void => {
+    port.onDisconnect.addListener(() => {
+      unsubscribe(id);
       subscription.unsubscribe();
-    };
+    });
+
+    return true;
   }
 
   private extrinsicSign (url: string, request: MessageExtrinsicSign): Promise<MessageExtrinsicSign$Response> {
@@ -58,7 +59,7 @@ export default class Tabs {
     return this._state.signQueue(url, request);
   }
 
-  async handle (type: MessageTypes, request: any, url: string, subscriber?: (data: any) => void): Promise<any> {
+  async handle (id: string, type: MessageTypes, request: any, url: string, port: chrome.runtime.Port): Promise<any> {
     switch (type) {
       case 'authorize.tab':
         return this.authorize(url, request);
@@ -67,7 +68,7 @@ export default class Tabs {
         return this.accountsList(url);
 
       case 'accounts.subscribe':
-        return this.accountsSubscribe(url, subscriber);
+        return this.accountsSubscribe(url, id, port);
 
       case 'extrinsic.sign':
         return this.extrinsicSign(url, request);

packages/extension/src/background/handlers/index.ts

@@ -15,13 +15,6 @@ const state = new State();
 const extension = new Extension(state);
 const tabs = new Tabs(state);
 
-function createSubscription (id: string, port: chrome.runtime.Port): (data: any) => void {
-  // FIXME We are not handling actual unsubscribes yet, this is an issue
-  return (subscription: any) => {
-    port.postMessage({ id, subscription });
-  };
-}
-
 export default function handler ({ id, message, request }: MessageRequest, port: chrome.runtime.Port): void {
   const isPopup = port.name === PORT_POPUP;
   const sender = port.sender as chrome.runtime.MessageSender;
@@ -32,27 +25,15 @@ export default function handler ({ id, message, request }: MessageRequest, port:
 
   console.log(` [in] ${source}`); // :: ${JSON.stringify(request)}`);
 
-  // This is not great - basically, based on the name (since there is only 1 atm),
-  // we create a subscription handler. Basically these handlers will continute stream
-  // results as they become available
-  const subscription = message.indexOf('.subscribe') !== -1
-    ? createSubscription(id, port)
-    : undefined;
   const promise = isPopup
-    ? extension.handle(message, request)
-    : tabs.handle(message, request, from || FALLBACK_URL, subscription);
+    ? extension.handle(id, message, request, port)
+    : tabs.handle(id, message, request, from || FALLBACK_URL, port);
 
   promise
     .then((response) => {
       console.log(`[out] ${source}`); // :: ${JSON.stringify(response)}`);
 
-      if (subscription) {
-        // TODO See unsub handling above, here we need to store the
-        // actual unsubscribe and handle appropriately
-        port.postMessage({ id, response: true });
-      } else {
-        port.postMessage({ id, response });
-      }
+      port.postMessage({ id, response });
     })
     .catch((error) => {
       console.log(`[err] ${source}:: ${error.message}`);

packages/extension/src/background/handlers/subscriptions.ts

@@ -0,0 +1,31 @@
+// Copyright 2019 @polkadot/extension authors & contributors
+// This software may be modified and distributed under the terms
+// of the Apache-2.0 license. See the LICENSE file for details.
+
+type Subscriptions = {
+  [index: string]: chrome.runtime.Port
+};
+
+const subscriptions: Subscriptions = {};
+
+// return a subscription callback, that will send the data to the caller via the port
+export function createSubscription (id: string, port: chrome.runtime.Port): (data: any) => void {
+  subscriptions[id] = port;
+
+  return (subscription: any) => {
+    if (subscriptions[id]) {
+      port.postMessage({ id, subscription });
+    }
+  };
+}
+
+// clear a previous subscriber
+export function unsubscribe (id: string): void {
+  if (subscriptions[id]) {
+    console.log(`Unsubscribing from ${id}`);
+
+    delete subscriptions[id];
+  } else {
+    console.error(`Unable to unsubscribe from ${id}`);
+  }
+}

packages/extension/src/background/types.ts

@@ -4,7 +4,7 @@
 
 import { KeypairType } from '@polkadot/util-crypto/types';
 
-export type MessageTypes = 'authorize.approve' | 'authorize.reject' | 'authorize.requests' | 'authorize.tab' | 'accounts.create' | 'accounts.edit' | 'accounts.forget' | 'accounts.list' | 'accounts.subscribe' | 'extrinsic.sign' | 'seed.create' | 'seed.validate' | 'signing.approve' | 'signing.cancel' | 'signing.requests';
+export type MessageTypes = 'authorize.approve' | 'authorize.reject' | 'authorize.requests' | 'authorize.subscribe' | 'authorize.tab' | 'accounts.create' | 'accounts.edit' | 'accounts.forget' | 'accounts.list' | 'accounts.subscribe' | 'extrinsic.sign' | 'seed.create' | 'seed.validate' | 'signing.approve' | 'signing.cancel' | 'signing.requests' | 'signing.subscribe';
 
 export type AuthorizeRequest = [string, MessageAuthorize, string];
 

packages/extension/src/page/index.ts

@@ -14,12 +14,14 @@ import Injected from './Injected';
 //  - this injector, listens on the events, maps it to the original
 //  - resolves/rejects the promise with the result (or sub data)
 
+type Handler = {
+  resolve: (data: any) => void,
+  reject: (error: Error) => void,
+  subscriber?: (data: any) => void
+};
+
 type Handlers = {
-  [index: string]: {
-    resolve: (data: any) => void,
-    reject: (error: Error) => void,
-    subscriber?: (data: any) => void
-  }
+  [index: string]: Handler
 };
 
 // small helper with the typescript types, just cast window
@@ -62,12 +64,11 @@ window.addEventListener('message', ({ data, source }) => {
 
   if (!handler.subscriber) {
     delete handlers[data.id];
-  } else if (data.subscription) {
-    handler.subscriber(data.subscription);
-    return;
   }
 
-  if (data.error) {
+  if (data.subscription) {
+    (handler.subscriber as Function)(data.subscription);
+  } else if (data.error) {
     handler.reject(new Error(data.error));
   } else {
     handler.resolve(data.response);