From d680065a8442a5bbade81a1063eca3938038ed85 Mon Sep 17 00:00:00 2001
From: Jenny Pollack <jennypollack3@gmail.com>
Date: Thu, 5 Sep 2019 17:50:49 +0200
Subject: [PATCH] update addressBook with entries by chainId, add isEns flag
 (#152)

update addressBook with entries by chainId
add isValidEns checking for address book name
---
 src/user/AddressBookController.ts   |  39 ++++--
 src/util.ts                         |   6 +
 tests/AddressBookController.test.ts | 177 +++++++++++++++++++++-------
 tests/ComposableController.test.ts  |  37 +++---
 tests/util.test.ts                  |  10 ++
 5 files changed, 201 insertions(+), 68 deletions(-)

diff --git a/src/user/AddressBookController.ts b/src/user/AddressBookController.ts
index 64725b9605..3956dc396b 100644
--- a/src/user/AddressBookController.ts
+++ b/src/user/AddressBookController.ts
@@ -1,4 +1,5 @@
 import { isValidAddress, toChecksumAddress } from 'ethereumjs-util';
+import { isValidEnsName } from '../util';
 import BaseController, { BaseConfig, BaseState } from '../BaseController';
 
 /**
@@ -23,12 +24,14 @@ export interface ContactEntry {
  * @property name - Nickname associated with this address
  * @property chainId - Chain id identifies the current chain
  * @property memo - User's note about address
+ * @property isEns - is the entry an ENS name
  */
 export interface AddressBookEntry {
 	address: string;
 	name: string;
-	chainId: number;
+	chainId: string;
 	memo: string;
+	isEns: boolean;
 }
 
 /**
@@ -39,7 +42,7 @@ export interface AddressBookEntry {
  * @property addressBook - Array of contact entry objects
  */
 export interface AddressBookState extends BaseState {
-	addressBook: { [address: string]: AddressBookEntry };
+	addressBook: { [chainId: string]: { [address: string]: AddressBookEntry } };
 }
 
 /**
@@ -77,14 +80,19 @@ export class AddressBookController extends BaseController<BaseConfig, AddressBoo
 	 *
 	 * @param address - Recipient address to delete
 	 */
-	delete(address: string) {
-		const normalizedAddress = toChecksumAddress(address);
-		if (!isValidAddress(normalizedAddress) || this.state.addressBook[normalizedAddress] === undefined) {
+	delete(chainId: string, address: string) {
+		address = toChecksumAddress(address);
+		if (!isValidAddress(address) || !this.state.addressBook[chainId] || !this.state.addressBook[chainId][address]) {
 			return false;
 		}
 
-		const addressBook: { [address: string]: AddressBookEntry } = Object.assign({}, this.state.addressBook);
-		delete addressBook[normalizedAddress];
+		const addressBook = Object.assign({}, this.state.addressBook);
+		delete addressBook[chainId][address];
+
+		if (Object.keys(addressBook[chainId]).length === 0) {
+			delete addressBook[chainId];
+		}
+
 		this.update({ addressBook });
 		return true;
 	}
@@ -98,7 +106,8 @@ export class AddressBookController extends BaseController<BaseConfig, AddressBoo
 	 * @param memo - User's note about address
 	 * @returns - Boolean indicating if the address was successfully set
 	 */
-	set(address: string, name: string, chainId = 1, memo = '') {
+	set(address: string, name: string, chainId = '1', memo = '') {
+		address = toChecksumAddress(address);
 		if (!isValidAddress(address)) {
 			return false;
 		}
@@ -106,11 +115,15 @@ export class AddressBookController extends BaseController<BaseConfig, AddressBoo
 		this.update({
 			addressBook: {
 				...this.state.addressBook,
-				[toChecksumAddress(address)]: {
-					address,
-					chainId,
-					memo,
-					name
+				[chainId]: {
+					...this.state.addressBook[chainId],
+					[address]: {
+						address,
+						chainId,
+						isEns: isValidEnsName(name),
+						memo,
+						name
+					}
 				}
 			}
 		});
diff --git a/src/util.ts b/src/util.ts
index 2a3baafff2..40d65cd9ed 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -320,6 +320,11 @@ export async function timeoutFetch(url: string, options?: RequestInit, timeout:
 	]);
 }
 
+export function isValidEnsName(ensName: string) {
+	const regex = /^.{7,}\.(eth|test)$/;
+	return regex.test(ensName);
+}
+
 export default {
 	BNToHex,
 	fractionBN,
@@ -328,6 +333,7 @@ export default {
 	hexToBN,
 	hexToText,
 	isSmartContractCode,
+	isValidEnsName,
 	normalizeTransaction,
 	safelyExecute,
 	timeoutFetch,
diff --git a/tests/AddressBookController.test.ts b/tests/AddressBookController.test.ts
index e863cfbe5f..0142d20fa3 100644
--- a/tests/AddressBookController.test.ts
+++ b/tests/AddressBookController.test.ts
@@ -11,11 +11,14 @@ describe('AddressBookController', () => {
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: '',
-					name: 'foo'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'foo'
+					}
 				}
 			}
 		});
@@ -23,14 +26,17 @@ describe('AddressBookController', () => {
 
 	it('should add a contact entry with chainId and memo', () => {
 		const controller = new AddressBookController();
-		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', 1, 'account 1');
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', '1', 'account 1');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: 'account 1',
-					name: 'foo'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: 'account 1',
+						name: 'foo'
+					}
 				}
 			}
 		});
@@ -38,14 +44,46 @@ describe('AddressBookController', () => {
 
 	it('should add a contact entry with chainId and memo', () => {
 		const controller = new AddressBookController();
-		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', 2, 'account 2');
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', '2', 'account 2');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 2,
-					memo: 'account 2',
-					name: 'foo'
+				2: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '2',
+						isEns: false,
+						memo: 'account 2',
+						name: 'foo'
+					}
+				}
+			}
+		});
+	});
+
+	it('should add multiple contact entries with different chainIds', () => {
+		const controller = new AddressBookController();
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', '1', 'account 2');
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo', '2', 'account 2');
+
+		expect(controller.state).toEqual({
+			addressBook: {
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: 'account 2',
+						name: 'foo'
+					}
+				},
+				2: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '2',
+						isEns: false,
+						memo: 'account 2',
+						name: 'foo'
+					}
 				}
 			}
 		});
@@ -57,11 +95,14 @@ describe('AddressBookController', () => {
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'bar');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: '',
-					name: 'bar'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'bar'
+					}
 				}
 			}
 		});
@@ -73,25 +114,75 @@ describe('AddressBookController', () => {
 		expect(controller.state).toEqual({ addressBook: {} });
 	});
 
-	it('should remove a contact entry', () => {
+	it('should remove one contact entry', () => {
 		const controller = new AddressBookController();
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
-		controller.delete('0x32Be343B94f860124dC4fEe278FDCBD38C102D88');
+		controller.delete('1', '0x32Be343B94f860124dC4fEe278FDCBD38C102D88');
+
 		expect(controller.state).toEqual({ addressBook: {} });
 	});
 
-	it('should remove a contact entry', () => {
+	it('should remove only one contact entry', () => {
+		const controller = new AddressBookController();
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
+		controller.set('0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d', 'bar');
+		controller.delete('1', '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d');
+
+		expect(controller.state).toEqual({
+			addressBook: {
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'foo'
+					}
+				}
+			}
+		});
+	});
+
+	it('should add two contact entries with the same chainId', () => {
 		const controller = new AddressBookController();
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
 		controller.set('0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d', 'bar');
-		controller.delete('0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d');
+
+		expect(controller.state).toEqual({
+			addressBook: {
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'foo'
+					},
+					'0xC38bF1aD06ef69F0c04E29DBeB4152B4175f0A8D': {
+						address: '0xC38bF1aD06ef69F0c04E29DBeB4152B4175f0A8D',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'bar'
+					}
+				}
+			}
+		});
+	});
+
+	it('should correctly mark ens entries', () => {
+		const controller = new AddressBookController();
+		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'metamask.eth');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: '',
-					name: 'foo'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: true,
+						memo: '',
+						name: 'metamask.eth'
+					}
 				}
 			}
 		});
@@ -118,27 +209,31 @@ describe('AddressBookController', () => {
 	it('should return true to indicate an address book entry has been deleted', () => {
 		const controller = new AddressBookController();
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
-		expect(controller.delete('0x32Be343B94f860124dC4fEe278FDCBD38C102D88')).toBeTruthy();
+		expect(controller.delete('1', '0x32Be343B94f860124dC4fEe278FDCBD38C102D88')).toBeTruthy();
 	});
 
 	it('should return false to indicate an address book entry has NOT been deleted', () => {
 		const controller = new AddressBookController();
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
-		expect(controller.delete('bar')).toBeFalsy();
+		expect(controller.delete('1', 'bar')).toBeFalsy();
 	});
 
-	it('should normalize addresses so adding a removing entries work across casings', () => {
+	it('should normalize addresses so adding and removing entries work across casings', () => {
 		const controller = new AddressBookController();
 		controller.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
-		expect(controller.set('0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d', 'bar')).toBeTruthy();
-		controller.delete('0xC38BF1AD06EF69F0C04E29DBEB4152B4175F0A8D');
+		controller.set('0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d', 'bar');
+
+		controller.delete('1', '0xC38BF1AD06EF69F0C04E29DBEB4152B4175F0A8D');
 		expect(controller.state).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: '',
-					name: 'foo'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'foo'
+					}
 				}
 			}
 		});
diff --git a/tests/ComposableController.test.ts b/tests/ComposableController.test.ts
index 3358fa0ccd..234a00edff 100644
--- a/tests/ComposableController.test.ts
+++ b/tests/ComposableController.test.ts
@@ -108,11 +108,14 @@ describe('ComposableController', () => {
 		addressContext.set('0x32Be343B94f860124dC4fEe278FDCBD38C102D88', 'foo');
 		expect(controller.flatState).toEqual({
 			addressBook: {
-				'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-					address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-					chainId: 1,
-					memo: '',
-					name: 'foo'
+				1: {
+					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+						chainId: '1',
+						isEns: false,
+						memo: '',
+						name: 'foo'
+					}
 				}
 			},
 			allCollectibleContracts: {},
@@ -152,10 +155,13 @@ describe('ComposableController', () => {
 			AddressBookController: {
 				addressBook: [
 					{
-						address: 'bar',
-						chainId: 1,
-						memo: '',
-						name: 'foo'
+						1: {
+							address: 'bar',
+							chainId: '1',
+							isEns: false,
+							memo: '',
+							name: 'foo'
+						}
 					}
 				]
 			}
@@ -174,11 +180,14 @@ describe('ComposableController', () => {
 		expect(listener.getCall(0).args[0]).toEqual({
 			AddressBookController: {
 				addressBook: {
-					'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
-						address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
-						chainId: 1,
-						memo: '',
-						name: 'foo'
+					1: {
+						'0x32Be343B94f860124dC4fEe278FDCBD38C102D88': {
+							address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
+							chainId: '1',
+							isEns: false,
+							memo: '',
+							name: 'foo'
+						}
 					}
 				}
 			}
diff --git a/tests/util.test.ts b/tests/util.test.ts
index 1acde1b1a9..6ed0c5b6fc 100644
--- a/tests/util.test.ts
+++ b/tests/util.test.ts
@@ -454,4 +454,14 @@ describe('util', () => {
 			expect(error.message).toBe('timeout');
 		});
 	});
+	describe('isValidEnsName', () => {
+		it('should return if the ens name is valid by current standards', async () => {
+			const valid = util.isValidEnsName('metamask.eth');
+			expect(valid).toBeTruthy();
+		});
+		it('should return if the ens name is invalid by current standards', async () => {
+			const invalid = util.isValidEnsName('me.eth');
+			expect(invalid).toBeFalsy();
+		});
+	});
 });