diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index 826a821..398fbc6 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -15,9 +15,11 @@ import {ISavingCircles} from '../interfaces/ISavingCircles.sol'; * @author @bagelface */ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { + uint256 public constant MINIMUM_MEMBERS = 2; + mapping(bytes32 id => Circle circle) public circles; - mapping(bytes32 id => mapping(address => uint256)) public balances; mapping(address token => bool status) public allowedTokens; + mapping(bytes32 id => mapping(address token => uint256 balance)) public balances; mapping(bytes32 id => mapping(address member => bool status)) public isMember; /// @custom:oz-upgrades-unsafe-allow constructor @@ -36,19 +38,19 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { function addCircle(Circle memory _circle) external override { bytes32 _id = keccak256(abi.encodePacked(_circle.name)); - if (circles[_id].members.length != 0) revert AlreadyExists(); - if (!allowedTokens[_circle.token]) revert InvalidToken(); - if (_circle.depositInterval == 0) revert InvalidInterval(); - if (_circle.depositAmount == 0) revert InvalidDeposit(); - if (_circle.members.length < 2) revert InvalidMembers(); - if (_circle.maxDeposits == 0) revert InvalidDeposit(); - if (_circle.circleStart == 0) revert InvalidStart(); - if (_circle.currentIndex != 0) revert InvalidIndex(); + 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(); - circles[_id] = _circle; for (uint256 i = 0; i < _circle.members.length; i++) { - isMember[_id][_circle.members[i]] = true; + address _member = _circle.members[i]; + if (_member == address(0)) revert InvalidCircle(); + isMember[_id][_member] = true; } + circles[_id] = _circle; emit CircleCreated( _id, _circle.name, _circle.members, _circle.token, _circle.depositAmount, _circle.depositInterval @@ -81,7 +83,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { function withdraw(bytes32 _id) external override nonReentrant { Circle storage _circle = circles[_id]; - if (!_circleWithdrawable(_id)) revert NotWithdrawable(); + if (!_withdrawable(_id)) revert NotWithdrawable(); if (_circle.members[_circle.currentIndex] != msg.sender) revert NotWithdrawable(); uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); @@ -141,15 +143,6 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { return allowedTokens[_token]; } - /** - * @notice Return the members of a specified circle - * @param _id Identifier of the circle - * @return _members Members of the circle - */ - function circleMembers(bytes32 _id) external view override returns (address[] memory _members) { - return circles[_id].members; - } - /** * @notice Return the info of a specified saving circle * @param _id Identifier of the circle @@ -190,7 +183,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id TODO * @param _member TODO */ - function withdrawable(bytes32 _id, address _member) external view override returns (bool) { + function withdrawableBy(bytes32 _id, address _member) external view override returns (bool) { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); @@ -203,8 +196,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @notice TODO * @param _id TODO */ - function circleWithdrawable(bytes32 _id) external view override returns (bool) { - return _circleWithdrawable(_id); + function withdrawable(bytes32 _id) external view override returns (bool) { + return _withdrawable(_id); } function _deposit(bytes32 _id, address _member, uint256 _value) internal { @@ -212,23 +205,12 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { if (_isDecommissioned(_circle)) revert NotCommissioned(); if (!isMember[_id][_member]) revert NotMember(); - if (block.timestamp < circles[_id].circleStart) revert InvalidDeposit(); - - // Check if deposit is within current interval window - if (block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * (circles[_id].currentIndex + 1))) - { - revert InvalidDeposit(); - } - - // Check if circle has not exceeded max number of deposits - if (block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].maxDeposits)) { - revert InvalidDeposit(); - } - - // Check if deposit amount does not exceed allowed deposit amount for member - if (balances[_id][_member] + _value > circles[_id].depositAmount) { - revert InvalidDeposit(); - } + 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(); balances[_id][_member] = balances[_id][_member] + _value; @@ -238,7 +220,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { emit DepositMade(_id, _member, _value); } - function _circleWithdrawable(bytes32 _id) internal view returns (bool) { + function _withdrawable(bytes32 _id) internal view returns (bool) { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); diff --git a/src/interfaces/ISavingCircles.sol b/src/interfaces/ISavingCircles.sol index 878e4a3..db94a68 100644 --- a/src/interfaces/ISavingCircles.sol +++ b/src/interfaces/ISavingCircles.sol @@ -25,11 +25,7 @@ interface ISavingCircles { error AlreadyDeposited(); error AlreadyExists(); error InvalidDeposit(); - error InvalidIndex(); - error InvalidInterval(); - error InvalidMembers(); - error InvalidStart(); - error InvalidToken(); + error InvalidCircle(); error NotCommissioned(); error NotMember(); error NotOwner(); @@ -49,7 +45,6 @@ interface ISavingCircles { function circle(bytes32 id) external view returns (Circle memory); function isTokenAllowed(address token) external view returns (bool); function balancesForCircle(bytes32 id) external view returns (address[] memory, uint256[] memory); - function circleWithdrawable(bytes32 id) external view returns (bool); - function circleMembers(bytes32 id) external view returns (address[] memory); - function withdrawable(bytes32 id, address member) external view returns (bool); + function withdrawable(bytes32 id) external view returns (bool); + function withdrawableBy(bytes32 id, address member) external view returns (bool); } diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index 6c28b25..90fcdcd 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -127,7 +127,7 @@ contract SavingCirclesIntegration is Test { address badToken = makeAddr('badToken'); baseCircle.token = badToken; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidToken.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); circle.addCircle(baseCircle); } diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol index 77d0a9e..4b84654 100644 --- a/test/unit/SavingCirclesUnit.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -293,8 +293,9 @@ contract SavingCirclesUnit is Test { vm.expectEmit(true, true, true, true); emit ISavingCircles.CircleDecommissioned(baseCircleId); savingCircles.decommissionCircle(baseCircleId); - address[] memory emptyMembers = savingCircles.circleMembers(baseCircleId); - assertEq(emptyMembers.length, 0); + + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); + savingCircles.circle(baseCircleId); } function test_AddCircleWhenCircleNameAlreadyExists() external { @@ -311,7 +312,7 @@ contract SavingCirclesUnit is Test { _invalidCircle.token = _notAllowedToken; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidToken.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); savingCircles.addCircle(_invalidCircle); } @@ -321,7 +322,7 @@ contract SavingCirclesUnit is Test { _invalidCircle.depositInterval = 0; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidInterval.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); savingCircles.addCircle(_invalidCircle); } @@ -331,7 +332,7 @@ contract SavingCirclesUnit is Test { _invalidCircle.depositAmount = 0; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); savingCircles.addCircle(_invalidCircle); } @@ -344,7 +345,7 @@ contract SavingCirclesUnit is Test { _invalidCircle.members = _oneMember; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidMembers.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); savingCircles.addCircle(_invalidCircle); } }