add test demonstrating fix for 44

Hats-Protocol · Mar 27, 2023 · 1d6b322 · 1d6b322
1 parent a29582c
commit 1d6b322
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 5 deletions.
diff --git a/src/HatsSignerGateBase.sol b/src/HatsSignerGateBase.sol
@@ -99,7 +99,8 @@ abstract contract HatsSignerGateBase is BaseGuard, SignatureDecoder, HatsOwnedIn
         if (_targetThreshold != targetThreshold) {
             _setTargetThreshold(_targetThreshold);
 
-            if (validSignerCount() > 1) _setSafeThreshold(_targetThreshold);
+            uint256 signerCount = validSignerCount();
+            if (signerCount > 1) _setSafeThreshold(_targetThreshold, signerCount);
 
             emit HSGLib.TargetThresholdSet(_targetThreshold);
         }
@@ -119,13 +120,14 @@ abstract contract HatsSignerGateBase is BaseGuard, SignatureDecoder, HatsOwnedIn
     /// @notice Internal function to set the threshold for the `safe`
     /// @dev Forwards the threshold-setting call to `safe.ExecTransactionFromModule`
     /// @param _threshold The threshold to set on the `safe`
-    function _setSafeThreshold(uint256 _threshold) internal {
+    /// @param _signerCount The number of valid signers on the `safe`; should be calculated from `validSignerCount()`
+    function _setSafeThreshold(uint256 _threshold, uint256 _signerCount) internal {
         uint256 newThreshold = _threshold;
-        uint256 signerCount = validSignerCount();
+        // uint256 signerCount = validSignerCount();
 
         // ensure that txs can't execute if fewer signers than target threshold
-        if (signerCount <= _threshold) {
-            newThreshold = signerCount;
+        if (_signerCount <= _threshold) {
+            newThreshold = _signerCount;
         }
         if (newThreshold != safe.getThreshold()) {
             bytes memory data = abi.encodeWithSignature("changeThreshold(uint256)", newThreshold);

diff --git a/test/HatsSignerGate.t.sol b/test/HatsSignerGate.t.sol
@@ -1010,6 +1010,20 @@ contract HatsSignerGateTest is HSGTestSetup {
         assertEq(hatsSignerGate.validSignerCount(), 5, "valid signer count");
     }
 
+    function testSetTargetThresholdUpdatesThresholdCorrectly() public {
+        // set target threshold to 5
+        mockIsWearerCall(address(this), ownerHat, true);
+        hatsSignerGate.setTargetThreshold(5);
+        // add 5 valid signers
+        addSigners(5);
+        // one loses their hat
+        mockIsWearerCall(addresses[4], signerHat, false);
+        // lower target threshold to 4
+        hatsSignerGate.setTargetThreshold(4);
+        // since hatsSignerGate.validSignerCount() is also 4, the threshold should also be 4
+        assertEq(safe.getThreshold(), 4, "threshold");
+    }
+
     // function testSignersCannotChangeModules() public {
     //     //
     // }