Merge pull request #5417 from paritytech/mh-publicnode-tests

Tests and tweaks for public node middleware

js/src/api/api.js

@@ -55,7 +55,7 @@ export default class Api extends EventEmitter {
         .nodeKind()
         .then((nodeKind) => {
           if (nodeKind.availability === 'public') {
-            return new LocalAccountsMiddleware(transport);
+            return LocalAccountsMiddleware;
           }
 
           return null;

js/src/api/local/ethkey/dummy.js

@@ -1,19 +0,0 @@
-// Copyright 2015-2017 Parity Technologies (UK) Ltd.
-// This file is part of Parity.
-
-// Parity is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-
-// Parity is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License
-// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
-
-export default function () {
-  // empty file included while building parity.js (don't include local keygen)
-}

js/src/api/local/ethkey/index.spec.js

@@ -18,8 +18,8 @@ import { randomPhrase } from '@parity/wordlist';
 import { phraseToAddress, phraseToWallet } from './';
 
 describe('api/local/ethkey', () => {
-  describe.skip('phraseToAddress', function () {
+  describe('phraseToAddress', function () {
-    this.timeout(10000);
+    this.timeout(30000);
 
     it('generates a valid address', () => {
       const phrase = randomPhrase(12);
@@ -37,8 +37,8 @@ describe('api/local/ethkey', () => {
     });
   });
 
-  describe.skip('phraseToWallet', function () {
+  describe('phraseToWallet', function () {
-    this.timeout(10000);
+    this.timeout(30000);
 
     it('generates a valid wallet object', () => {
       const phrase = randomPhrase(12);

js/src/api/local/ethkey/worker.js

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-import secp256k1 from 'secp256k1/js';
+import secp256k1 from 'secp256k1';
 import { keccak_256 as keccak256 } from 'js-sha3';
 import { bytesToHex } from '~/api/util/format';
 
@@ -28,11 +28,9 @@ if (!isWorker) {
 }
 
 // keythereum should never be used outside of the browser
-let keythereum = null;
+let keythereum = require('keythereum');
 
 if (isWorker) {
-  require('keythereum/dist/keythereum');
-
   keythereum = self.keythereum;
 }
 
@@ -109,9 +107,13 @@ const actions = {
 };
 
 self.onmessage = function ({ data }) {
+  try {
     const result = route(data);
 
-  postMessage(result);
+    postMessage([null, result]);
+  } catch (err) {
+    postMessage([err, null]);
+  }
 };
 
 // Emulate a web worker in Node.js
@@ -119,9 +121,13 @@ class KeyWorker {
   postMessage (data) {
     // Force async
     setTimeout(() => {
+      try {
         const result = route(data);
 
-      this.onmessage({ data: result });
+        this.onmessage({ data: [null, result] });
+      } catch (err) {
+        this.onmessage({ data: [err, null] });
+      }
     }, 0);
   }
 

js/src/api/local/ethkey/workerPool.js

@@ -33,8 +33,15 @@ class WorkerContainer {
     return new Promise((resolve, reject) => {
       this._worker.postMessage({ action, payload });
       this._worker.onmessage = ({ data }) => {
+        const [err, result] = data;
+
         this.busy = false;
-        resolve(data);
+
+        if (err) {
+          reject(err);
+        } else {
+          resolve(result);
+        }
       };
     });
   }

js/src/api/local/index.js

@@ -14,4 +14,4 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-export LocalAccountsMiddleware from './middleware';
+export LocalAccountsMiddleware from './localAccountsMiddleware';

js/src/api/local/localAccountsMiddleware.js

@@ -23,15 +23,6 @@ import { phraseToWallet, phraseToAddress, verifySecret } from './ethkey';
 import { randomPhrase } from '@parity/wordlist';
 
 export default class LocalAccountsMiddleware extends Middleware {
-  // Maps transaction requests to transaction hashes.
-  // This allows the locally-signed transactions to emulate the signer.
-  transactionHashes = {};
-  transactions = {};
-
-  // Current transaction id. This doesn't need to be stored, as it's
-  // only relevant for the current the session.
-  transactionId = 1;
-
   constructor (transport) {
     super(transport);
 
@@ -170,13 +161,27 @@ export default class LocalAccountsMiddleware extends Middleware {
         data
       } = Object.assign(transactions.get(id), modify);
 
+      transactions.lock(id);
+
       const account = accounts.get(from);
 
       return Promise.all([
         this.rpcRequest('parity_nextNonce', [from]),
         account.decryptPrivateKey(password)
       ])
+      .catch((err) => {
+        transactions.unlock(id);
+
+        // transaction got unlocked, can propagate rejection further
+        throw err;
+      })
       .then(([nonce, privateKey]) => {
+        if (!privateKey) {
+          transactions.unlock(id);
+
+          throw new Error('Invalid password');
+        }
+
         const tx = new EthereumTx({
           nonce,
           to,

js/src/api/local/localAccountsMiddleware.spec.js

@@ -0,0 +1,154 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+import LocalAccountsMiddleware from './localAccountsMiddleware';
+import JsonRpcBase from '../transport/jsonRpcBase';
+
+const RPC_RESPONSE = Symbol('RPC response');
+const ADDRESS = '0x00a329c0648769a73afac7f9381e08fb43dbea72';
+const SECRET = '0x4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7';
+const PASSWORD = 'password';
+
+const FOO_PHRASE = 'foobar';
+const FOO_PASSWORD = 'foopass';
+const FOO_ADDRESS = '0x007ef7ac1058e5955e366ab9d6b6c4ebcc937e7e';
+
+class MockedTransport extends JsonRpcBase {
+  _execute (method, params) {
+    return RPC_RESPONSE;
+  }
+}
+
+describe('api/local/LocalAccountsMiddleware', function () {
+  this.timeout(30000);
+
+  let transport;
+
+  beforeEach(() => {
+    transport = new MockedTransport();
+    transport.addMiddleware(LocalAccountsMiddleware);
+
+    // Same as `parity_newAccountFromPhrase` with empty phrase
+    return transport
+      .execute('parity_newAccountFromSecret', SECRET, PASSWORD)
+      .catch((_err) => {
+        // Ignore the error - all instances of LocalAccountsMiddleware
+        // share account storage
+      });
+  });
+
+  it('registers all necessary methods', () => {
+    return Promise
+      .all([
+        'eth_accounts',
+        'eth_coinbase',
+        'parity_accountsInfo',
+        'parity_allAccountsInfo',
+        'parity_changePassword',
+        'parity_checkRequest',
+        'parity_defaultAccount',
+        'parity_generateSecretPhrase',
+        'parity_getNewDappsAddresses',
+        'parity_hardwareAccountsInfo',
+        'parity_newAccountFromPhrase',
+        'parity_newAccountFromSecret',
+        'parity_setAccountMeta',
+        'parity_setAccountName',
+        'parity_postTransaction',
+        'parity_phraseToAddress',
+        'parity_useLocalAccounts',
+        'parity_listGethAccounts',
+        'parity_listRecentDapps',
+        'parity_killAccount',
+        'parity_testPassword',
+        'signer_confirmRequest',
+        'signer_rejectRequest',
+        'signer_requestsToConfirm'
+      ].map((method) => {
+        return transport
+          .execute(method)
+          .then((result) => {
+            expect(result).not.to.be.equal(RPC_RESPONSE);
+          })
+          // Some errors are expected here since we are calling methods
+          // without parameters.
+          .catch((_) => {});
+      }));
+  });
+
+  it('allows non-registered methods through', () => {
+    return transport
+      .execute('eth_getBalance', '0x407d73d8a49eeb85d32cf465507dd71d507100c1')
+      .then((result) => {
+        expect(result).to.be.equal(RPC_RESPONSE);
+      });
+  });
+
+  it('can handle `eth_accounts`', () => {
+    return transport
+      .execute('eth_accounts')
+      .then((accounts) => {
+        expect(accounts.length).to.be.equal(1);
+        expect(accounts[0]).to.be.equal(ADDRESS);
+      });
+  });
+
+  it('can handle `parity_defaultAccount`', () => {
+    return transport
+      .execute('parity_defaultAccount')
+      .then((address) => {
+        expect(address).to.be.equal(ADDRESS);
+      });
+  });
+
+  it('can handle `parity_phraseToAddress`', () => {
+    return transport
+      .execute('parity_phraseToAddress', '')
+      .then((address) => {
+        expect(address).to.be.equal(ADDRESS);
+
+        return transport.execute('parity_phraseToAddress', FOO_PHRASE);
+      })
+      .then((address) => {
+        expect(address).to.be.equal(FOO_ADDRESS);
+      });
+  });
+
+  it('can create and kill an account', () => {
+    return transport
+      .execute('parity_newAccountFromPhrase', FOO_PHRASE, FOO_PASSWORD)
+      .then((address) => {
+        expect(address).to.be.equal(FOO_ADDRESS);
+
+        return transport.execute('eth_accounts');
+      })
+      .then((accounts) => {
+        expect(accounts.length).to.be.equal(2);
+        expect(accounts.includes(FOO_ADDRESS)).to.be.true;
+
+        return transport.execute('parity_killAccount', FOO_ADDRESS, FOO_PASSWORD);
+      })
+      .then((result) => {
+        expect(result).to.be.true;
+
+        return transport.execute('eth_accounts');
+      })
+      .then((accounts) => {
+        expect(accounts.length).to.be.equal(1);
+        expect(accounts.includes(FOO_ADDRESS)).to.be.false;
+      });
+  });
+});

js/src/api/local/transactions.js

@@ -18,6 +18,7 @@ import { toHex } from '../util/format';
 import { TransportError } from '../transport';
 
 const AWAITING = Symbol('awaiting');
+const LOCKED = Symbol('locked');
 const CONFIRMED = Symbol('confirmed');
 const REJECTED = Symbol('rejected');
 
@@ -57,6 +58,26 @@ class Transactions {
     return state.transaction;
   }
 
+  lock (id) {
+    const state = this._states[id];
+
+    if (!state || state.status !== AWAITING) {
+      throw new Error('Trying to lock an invalid transaction');
+    }
+
+    state.status = LOCKED;
+  }
+
+  unlock (id) {
+    const state = this._states[id];
+
+    if (!state || state.status !== LOCKED) {
+      throw new Error('Trying to unlock an invalid transaction');
+    }
+
+    state.status = AWAITING;
+  }
+
   hash (id) {
     const state = this._states[id];
 
@@ -76,9 +97,12 @@ class Transactions {
 
   confirm (id, hash) {
     const state = this._states[id];
+    const status = state ? state.status : null;
 
-    if (!state || state.status !== AWAITING) {
+    switch (status) {
-      throw new Error('Trying to confirm an invalid transaction');
+      case AWAITING: break;
+      case LOCKED: break;
+      default: throw new Error('Trying to confirm an invalid transaction');
     }
 
     state.hash = hash;

js/src/api/local/transactions.spec.js

@@ -65,4 +65,21 @@ describe('api/local/transactions', () => {
     expect(requests.length).to.be.equal(0);
     expect(() => transactions.hash(id)).to.throw(TransportError);
   });
+
+  it('can lock and confirm transactions', () => {
+    const id = transactions.add(DUMMY_TX);
+    const hash = '0x1111111111111111111111111111111111111111';
+
+    transactions.lock(id);
+
+    const requests = transactions.requestsToConfirm();
+
+    expect(requests.length).to.be.equal(0);
+    expect(transactions.get(id)).to.be.null;
+    expect(transactions.hash(id)).to.be.null;
+
+    transactions.confirm(id, hash);
+
+    expect(transactions.hash(id)).to.be.equal(hash);
+  });
 });

js/src/api/transport/jsonRpcBase.js

@@ -38,20 +38,20 @@ export default class JsonRpcBase extends EventEmitter {
     return json;
   }
 
-  addMiddleware (middleware) {
+  addMiddleware (Middleware) {
     this._middlewareList = Promise
       .all([
-        middleware,
+        Middleware,
         this._middlewareList
       ])
-      .then(([middleware, middlewareList]) => {
+      .then(([Middleware, middlewareList]) => {
         // Do nothing if `handlerPromise` resolves to a null-y value.
-        if (middleware == null) {
+        if (Middleware == null) {
           return middlewareList;
         }
 
         // don't mutate the original array
-        return middlewareList.concat([middleware]);
+        return middlewareList.concat([new Middleware(this)]);
       });
   }
 
@@ -80,8 +80,8 @@ export default class JsonRpcBase extends EventEmitter {
         const res = middleware.handle(method, params);
 
         if (res != null) {
-          // If `res` isn't a promise, we need to wrap it
+          return Promise
-          return Promise.resolve(res)
+            .resolve(res)
             .then((res) => {
               const result = this._wrapSuccessResult(res);
               const json = this.encode(method, params);

js/src/api/transport/middleware.js

@@ -28,9 +28,7 @@ export default class Middleware {
     const handler = this._handlers[method];
 
     if (handler != null) {
-      const response = handler(params);
+      return handler(params);
-
-      return response;
     }
 
     return null;

js/src/api/transport/middleware.spec.js

@@ -25,17 +25,21 @@ class MockTransport extends JsonRpcBase {
   }
 }
 
+class MockMiddleware extends Middleware {
+  constructor (transport) {
+    super(transport);
+
+    this.register('mock_rpc', ([num]) => num);
+    this.register('mock_null', () => null);
+  }
+}
+
 describe('api/transport/Middleware', () => {
-  let middleware;
   let transport;
 
   beforeEach(() => {
     transport = new MockTransport();
-    middleware = new Middleware(transport);
+    transport.addMiddleware(MockMiddleware);
-
-    middleware.register('mock_rpc', ([num]) => num);
-    middleware.register('mock_null', () => null);
-    transport.addMiddleware(middleware);
   });
 
   it('Routes requests to middleware', () => {

js/webpack/app.js

@@ -138,7 +138,9 @@ module.exports = {
 
   resolve: {
     alias: {
-      '~': path.resolve(__dirname, '../src')
+      '~': path.resolve(__dirname, '../src'),
+      'secp256k1': path.resolve(__dirname, '../node_modules/secp256k1/js'),
+      'keythereum': path.resolve(__dirname, '../node_modules/keythereum/dist/keythereum')
     },
     modules: [
       path.join(__dirname, '../node_modules')

js/webpack/libraries.js

@@ -41,7 +41,9 @@ module.exports = {
 
   resolve: {
     alias: {
-      '~': path.resolve(__dirname, '../src')
+      '~': path.resolve(__dirname, '../src'),
+      'secp256k1': path.resolve(__dirname, '../node_modules/secp256k1/js'),
+      'keythereum': path.resolve(__dirname, '../node_modules/keythereum/dist/keythereum')
     }
   },
 

js/webpack/npm.js

@@ -76,7 +76,6 @@ module.exports = {
 
   resolve: {
     alias: {
-      'secp256k1/js': path.resolve(__dirname, '../src/api/local/ethkey/dummy.js'),
       '~': path.resolve(__dirname, '../src')
     },
     modules: [