From d7feac802fef3960e8e4a98bcaa55dece3e81465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kru=CC=88ger?= Date: Tue, 13 Feb 2018 17:18:24 +0100 Subject: [PATCH] verifySignatures -> hasEnoughValidSignatures and make it return bool --- contracts/bridge.sol | 31 ++++++++++++++++----------- truffle/test/helpers.js | 46 ++++++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/contracts/bridge.sol b/contracts/bridge.sol index dcae114..adb3ed5 100644 --- a/contracts/bridge.sol +++ b/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)); - encountered[i] = a; + if (addressArrayContains(encountered_addresses, recovered_address)) { + 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 { - return Helpers.verifySignatures(message, vs, rs, ss, addresses, requiredSignatures); + function hasEnoughValidSignatures(bytes message, uint8[] vs, bytes32[] rs, bytes32[] ss, address[] addresses, uint requiredSignatures) public pure returns (bool) { + 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); diff --git a/truffle/test/helpers.js b/truffle/test/helpers.js index a15758b..ddd7390 100644 --- a/truffle/test/helpers.js +++ b/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() { - assert(false, "should fail"); - }, helpers.ignoreExpectedError) + ).then(function(result) { + assert.equal(result, false, "should return false"); + }) }) }) @@ -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() { - assert(false, "should fail"); - }, helpers.ignoreExpectedError) + ).then(function(result) { + assert.equal(result, false, "should return false"); + }) }) }) @@ -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() { - assert(false, "should fail"); - }, helpers.ignoreExpectedError) + ).then(function(result) { + assert.equal(result, false, "should return false"); + }) }) }) @@ -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() { - assert(false, "should fail"); - }, helpers.ignoreExpectedError) + ).then(function(result) { + assert.equal(result, false, "should return false"); + }) }) }) })