Merge pull request #86 from Adamant-im/chore/tests

Chore/tests

.github/CONTRIBUTING.md

@@ -0,0 +1,84 @@
+## Tests
+
+To be merged, pull requests **MUST** include tests for any new features or bug fixes.
+
+The tests are located in the test/ directory. The structure of the directory is as follows:
+- `api/` - End-to-end tests that require running a local test node.
+- `unit/` - Unit tests that DO NOT require running a test node.
+- `common/` - Contains stub objects and utilities for the tests.
+- `node.js` - Package for making requests to the local test node.
+- `config.json` - Configuration file to run a local test node; copy `config.default.json`
+- `genesisBlock.json` - Genesis block data.
+- `genesisDelegates.json` - Genesis delegate accounts.
+- `genesisPasses.json` - Passphrases for the genesis accounts.
+
+All tests inside `api/` and `unit/` should mirror (as much as possible) the structure of the project. For example, unit tests for the `modules/blocks.js` module should be located in the `test/unit/modules/blocks.js` file.
+
+### Commands
+
+To run a single test file, use the following command:
+
+```
+npm run test:single test/path/to/the/test.js
+```
+
+If you have changed any common files (e.g., files inside `test/common/`, `test/node.js` package, etc.), consider running all tests:
+
+```
+npm run test:all
+```
+
+### Convention for tests
+
+Since we use the Chai package for assertions, we have a few rules for consistency:
+
+- **Use proper English grammar in assertions**
+
+  ```js
+  // ❌
+  expect({}).to.be.a('object');
+  // ✅
+  expect(true).to.be.an('object');
+  ```
+
+- **Use `to.be.true` instead of `to.be.ok`**
+
+  Boolean values should be strictly asserted:
+
+  ```js
+  // ❌
+  expect(true).to.be.ok;
+  // ✅
+  expect(true).to.be.true;
+  ```
+
+- **Use `to.not` instead of `not.to`**
+
+  Prefer `not.to` for convention:
+
+  ```js
+  // ❌
+  expect(true).to.not.equal(false);
+  // ✅
+  expect(true).not.to.be.false;
+  ```
+
+- **Use `.equal()` instead of `.eql()` for `===`**
+
+  Use `.eql()` **only** for deep assertion.
+
+  ```js
+  // ❌
+  expect(true).to.eql(true);
+  // ✅
+  expect(true).to.be.true;
+  ```
+
+- **Use parentheses for functions and methods in `describe` names**
+
+  ```js
+  // ❌
+  describe(`functionName`, () => { /* ... */ })
+  // ✅
+  describe(`functionName()`, () => { /* ... */ })
+  ```

helpers/accounts.js

@@ -8,18 +8,23 @@ var bignum = require('./bignum.js');
 var sodium = require('sodium-browserify-tweetnacl');
 
 let Mnemonic = require('bitcore-mnemonic');
+const { isPublicKey } = require('./publicKey.js');
 
 var accounts = {};
 
 /**
- * Gets address by public
+ * Gets address by public key
  * @private
  * @implements {crypto.createHash}
  * @implements {bignum.fromBuffer}
- * @param {publicKey} publicKey
- * @return {address} address
+ * @param {string} publicKey
+ * @return {string} The address matching the public key, or an empty string if an invalid public key was provided
  */
 accounts.getAddressByPublicKey = function (publicKey) {
+  if (!isPublicKey(publicKey)) {
+    return '';
+  }
+
   var publicKeyHash = crypto.createHash('sha256').update(publicKey, 'hex').digest();
   var temp = Buffer.alloc(8);
 

helpers/ed.js

@@ -14,6 +14,24 @@ var crypto = require('crypto');
  */
 var ed = {};
 
+/**
+ * Returns whether the passphrase is valid mnemonic
+ * @param {string} passphrase passhraase to test
+ * @returns {boolean}
+ */
+ed.isValidPassphrase = function(passphrase) {
+  return mnemonic.isValid(passphrase, mnemonic.Words.ENGLISH);
+}
+
+/**
+ * Generates a new passphrase
+ * @returns {string} passphrase
+ */
+ed.generatePassphrase = function() {
+  const secretMnemonic = new mnemonic(mnemonic.Words.ENGLISH);
+  return secretMnemonic.phrase;
+}
+
 /**
  * Creates a hash based on a passphrase.
  * @param {string} passPhrase

helpers/publicKey.js

@@ -0,0 +1,8 @@
+/**
+ * Tests if the provided value is a valid public key
+ * @param {any} str string to test
+ * @returns {boolean} whether the string can be considered a public key
+ */
+exports.isPublicKey = (value) => {
+  return typeof value === "string" && Buffer.from(value, "hex").length === 32;
+};

helpers/request-limiter.js

@@ -24,7 +24,9 @@ var defaults = {
  * @param {Object} [limits]
  * @return {Object} max, delayMs, delayAfter, windowMs
  */
-function applyLimits (limits) {
+function applyLimits(config) {
+  const limits = config ?? defaults;
+
   if (typeof limits === 'object') {
     const settings = {
       max: Math.floor(limits.max) || defaults.max,
@@ -35,7 +37,7 @@ function applyLimits (limits) {
       windowMs: Math.floor(limits.windowMs) || defaults.windowMs
     };
 
-    if (!limits.max) {
+    if (!limits.delayAfter) {
       settings.skip = skip;
     }
 

helpers/sequence.js

@@ -18,15 +18,20 @@ function Sequence (config) {
   _default = extend(_default, config);
   var self = this;
   this.sequence = [];
+  this.isTicking = false;
+
+  this.nextSequenceTick = function () {
+    if (!self.sequence.length) {
+      self.isTicking = false;
+      return;
+    }
 
-  setImmediate(function nextSequenceTick () {
     if (_default.onWarning && self.sequence.length >= _default.warningLimit) {
       _default.onWarning(self.sequence.length, _default.warningLimit);
     }
-    self.__tick(function () {
-      setTimeout(nextSequenceTick, 3);
-    });
-  });
+
+    self.__tick(self.nextSequenceTick);
+  };
 }
 
 /**
@@ -68,6 +73,11 @@ Sequence.prototype.add = function (worker, args, done) {
       task.args = args;
     }
     this.sequence.push(task);
+
+    if (!this.isTicking) {
+      this.isTicking = true;
+      setImmediate(this.nextSequenceTick);
+    }
   }
 };
 

helpers/z_schema.js

@@ -23,6 +23,7 @@ var ip = require('neoip');
  * @return {Boolean} True if the format is valid
  */
 var z_schema = require('z-schema');
+const { isPublicKey } = require('./publicKey.js');
 
 z_schema.registerFormat('id', function (str) {
   if (str.length === 0) {
@@ -62,13 +63,7 @@ z_schema.registerFormat('publicKey', function (str) {
     return true;
   }
 
-  try {
-    var publicKey = Buffer.from(str, 'hex');
-
-    return publicKey.length === 32;
-  } catch (e) {
-    return false;
-  }
+  return isPublicKey(str)
 });
 
 z_schema.registerFormat('csv', function (str) {

logic/account.js

@@ -509,9 +509,7 @@ Account.prototype.verifyPublicKey = function (publicKey) {
       throw 'Invalid public key, must be 64 characters long';
     }
     // Check format
-    try {
-      Buffer.from(publicKey, 'hex');
-    } catch (e) {
+    if (!/^[0-9A-Fa-f]+$/.test(publicKey)) {
       throw 'Invalid public key, must be a hex string';
     }
   }
@@ -680,7 +678,7 @@ Account.prototype.merge = function (address, diff, cb) {
   // Normalize address
   address = String(address).toUpperCase();
 
-  this.editable.forEach(function (value) {
+  for (const value of this.editable) {
     var val, i;
 
     if (diff[value] !== undefined) {
@@ -691,7 +689,9 @@ Account.prototype.merge = function (address, diff, cb) {
           break;
         case Number:
           if (isNaN(trueValue) || trueValue === Infinity) {
-            return setImmediate(cb, 'Encountered unsane number: ' + trueValue);
+            const error = new Error(`Encountered unsafe number: ${trueValue}`)
+            library.logger.error(error.stack, diff);
+            return setImmediate(cb, error.message);
           } else if (Math.abs(trueValue) === trueValue && trueValue !== 0) {
             update[value] = knex.raw('?? + ?', [value, Math.floor(trueValue)])
 
@@ -800,7 +800,7 @@ Account.prototype.merge = function (address, diff, cb) {
           break;
       }
     }
-  });
+  }
 
   var sqles = [];
 

modules/accounts.js

@@ -95,6 +95,10 @@ __private.newAccount = function (publicKey, cb) {
  * @return {setImmediateCallback} As per logic new|current account data object.
  */
 __private.openAccount = function (secret, cb) {
+  if (!library.ed.isValidPassphrase(secret)) {
+    return setImmediate(cb, `Mnemonic string is invalid: ${secret}`);
+  }
+
   var hash = library.ed.createPassPhraseHash(secret);
   var keypair = library.ed.makeKeypair(hash);
   var publicKey = keypair.publicKey.toString('hex');
@@ -143,14 +147,36 @@ Accounts.prototype.generateAddressByPublicKey = function (publicKey) {
 
 /**
  * Gets account information, calls logic.account.get().
- * @implements module:accounts#Account~get
+ * @overload
+ * @param {Object} filter - Contains publicKey.
+ * @param {Array} fields - Fields to get.
+ * @param {function} cb - Callback function.
+ */
+
+/**
+ * Gets account information, calls logic.account.get().
+ * @overload
  * @param {Object} filter - Contains publicKey.
- * @param {function} fields - Fields to get.
  * @param {function} cb - Callback function.
  */
+
+/**
+ * Gets account information, calls logic.account.get().
+ * @implements module:accounts#Account~get
+ * @param {Object} filter - Contains publicKey.
+ * @param {Array | function} fields - Fields to get or callback function.
+ * @param {function} [cb] - Callback function.
+ */
 Accounts.prototype.getAccount = function (filter, fields, cb) {
   if (filter.publicKey) {
-    filter.address = self.generateAddressByPublicKey(filter.publicKey);
+    try {
+      filter.address = self.generateAddressByPublicKey(filter.publicKey);
+    } catch (error) {
+      if (typeof fields === 'function') {
+        return setImmediate(fields, error);
+      }
+      return setImmediate(cb, error);
+    }
     delete filter.publicKey;
   }
 
@@ -189,10 +215,6 @@ Accounts.prototype.setAccountAndGet = function (data, cb) {
     }
   }
 
-  if (!address) {
-    err = 'Invalid public key';
-  }
-
   if (err) {
     if (typeof cb === 'function') {
       return setImmediate(cb, err);
@@ -230,10 +252,6 @@ Accounts.prototype.mergeAccountAndGet = function (data, cb) {
     }
   }
 
-  if (!address) {
-    err = 'Invalid public key';
-  }
-
   if (err) {
     if (typeof cb === 'function') {
       return setImmediate(cb, err);