From 9510bb47d9eb37d0656fad0c90665ab03bce7966 Mon Sep 17 00:00:00 2001
From: Csongor Kiss <ckiss@jumptrading.com>
Date: Fri, 12 Aug 2022 19:36:56 +0100
Subject: [PATCH] ethereum: Add property test on quorum

It should always be possible to reach quorum
---
 ethereum/contracts/Messages.sol    |  2 ++
 ethereum/forge-test/Messages.t.sol | 15 +++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/ethereum/contracts/Messages.sol b/ethereum/contracts/Messages.sol
index 7b9dfd9a8..c41a730c0 100644
--- a/ethereum/contracts/Messages.sol
+++ b/ethereum/contracts/Messages.sol
@@ -159,6 +159,8 @@ contract Messages is Getters {
      * @dev quorum serves solely to determine the number of signatures required to acheive quorum
      */
     function quorum(uint numGuardians) public pure virtual returns (uint numSignaturesRequiredForQuorum) {
+        // The max number of guardians is 255
+        require(numGuardians < 256, "too many guardians");
         return ((numGuardians * 2) / 3) + 1;
     }
 }
diff --git a/ethereum/forge-test/Messages.t.sol b/ethereum/forge-test/Messages.t.sol
index a032ce4ad..bef999dce 100644
--- a/ethereum/forge-test/Messages.t.sol
+++ b/ethereum/forge-test/Messages.t.sol
@@ -28,6 +28,17 @@ contract TestMessages is Messages, Test {
     assertEq(quorum(20), 14);
   }
 
+  function testQuorumCanAlwaysBeReached(uint numGuardians) public {
+    if (numGuardians == 0) {
+      return;
+    }
+    if (numGuardians >= 256) {
+      vm.expectRevert("too many guardians");
+    }
+    // test that quorums is never greater than the number of guardians
+    assert(quorum(numGuardians) <= numGuardians);
+  }
+
   // This test ensures that submitting invalid signatures for non-existent
   // guardians fails.
   //
@@ -41,7 +52,7 @@ contract TestMessages is Messages, Test {
     address[] memory keys = new address[](1);
     keys[0] = testGuardianPub;
     Structs.GuardianSet memory guardianSet = Structs.GuardianSet(keys, 0);
-    assertEq(quorum(guardianSet.keys.length), 1);
+    require(quorum(guardianSet.keys.length) == 1, "Quorum should be 1");
 
     // Two invalid signatures, for guardian index 2 and 3 respectively.
     // These guardian indices are out of bounds for the guardian set.
@@ -49,7 +60,7 @@ contract TestMessages is Messages, Test {
     Structs.Signature memory bad1 = Structs.Signature(message, 0, 0, 2);
     Structs.Signature memory bad2 = Structs.Signature(message, 0, 0, 3);
     // ecrecover on an invalid signature returns 0 instead of reverting
-    assertEq(ecrecover(message, bad1.v, bad1.r, bad1.s), address(0));
+    require(ecrecover(message, bad1.v, bad1.r, bad1.s) == address(0), "ecrecover should return the 0 address for an invalid signature");
 
     Structs.Signature[] memory badSigs = new Structs.Signature[](2);
     badSigs[0] = bad1;