Merge 5109560 into 8f11c7d

contracts/libraries/Message.sol

@@ -96,31 +96,19 @@ library Message {
         }
     }
 
-    function hasEnoughValidSignatures(
-        bytes _message,
-        uint8[] _vs,
-        bytes32[] _rs,
-        bytes32[] _ss,
-        IBridgeValidators _validatorContract,
-        bool isAMBMessage
-    ) internal view {
-        require(isAMBMessage || isMessageValid(_message));
-        uint256 requiredSignatures = _validatorContract.requiredSignatures();
-        // It is not necessary to check that arrays have the same length since it will be handled
-        // during attempt to access to the corresponding elements in the loop and the call will be reverted.
-        // It will save gas for the rational validators actions and still be safe enough from security point of view
-        require(_vs.length >= requiredSignatures);
-        bytes32 hash = hashMessage(_message, isAMBMessage);
-        address[] memory encounteredAddresses = new address[](requiredSignatures);
-
-        for (uint256 i = 0; i < requiredSignatures; i++) {
-            address recoveredAddress = ecrecover(hash, _vs[i], _rs[i], _ss[i]);
-            require(_validatorContract.isValidator(recoveredAddress));
-            require(!addressArrayContains(encounteredAddresses, recoveredAddress));
-            encounteredAddresses[i] = recoveredAddress;
-        }
-    }
-
+    /**
+    * @dev Validates provided signatures, only first requiredSignatures() number
+    * of signatures are going to be validated, these signatures should be from different validators.
+    * @param _message bytes message used to generate signatures
+    * @param _signatures bytes blob with signatures to be validated.
+    * First byte X is a number of signatures in a blob,
+    * next X bytes are v components of signatures,
+    * next 32 * X bytes are r components of signatures,
+    * next 32 * X bytes are s components of signatures.
+    * @param _validatorContract contract, which conforms to the IBridgeValidators interface,
+    * where info about current validators and required signatures is stored.
+    * @param isAMBMessage true if _message is an AMB message with arbitrary length.
+    */
     function hasEnoughValidSignatures(
         bytes _message,
         bytes _signatures,

contracts/upgradeable_contracts/BasicForeignBridge.sol

@@ -15,8 +15,14 @@ contract BasicForeignBridge is EternalStorage, Validatable, BasicBridge, BasicTo
     event RelayedMessage(address recipient, uint256 value, bytes32 transactionHash);
     event UserRequestForAffirmation(address recipient, uint256 value);
 
-    function executeSignatures(uint8[] vs, bytes32[] rs, bytes32[] ss, bytes message) external {
-        Message.hasEnoughValidSignatures(message, vs, rs, ss, validatorContract(), false);
+    /**
+    * @dev Validates provided signatures and relays a given message
+    * @param message bytes to be relayed
+    * @param signatures bytes blob with signatures to be validated
+    */
+    function executeSignatures(bytes message, bytes signatures) external {
+        Message.hasEnoughValidSignatures(message, signatures, validatorContract(), false);
+
         address recipient;
         uint256 amount;
         bytes32 txHash;

contracts/upgradeable_contracts/VersionableBridge.sol

@@ -2,7 +2,7 @@ pragma solidity 0.4.24;
 
 contract VersionableBridge {
     function getBridgeInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) {
-        return (2, 5, 0);
+        return (3, 0, 0);
     }
 
     /* solcov ignore next */

contracts/upgradeable_contracts/arbitrary_message/BasicForeignAMB.sol

@@ -8,6 +8,11 @@ import "./MessageProcessor.sol";
 import "../MessageRelay.sol";
 
 contract BasicForeignAMB is BasicAMB, MessageRelay, MessageDelivery, MessageProcessor {
+    /**
+    * @dev Validates provided signatures and relays a given message
+    * @param _data bytes to be relayed
+    * @param _signatures bytes blob with signatures to be validated
+    */
     function executeSignatures(bytes _data, bytes _signatures) external {
         Message.hasEnoughValidSignatures(_data, _signatures, validatorContract(), true);
 

test/arbitrary_message/foreign_bridge.test.js

@@ -9,7 +9,7 @@ const {
   ether,
   expectEventInLogs,
   addTxHashToAMBData,
-  signatureToVRSAMB,
+  signatureToVRS,
   packSignatures,
   createFullAccounts
 } = require('../helpers/helpers')
@@ -244,7 +244,7 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature = await sign(authorities[0], message)
-      const vrs = signatureToVRSAMB(signature)
+      const vrs = signatureToVRS(signature)
       const signatures = packSignatures([vrs])
 
       const { logs } = await foreignBridge.executeSignatures(message, signatures, {
@@ -300,13 +300,13 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature1 = await sign(authoritiesFiveAccs[0], message)
-      const vrs = signatureToVRSAMB(signature1)
+      const vrs = signatureToVRS(signature1)
 
       const signature2 = await sign(authoritiesFiveAccs[1], message)
-      const vrs2 = signatureToVRSAMB(signature2)
+      const vrs2 = signatureToVRS(signature2)
 
       const signature3 = await sign(authoritiesFiveAccs[2], message)
-      const vrs3 = signatureToVRSAMB(signature3)
+      const vrs3 = signatureToVRS(signature3)
       const oneSignature = packSignatures([vrs])
       const twoSignatures = packSignatures([vrs, vrs2])
       const threeSignatures = packSignatures([vrs, vrs2, vrs3])
@@ -376,7 +376,7 @@ contract('ForeignAMB', async accounts => {
       const vrsList = []
       for (let i = 0; i < MAX_SIGNATURES; i++) {
         const { signature } = await authorities[i].sign(message)
-        vrsList[i] = signatureToVRSAMB(signature)
+        vrsList[i] = signatureToVRS(signature)
       }
 
       const signatures = packSignatures(vrsList)
@@ -400,7 +400,7 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature = await sign(authorities[0], message)
-      const vrs = signatureToVRSAMB(signature)
+      const vrs = signatureToVRS(signature)
       const signatures = packSignatures([vrs])
 
       const { logs } = await foreignBridge.executeSignatures(message, signatures, {
@@ -434,7 +434,7 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature = await sign(authorities[0], message)
-      const vrs = signatureToVRSAMB(signature)
+      const vrs = signatureToVRS(signature)
       const signatures = packSignatures([vrs])
 
       const { logs } = await foreignBridge.executeSignatures(message, signatures, {
@@ -467,7 +467,7 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature = await sign(authorities[0], message)
-      const vrs = signatureToVRSAMB(signature)
+      const vrs = signatureToVRS(signature)
       const signatures = packSignatures([vrs])
 
       const { logs } = await foreignBridge.executeSignatures(message, signatures, {
@@ -509,7 +509,7 @@ contract('ForeignAMB', async accounts => {
       const message = addTxHashToAMBData(encodedData, resultPassMessageTx.tx)
 
       const signature = await sign(authorities[0], message)
-      const vrs = signatureToVRSAMB(signature)
+      const vrs = signatureToVRS(signature)
       const signatures = packSignatures([vrs])
 
       const { logs } = await foreignBridge.executeSignatures(message, signatures, {

test/erc_to_erc/foreign_bridge.test.js

@@ -14,7 +14,8 @@ const {
   ether,
   getEvents,
   expectEventInLogs,
-  createFullAccounts
+  createFullAccounts,
+  packSignatures
 } = require('../helpers/helpers')
 
 const oneEther = ether('1')
@@ -217,8 +218,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, value, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
       false.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
-      const { logs } = await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.fulfilled
+      const { logs } = await foreignBridge.executeSignatures(message, oneSignature).should.be.fulfilled
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipientAccount)
       logs[0].args.value.should.be.bignumber.equal(value)
@@ -237,16 +239,18 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, value, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
       false.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
-      await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.fulfilled
+      await foreignBridge.executeSignatures(message, oneSignature).should.be.fulfilled
       // tx 2
       await token.mint(foreignBridge.address, value)
       const transactionHash2 = '0x77a496628a776a03d58d7e6059a5937f04bebd8ba4ff89f76dd4bb8ba7e291ee'
       const message2 = createMessage(recipientAccount, value, transactionHash2, foreignBridge.address)
       const signature2 = await sign(authorities[0], message2)
       const vrs2 = signatureToVRS(signature2)
+      const oneSignature2 = packSignatures([vrs2])
       false.should.be.equal(await foreignBridge.relayedMessages(transactionHash2))
-      const { logs } = await foreignBridge.executeSignatures([vrs2.v], [vrs2.r], [vrs2.s], message2).should.be.fulfilled
+      const { logs } = await foreignBridge.executeSignatures(message2, oneSignature2).should.be.fulfilled
 
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipientAccount)
@@ -264,15 +268,17 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, value, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
       false.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
-      await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.fulfilled
+      await foreignBridge.executeSignatures(message, oneSignature).should.be.fulfilled
       // tx 2
       await token.mint(foreignBridge.address, value)
       const message2 = createMessage(accounts[4], value, transactionHash, foreignBridge.address)
       const signature2 = await sign(authorities[0], message2)
       const vrs2 = signatureToVRS(signature2)
+      const oneSignature2 = packSignatures([vrs2])
       true.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
-      await foreignBridge.executeSignatures([vrs2.v], [vrs2.r], [vrs2.s], message2).should.be.rejectedWith(ERROR_MSG)
+      await foreignBridge.executeSignatures(message2, oneSignature2).should.be.rejectedWith(ERROR_MSG)
     })
 
     it('should not allow withdraw over home max tx limit', async () => {
@@ -284,8 +290,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, invalidValue, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
 
-      await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.rejectedWith(ERROR_MSG)
+      await foreignBridge.executeSignatures(message, oneSignature).should.be.rejectedWith(ERROR_MSG)
     })
 
     it('should not allow withdraw over daily home limit', async () => {
@@ -296,22 +303,25 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, halfEther, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
 
-      await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.fulfilled
+      await foreignBridge.executeSignatures(message, oneSignature).should.be.fulfilled
 
       const transactionHash2 = '0x69debd8fd1923c9cb3cd8ef6461e2740b2d037943b941729d5a47671a2bb8712'
       const message2 = createMessage(recipientAccount, halfEther, transactionHash2, foreignBridge.address)
       const signature2 = await sign(authorities[0], message2)
       const vrs2 = signatureToVRS(signature2)
+      const oneSignature2 = packSignatures([vrs2])
 
-      await foreignBridge.executeSignatures([vrs2.v], [vrs2.r], [vrs2.s], message2).should.be.fulfilled
+      await foreignBridge.executeSignatures(message2, oneSignature2).should.be.fulfilled
 
       const transactionHash3 = '0x022695428093bb292db8e48bd1417c5e1b84c0bf673bd0fff23ed0fb6495b872'
       const message3 = createMessage(recipientAccount, halfEther, transactionHash3, foreignBridge.address)
       const signature3 = await sign(authorities[0], message3)
       const vrs3 = signatureToVRS(signature3)
+      const oneSignature3 = packSignatures([vrs3])
 
-      await foreignBridge.executeSignatures([vrs3.v], [vrs3.r], [vrs3.s], message3).should.be.rejectedWith(ERROR_MSG)
+      await foreignBridge.executeSignatures(message3, oneSignature3).should.be.rejectedWith(ERROR_MSG)
     })
   })
   describe('#withdraw with 2 minimum signatures', async () => {
@@ -349,19 +359,15 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, value, transactionHash, foreignBridgeWithMultiSignatures.address)
       const signature = await sign(twoAuthorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
       false.should.be.equal(await foreignBridgeWithMultiSignatures.relayedMessages(transactionHash))
-      await foreignBridgeWithMultiSignatures
-        .executeSignatures([vrs.v], [vrs.r], [vrs.s], message)
-        .should.be.rejectedWith(ERROR_MSG)
+      await foreignBridgeWithMultiSignatures.executeSignatures(message, oneSignature).should.be.rejectedWith(ERROR_MSG)
       // msg 2
       const signature2 = await sign(twoAuthorities[1], message)
       const vrs2 = signatureToVRS(signature2)
-      const { logs } = await foreignBridgeWithMultiSignatures.executeSignatures(
-        [vrs.v, vrs2.v],
-        [vrs.r, vrs2.r],
-        [vrs.s, vrs2.s],
-        message
-      ).should.be.fulfilled
+      const twoSignatures = packSignatures([vrs, vrs2])
+      const { logs } = await foreignBridgeWithMultiSignatures.executeSignatures(message, twoSignatures).should.be
+        .fulfilled
 
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipientAccount)
@@ -374,10 +380,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, value, transactionHash, foreignBridgeWithMultiSignatures.address)
       const signature = await sign(twoAuthorities[0], message)
       const vrs = signatureToVRS(signature)
+      const twoSignatures = packSignatures([vrs, vrs])
       false.should.be.equal(await foreignBridgeWithMultiSignatures.relayedMessages(transactionHash))
-      await foreignBridgeWithMultiSignatures
-        .executeSignatures([vrs.v, vrs.v], [vrs.r, vrs.r], [vrs.s, vrs.s], message)
-        .should.be.rejectedWith(ERROR_MSG)
+      await foreignBridgeWithMultiSignatures.executeSignatures(message, twoSignatures).should.be.rejectedWith(ERROR_MSG)
     })
 
     it('works with 5 validators and 3 required signatures', async () => {
@@ -417,12 +422,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const signature3 = await sign(authoritiesFiveAccs[2], message)
       const vrs3 = signatureToVRS(signature3)
 
-      const { logs } = await foreignBridgeWithThreeSigs.executeSignatures(
-        [vrs.v, vrs2.v, vrs3.v],
-        [vrs.r, vrs2.r, vrs3.r],
-        [vrs.s, vrs2.s, vrs3.s],
-        message
-      ).should.be.fulfilled
+      const threeSignatures = packSignatures([vrs, vrs2, vrs3])
+
+      const { logs } = await foreignBridgeWithThreeSigs.executeSignatures(message, threeSignatures).should.be.fulfilled
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipient)
       logs[0].args.value.should.be.bignumber.equal(value)
@@ -462,12 +464,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
         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
+      const maxSignatures = packSignatures(vrsList)
+
+      const { receipt } = await foreignBridgeWithMaxSigs.executeSignatures(message, maxSignatures).should.be.fulfilled
       expect(receipt.gasUsed).to.be.lte(MAX_GAS)
     })
   })
@@ -798,8 +797,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const message = createMessage(recipientAccount, valueOnHome, transactionHash, foreignBridge.address)
       const signature = await sign(authorities[0], message)
       const vrs = signatureToVRS(signature)
+      const oneSignature = packSignatures([vrs])
       false.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
-      const { logs } = await foreignBridge.executeSignatures([vrs.v], [vrs.r], [vrs.s], message).should.be.fulfilled
+      const { logs } = await foreignBridge.executeSignatures(message, oneSignature).should.be.fulfilled
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipientAccount)
       logs[0].args.value.should.be.bignumber.equal(valueOnHome)
@@ -853,12 +853,9 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
       const signature3 = await sign(authoritiesFiveAccs[2], message)
       const vrs3 = signatureToVRS(signature3)
 
-      const { logs } = await foreignBridgeWithThreeSigs.executeSignatures(
-        [vrs.v, vrs2.v, vrs3.v],
-        [vrs.r, vrs2.r, vrs3.r],
-        [vrs.s, vrs2.s, vrs3.s],
-        message
-      ).should.be.fulfilled
+      const threeSignatures = packSignatures([vrs, vrs2, vrs3])
+
+      const { logs } = await foreignBridgeWithThreeSigs.executeSignatures(message, threeSignatures).should.be.fulfilled
       logs[0].event.should.be.equal('RelayedMessage')
       logs[0].args.recipient.should.be.equal(recipient)
       logs[0].args.value.should.be.bignumber.equal(valueOnHome)