chore: adding errors and breaking up big if"

BreadchainCoop · Jan 20, 2025 · 352bbee · 352bbee
1 parent 8b3fc57
commit 352bbee
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 19 deletions.
diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol
@@ -41,15 +41,18 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable {
     _id = nextId++;
 
     if (circles[_id].owner != address(0)) revert AlreadyExists();
-    if (
-      !allowedTokens[_circle.token] || _circle.depositInterval == 0 || _circle.depositAmount == 0
-        || _circle.maxDeposits == 0 || _circle.circleStart == 0 || _circle.currentIndex != 0
-        || _circle.owner == address(0) || _circle.members.length < MINIMUM_MEMBERS
-    ) revert InvalidCircle();
+    if (!allowedTokens[_circle.token]) revert TokenNotAllowed();
+    if (_circle.depositInterval == 0) revert InvalidDepositInterval();
+    if (_circle.depositAmount == 0) revert InvalidDepositAmount();
+    if (_circle.maxDeposits == 0) revert InvalidMaxDeposits();
+    if (_circle.circleStart == 0) revert InvalidCircleStartTime();
+    if (_circle.currentIndex != 0) revert InvalidCurrentIndex();
+    if (_circle.owner == address(0)) revert InvalidOwner();
+    if (_circle.members.length < MINIMUM_MEMBERS) revert InvalidMemberCount();
 
     for (uint256 i = 0; i < _circle.members.length; i++) {
       address _member = _circle.members[i];
-      if (_member == address(0)) revert InvalidCircle();
+      if (_member == address(0)) revert InvalidMemberAddress();
       isMember[_id][_member] = true;
       memberCircles[_member].push(_id);
     }
@@ -292,12 +295,19 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable {
 
     if (_isDecommissioned(_circle)) revert NotCommissioned();
     if (!isMember[_id][_member]) revert NotMember();
-    if (
-      block.timestamp < circles[_id].circleStart
-        || block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * (circles[_id].currentIndex + 1))
-        || block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].maxDeposits)
-        || balances[_id][_member] + _value > circles[_id].depositAmount
-    ) revert InvalidDeposit();
+    if (block.timestamp < circles[_id].circleStart) {
+      revert DepositBeforeCircleStart();
+    }
+    if (block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * (circles[_id].currentIndex + 1)))
+    {
+      revert DepositWindowClosed();
+    }
+    if (block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].maxDeposits)) {
+      revert CircleExpired();
+    }
+    if (balances[_id][_member] + _value > circles[_id].depositAmount) {
+      revert ExceedsDepositAmount();
+    }
 
     balances[_id][_member] = balances[_id][_member] + _value;
 

diff --git a/src/interfaces/ISavingCircles.sol b/src/interfaces/ISavingCircles.sol
@@ -42,6 +42,19 @@ interface ISavingCircles {
   error NotDecommissionable();
   error NotWithdrawable();
   error TransferFailed();
+  error DepositWindowClosed();
+  error CircleExpired();
+  error ExceedsDepositAmount();
+  error DepositBeforeCircleStart();
+  error TokenNotAllowed();
+  error InvalidDepositInterval();
+  error InvalidDepositAmount();
+  error InvalidMaxDeposits();
+  error InvalidCircleStartTime();
+  error InvalidCurrentIndex();
+  error InvalidOwner();
+  error InvalidMemberCount();
+  error InvalidMemberAddress();
 
   /*///////////////////////////////////////////////////////////////
                             VIEW

diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol
@@ -57,7 +57,7 @@ contract SavingCirclesIntegration is IntegrationBase {
     address badToken = makeAddr('badToken');
     baseCircle.token = badToken;
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.TokenNotAllowed.selector));
     circle.create(baseCircle);
   }
 

diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol
@@ -132,7 +132,7 @@ contract SavingCirclesUnit is Test {
     savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT);
 
     // Second deposit attempt should fail since member has already deposited max amount
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.ExceedsDepositAmount.selector));
     savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT);
     vm.stopPrank();
   }
@@ -161,7 +161,7 @@ contract SavingCirclesUnit is Test {
     vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1);
 
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.DepositWindowClosed.selector));
     savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT);
   }
 
@@ -394,7 +394,7 @@ contract SavingCirclesUnit is Test {
     _invalidCircle.token = _notAllowedToken;
 
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.TokenNotAllowed.selector));
     savingCircles.create(_invalidCircle);
   }
 
@@ -403,7 +403,7 @@ contract SavingCirclesUnit is Test {
     _invalidCircle.depositInterval = 0;
 
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDepositInterval.selector));
     savingCircles.create(_invalidCircle);
   }
 
@@ -412,7 +412,7 @@ contract SavingCirclesUnit is Test {
     _invalidCircle.depositAmount = 0;
 
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDepositAmount.selector));
     savingCircles.create(_invalidCircle);
   }
 
@@ -424,7 +424,7 @@ contract SavingCirclesUnit is Test {
     _invalidCircle.members = _oneMember;
 
     vm.prank(alice);
-    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector));
+    vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidMemberCount.selector));
     savingCircles.create(_invalidCircle);
   }