Use custom errors for the keyper set manager contract

shutter-network · Jul 1, 2023 · 49203bd · 49203bd
1 parent e9ebd35
commit 49203bd
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 15 deletions.
diff --git a/src/IKeyperSetManager.sol b/src/IKeyperSetManager.sol
@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.20;
 
+error KeyperSetNotFinalized();
+error AlreadyHaveKeyperSet();
+error NoActiveKeyperSet();
+
 interface IKeyperSetManager {
     function addKeyperSet(
         uint64 activationSlot,

diff --git a/src/KeyperSetManager.sol b/src/KeyperSetManager.sol
@@ -13,15 +13,15 @@ contract KeyperSetManager is IKeyperSetManager, Ownable {
         uint64 activationSlot,
         address keyperSetContract
     ) external onlyOwner {
-        require(
-            contracts.length == 0 ||
-                activationSlot >= activationSlots[activationSlots.length - 1],
-            "keyper set already added for this activation slot"
-        );
-        require(
-            IKeyperSet(keyperSetContract).isFinalized(),
-            "keyper set contract not finalized"
-        );
+        if (
+            contracts.length > 0 &&
+            activationSlot < activationSlots[activationSlots.length - 1]
+        ) {
+            revert AlreadyHaveKeyperSet();
+        }
+        if (!IKeyperSet(keyperSetContract).isFinalized()) {
+            revert KeyperSetNotFinalized();
+        }
         activationSlots.push(activationSlot);
         contracts.push(keyperSetContract);
         emit KeyperSetAdded(activationSlot, keyperSetContract);
@@ -39,7 +39,7 @@ contract KeyperSetManager is IKeyperSetManager, Ownable {
                 return uint64(i - 1);
             }
         }
-        revert("no active keyper set found for given slot");
+        revert NoActiveKeyperSet();
     }
 
     function getKeyperSetAddress(uint64 index) external view returns (address) {

diff --git a/test/KeyperSetManager.t.sol b/test/KeyperSetManager.t.sol
@@ -34,30 +34,30 @@ contract KeyperSetManagerTest is Test {
 
     function testAddKeyperSetRequiresFinalizedSet() public {
         KeyperSet ks = new KeyperSet();
-        vm.expectRevert("keyper set contract not finalized");
+        vm.expectRevert(KeyperSetNotFinalized.selector);
         keyperSetManager.addKeyperSet(0, address(ks));
     }
 
     function testAddKeyperSetRequiresIncreasingActivationBlock() public {
         keyperSetManager.addKeyperSet(1000, address(members0));
-        vm.expectRevert("keyper set already added for this activation slot");
+        vm.expectRevert(AlreadyHaveKeyperSet.selector);
         keyperSetManager.addKeyperSet(999, address(members1));
         keyperSetManager.addKeyperSet(1000, address(members1));
     }
 
     function testGetKeyperSetIndexBySlotEmpty() public {
-        vm.expectRevert("no active keyper set found for given slot");
+        vm.expectRevert(NoActiveKeyperSet.selector);
         keyperSetManager.getKeyperSetIndexBySlot(0);
     }
 
     function testGetKeyperSetIndexBySlot() public {
         keyperSetManager.addKeyperSet(1000, address(members0));
         keyperSetManager.addKeyperSet(1100, address(members1));
 
-        vm.expectRevert("no active keyper set found for given slot");
+        vm.expectRevert(NoActiveKeyperSet.selector);
         keyperSetManager.getKeyperSetIndexBySlot(0);
 
-        vm.expectRevert("no active keyper set found for given slot");
+        vm.expectRevert(NoActiveKeyperSet.selector);
         keyperSetManager.getKeyperSetIndexBySlot(999);
 
         assertEq(keyperSetManager.getKeyperSetIndexBySlot(1000), 0);