verifySignatures -> hasEnoughValidSignatures and make it return bool

contracts/bridge.sol

@@ -36,25 +36,32 @@ library Helpers {
         return string(result);
     }
 
-    /// reverts unless signatures (whose components are in `vs`, `rs`, `ss`)
+    /// returns whether signatures (whose components are in `vs`, `rs`, `ss`)
     /// contain `requiredSignatures` distinct correct signatures
-    /// where signer is in `addresses`
+    /// where signer is in `allowed_signers`
     /// that signed `message`
-    function verifySignatures(bytes message, uint8[] vs, bytes32[] rs, bytes32[] ss, address[] addresses, uint requiredSignatures) internal pure {
+    function hasEnoughValidSignatures(bytes message, uint8[] vs, bytes32[] rs, bytes32[] ss, address[] allowed_signers, uint requiredSignatures) internal pure returns (bool) {
         // not enough signatures
-        require(requiredSignatures <= vs.length);
+        if (vs.length < requiredSignatures) {
+            return false;
+        }
 
         var hash = MessageSigning.hashMessage(message);
-        var encountered = new address[](addresses.length);
+        var encountered_addresses = new address[](allowed_signers.length);
 
         for (uint i = 0; i < requiredSignatures; i++) {
-            var a = ecrecover(hash, vs[i], rs[i], ss[i]);
+            var recovered_address = ecrecover(hash, vs[i], rs[i], ss[i]);
             // only signatures by addresses in `addresses` are allowed
-            require(addressArrayContains(addresses, a));
+            if (!addressArrayContains(allowed_signers, recovered_address)) {
+                return false;
+            }
             // duplicate signatures are not allowed
-            require(!addressArrayContains(encountered, a));
+            if (addressArrayContains(encountered_addresses, recovered_address)) {
-            encountered[i] = a;
+                return false;
+            }
+            encountered_addresses[i] = recovered_address;
         }
+        return true;
     }
 
 }
@@ -70,8 +77,8 @@ library HelpersTest {
         return Helpers.uintToString(inputValue);
     }
 
-    function verifySignatures(bytes message, uint8[] vs, bytes32[] rs, bytes32[] ss, address[] addresses, uint requiredSignatures) public pure {
+    function hasEnoughValidSignatures(bytes message, uint8[] vs, bytes32[] rs, bytes32[] ss, address[] addresses, uint requiredSignatures) public pure returns (bool) {
-        return Helpers.verifySignatures(message, vs, rs, ss, addresses, requiredSignatures);
+        return Helpers.hasEnoughValidSignatures(message, vs, rs, ss, addresses, requiredSignatures);
     }
 }
 
@@ -244,7 +251,7 @@ contract HomeBridge {
         require(message.length == 116);
 
         // check that at least `requiredSignatures` `authorities` have signed `message`
-        Helpers.verifySignatures(message, vs, rs, ss, authorities, requiredSignatures);
+        require(Helpers.hasEnoughValidSignatures(message, vs, rs, ss, authorities, requiredSignatures));
 
         address recipient = Message.getRecipient(message);
         uint value = Message.getValue(message);

truffle/test/helpers.js

@@ -81,7 +81,7 @@ contract("Helpers", function(accounts) {
     })
   })
 
-  it("`verifySignatures` should pass for 1 required signature", function() {
+  it("`hasEnoughValidSignatures` should pass for 1 required signature", function() {
     var library;
     var signature;
     var requiredSignatures = 1;
@@ -99,14 +99,16 @@ contract("Helpers", function(accounts) {
       signature = result;
       var vrs = helpers.signatureToVRS(signature);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message,
         [vrs.v],
         [vrs.r],
         [vrs.s],
 		authorities,
 		requiredSignatures
-	  )
+	  ).then(function(result) {
+		assert(result, "should return true");
+	  })
 	})
   })
 
@@ -141,14 +143,16 @@ contract("Helpers", function(accounts) {
       vrs[1] = helpers.signatureToVRS(signatures[1]);
       vrs[2] = helpers.signatureToVRS(signatures[2]);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message,
         [vrs[0].v, vrs[1].v, vrs[2].v],
         [vrs[0].r, vrs[1].r, vrs[2].r],
         [vrs[0].s, vrs[1].s, vrs[2].s],
 		authorities,
 		requiredSignatures
-	  )
+	  ).then(function(result) {
+		assert(result, "should return true");
+	  })
 	})
   })
 
@@ -172,16 +176,16 @@ contract("Helpers", function(accounts) {
       signature = result;
       var vrs = helpers.signatureToVRS(signature);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message2,
         [vrs.v],
         [vrs.r],
         [vrs.s],
 		authorities,
 		requiredSignatures
-      ).then(function() {
+      ).then(function(result) {
-        assert(false, "should fail");
+        assert.equal(result, false, "should return false");
-      }, helpers.ignoreExpectedError)
+	  })
 	})
   })
 
@@ -203,16 +207,16 @@ contract("Helpers", function(accounts) {
       signature = result;
       var vrs = helpers.signatureToVRS(signature);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message,
         [vrs.v],
         [vrs.r],
         [vrs.s],
 		authorities,
 		requiredSignatures
-      ).then(function() {
+      ).then(function(result) {
-        assert(false, "should fail");
+        assert.equal(result, false, "should return false");
-      }, helpers.ignoreExpectedError)
+	  })
 	})
   })
 
@@ -234,16 +238,16 @@ contract("Helpers", function(accounts) {
       signature = result;
       var vrs = helpers.signatureToVRS(signature);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message,
         [vrs.v],
         [vrs.r],
         [vrs.s],
 		authorities,
 		requiredSignatures
-      ).then(function() {
+      ).then(function(result) {
-        assert(false, "should fail");
+        assert.equal(result, false, "should return false");
-      }, helpers.ignoreExpectedError)
+	  })
 	})
   })
 
@@ -265,16 +269,16 @@ contract("Helpers", function(accounts) {
       signature = result;
       var vrs = helpers.signatureToVRS(signature);
 
-	  return library.verifySignatures.call(
+	  return library.hasEnoughValidSignatures.call(
 		message,
         [vrs.v, vrs.v],
         [vrs.r, vrs.r],
         [vrs.s, vrs.r],
 		authorities,
 		requiredSignatures
-      ).then(function() {
+      ).then(function(result) {
-        assert(false, "should fail");
+        assert.equal(result, false, "should return false");
-      }, helpers.ignoreExpectedError)
+	  })
 	})
   })
 })