Changed before for beforeAll, refactored Bouncer tests. (#1094)

* Changed before for beforeAll, refactored Bouncer tests.

* Fixed linter errors.

* fix: updates for SignatureBouncer tests and voucher construction

contracts/access/SignatureBouncer.sol

@@ -34,8 +34,8 @@ contract SignatureBouncer is Ownable, RBAC {
 
   string public constant ROLE_BOUNCER = "bouncer";
   uint constant METHOD_ID_SIZE = 4;
-  // (signature length size) 32 bytes + (signature size 65 bytes padded) 96 bytes
-  uint constant SIGNATURE_SIZE = 128;
+  // signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes
+  uint constant SIGNATURE_SIZE = 96;
 
   /**
    * @dev requires that a valid signature of a bouncer was provided

contracts/mocks/BouncerMock.sol

@@ -36,7 +36,12 @@ contract SignatureBouncerMock is SignatureBouncer {
 
   }
 
-  function checkValidSignatureAndData(address _address, bytes _bytes, uint _val, bytes _sig)
+  function checkValidSignatureAndData(
+    address _address,
+    bytes,
+    uint,
+    bytes _sig
+  )
     public
     view
     returns (bool)
@@ -44,11 +49,18 @@ contract SignatureBouncerMock is SignatureBouncer {
     return isValidSignatureAndData(_address, _sig);
   }
 
-  function onlyWithValidSignatureAndData(uint _val, bytes _sig)
+  function onlyWithValidSignatureAndData(uint, bytes _sig)
     onlyValidSignatureAndData(_sig)
     public
     view
   {
 
   }
+
+  function theWrongMethod(bytes)
+    public
+    pure
+  {
+
+  }
 }

test/access/SignatureBouncer.test.js

@@ -1,5 +1,5 @@
 const { assertRevert } = require('../helpers/assertRevert');
-const { signHex } = require('../helpers/sign');
+const { getBouncerSigner } = require('../helpers/sign');
 
 const Bouncer = artifacts.require('SignatureBouncerMock');
 
@@ -7,274 +7,239 @@ require('chai')
   .use(require('chai-as-promised'))
   .should();
 
-const getSigner = (contract, signer, data = '') => (addr) => {
-  // via: https://github.com/OpenZeppelin/zeppelin-solidity/pull/812/files
-  const message = contract.address.substr(2) + addr.substr(2) + data;
-  // ^ substr to remove `0x` because in solidity the address is a set of byes, not a string `0xabcd`
-  return signHex(signer, message);
-};
-
-const getMethodId = (methodName, ...paramTypes) => {
-  // methodId is a sha3 of the first 4 bytes after 0x of 'method(paramType1,...)'
-  return web3.sha3(`${methodName}(${paramTypes.join(',')})`).substr(2, 8);
-};
-
-const stripAndPadHexValue = (hexVal, sizeInBytes, start = true) => {
-  // strip 0x from the font and pad with 0's for
-  const strippedHexVal = hexVal.substr(2);
-  return start ? strippedHexVal.padStart(sizeInBytes * 2, 0) : strippedHexVal.padEnd(sizeInBytes * 2, 0);
-};
-
-contract('Bouncer', ([_, owner, authorizedUser, anyone, bouncerAddress, newBouncer]) => {
-  before(async function () {
+const UINT_VALUE = 23;
+const BYTES_VALUE = web3.toHex('test');
+const INVALID_SIGNATURE = '0xabcd';
+
+contract('Bouncer', ([_, owner, anyone, bouncerAddress, authorizedUser]) => {
+  beforeEach(async function () {
     this.bouncer = await Bouncer.new({ from: owner });
     this.roleBouncer = await this.bouncer.ROLE_BOUNCER();
-    this.genSig = getSigner(this.bouncer, bouncerAddress);
-    this.uintValue = 23;
-    this.checkValidSignatureAndMethodId = getMethodId('checkValidSignatureAndMethod', 'address', 'bytes');
-    this.uintValueData = stripAndPadHexValue(web3.toHex(this.uintValue), 32);
-    this.authorizedUserData = stripAndPadHexValue(authorizedUser, 32);
-    this.bytesValue = web3.toHex('bytesValue');
-    this.validateSignatureAndDataMsgData = [
-      getMethodId('checkValidSignatureAndData', 'address', 'bytes', 'uint256', 'bytes'),
-      stripAndPadHexValue(authorizedUser, 32),
-      stripAndPadHexValue(web3.toHex(32 * 4), 32), // bytesValue location
-      this.uintValueData,
-      stripAndPadHexValue(web3.toHex(32 * 6), 32), // sig location
-      stripAndPadHexValue(web3.toHex(this.bytesValue.substr(2).length / 2), 32), // bytesValue size
-      stripAndPadHexValue(this.bytesValue, 32, false), // bytesValue
-    ];
-  });
-
-  it('should have a default owner of self', async function () {
-    const theOwner = await this.bouncer.owner();
-    theOwner.should.eq(owner);
-  });
-
-  it('should allow owner to add a bouncer', async function () {
-    await this.bouncer.addBouncer(bouncerAddress, { from: owner });
-    const hasRole = await this.bouncer.hasRole(bouncerAddress, this.roleBouncer);
-    hasRole.should.eq(true);
-  });
-
-  it('should not allow anyone to add a bouncer', async function () {
-    await assertRevert(
-      this.bouncer.addBouncer(bouncerAddress, { from: anyone })
-    );
   });
 
-  context('modifiers', () => {
-    it('should allow valid signature for sender', async function () {
-      await this.bouncer.onlyWithValidSignature(
-        this.genSig(authorizedUser),
-        { from: authorizedUser }
-      );
-    });
-    it('should not allow invalid signature for sender', async function () {
-      await assertRevert(
-        this.bouncer.onlyWithValidSignature(
-          'abcd',
-          { from: authorizedUser }
-        )
-      );
-    });
-    it('should allow valid signature with a valid method for sender', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        getMethodId('onlyWithValidSignatureAndMethod', 'bytes')
-      )(authorizedUser);
-      await this.bouncer.onlyWithValidSignatureAndMethod(
-        sig,
-        { from: authorizedUser }
-      );
-    });
-    it('should not allow invalid signature with method for sender', async function () {
-      await assertRevert(
-        this.bouncer.onlyWithValidSignatureAndMethod(
-          'abcd',
-          { from: authorizedUser }
-        )
-      );
-    });
-    it('should allow valid signature with a valid data for sender', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        [
-          getMethodId('onlyWithValidSignatureAndData', 'uint256', 'bytes'),
-          this.uintValueData,
-          stripAndPadHexValue(web3.toHex(64), 32),
-        ].join('')
-      )(authorizedUser);
-      await this.bouncer.onlyWithValidSignatureAndData(
-        this.uintValue,
-        sig,
-        { from: authorizedUser }
-      );
-    });
-    it('should not allow invalid signature with data for sender', async function () {
-      await assertRevert(
-        this.bouncer.onlyWithValidSignatureAndData(
-          this.uintValue,
-          'abcd',
-          { from: authorizedUser }
-        )
-      );
+  context('management', () => {
+    it('has a default owner of self', async function () {
+      (await this.bouncer.owner()).should.eq(owner);
     });
-  });
 
-  context('signatures', () => {
-    it('should accept valid message for valid user', async function () {
-      const isValid = await this.bouncer.checkValidSignature(
-        authorizedUser,
-        this.genSig(authorizedUser)
-      );
-      isValid.should.eq(true);
-    });
-    it('should not accept invalid message for valid user', async function () {
-      const isValid = await this.bouncer.checkValidSignature(
-        authorizedUser,
-        this.genSig(anyone)
-      );
-      isValid.should.eq(false);
+    it('allows the owner to add a bouncer', async function () {
+      await this.bouncer.addBouncer(bouncerAddress, { from: owner });
+      (await this.bouncer.hasRole(bouncerAddress, this.roleBouncer)).should.eq(true);
     });
-    it('should not accept invalid message for invalid user', async function () {
-      const isValid = await this.bouncer.checkValidSignature(
-        anyone,
-        'abcd'
-      );
-      isValid.should.eq(false);
-    });
-    it('should not accept valid message for invalid user', async function () {
-      const isValid = await this.bouncer.checkValidSignature(
-        anyone,
-        this.genSig(authorizedUser)
-      );
-      isValid.should.eq(false);
-    });
-    it('should accept valid message with valid method for valid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        getMethodId('checkValidSignatureAndMethod', 'address', 'bytes')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndMethod(
-        authorizedUser,
-        sig
-      );
-      isValid.should.eq(true);
-    });
-    it('should not accept valid message with an invalid method for valid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        getMethodId('invalidMethod', 'address', 'bytes')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndMethod(
-        authorizedUser,
-        sig
-      );
-      isValid.should.eq(false);
-    });
-    it('should not accept valid message with a valid method for an invalid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        this.checkValidSignatureAndMethodId
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndMethod(
-        anyone,
-        sig
-      );
-      isValid.should.eq(false);
-    });
-    it('should accept valid method with valid params for valid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        this.validateSignatureAndDataMsgData.join('')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndData(
-        authorizedUser,
-        this.bytesValue,
-        this.uintValue,
-        sig
-      );
-      isValid.should.eq(true);
-    });
-    it('should not accept an invalid method with valid params for valid user', async function () {
-      this.validateSignatureAndDataMsgData[0] = getMethodId('invalidMethod', 'address', 'bytes', 'uint256', 'bytes');
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        this.validateSignatureAndDataMsgData.join('')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndData(
-        authorizedUser,
-        this.bytesValue,
-        this.uintValue,
-        sig
-      );
-      isValid.should.eq(false);
-    });
-    it('should not accept valid method with invalid params for valid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        this.validateSignatureAndDataMsgData.join('')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndData(
-        authorizedUser,
-        this.bytesValue,
-        500,
-        sig
-      );
-      isValid.should.eq(false);
-    });
-    it('should not accept valid method with valid params for invalid user', async function () {
-      const sig = getSigner(
-        this.bouncer,
-        bouncerAddress,
-        this.validateSignatureAndDataMsgData.join('')
-      )(authorizedUser);
-      const isValid = await this.bouncer.checkValidSignatureAndData(
-        anyone,
-        this.bytesValue,
-        this.uintValue,
-        sig
-      );
-      isValid.should.eq(false);
-    });
-  });
 
-  context('management', () => {
-    it('should not allow anyone to add bouncers', async function () {
+    it('does not allow adding an invalid address', async function () {
       await assertRevert(
-        this.bouncer.addBouncer(newBouncer, { from: anyone })
+        this.bouncer.addBouncer('0x0', { from: owner })
       );
     });
 
-    it('should be able to add bouncers', async function () {
-      await this.bouncer.addBouncer(newBouncer, { from: owner })
-        .should.be.fulfilled;
+    it('allows the owner to remove a bouncer', async function () {
+      await this.bouncer.addBouncer(bouncerAddress, { from: owner });
+
+      await this.bouncer.removeBouncer(bouncerAddress, { from: owner });
+      (await this.bouncer.hasRole(bouncerAddress, this.roleBouncer)).should.eq(false);
     });
 
-    it('should not allow adding invalid address', async function () {
+    it('does not allow anyone to add a bouncer', async function () {
       await assertRevert(
-        this.bouncer.addBouncer('0x0', { from: owner })
+        this.bouncer.addBouncer(bouncerAddress, { from: anyone })
       );
     });
 
-    it('should not allow anyone to remove bouncer', async function () {
+    it('does not allow anyone to remove a bouncer', async function () {
+      await this.bouncer.addBouncer(bouncerAddress, { from: owner });
+
       await assertRevert(
-        this.bouncer.removeBouncer(newBouncer, { from: anyone })
+        this.bouncer.removeBouncer(bouncerAddress, { from: anyone })
       );
     });
+  });
 
-    it('should be able to remove bouncers', async function () {
-      await this.bouncer.removeBouncer(newBouncer, { from: owner })
-        .should.be.fulfilled;
+  context('with bouncer address', () => {
+    beforeEach(async function () {
+      await this.bouncer.addBouncer(bouncerAddress, { from: owner });
+      this.signFor = getBouncerSigner(this.bouncer, bouncerAddress);
+    });
+
+    describe('modifiers', () => {
+      context('plain signature', () => {
+        it('allows valid signature for sender', async function () {
+          await this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: authorizedUser });
+        });
+
+        it('does not allow invalid signature for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignature(INVALID_SIGNATURE, { from: authorizedUser })
+          );
+        });
+
+        it('does not allow valid signature for other sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: anyone })
+          );
+        });
+
+        it('does not allow valid signature for method for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser, 'onlyWithValidSignature'),
+              { from: authorizedUser })
+          );
+        });
+      });
+
+      context('method signature', () => {
+        it('allows valid signature with correct method for sender', async function () {
+          await this.bouncer.onlyWithValidSignatureAndMethod(
+            this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: authorizedUser }
+          );
+        });
+
+        it('does not allow invalid signature with correct method for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndMethod(INVALID_SIGNATURE, { from: authorizedUser })
+          );
+        });
+
+        it('does not allow valid signature with correct method for other sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndMethod(
+              this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: anyone }
+            )
+          );
+        });
+
+        it('does not allow valid method signature with incorrect method for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser, 'theWrongMethod'),
+              { from: authorizedUser })
+          );
+        });
+
+        it('does not allow valid non-method signature method for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser), { from: authorizedUser })
+          );
+        });
+      });
+
+      context('method and data signature', () => {
+        it('allows valid signature with correct method and data for sender', async function () {
+          await this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE,
+            this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), { from: authorizedUser }
+          );
+        });
+
+        it('does not allow invalid signature with correct method and data for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, INVALID_SIGNATURE, { from: authorizedUser })
+          );
+        });
+
+        it('does not allow valid signature with correct method and incorrect data for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE + 10,
+              this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]),
+              { from: authorizedUser }
+            )
+          );
+        });
+
+        it('does not allow valid signature with correct method and data for other sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE,
+              this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]),
+              { from: anyone }
+            )
+          );
+        });
+
+        it('does not allow valid non-method signature for sender', async function () {
+          await assertRevert(
+            this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE,
+              this.signFor(authorizedUser), { from: authorizedUser }
+            )
+          );
+        });
+      });
+    });
+
+    context('signature validation', () => {
+      context('plain signature', () => {
+        it('validates valid signature for valid user', async function () {
+          (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser))).should.eq(true);
+        });
+
+        it('does not validate invalid signature for valid user', async function () {
+          (await this.bouncer.checkValidSignature(authorizedUser, INVALID_SIGNATURE)).should.eq(false);
+        });
+
+        it('does not validate valid signature for anyone', async function () {
+          (await this.bouncer.checkValidSignature(anyone, this.signFor(authorizedUser))).should.eq(false);
+        });
+
+        it('does not validate valid signature for method for valid user', async function () {
+          (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser, 'checkValidSignature'))
+          ).should.eq(false);
+        });
+      });
+
+      context('method signature', () => {
+        it('validates valid signature with correct method for valid user', async function () {
+          (await this.bouncer.checkValidSignatureAndMethod(authorizedUser,
+            this.signFor(authorizedUser, 'checkValidSignatureAndMethod'))
+          ).should.eq(true);
+        });
+
+        it('does not validate invalid signature with correct method for valid user', async function () {
+          (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, INVALID_SIGNATURE)).should.eq(false);
+        });
+
+        it('does not validate valid signature with correct method for anyone', async function () {
+          (await this.bouncer.checkValidSignatureAndMethod(anyone,
+            this.signFor(authorizedUser, 'checkValidSignatureAndMethod'))
+          ).should.eq(false);
+        });
+
+        it('does not validate valid non-method signature with correct method for valid user', async function () {
+          (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, this.signFor(authorizedUser))
+          ).should.eq(false);
+        });
+      });
+
+      context('method and data signature', () => {
+        it('validates valid signature with correct method and data for valid user', async function () {
+          (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE,
+            this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE]))
+          ).should.eq(true);
+        });
+
+        it('does not validate invalid signature with correct method and data for valid user', async function () {
+          (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, INVALID_SIGNATURE)
+          ).should.eq(false);
+        });
+
+        it('does not validate valid signature with correct method and incorrect data for valid user',
+          async function () {
+            (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE + 10,
+              this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE]))
+            ).should.eq(false);
+          }
+        );
+
+        it('does not validate valid signature with correct method and data for anyone', async function () {
+          (await this.bouncer.checkValidSignatureAndData(anyone, BYTES_VALUE, UINT_VALUE,
+            this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE]))
+          ).should.eq(false);
+        });
+
+        it('does not validate valid non-method-data signature with correct method and data for valid user',
+          async function () {
+            (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE,
+              this.signFor(authorizedUser, 'checkValidSignatureAndData'))
+            ).should.eq(false);
+          }
+        );
+      });
     });
   });
 });

test/helpers/sign.js

@@ -1,4 +1,10 @@
 const utils = require('ethereumjs-util');
+const { soliditySha3 } = require('web3-utils');
+
+const REAL_SIGNATURE_SIZE = 2 * 65; // 65 bytes in hexadecimal string legnth
+const PADDED_SIGNATURE_SIZE = 2 * 96; // 96 bytes in hexadecimal string length
+
+const DUMMY_SIGNATURE = `0x${web3.padLeft('', REAL_SIGNATURE_SIZE)}`;
 
 /**
  * Hash and add same prefix to the hash that ganache use.
@@ -11,18 +17,63 @@ function hashMessage (message) {
   return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex])));
 }
 
-// signs message using web3 (auto-applies prefix)
-function signMessage (signer, message = '', options = {}) {
-  return web3.eth.sign(signer, web3.sha3(message, options));
-}
+// signs message in node (auto-applies prefix)
+// message must be in hex already! will not be autoconverted!
+const signMessage = (signer, message = '') => {
+  return web3.eth.sign(signer, message);
+};
 
-// signs hex string using web3 (auto-applies prefix)
-function signHex (signer, message = '') {
-  return signMessage(signer, message, { encoding: 'hex' });
-}
+// @TODO - remove this when we migrate to web3-1.0.0
+const transformToFullName = function (json) {
+  if (json.name.indexOf('(') !== -1) {
+    return json.name;
+  }
+
+  var typeName = json.inputs.map(function (i) { return i.type; }).join();
+  return json.name + '(' + typeName + ')';
+};
+
+/**
+ * Create a signer between a contract and a signer for a voucher of method, args, and redeemer
+ * Note that `method` is the web3 method, not the truffle-contract method
+ * Well truffle is terrible, but luckily (?) so is web3 < 1.0, so we get to make our own method id
+ *   fetcher because the method on the contract isn't actually the SolidityFunction object ಠ_ಠ
+ * @param contract TruffleContract
+ * @param signer address
+ * @param redeemer address
+ * @param methodName string
+ * @param methodArgs any[]
+ */
+const getBouncerSigner = (contract, signer) => (redeemer, methodName, methodArgs = []) => {
+  const parts = [
+    contract.address,
+    redeemer,
+  ];
+
+  // if we have a method, add it to the parts that we're signing
+  if (methodName) {
+    if (methodArgs.length > 0) {
+      parts.push(
+        contract.contract[methodName].getData(...methodArgs.concat([DUMMY_SIGNATURE])).slice(
+          0,
+          -1 * PADDED_SIGNATURE_SIZE
+        )
+      );
+    } else {
+      const abi = contract.abi.find(abi => abi.name === methodName);
+      const name = transformToFullName(abi);
+      const signature = web3.sha3(name).slice(0, 10);
+      parts.push(signature);
+    }
+  }
+
+  // ^ substr to remove `0x` because in solidity the address is a set of byes, not a string `0xabcd`
+  const hashOfMessage = soliditySha3(...parts);
+  return signMessage(signer, hashOfMessage);
+};
 
 module.exports = {
   hashMessage,
   signMessage,
-  signHex,
+  getBouncerSigner,
 };

test/introspection/SupportsInterfaceWithLookup.test.js

@@ -8,7 +8,7 @@ require('chai')
   .should();
 
 contract('SupportsInterfaceWithLookup', function (accounts) {
-  before(async function () {
+  beforeEach(async function () {
     this.mock = await SupportsInterfaceWithLookup.new();
   });
 

test/library/ECRecovery.test.js

@@ -11,7 +11,7 @@ contract('ECRecovery', function (accounts) {
   let ecrecovery;
   const TEST_MESSAGE = 'OpenZeppelin';
 
-  before(async function () {
+  beforeEach(async function () {
     ecrecovery = await ECRecoveryMock.new();
   });
 
@@ -37,7 +37,7 @@ contract('ECRecovery', function (accounts) {
 
   it('recover using web3.eth.sign()', async function () {
     // Create the signature using account[0]
-    const signature = signMessage(accounts[0], TEST_MESSAGE);
+    const signature = signMessage(accounts[0], web3.sha3(TEST_MESSAGE));
 
     // Recover the signer address from the generated message and signature.
     const addrRecovered = await ecrecovery.recover(
@@ -49,7 +49,7 @@ contract('ECRecovery', function (accounts) {
 
   it('recover using web3.eth.sign() should return wrong signer', async function () {
     // Create the signature using account[0]
-    const signature = signMessage(accounts[0], TEST_MESSAGE);
+    const signature = signMessage(accounts[0], web3.sha3(TEST_MESSAGE));
 
     // Recover the signer address from the generated message and wrong signature.
     const addrRecovered = await ecrecovery.recover(hashMessage('Nope'), signature);

test/library/Math.test.js

@@ -3,7 +3,7 @@ var MathMock = artifacts.require('MathMock');
 contract('Math', function (accounts) {
   let math;
 
-  before(async function () {
+  beforeEach(async function () {
     math = await MathMock.new();
   });
 

test/library/MerkleProof.test.js

@@ -6,7 +6,7 @@ var MerkleProofWrapper = artifacts.require('MerkleProofWrapper');
 contract('MerkleProof', function (accounts) {
   let merkleProof;
 
-  before(async function () {
+  beforeEach(async function () {
     merkleProof = await MerkleProofWrapper.new();
   });
 

test/lifecycle/Destructible.test.js

@@ -2,20 +2,21 @@ var Destructible = artifacts.require('Destructible');
 const { ethGetBalance } = require('../helpers/web3');
 
 contract('Destructible', function (accounts) {
+  beforeEach(async function () {
+    this.destructible = await Destructible.new({ from: accounts[0], value: web3.toWei('10', 'ether') });
+    this.owner = await this.destructible.owner();
+  });
+
   it('should send balance to owner after destruction', async function () {
-    let destructible = await Destructible.new({ from: accounts[0], value: web3.toWei('10', 'ether') });
-    let owner = await destructible.owner();
-    let initBalance = await ethGetBalance(owner);
-    await destructible.destroy({ from: owner });
-    let newBalance = await ethGetBalance(owner);
+    let initBalance = await ethGetBalance(this.owner);
+    await this.destructible.destroy({ from: this.owner });
+    let newBalance = await ethGetBalance(this.owner);
     assert.isTrue(newBalance > initBalance);
   });
 
   it('should send balance to recepient after destruction', async function () {
-    let destructible = await Destructible.new({ from: accounts[0], value: web3.toWei('10', 'ether') });
-    let owner = await destructible.owner();
     let initBalance = await ethGetBalance(accounts[1]);
-    await destructible.destroyAndSend(accounts[1], { from: owner });
+    await this.destructible.destroyAndSend(accounts[1], { from: this.owner });
     let newBalance = await ethGetBalance(accounts[1]);
     assert.isTrue(newBalance.greaterThan(initBalance));
   });

test/lifecycle/Pausable.test.js

@@ -2,61 +2,59 @@ const { assertRevert } = require('../helpers/assertRevert');
 const PausableMock = artifacts.require('PausableMock');
 
 contract('Pausable', function (accounts) {
+  beforeEach(async function () {
+    this.Pausable = await PausableMock.new();
+  });
+
   it('can perform normal process in non-pause', async function () {
-    let Pausable = await PausableMock.new();
-    let count0 = await Pausable.count();
+    let count0 = await this.Pausable.count();
     assert.equal(count0, 0);
 
-    await Pausable.normalProcess();
-    let count1 = await Pausable.count();
+    await this.Pausable.normalProcess();
+    let count1 = await this.Pausable.count();
     assert.equal(count1, 1);
   });
 
   it('can not perform normal process in pause', async function () {
-    let Pausable = await PausableMock.new();
-    await Pausable.pause();
-    let count0 = await Pausable.count();
+    await this.Pausable.pause();
+    let count0 = await this.Pausable.count();
     assert.equal(count0, 0);
 
-    await assertRevert(Pausable.normalProcess());
-    let count1 = await Pausable.count();
+    await assertRevert(this.Pausable.normalProcess());
+    let count1 = await this.Pausable.count();
     assert.equal(count1, 0);
   });
 
   it('can not take drastic measure in non-pause', async function () {
-    let Pausable = await PausableMock.new();
-    await assertRevert(Pausable.drasticMeasure());
-    const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
+    await assertRevert(this.Pausable.drasticMeasure());
+    const drasticMeasureTaken = await this.Pausable.drasticMeasureTaken();
     assert.isFalse(drasticMeasureTaken);
   });
 
   it('can take a drastic measure in a pause', async function () {
-    let Pausable = await PausableMock.new();
-    await Pausable.pause();
-    await Pausable.drasticMeasure();
-    let drasticMeasureTaken = await Pausable.drasticMeasureTaken();
+    await this.Pausable.pause();
+    await this.Pausable.drasticMeasure();
+    let drasticMeasureTaken = await this.Pausable.drasticMeasureTaken();
 
     assert.isTrue(drasticMeasureTaken);
   });
 
   it('should resume allowing normal process after pause is over', async function () {
-    let Pausable = await PausableMock.new();
-    await Pausable.pause();
-    await Pausable.unpause();
-    await Pausable.normalProcess();
-    let count0 = await Pausable.count();
+    await this.Pausable.pause();
+    await this.Pausable.unpause();
+    await this.Pausable.normalProcess();
+    let count0 = await this.Pausable.count();
 
     assert.equal(count0, 1);
   });
 
   it('should prevent drastic measure after pause is over', async function () {
-    let Pausable = await PausableMock.new();
-    await Pausable.pause();
-    await Pausable.unpause();
+    await this.Pausable.pause();
+    await this.Pausable.unpause();
 
-    await assertRevert(Pausable.drasticMeasure());
+    await assertRevert(this.Pausable.drasticMeasure());
 
-    const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
+    const drasticMeasureTaken = await this.Pausable.drasticMeasureTaken();
     assert.isFalse(drasticMeasureTaken);
   });
 });

test/lifecycle/TokenDestructible.test.js

@@ -5,16 +5,18 @@ var StandardTokenMock = artifacts.require('StandardTokenMock');
 
 contract('TokenDestructible', function (accounts) {
   let destructible;
+  let owner;
 
   beforeEach(async function () {
     destructible = await TokenDestructible.new({
       from: accounts[0],
       value: web3.toWei('10', 'ether'),
     });
+
+    owner = await destructible.owner();
   });
 
   it('should send balance to owner after destruction', async function () {
-    let owner = await destructible.owner();
     let initBalance = await ethGetBalance(owner);
     await destructible.destroy([], { from: owner });
     let newBalance = await ethGetBalance(owner);
@@ -22,7 +24,6 @@ contract('TokenDestructible', function (accounts) {
   });
 
   it('should send tokens to owner after destruction', async function () {
-    let owner = await destructible.owner();
     let token = await StandardTokenMock.new(destructible.address, 100);
     let initContractBalance = await token.balanceOf(destructible.address);
     let initOwnerBalance = await token.balanceOf(owner);

test/math/SafeMath.test.js

@@ -9,7 +9,7 @@ require('chai')
 contract('SafeMath', () => {
   const MAX_UINT = new BigNumber('115792089237316195423570985008687907853269984665640564039457584007913129639935');
 
-  before(async function () {
+  beforeEach(async function () {
     this.safeMath = await SafeMathMock.new();
   });
 

test/ownership/HasNoEther.test.js

@@ -7,7 +7,7 @@ const ForceEther = artifacts.require('ForceEther');
 contract('HasNoEther', function (accounts) {
   const amount = web3.toWei('1', 'ether');
 
-  it('should be constructorable', async function () {
+  it('should be constructible', async function () {
     await HasNoEtherTest.new();
   });
 

test/ownership/Whitelist.test.js

@@ -17,7 +17,7 @@ contract('Whitelist', function (accounts) {
 
   const whitelistedAddresses = [whitelistedAddress1, whitelistedAddress2];
 
-  before(async function () {
+  beforeEach(async function () {
     this.mock = await WhitelistMock.new();
     this.role = await this.mock.ROLE_WHITELISTED();
   });

test/ownership/rbac/RBAC.test.js

@@ -19,7 +19,7 @@ contract('RBAC', function (accounts) {
     ...advisors
   ] = accounts;
 
-  before(async () => {
+  beforeEach(async () => {
     mock = await RBACMock.new(advisors, { from: admin });
   });
 

test/proposals/ERC1046/TokenMetadata.test.js

@@ -7,7 +7,7 @@ require('chai')
 const metadataURI = 'https://example.com';
 
 describe('ERC20WithMetadata', function () {
-  before(async function () {
+  beforeEach(async function () {
     this.token = await ERC20WithMetadata.new(metadataURI);
   });