Replace withAppKeyOrigin option with getAppKeying

The new `getAppKeyring` method will return a new SimpleKeyring created
with an app key derived from the provided origin. This serves as an
alternative to optionally augmenting each wallet method with a set of
options that may contain a `withAppKeyOrigin` value, which would be
used to generate a new wallet on-demand.

Moving app key generation to a single place makes it easier to limit
access to sensitive information, such as the primary private keys.
It also allows the consumer to use app keys more efficiently by using
the same wallet for successive operations, rather than generating a
new private key each time with `keccak`.

index.js

@@ -127,28 +127,35 @@ class SimpleKeyring extends EventEmitter {
     return Promise.resolve(sig)
   }
 
-  getPrivateKeyFor (address, opts = {}) {
+  getPrivateKeyFor (address) {
     if (!address) {
       throw new Error('Must specify address.');
     }
-    const wallet = this._getWalletForAccount(address, opts)
+    const wallet = this._getWalletForAccount(address)
     const privKey = ethUtil.toBuffer(wallet.getPrivateKey())
     return privKey;
   }
 
-  // returns an address specific to an app
-  getAppKeyAddress (address, origin) {
-    return new Promise((resolve, reject) => {
-      try {
-        const wallet = this._getWalletForAccount(address, {
-          withAppKeyOrigin: origin,
-        })
-        const appKeyAddress = sigUtil.normalize(wallet.getAddress().toString('hex'))
-        return resolve(appKeyAddress)
-      } catch (e) {
-        return reject(e)
-      }
-    })
+  getAppKeyring (address, origin) {
+    if (
+      !origin ||
+      typeof origin !== 'string'
+    ) {
+      throw new Error(`'origin' must be a non-empty string`)
+    }
+
+    const wallet = this._getWalletForAccount(address)
+    const privKey = wallet.getPrivateKey()
+    const appKeyOriginBuffer = Buffer.from(origin, 'utf8')
+    const appKeyBuffer = Buffer.concat([privKey, appKeyOriginBuffer])
+    const appKeyPrivKey = ethUtil.keccak(appKeyBuffer, 256)
+    return new SimpleKeyring([appKeyPrivKey.toString('hex')])
+  }
+
+  async getAppKeyAddress (address, origin) {
+    const keyring = this.getAppKeyring(address, origin)
+    const accounts = await keyring.getAccounts()
+    return accounts[0]
   }
 
   // exportAccount should return a hex-encoded private key:
@@ -166,19 +173,10 @@ class SimpleKeyring extends EventEmitter {
 
   /* PRIVATE METHODS */
 
-  _getWalletForAccount (account, opts = {}) {
+  _getWalletForAccount (account) {
     const address = sigUtil.normalize(account)
     let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === address)
     if (!wallet) throw new Error('Simple Keyring - Unable to find matching address.')
-
-    if (opts.withAppKeyOrigin) {
-      const privKey = wallet.getPrivateKey()
-      const appKeyOriginBuffer = Buffer.from(opts.withAppKeyOrigin, 'utf8')
-      const appKeyBuffer = Buffer.concat([privKey, appKeyOriginBuffer])
-      const appKeyPrivKey = ethUtil.keccak(appKeyBuffer, 256)
-      wallet = Wallet.fromPrivateKey(appKeyPrivKey)
-    }
-
     return wallet
   }
 

test/index.js

@@ -415,44 +415,79 @@ describe('simple-keyring', () => {
     })
   })
 
-  describe('signing methods withAppKeyOrigin option', function () {
-    it('should signPersonalMessage with the expected key when passed a withAppKeyOrigin', async function () {
+  describe('getAppKeyring', function () {
+    it('should return a SimpleKeyring custom to the provided app origin', async function () {
       const address = testAccount.address
-      const message = '0x68656c6c6f20776f726c64'
+      const keyring = new SimpleKeyring([testAccount.key])
+
+      const appKeyring = await keyring.getAppKeyring(address, 'someapp.origin.io')
 
-      const privateKeyHex = '4fbe006f0e9c2374f53eb1aef1b6970d20206c61ea05ad9591ef42176eb842c0'
-      const privateKeyBuffer = new Buffer(privateKeyHex, 'hex')
-      const expectedSig = sigUtil.personalSign(privateKeyBuffer, { data: message })
+      assert(appKeyring instanceof SimpleKeyring)
 
+      const appKeyAddress = (await appKeyring.getAccounts())[0]
+
+      assert.notEqual(address, appKeyAddress)
+      assert(ethUtil.isValidAddress(appKeyAddress))
+    })
+
+    it('should return different keyrings when provided different app origins', async function () {
+      const address = testAccount.address
       const keyring = new SimpleKeyring([testAccount.key])
-      const sig = await keyring.signPersonalMessage(address, message, {
-        withAppKeyOrigin: 'someapp.origin.io',
-      })
 
-      assert.equal(expectedSig, sig, 'sign with app key generated private key')
+      const appKeyring1 = await keyring.getAppKeyring(address, 'someapp.origin.io')
+      const appKeyAddress1 = (await appKeyring1.getAccounts())[0]
+
+      assert(ethUtil.isValidAddress(appKeyAddress1))
+
+      const appKeyring2 = await keyring.getAppKeyring(address, 'anotherapp.origin.io')
+      const appKeyAddress2 = (await appKeyring2.getAccounts())[0]
+
+      assert(ethUtil.isValidAddress(appKeyAddress2))
+
+      assert.notEqual(appKeyAddress1, appKeyAddress2)
     })
 
-    it('should signTypedData_v3 with the expected key when passed a withAppKeyOrigin', async function () {
+    it('should return the same keyring when called multiple times with the same params', async function () {
       const address = testAccount.address
-      const typedData = {
-        types: {
-          EIP712Domain: []
-        },
-        domain: {},
-        primaryType: 'EIP712Domain',
-        message: {}
-      }
+      const keyring = new SimpleKeyring([testAccount.key])
+
+      const appKeyring1 = await keyring.getAppKeyring(address, 'someapp.origin.io')
+      const appKeyAddress1 = (await appKeyring1.getAccounts())[0]
 
-      const privateKeyHex = '4fbe006f0e9c2374f53eb1aef1b6970d20206c61ea05ad9591ef42176eb842c0'
-      const privateKeyBuffer = new Buffer(privateKeyHex, 'hex')
-      const expectedSig = sigUtil.signTypedData(privateKeyBuffer, { data: typedData })
+      assert(ethUtil.isValidAddress(appKeyAddress1))
 
+      const appKeyring2 = await keyring.getAppKeyring(address, 'someapp.origin.io')
+      const appKeyAddress2 = (await appKeyring2.getAccounts())[0]
+
+      assert(ethUtil.isValidAddress(appKeyAddress2))
+
+      assert.equal(appKeyAddress1, appKeyAddress2)
+    })
+
+    it('should throw error if the provided origin is not a string', async function () {
+      const address = testAccount.address
       const keyring = new SimpleKeyring([testAccount.key])
-      const sig = await keyring.signTypedData_v3(address, typedData, {
-        withAppKeyOrigin: 'someapp.origin.io',
-      })
 
-      assert.equal(expectedSig, sig, 'sign with app key generated private key')
+      try {
+        await keyring.getAppKeyring(address, [])
+      } catch (error) {
+        assert(error instanceof Error, 'Value thrown is not an error')
+        return
+      }
+      assert.fail('Should have thrown error')
+    })
+
+    it('should throw error if the provided origin is an empty string', async function () {
+      const address = testAccount.address
+      const keyring = new SimpleKeyring([testAccount.key])
+
+      try {
+        await keyring.getAppKeyring(address, '')
+      } catch (error) {
+        assert(error instanceof Error, 'Value thrown is not an error')
+        return
+      }
+      assert.fail('Should have thrown error')
     })
   })
 })