Tests to cover max number of signatures (#335)

* Fixed Message library bug
* Added tests for covering max number of signatures
* Increased heap memory limit for coverage script

contracts/libraries/Message.sol

@@ -129,9 +129,9 @@ library Message {
     ) internal view {
         require(isAMBMessage || (!isAMBMessage && isMessageValid(_message)));
         uint256 requiredSignatures = _validatorContract.requiredSignatures();
-        uint8 amount;
+        uint256 amount;
         assembly {
-            amount := mload(add(_signatures, 1))
+            amount := and(mload(add(_signatures, 1)), 0xff)
         }
         require(amount >= requiredSignatures);
         bytes32 hash = hashMessage(_message, isAMBMessage);

scripts/test.sh

@@ -56,7 +56,7 @@ if [ "$SOLIDITY_COVERAGE" != true ]; then
 fi
 
 if [ "$SOLIDITY_COVERAGE" = true ]; then
-  node_modules/.bin/truffle test; istanbul report lcov
+  node --max-old-space-size=4096 node_modules/.bin/truffle test; istanbul report lcov
 
   if [ "$CONTINUOUS_INTEGRATION" = true ]; then
     cat coverage/lcov.info | node_modules/.bin/coveralls

test/arbitrary_message/foreign_bridge.test.js

@@ -10,7 +10,8 @@ const {
   expectEventInLogs,
   addTxHashToAMBData,
   signatureToVRSAMB,
-  packSignatures
+  packSignatures,
+  createFullAccounts
 } = require('../helpers/helpers')
 const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
 
@@ -19,6 +20,9 @@ const gasPrice = web3.utils.toWei('1', 'gwei')
 const oneEther = ether('1')
 const twoEther = ether('2')
 const ZERO = toBN(0)
+const MAX_VALIDATORS = 50
+const MAX_SIGNATURES = MAX_VALIDATORS
+const MAX_GAS = 8000000
 
 contract('ForeignAMB', async accounts => {
   let validatorContract
@@ -334,6 +338,55 @@ contract('ForeignAMB', async accounts => {
       expect(await box.txHash()).to.be.equal(resultPassMessageTx.tx)
       expect(await foreignBridge.messageSender()).to.be.equal(ZERO_ADDRESS)
     })
+    it('test with max allowed number of signatures required', async () => {
+      // set validator
+      const validatorContract = await BridgeValidators.new()
+      const authorities = createFullAccounts(web3, MAX_VALIDATORS)
+      const addresses = authorities.map(account => account.address)
+      const ownerOfValidators = accounts[0]
+      await validatorContract.initialize(MAX_SIGNATURES, addresses, ownerOfValidators)
+
+      // set bridge
+      const foreignBridgeWithMaxSigs = await ForeignBridge.new()
+      await foreignBridgeWithMaxSigs.initialize(
+        validatorContract.address,
+        oneEther,
+        gasPrice,
+        requiredBlockConfirmations,
+        owner
+      )
+
+      const user = accounts[8]
+
+      const boxInitialValue = await box.value()
+      boxInitialValue.should.be.bignumber.equal(ZERO)
+
+      // Use these calls to simulate home bridge on home network
+      const resultPassMessageTx = await foreignBridgeWithMaxSigs.requireToPassMessage(
+        box.address,
+        setValueData,
+        821254,
+        { from: user }
+      )
+
+      // Validator on token-bridge add txHash to message
+      const { encodedData } = resultPassMessageTx.logs[0].args
+      const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
+
+      const vrsList = []
+      for (let i = 0; i < MAX_SIGNATURES; i++) {
+        const { signature } = await authorities[i].sign(message)
+        vrsList[i] = signatureToVRSAMB(signature)
+      }
+
+      const signatures = packSignatures(vrsList)
+
+      const { receipt } = await foreignBridgeWithMaxSigs.executeSignatures(message, signatures, {
+        from: accounts[0],
+        gasPrice
+      }).should.be.fulfilled
+      expect(receipt.gasUsed).to.be.lte(MAX_GAS)
+    })
     it('should not allow to double execute signatures', async () => {
       const user = accounts[8]
 

test/erc_to_erc/foreign_bridge.test.js

@@ -7,7 +7,15 @@ const ERC677BridgeToken = artifacts.require('ERC677BridgeToken.sol')
 
 const { expect } = require('chai')
 const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
-const { createMessage, sign, signatureToVRS, ether, getEvents, expectEventInLogs } = require('../helpers/helpers')
+const {
+  createMessage,
+  sign,
+  signatureToVRS,
+  ether,
+  getEvents,
+  expectEventInLogs,
+  createFullAccounts
+} = require('../helpers/helpers')
 
 const oneEther = ether('1')
 const halfEther = ether('0.5')
@@ -19,6 +27,9 @@ const maxPerTx = halfEther
 const minPerTx = ether('0.01')
 const dailyLimit = oneEther
 const ZERO = toBN(0)
+const MAX_VALIDATORS = 50
+const MAX_SIGNATURES = MAX_VALIDATORS
+const MAX_GAS = 8000000
 const decimalShiftZero = 0
 
 contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
@@ -417,6 +428,48 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       logs[0].args.value.should.be.bignumber.equal(value)
       true.should.be.equal(await foreignBridgeWithThreeSigs.relayedMessages(txHash))
     })
+
+    it('works with max allowed number of signatures required', async () => {
+      const recipient = accounts[8]
+      const value = halfEther
+      const validatorContract = await BridgeValidators.new()
+      const authorities = createFullAccounts(web3, MAX_VALIDATORS)
+      const addresses = authorities.map(account => account.address)
+      const ownerOfValidators = accounts[0]
+
+      await validatorContract.initialize(MAX_SIGNATURES, addresses, ownerOfValidators)
+      const erc20Token = await ERC677BridgeToken.new('Some ERC20', 'RSZT', 18)
+      const foreignBridgeWithMaxSigs = await ForeignBridge.new()
+
+      await foreignBridgeWithMaxSigs.initialize(
+        validatorContract.address,
+        erc20Token.address,
+        requireBlockConfirmations,
+        gasPrice,
+        [dailyLimit, maxPerTx, minPerTx],
+        [homeDailyLimit, homeMaxPerTx],
+        owner,
+        decimalShiftZero
+      )
+      await erc20Token.mint(foreignBridgeWithMaxSigs.address, value)
+
+      const txHash = '0x35d3818e50234655f6aebb2a1cfbf30f59568d8a4ec72066fac5a25dbe7b8121'
+      const message = createMessage(recipient, value, txHash, foreignBridgeWithMaxSigs.address)
+
+      const vrsList = []
+      for (let i = 0; i < MAX_SIGNATURES; i++) {
+        const { signature } = await authorities[i].sign(message)
+        vrsList[i] = signatureToVRS(signature)
+      }
+
+      const { receipt } = await foreignBridgeWithMaxSigs.executeSignatures(
+        vrsList.map(vrs => vrs.v),
+        vrsList.map(vrs => vrs.r),
+        vrsList.map(vrs => vrs.s),
+        message
+      ).should.be.fulfilled
+      expect(receipt.gasUsed).to.be.lte(MAX_GAS)
+    })
   })
   describe('#upgradeable', async () => {
     it('can be upgraded', async () => {

test/erc_to_native/foreign_bridge.test.js

@@ -11,7 +11,15 @@ const ForeignBridgeErcToNativeMock = artifacts.require('ForeignBridgeErcToNative
 
 const { expect } = require('chai')
 const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
-const { createMessage, sign, signatureToVRS, ether, expectEventInLogs, getEvents } = require('../helpers/helpers')
+const {
+  createMessage,
+  sign,
+  signatureToVRS,
+  ether,
+  expectEventInLogs,
+  getEvents,
+  createFullAccounts
+} = require('../helpers/helpers')
 
 const halfEther = ether('0.5')
 const requireBlockConfirmations = 8
@@ -24,6 +32,9 @@ const dailyLimit = oneEther
 const maxPerTx = halfEther
 const minPerTx = ether('0.01')
 const ZERO = toBN(0)
+const MAX_VALIDATORS = 50
+const MAX_SIGNATURES = MAX_VALIDATORS
+const MAX_GAS = 8000000
 const decimalShiftZero = 0
 
 contract('ForeignBridge_ERC20_to_Native', async accounts => {
@@ -459,6 +470,48 @@ contract('ForeignBridge_ERC20_to_Native', async accounts => {
       logs[0].args.value.should.be.bignumber.equal(value)
       true.should.be.equal(await foreignBridgeWithThreeSigs.relayedMessages(txHash))
     })
+    it('works with max allowed number of signatures required', async () => {
+      const recipient = accounts[8]
+      const value = halfEther
+      const validatorContract = await BridgeValidators.new()
+      const authorities = createFullAccounts(web3, MAX_VALIDATORS)
+      const addresses = authorities.map(account => account.address)
+      const ownerOfValidators = accounts[0]
+
+      await validatorContract.initialize(MAX_SIGNATURES, addresses, ownerOfValidators)
+      const erc20Token = await ERC677BridgeToken.new('Some ERC20', 'RSZT', 18)
+      const foreignBridgeWithMaxSigs = await ForeignBridgeErcToNativeMock.new()
+
+      await foreignBridgeWithMaxSigs.initialize(
+        validatorContract.address,
+        erc20Token.address,
+        requireBlockConfirmations,
+        gasPrice,
+        [dailyLimit, maxPerTx, minPerTx],
+        [homeDailyLimit, homeMaxPerTx],
+        owner,
+        decimalShiftZero,
+        otherSideBridge.address
+      )
+      await erc20Token.mint(foreignBridgeWithMaxSigs.address, value)
+
+      const txHash = '0x35d3818e50234655f6aebb2a1cfbf30f59568d8a4ec72066fac5a25dbe7b8121'
+      const message = createMessage(recipient, value, txHash, foreignBridgeWithMaxSigs.address)
+
+      const vrsList = []
+      for (let i = 0; i < MAX_SIGNATURES; i++) {
+        const { signature } = await authorities[i].sign(message)
+        vrsList[i] = signatureToVRS(signature)
+      }
+
+      const { receipt } = await foreignBridgeWithMaxSigs.executeSignatures(
+        vrsList.map(vrs => vrs.v),
+        vrsList.map(vrs => vrs.r),
+        vrsList.map(vrs => vrs.s),
+        message
+      ).should.be.fulfilled
+      expect(receipt.gasUsed).to.be.lte(MAX_GAS)
+    })
   })
   describe('#upgradeable', async () => {
     it('can be upgraded', async () => {

test/helpers/helpers.js

@@ -198,6 +198,16 @@ function createAccounts(web3, amount) {
 
 module.exports.createAccounts = createAccounts
 
+function createFullAccounts(web3, amount) {
+  const array = []
+  for (let i = 0; i < amount; i++) {
+    array[i] = web3.eth.accounts.create()
+  }
+  return array
+}
+
+module.exports.createFullAccounts = createFullAccounts
+
 function addTxHashToAMBData(encodedData, transactionHash) {
   return encodedData.slice(0, 2) + strip0x(transactionHash) + encodedData.slice(2)
 }

test/native_to_erc/foreign_bridge_test.js

@@ -17,7 +17,8 @@ const {
   getEvents,
   ether,
   expectEventInLogs,
-  createAccounts
+  createAccounts,
+  createFullAccounts
 } = require('../helpers/helpers')
 
 const oneEther = ether('1')
@@ -30,6 +31,7 @@ const homeMaxPerTx = halfEther
 const ZERO = toBN(0)
 const MAX_GAS = 8000000
 const MAX_VALIDATORS = 50
+const MAX_SIGNATURES = MAX_VALIDATORS
 const decimalShiftZero = 0
 
 contract('ForeignBridge', async accounts => {
@@ -432,6 +434,48 @@ contract('ForeignBridge', async accounts => {
       logs[0].args.value.should.be.bignumber.equal(value)
       true.should.be.equal(await foreignBridgeWithThreeSigs.relayedMessages(txHash))
     })
+    it('works with max allowed number of signatures required', async () => {
+      const recipient = accounts[8]
+      const value = halfEther
+      const validatorContract = await BridgeValidators.new()
+      const authorities = createFullAccounts(web3, MAX_VALIDATORS)
+      const addresses = authorities.map(account => account.address)
+      const ownerOfValidators = accounts[0]
+
+      await validatorContract.initialize(MAX_SIGNATURES, addresses, ownerOfValidators)
+      const erc20Token = await POA20.new('Some ERC20', 'RSZT', 18)
+      const foreignBridgeWithMaxSigs = await ForeignBridge.new()
+
+      await foreignBridgeWithMaxSigs.initialize(
+        validatorContract.address,
+        erc20Token.address,
+        [oneEther, halfEther, minPerTx],
+        gasPrice,
+        requireBlockConfirmations,
+        [homeDailyLimit, homeMaxPerTx],
+        owner,
+        decimalShiftZero,
+        otherSideBridgeAddress
+      )
+      await erc20Token.transferOwnership(foreignBridgeWithMaxSigs.address)
+
+      const txHash = '0x35d3818e50234655f6aebb2a1cfbf30f59568d8a4ec72066fac5a25dbe7b8121'
+      const message = createMessage(recipient, value, txHash, foreignBridgeWithMaxSigs.address)
+
+      const vrsList = []
+      for (let i = 0; i < MAX_SIGNATURES; i++) {
+        const { signature } = await authorities[i].sign(message)
+        vrsList[i] = signatureToVRS(signature)
+      }
+
+      const { receipt } = await foreignBridgeWithMaxSigs.executeSignatures(
+        vrsList.map(vrs => vrs.v),
+        vrsList.map(vrs => vrs.r),
+        vrsList.map(vrs => vrs.s),
+        message
+      ).should.be.fulfilled
+      expect(receipt.gasUsed).to.be.lte(MAX_GAS)
+    })
     it('Should fail if length of signatures is not equal', async () => {
       const recipient = accounts[8]
       const authoritiesFiveAccs = [accounts[1], accounts[2], accounts[3], accounts[4], accounts[5]]