From 35676805aa8a158afb2859441b9b504f2fbaa151 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Fri, 19 Feb 2021 14:24:57 -0330
Subject: [PATCH 1/8] Add BaseControllerV2 schema

A schema has been added to the new BaseController class as a required
constructor parameter. The schema describes which pieces of state
should be persisted, and how to get an 'anonymized' snapshot of the
controller state.

This is part of the controller redesign (#337).
---
 src/BaseControllerV2.test.ts | 139 ++++++++++++++++++++++++++++++++---
 src/BaseControllerV2.ts      |  44 ++++++++++-
 2 files changed, 170 insertions(+), 13 deletions(-)

diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts
index 5dd8f66aa23..8da80dba06c 100644
--- a/src/BaseControllerV2.test.ts
+++ b/src/BaseControllerV2.test.ts
@@ -1,12 +1,19 @@
 import type { Draft } from 'immer';
 import * as sinon from 'sinon';
 
-import { BaseController } from './BaseControllerV2';
+import { BaseController, getAnonymizedState, getPersistentState } from './BaseControllerV2';
 
 type MockControllerState = {
   count: number;
 };
 
+const mockControllerSchema = {
+  count: {
+    persist: true,
+    anonymous: true,
+  },
+};
+
 class MockController extends BaseController<MockControllerState> {
   update(callback: (state: Draft<MockControllerState>) => void | MockControllerState) {
     super.update(callback);
@@ -19,13 +26,19 @@ class MockController extends BaseController<MockControllerState> {
 
 describe('BaseController', () => {
   it('should set initial state', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
 
     expect(controller.state).toEqual({ count: 0 });
   });
 
+  it('should set initial schema', () => {
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
+
+    expect(controller.schema).toEqual(mockControllerSchema);
+  });
+
   it('should not allow mutating state directly', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
 
     expect(() => {
       controller.state = { count: 1 };
@@ -33,7 +46,7 @@ describe('BaseController', () => {
   });
 
   it('should allow updating state by modifying draft', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
 
     controller.update((draft) => {
       draft.count += 1;
@@ -43,7 +56,7 @@ describe('BaseController', () => {
   });
 
   it('should allow updating state by return a value', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
 
     controller.update(() => {
       return { count: 1 };
@@ -53,7 +66,7 @@ describe('BaseController', () => {
   });
 
   it('should throw an error if update callback modifies draft and returns value', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
 
     expect(() => {
       controller.update((draft) => {
@@ -64,7 +77,7 @@ describe('BaseController', () => {
   });
 
   it('should inform subscribers of state changes', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
     const listener2 = sinon.stub();
 
@@ -81,7 +94,7 @@ describe('BaseController', () => {
   });
 
   it('should inform a subscriber of each state change once even after multiple subscriptions', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -95,7 +108,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer inform a subscriber about state changes after unsubscribing', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -108,7 +121,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer inform a subscriber about state changes after unsubscribing once, even if they subscribed many times', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -122,7 +135,7 @@ describe('BaseController', () => {
   });
 
   it('should allow unsubscribing listeners who were never subscribed', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
 
     expect(() => {
@@ -131,7 +144,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer update subscribers after being destroyed', () => {
-    const controller = new MockController({ count: 0 });
+    const controller = new MockController({ count: 0 }, mockControllerSchema);
     const listener1 = sinon.stub();
     const listener2 = sinon.stub();
 
@@ -146,3 +159,105 @@ describe('BaseController', () => {
     expect(listener2.callCount).toEqual(0);
   });
 });
+
+describe('getAnonymizedState', () => {
+  it('should return empty state', () => {
+    expect(getAnonymizedState({}, {})).toEqual({});
+  });
+
+  it('should return empty state when no properties are anonymized', () => {
+    const anonymizedState = getAnonymizedState({ count: 1 }, { count: { anonymous: false, persist: false } });
+    expect(anonymizedState).toEqual({});
+  });
+
+  it('should return state that is already anonymized', () => {
+    const anonymizedState = getAnonymizedState(
+      {
+        password: 'secret password',
+        privateKey: '123',
+        network: 'mainnet',
+        tokens: ['DAI', 'USDC'],
+      },
+      {
+        password: {
+          anonymous: false,
+          persist: false,
+        },
+        privateKey: {
+          anonymous: false,
+          persist: false,
+        },
+        network: {
+          anonymous: true,
+          persist: false,
+        },
+        tokens: {
+          anonymous: true,
+          persist: false,
+        },
+      },
+    );
+    expect(anonymizedState).toEqual({ network: 'mainnet', tokens: ['DAI', 'USDC'] });
+  });
+
+  it('should use anonymizing function to anonymize state', () => {
+    const anonymizeTransactionHash = (hash: string) => {
+      return hash.split('').reverse().join('');
+    };
+
+    const anonymizedState = getAnonymizedState(
+      {
+        transactionHash: '0x1234',
+      },
+      {
+        transactionHash: {
+          anonymous: anonymizeTransactionHash,
+          persist: false,
+        },
+      },
+    );
+
+    expect(anonymizedState).toEqual({ transactionHash: '4321x0' });
+  });
+});
+
+describe('getPersistentState', () => {
+  it('should return empty state', () => {
+    expect(getPersistentState({}, {})).toEqual({});
+  });
+
+  it('should return empty state when no properties are persistent', () => {
+    const persistentState = getPersistentState({ count: 1 }, { count: { anonymous: false, persist: false } });
+    expect(persistentState).toEqual({});
+  });
+
+  it('should return persistent state', () => {
+    const persistentState = getPersistentState(
+      {
+        password: 'secret password',
+        privateKey: '123',
+        network: 'mainnet',
+        tokens: ['DAI', 'USDC'],
+      },
+      {
+        password: {
+          anonymous: false,
+          persist: true,
+        },
+        privateKey: {
+          anonymous: false,
+          persist: true,
+        },
+        network: {
+          anonymous: false,
+          persist: false,
+        },
+        tokens: {
+          anonymous: false,
+          persist: false,
+        },
+      },
+    );
+    expect(persistentState).toEqual({ password: 'secret password', privateKey: '123' });
+  });
+});
diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index 5c57ef227fc..7372be9d0cd 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -11,6 +11,15 @@ enablePatches();
  */
 export type Listener<T> = (state: T, patches: Patch[]) => void;
 
+export type Anonymizer<T> = (value: T) => T;
+
+export type Schema<T> = {
+  [P in keyof T]: {
+    persist: boolean;
+    anonymous: boolean | Anonymizer<T[P]>;
+  };
+};
+
 /**
  * Controller class that provides state management and subscriptions
  */
@@ -19,13 +28,18 @@ export class BaseController<S extends Record<string, unknown>> {
 
   private internalListeners: Set<Listener<S>> = new Set();
 
+  public readonly schema: Schema<S>;
+
   /**
    * Creates a BaseController instance.
    *
    * @param state - Initial controller state
+   * @param schema - State schema, describing how to "anonymize" the state,
+   *   and which parts should be persisted.
    */
-  constructor(state: S) {
+  constructor(state: S, schema: Schema<S>) {
     this.internalState = state;
+    this.schema = schema;
   }
 
   /**
@@ -89,3 +103,31 @@ export class BaseController<S extends Record<string, unknown>> {
     this.internalListeners.clear();
   }
 }
+
+// This function acts as a type guard. Using a `typeof` conditional didn't seem to work.
+function isAnonymizingFunction<T>(x: boolean | Anonymizer<T>): x is Anonymizer<T> {
+  return typeof x === 'function';
+}
+
+export function getAnonymizedState<S extends Record<string, any>>(state: S, schema: Schema<S>) {
+  return Object.keys(state).reduce((anonymizedState, _key) => {
+    const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
+    const schemaValue = schema[key].anonymous;
+    if (isAnonymizingFunction(schemaValue)) {
+      anonymizedState[key] = schemaValue(state[key]);
+    } else if (schemaValue) {
+      anonymizedState[key] = state[key];
+    }
+    return anonymizedState;
+  }, {} as Partial<S>);
+}
+
+export function getPersistentState<S extends Record<string, any>>(state: S, schema: Schema<S>) {
+  return Object.keys(state).reduce((persistedState, _key) => {
+    const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
+    if (schema[key].persist) {
+      persistedState[key] = state[key];
+    }
+    return persistedState;
+  }, {} as Partial<S>);
+}

From 64c910f67c5c9440750cce53710ebd0aa2a2425f Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Wed, 24 Feb 2021 09:32:41 -0330
Subject: [PATCH 2/8] Rename `scheme` to `metadata`

Really this is state metadata, not a schema. The word "schema" never
described this well.
---
 src/BaseControllerV2.test.ts | 28 ++++++++++++++--------------
 src/BaseControllerV2.ts      | 24 ++++++++++++------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts
index 8da80dba06c..f9b42787073 100644
--- a/src/BaseControllerV2.test.ts
+++ b/src/BaseControllerV2.test.ts
@@ -7,7 +7,7 @@ type MockControllerState = {
   count: number;
 };
 
-const mockControllerSchema = {
+const mockControllerStateMetadata = {
   count: {
     persist: true,
     anonymous: true,
@@ -26,19 +26,19 @@ class MockController extends BaseController<MockControllerState> {
 
 describe('BaseController', () => {
   it('should set initial state', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
     expect(controller.state).toEqual({ count: 0 });
   });
 
   it('should set initial schema', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
-    expect(controller.schema).toEqual(mockControllerSchema);
+    expect(controller.metadata).toEqual(mockControllerStateMetadata);
   });
 
   it('should not allow mutating state directly', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
     expect(() => {
       controller.state = { count: 1 };
@@ -46,7 +46,7 @@ describe('BaseController', () => {
   });
 
   it('should allow updating state by modifying draft', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
     controller.update((draft) => {
       draft.count += 1;
@@ -56,7 +56,7 @@ describe('BaseController', () => {
   });
 
   it('should allow updating state by return a value', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
     controller.update(() => {
       return { count: 1 };
@@ -66,7 +66,7 @@ describe('BaseController', () => {
   });
 
   it('should throw an error if update callback modifies draft and returns value', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
 
     expect(() => {
       controller.update((draft) => {
@@ -77,7 +77,7 @@ describe('BaseController', () => {
   });
 
   it('should inform subscribers of state changes', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
     const listener2 = sinon.stub();
 
@@ -94,7 +94,7 @@ describe('BaseController', () => {
   });
 
   it('should inform a subscriber of each state change once even after multiple subscriptions', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -108,7 +108,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer inform a subscriber about state changes after unsubscribing', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -121,7 +121,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer inform a subscriber about state changes after unsubscribing once, even if they subscribed many times', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
 
     controller.subscribe(listener1);
@@ -135,7 +135,7 @@ describe('BaseController', () => {
   });
 
   it('should allow unsubscribing listeners who were never subscribed', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
 
     expect(() => {
@@ -144,7 +144,7 @@ describe('BaseController', () => {
   });
 
   it('should no longer update subscribers after being destroyed', () => {
-    const controller = new MockController({ count: 0 }, mockControllerSchema);
+    const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
     const listener1 = sinon.stub();
     const listener2 = sinon.stub();
 
diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index 7372be9d0cd..85d27878d2a 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -13,7 +13,7 @@ export type Listener<T> = (state: T, patches: Patch[]) => void;
 
 export type Anonymizer<T> = (value: T) => T;
 
-export type Schema<T> = {
+export type StateMetadata<T> = {
   [P in keyof T]: {
     persist: boolean;
     anonymous: boolean | Anonymizer<T[P]>;
@@ -28,18 +28,18 @@ export class BaseController<S extends Record<string, unknown>> {
 
   private internalListeners: Set<Listener<S>> = new Set();
 
-  public readonly schema: Schema<S>;
+  public readonly metadata: StateMetadata<S>;
 
   /**
    * Creates a BaseController instance.
    *
    * @param state - Initial controller state
-   * @param schema - State schema, describing how to "anonymize" the state,
+   * @param metadata - State metadata, describing how to "anonymize" the state,
    *   and which parts should be persisted.
    */
-  constructor(state: S, schema: Schema<S>) {
+  constructor(state: S, metadata: StateMetadata<S>) {
     this.internalState = state;
-    this.schema = schema;
+    this.metadata = metadata;
   }
 
   /**
@@ -109,23 +109,23 @@ function isAnonymizingFunction<T>(x: boolean | Anonymizer<T>): x is Anonymizer<T
   return typeof x === 'function';
 }
 
-export function getAnonymizedState<S extends Record<string, any>>(state: S, schema: Schema<S>) {
+export function getAnonymizedState<S extends Record<string, any>>(state: S, metadata: StateMetadata<S>) {
   return Object.keys(state).reduce((anonymizedState, _key) => {
     const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
-    const schemaValue = schema[key].anonymous;
-    if (isAnonymizingFunction(schemaValue)) {
-      anonymizedState[key] = schemaValue(state[key]);
-    } else if (schemaValue) {
+    const metadataValue = metadata[key].anonymous;
+    if (isAnonymizingFunction(metadataValue)) {
+      anonymizedState[key] = metadataValue(state[key]);
+    } else if (metadataValue) {
       anonymizedState[key] = state[key];
     }
     return anonymizedState;
   }, {} as Partial<S>);
 }
 
-export function getPersistentState<S extends Record<string, any>>(state: S, schema: Schema<S>) {
+export function getPersistentState<S extends Record<string, any>>(state: S, metadata: StateMetadata<S>) {
   return Object.keys(state).reduce((persistedState, _key) => {
     const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
-    if (schema[key].persist) {
+    if (metadata[key].persist) {
       persistedState[key] = state[key];
     }
     return persistedState;

From 4cfeb3478c337753b90e3deb93619f539421b07d Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Wed, 24 Feb 2021 09:50:23 -0330
Subject: [PATCH 3/8] Improve anonymizer return type

The anonymizer function can now return a _recursive partial_ of the
input type. So any properties can be omitted in the state property
directly or in any nested object.

Note that for primitive values and arrays, the return type must still
match the state. Similarly, any objects still need to be returned as
objects, though they're allowed to have a few less properties.
---
 src/BaseControllerV2.ts | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index 85d27878d2a..40ee5e2a006 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -11,7 +11,18 @@ enablePatches();
  */
 export type Listener<T> = (state: T, patches: Patch[]) => void;
 
-export type Anonymizer<T> = (value: T) => T;
+type Primitive = boolean | string | number | null;
+
+// Based upon this StackOverflow answer: https://stackoverflow.com/a/64060332
+type RecursivePartial<T> = {
+  [P in keyof T]?: T[P] extends (infer U)[]
+    ? RecursivePartial<U>[]
+    : T[P] extends Primitive
+    ? T[P]
+    : RecursivePartial<T>;
+};
+
+export type Anonymizer<T> = (value: T) => T extends Primitive ? T : RecursivePartial<T>;
 
 export type StateMetadata<T> = {
   [P in keyof T]: {
@@ -109,7 +120,10 @@ function isAnonymizingFunction<T>(x: boolean | Anonymizer<T>): x is Anonymizer<T
   return typeof x === 'function';
 }
 
-export function getAnonymizedState<S extends Record<string, any>>(state: S, metadata: StateMetadata<S>) {
+export function getAnonymizedState<S extends Record<string, any>>(
+  state: S,
+  metadata: StateMetadata<S>,
+): RecursivePartial<S> {
   return Object.keys(state).reduce((anonymizedState, _key) => {
     const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
     const metadataValue = metadata[key].anonymous;
@@ -119,15 +133,18 @@ export function getAnonymizedState<S extends Record<string, any>>(state: S, meta
       anonymizedState[key] = state[key];
     }
     return anonymizedState;
-  }, {} as Partial<S>);
+  }, {} as RecursivePartial<S>);
 }
 
-export function getPersistentState<S extends Record<string, any>>(state: S, metadata: StateMetadata<S>) {
+export function getPersistentState<S extends Record<string, any>>(
+  state: S,
+  metadata: StateMetadata<S>,
+): RecursivePartial<S> {
   return Object.keys(state).reduce((persistedState, _key) => {
     const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
     if (metadata[key].persist) {
       persistedState[key] = state[key];
     }
     return persistedState;
-  }, {} as Partial<S>);
+  }, {} as RecursivePartial<S>);
 }

From e2b22c2cc8b6c45467b81d1994a883c8712e92e2 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Wed, 24 Feb 2021 09:56:07 -0330
Subject: [PATCH 4/8] Add doc comments for all public functions and types

Note that the metadata type was split up to make it easier to use the
`@property` JSDoc directive.
---
 src/BaseControllerV2.ts | 67 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 6 deletions(-)

diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index 40ee5e2a006..b647908ff16 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -7,7 +7,15 @@ import type { Draft, Patch } from 'immer';
 enablePatches();
 
 /**
- * State change callbacks
+ * A state change listener.
+ *
+ * This function will get called for each state change, and is given a copy of
+ * the new state along with a set of patches describing the changes since the
+ * last update.
+ *
+ * @param state - The new controller state
+ * @param patches - A list of patches describing any changes (see here for more
+ *   information: https://immerjs.github.io/immer/docs/patches)
  */
 export type Listener<T> = (state: T, patches: Patch[]) => void;
 
@@ -22,17 +30,46 @@ type RecursivePartial<T> = {
     : RecursivePartial<T>;
 };
 
+/**
+ * An anonymizing function
+ *
+ * This function will accept one piece of the controller state (one property),
+ * and will return an anonymized representation of this state. By "anonymized",
+ * we mean that it should not contain any information that could be personally
+ * identifiable.
+ *
+ * @param value - A piece of controller state
+ * @returns An anonymized representation of the given state
+ */
 export type Anonymizer<T> = (value: T) => T extends Primitive ? T : RecursivePartial<T>;
 
+/**
+ * State metadata.
+ *
+ * This metadata describes which parts of state should be persisted, and how to
+ * get an anonymized representation of the state.
+ */
 export type StateMetadata<T> = {
-  [P in keyof T]: {
-    persist: boolean;
-    anonymous: boolean | Anonymizer<T[P]>;
-  };
+  [P in keyof T]: StatePropertyMetadata<T[P]>;
 };
 
 /**
- * Controller class that provides state management and subscriptions
+ * Metadata for a single state property
+ *
+ * @property persist - Indicates whether this property should be persisted
+ *   (`true` for persistent, `false` for transient)
+ * @property anonymous - Indicates whether this property is already anonymous,
+ *   (`true` for anonymous, `false` if it has potential to be personally
+ *   identifiable), or is set to a function that returns an anonymized
+ *   representation of this state.
+ */
+export interface StatePropertyMetadata<P> {
+  persist: boolean;
+  anonymous: boolean | Anonymizer<P>;
+}
+
+/**
+ * Controller class that provides state management, subscriptions, and state metadata
  */
 export class BaseController<S extends Record<string, unknown>> {
   private internalState: S;
@@ -120,6 +157,17 @@ function isAnonymizingFunction<T>(x: boolean | Anonymizer<T>): x is Anonymizer<T
   return typeof x === 'function';
 }
 
+/**
+ * Returns an anonymized representation of the controller state.
+ *
+ * By "anonymized" we mean that it should not contain any information that could be personally
+ * identifiable.
+ *
+ * @param state - The controller state
+ * @param metadata - The controller state metadata, which describes how to derive the
+ *   anonymized state
+ * @returns The anonymized controller state
+ */
 export function getAnonymizedState<S extends Record<string, any>>(
   state: S,
   metadata: StateMetadata<S>,
@@ -136,6 +184,13 @@ export function getAnonymizedState<S extends Record<string, any>>(
   }, {} as RecursivePartial<S>);
 }
 
+/**
+ * Returns the subset of state that should be persisted
+ *
+ * @param state - The controller state
+ * @param metadata - The controller state metadata, which describes which pieces of state should be persisted
+ * @returns The subset of controller state that should be persisted
+ */
 export function getPersistentState<S extends Record<string, any>>(
   state: S,
   metadata: StateMetadata<S>,

From 3e82037329eb57fdc35367fe6833d2db2002db0c Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Wed, 24 Feb 2021 10:19:54 -0330
Subject: [PATCH 5/8] Add tests for recursive partial anonymizing function

Tests have been added to ensure that the anonymizing function works as
expected when the return type omits properties from the top-level
property state or from a nested object.
---
 src/BaseControllerV2.test.ts | 61 ++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts
index f9b42787073..a4f9cdd2e3e 100644
--- a/src/BaseControllerV2.test.ts
+++ b/src/BaseControllerV2.test.ts
@@ -219,6 +219,67 @@ describe('getAnonymizedState', () => {
 
     expect(anonymizedState).toEqual({ transactionHash: '4321x0' });
   });
+
+  it('should allow returning a partial object from an anonymizing function', () => {
+    const anonymizeTransactionHash = (txMeta: { hash: string; value: number }) => {
+      return { value: txMeta.value };
+    };
+
+    const anonymizedState = getAnonymizedState(
+      {
+        txMeta: {
+          hash: '0x123',
+          value: 10,
+        },
+      },
+      {
+        txMeta: {
+          anonymous: anonymizeTransactionHash,
+          persist: false,
+        },
+      },
+    );
+
+    expect(anonymizedState).toEqual({ txMeta: { value: 10 } });
+  });
+
+  it('should allow returning a ntested partial object from an anonymizing function', () => {
+    const anonymizeTransactionHash = (txMeta: {
+      hash: string;
+      value: number;
+      history: { hash: string; value: number }[];
+    }) => {
+      return {
+        history: txMeta.history.map((entry) => {
+          return { value: entry.value };
+        }),
+        value: txMeta.value,
+      };
+    };
+
+    const anonymizedState = getAnonymizedState(
+      {
+        txMeta: {
+          hash: '0x123',
+          history: [
+            {
+              hash: '0x123',
+              value: 9,
+            },
+          ],
+          value: 10,
+        },
+      },
+      {
+        txMeta: {
+          anonymous: anonymizeTransactionHash,
+          persist: false,
+        },
+      },
+    );
+
+    expect(anonymizedState).toEqual({ txMeta: { history: [{ value: 9 }], value: 10 } });
+  });
 });
 
 describe('getPersistentState', () => {

From a0fa83b8980948c0ba7e397395def054fdd774be Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Thu, 25 Feb 2021 01:42:55 -0330
Subject: [PATCH 6/8] Use `Partial` instead of `RecursivePartial` for persisted
 state

The current state metadata takes an all-or-nothing approach to
persisting each state property. There is no mechanism for omitting
nested properties from the persisted state. As such, `Partial` is a
better description of the return type than `RecursivePartial`.
---
 src/BaseControllerV2.ts | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index b647908ff16..bb0d72e93d3 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -191,15 +191,12 @@ export function getAnonymizedState<S extends Record<string, any>>(
  * @param metadata - The controller state metadata, which describes which pieces of state should be persisted
  * @returns The subset of controller state that should be persisted
  */
-export function getPersistentState<S extends Record<string, any>>(
-  state: S,
-  metadata: StateMetadata<S>,
-): RecursivePartial<S> {
+export function getPersistentState<S extends Record<string, any>>(state: S, metadata: StateMetadata<S>): Partial<S> {
   return Object.keys(state).reduce((persistedState, _key) => {
     const key: keyof S = _key; // https://stackoverflow.com/questions/63893394/string-cannot-be-used-to-index-type-t
     if (metadata[key].persist) {
       persistedState[key] = state[key];
     }
     return persistedState;
-  }, {} as RecursivePartial<S>);
+  }, {} as Partial<S>);
 }

From d113433bb5f0f81f71a562caafc1dc89acb272f2 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Thu, 25 Feb 2021 01:50:22 -0330
Subject: [PATCH 7/8] Fix mistake in RecursivePartial type

---
 src/BaseControllerV2.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index bb0d72e93d3..c8e4e2b15ce 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -27,7 +27,7 @@ type RecursivePartial<T> = {
     ? RecursivePartial<U>[]
     : T[P] extends Primitive
     ? T[P]
-    : RecursivePartial<T>;
+    : RecursivePartial<T[P]>;
 };
 
 /**

From ef02cccf1c756d3fa99c01a275dc016cff6671b4 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Thu, 25 Feb 2021 10:38:15 -0330
Subject: [PATCH 8/8] Fix typo and use T for generic for consistency

---
 src/BaseControllerV2.test.ts | 2 +-
 src/BaseControllerV2.ts      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts
index a4f9cdd2e3e..697a5624a94 100644
--- a/src/BaseControllerV2.test.ts
+++ b/src/BaseControllerV2.test.ts
@@ -243,7 +243,7 @@ describe('getAnonymizedState', () => {
     expect(anonymizedState).toEqual({ txMeta: { value: 10 } });
   });
 
-  it('should allow returning a ntested partial object from an anonymizing function', () => {
+  it('should allow returning a nested partial object from an anonymizing function', () => {
     const anonymizeTransactionHash = (txMeta: {
       hash: string;
       value: number;
diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts
index c8e4e2b15ce..6cc5206bba3 100644
--- a/src/BaseControllerV2.ts
+++ b/src/BaseControllerV2.ts
@@ -63,9 +63,9 @@ export type StateMetadata<T> = {
  *   identifiable), or is set to a function that returns an anonymized
  *   representation of this state.
  */
-export interface StatePropertyMetadata<P> {
+export interface StatePropertyMetadata<T> {
   persist: boolean;
-  anonymous: boolean | Anonymizer<P>;
+  anonymous: boolean | Anonymizer<T>;
 }
 
 /**