From 1b6cc9584516ee4175d616f44a4effdc0d636b4c Mon Sep 17 00:00:00 2001 From: bagelface Date: Fri, 20 Dec 2024 12:18:21 -0500 Subject: [PATCH 01/10] refactor: bump compiler version --- foundry.toml | 2 +- script/Common.sol | 2 +- script/Deploy.sol | 2 +- script/Registry.sol | 2 +- src/contracts/Greeter.sol | 2 +- src/interfaces/IGreeter.sol | 2 +- test/.solhint.json | 11 ++++++----- test/integration/Greeter.t.sol | 2 +- test/integration/IntegrationBase.sol | 2 +- test/invariants/fuzz/Greeter.t.sol | 2 +- test/invariants/symbolic/Greeter.t.sol | 2 +- test/unit/Greeter.t.sol | 2 +- test/unit/SavingCircleTest.t.sol | 19 ++++++++++--------- 13 files changed, 27 insertions(+), 25 deletions(-) diff --git a/foundry.toml b/foundry.toml index ce135c6..d24224c 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ multiline_func_header = 'params_first_multi' sort_imports = true [profile.default] -solc_version = '0.8.23' +solc_version = '0.8.28' libs = ['node_modules', 'lib'] optimizer_runs = 10_000 diff --git a/script/Common.sol b/script/Common.sol index a578893..d48fccd 100644 --- a/script/Common.sol +++ b/script/Common.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Greeter, IGreeter} from 'contracts/Greeter.sol'; diff --git a/script/Deploy.sol b/script/Deploy.sol index 9bfcd0c..28bef9e 100644 --- a/script/Deploy.sol +++ b/script/Deploy.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {Common} from 'script/Common.sol'; diff --git a/script/Registry.sol b/script/Registry.sol index 613a708..ac67876 100644 --- a/script/Registry.sol +++ b/script/Registry.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; /// @dev Example of addresses that may be stored in the Registry for use throughout the repository diff --git a/src/contracts/Greeter.sol b/src/contracts/Greeter.sol index 54e3c66..8a1c80e 100644 --- a/src/contracts/Greeter.sol +++ b/src/contracts/Greeter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {IGreeter} from 'interfaces/IGreeter.sol'; diff --git a/src/interfaces/IGreeter.sol b/src/interfaces/IGreeter.sol index 00b1180..4be5ae1 100644 --- a/src/interfaces/IGreeter.sol +++ b/src/interfaces/IGreeter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; diff --git a/test/.solhint.json b/test/.solhint.json index 3ec7f11..c5f483f 100644 --- a/test/.solhint.json +++ b/test/.solhint.json @@ -3,14 +3,15 @@ "style-guide-casing": [ "warn", { - "ignorePublicFunctions":true, - "ignoreExternalFunctions":true, - "ignoreContracts":true + "ignorePublicFunctions": true, + "ignoreExternalFunctions": true, + "ignoreContracts": true } ], "no-global-import": "off", "max-states-count": "off", "ordering": "off", - "one-contract-per-file": "off" + "one-contract-per-file": "off", + "func-name-mixedcase": "off" } -} +} \ No newline at end of file diff --git a/test/integration/Greeter.t.sol b/test/integration/Greeter.t.sol index ef65ff3..6bd0bdc 100644 --- a/test/integration/Greeter.t.sol +++ b/test/integration/Greeter.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IntegrationBase} from 'test/integration/IntegrationBase.sol'; diff --git a/test/integration/IntegrationBase.sol b/test/integration/IntegrationBase.sol index 7531ce7..1d71ce4 100644 --- a/test/integration/IntegrationBase.sol +++ b/test/integration/IntegrationBase.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Test} from 'forge-std/Test.sol'; diff --git a/test/invariants/fuzz/Greeter.t.sol b/test/invariants/fuzz/Greeter.t.sol index 96e2b1e..25f9e36 100644 --- a/test/invariants/fuzz/Greeter.t.sol +++ b/test/invariants/fuzz/Greeter.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {Greeter, IERC20} from 'contracts/Greeter.sol'; diff --git a/test/invariants/symbolic/Greeter.t.sol b/test/invariants/symbolic/Greeter.t.sol index 16a7edd..5a7db40 100644 --- a/test/invariants/symbolic/Greeter.t.sol +++ b/test/invariants/symbolic/Greeter.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {Greeter, IERC20} from 'contracts/Greeter.sol'; diff --git a/test/unit/Greeter.t.sol b/test/unit/Greeter.t.sol index c9a7f48..5a7b5a7 100644 --- a/test/unit/Greeter.t.sol +++ b/test/unit/Greeter.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Greeter, IGreeter} from 'contracts/Greeter.sol'; diff --git a/test/unit/SavingCircleTest.t.sol b/test/unit/SavingCircleTest.t.sol index 7ec1efd..3fc453d 100644 --- a/test/unit/SavingCircleTest.t.sol +++ b/test/unit/SavingCircleTest.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.23; +pragma solidity 0.8.28; import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgradeable.sol'; import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; @@ -13,19 +13,20 @@ contract SavingsCircleTest is Test { uint256 public constant DEPOSIT_INTERVAL = 1 days; uint256 public constant CIRCLE_DURATION = 30 days; + SavingsCircle public savingcircles; + IERC20 public token; + // Test addresses address public owner; address public alice; address public bob; address public carol; - address immutable stranger = makeAddr('stranger'); - IERC20 token; - SavingsCircle savingcircles; + address public immutable STRANGER = makeAddr('stranger'); // Test data - bytes32 baseCircleId; - address[] members; - ISavingsCircle.Circle baseCircle; + bytes32 public baseCircleId; + address[] public members; + ISavingsCircle.Circle public baseCircle; function setUp() external { // Setup test addresses @@ -114,7 +115,7 @@ contract SavingsCircleTest is Test { function test_DepositWhenMemberHasAlreadyDeposited() external { // First deposit - vm.startPrank(stranger); + vm.startPrank(STRANGER); vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); @@ -126,7 +127,7 @@ contract SavingsCircleTest is Test { } function test_DepositWhenParametersAreValid() external { - vm.startPrank(stranger); + vm.startPrank(STRANGER); // Mock token transfer vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); From b94baaaaeeb44ebc88cd79eb52c627628a572cac Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 23 Dec 2024 12:24:29 -0500 Subject: [PATCH 02/10] refactor: add natspec comments, refactor variable, function, etc. names, get test set up working --- src/contracts/SavingsCircle.sol | 320 +++++++++++++++++------------- src/interfaces/ISavingsCircle.sol | 72 +++---- test/SavingsCircle.t.sol | 85 ++++---- test/unit/Greeter.t.sol | 2 + test/unit/SavingCircleTest.t.sol | 51 +++-- 5 files changed, 282 insertions(+), 248 deletions(-) diff --git a/src/contracts/SavingsCircle.sol b/src/contracts/SavingsCircle.sol index 59f1100..160b637 100644 --- a/src/contracts/SavingsCircle.sol +++ b/src/contracts/SavingsCircle.sol @@ -1,212 +1,256 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -import {ISavingsCircle} from '../interfaces/ISavingsCircle.sol'; import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgradeable.sol'; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {ReentrancyGuard} from '@openzeppelin/utils/ReentrancyGuard.sol'; +import {ISavingsCircle} from '../interfaces/ISavingsCircle.sol'; + +/** + * @title Savings Circle + * @notice TODO + * @author Breadchain Collective + * @author @RonTuretzky + * @author @bagelface + */ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { - mapping(bytes32 circleIdentifier => Circle circle) public circles; - mapping(bytes32 circleIdentifier => mapping(address => uint256)) public circleBalances; - mapping(address token => bool status) public allowlistedTokens; - mapping(bytes32 circleIdentifier => mapping(address member => bool status)) public isMember; + mapping(bytes32 id => Circle circle) public circles; + mapping(bytes32 id => mapping(address => uint256)) public circleBalances; + mapping(address token => bool status) public allowedTokens; + mapping(bytes32 id => mapping(address member => bool status)) public isMember; - modifier validDeposit(bytes32 circleIdentifier, uint256 value, address member) { + modifier validDeposit(bytes32 _id, uint256 _value, address _member) { if ( - block.timestamp - >= circles[circleIdentifier].circleStart - + (circles[circleIdentifier].depositInterval * circles[circleIdentifier].currentIndex) - || block.timestamp - >= circles[circleIdentifier].circleStart - + (circles[circleIdentifier].depositInterval * circles[circleIdentifier].maxDeposits) - || circleBalances[circleIdentifier][member] + value >= circles[circleIdentifier].depositAmount + block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].currentIndex) + || block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].maxDeposits) + || circleBalances[_id][_member] + _value >= circles[_id].depositAmount ) revert InvalidDeposit(); _; } - /// @custom:oz-upgrades-unsafe-allow constructor + /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); } - function initialize() external override initializer { - __Ownable_init_unchained(msg.sender); + function initialize(address _owner) external override initializer { + __Ownable_init_unchained(_owner); } - function addCircle(Circle memory circle) external override { - bytes32 circleIdentifier = keccak256(abi.encodePacked(circle.name)); - if (circles[circleIdentifier].members.length != 0) revert CircleExists(); - if (!allowlistedTokens[circle.tokenAddress]) 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(); - - circles[circleIdentifier] = circle; - Circle storage newCircle = circles[circleIdentifier]; - for (uint256 i = 0; i < newCircle.members.length; i++) { - isMember[circleIdentifier][newCircle.members[i]] = true; + /** + * @notice Commission a new savings circle + * @param _circle A new savings circle + */ + function addCircle(Circle memory _circle) external override { + bytes32 _id = keccak256(abi.encodePacked(_circle.name)); + if (circles[_id].members.length != 0) revert CircleExists(); + 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(); + + circles[_id] = _circle; + Circle storage _newCircle = circles[_id]; + for (uint256 i = 0; i < _newCircle.members.length; i++) { + isMember[_id][_newCircle.members[i]] = true; } emit CircleCreated( - circleIdentifier, circle.name, circle.members, circle.tokenAddress, circle.depositAmount, circle.depositInterval + _id, _circle.name, _circle.members, _circle.token, _circle.depositAmount, _circle.depositInterval ); } - function allowlistToken(address token) external override onlyOwner { - allowlistedTokens[token] = true; - emit TokenAllowlisted(token); - } - - function denylistToken(address token) external override onlyOwner { - allowlistedTokens[token] = false; - emit TokenDenylisted(token); - } + /** + * @notice Make a deposit into a specified circle + * @param _id Identifier of the circle + * @param _value Amount of the token to deposit + */ + function deposit(bytes32 _id, uint256 _value) external override nonReentrant validDeposit(_id, _value, msg.sender) { + if (!isMember[_id][msg.sender]) revert NotMember(); - function deposit( - bytes32 circleIdentifier, - uint256 value - ) external override nonReentrant validDeposit(circleIdentifier, value, msg.sender) { - if (!isMember[circleIdentifier][msg.sender]) revert NotMember(); - _deposit(circleIdentifier, msg.sender, value); + _deposit(_id, msg.sender, _value); } + /** + * @notice Make a deposit on behalf of another member + * @param _id Identifier of the circle + * @param _member Address to make a deposit for + * @param _value Amount of the token to deposit + */ function depositFor( - bytes32 circleIdentifier, - address member, - uint256 value - ) external override nonReentrant validDeposit(circleIdentifier, value, member) { - _deposit(circleIdentifier, member, value); + bytes32 _id, + address _member, + uint256 _value + ) external override nonReentrant validDeposit(_id, _value, _member) { + _deposit(_id, _member, _value); } - function withdraw(bytes32 circleIdentifier) external override nonReentrant { - if (!circleWithdrawable(circleIdentifier)) revert NotWithdrawable(); - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); - if (circle.members[circle.currentIndex] != msg.sender) revert NotWithdrawable(); + /** + * @notice Make a withdrawal from a specified circle + * @param _id Identifier of the circle + */ + function withdraw(bytes32 _id) external override nonReentrant { + Circle storage _circle = circles[_id]; - uint256 withdrawAmount = circle.depositAmount * (circle.members.length); + if (!_circleWithdrawable(_id)) revert NotWithdrawable(); + if (_circle.members.length == 0) revert CircleNotFound(); + if (_circle.members[_circle.currentIndex] != msg.sender) revert NotWithdrawable(); - for (uint256 i = 0; i < circle.members.length; i++) { - circleBalances[circleIdentifier][circle.members[i]] = 0; + uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); + + for (uint256 i = 0; i < _circle.members.length; i++) { + circleBalances[_id][_circle.members[i]] = 0; } - circle.currentIndex = (circle.currentIndex + 1) % circle.members.length; - IERC20(circle.tokenAddress).transfer(msg.sender, withdrawAmount); - emit WithdrawalMade(circleIdentifier, msg.sender, withdrawAmount); + _circle.currentIndex = (_circle.currentIndex + 1) % _circle.members.length; + bool success = IERC20(_circle.token).transfer(msg.sender, _withdrawAmount); + if (!success) revert TransferFailed(); + + emit WithdrawalMade(_id, msg.sender, _withdrawAmount); } - function decommissionCircle(bytes32 circleIdentifier) external override onlyOwner { - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); - if (circle.owner != msg.sender) revert NotOwner(); + /** + * @notice Set if a token can be used for savings circles + * @param _token Token to update the status of + * @param _allowed Can be used for savings circles + */ + function setTokenAllowed(address _token, bool _allowed) external override onlyOwner { + allowedTokens[_token] = _allowed; - // Return all deposits to members - for (uint256 i = 0; i < circle.members.length; i++) { - address member = circle.members[i]; - uint256 balance = circleBalances[circleIdentifier][member]; - if (balance > 0) { - circleBalances[circleIdentifier][member] = 0; - IERC20(circle.tokenAddress).transfer(member, balance); + emit TokenAllowed(_token, _allowed); + } + + /** + * @notice Decommission an existing savings circle + * @dev Returns all deposits to members + * @param _id Identifier of the circle + */ + function decommissionCircle(bytes32 _id) external override onlyOwner { + Circle storage _circle = circles[_id]; + if (_circle.members.length == 0) revert CircleNotFound(); + if (_circle.owner != msg.sender) revert NotOwner(); + + for (uint256 i = 0; i < _circle.members.length; i++) { + address _member = _circle.members[i]; + uint256 _balance = circleBalances[_id][_member]; + if (_balance > 0) { + circleBalances[_id][_member] = 0; + IERC20(_circle.token).transfer(_member, _balance); } } - delete circles[circleIdentifier]; - emit CircleDecommissioned(circleIdentifier); + delete circles[_id]; + + emit CircleDecommissioned(_id); } - function isTokenAllowlisted(address token) external view override returns (bool) { - return allowlistedTokens[token]; + /** + * @notice Return if a token is allowed to be used for saving circles + * @param _token Address of a token + * @return bool Token allowed + */ + function isTokenAllowed(address _token) external view override returns (bool) { + return allowedTokens[_token]; } - function circleMembers(bytes32 circleIdentifier) external view override returns (address[] memory) { - return circles[circleIdentifier].members; + /** + * @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; } - function circleInfo(bytes32 circleIdentifier) - external - view - override - returns ( - string memory name, - address[] memory members, - address tokenAddress, - uint256 depositAmount, - uint256 depositInterval, - uint256 circleStart, - uint256 numWithdrawals, - uint256 currentIndex - ) - { - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); - return ( - circle.name, - circle.members, - circle.tokenAddress, - circle.depositAmount, - circle.depositInterval, - circle.circleStart, - circle.currentIndex, - circle.maxDeposits - ); + /** + * @notice Return the info of a specified savings circle + * @param _id Identifier of the circle + * @return _circle Savings circle + */ + function circle(bytes32 _id) external view override returns (Circle memory _circle) { + _circle = circles[_id]; + + if (_circle.members.length == 0) revert CircleNotFound(); + + return _circle; } - function balancesForCircle(bytes32 circleIdentifier) + /** + * @notice TODO + * @param _id TODO + */ + function balancesForCircle(bytes32 _id) external view override - returns (address[] memory, uint256[] memory) + returns (address[] memory _members, uint256[] memory _balances) { - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); + Circle storage _circle = circles[_id]; + if (_circle.members.length == 0) revert CircleNotFound(); - uint256[] memory balances = new uint256[](circle.members.length); - for (uint256 i = 0; i < circle.members.length; i++) { - balances[i] = circleBalances[circleIdentifier][circle.members[i]]; + _balances = new uint256[](_circle.members.length); + for (uint256 i = 0; i < _circle.members.length; i++) { + _balances[i] = circleBalances[_id][_circle.members[i]]; } - return (circle.members, balances); + return (_circle.members, _balances); } - function withdrawable(bytes32 circleIdentifier, address member) external view override returns (bool) { - if (!isMember[circleIdentifier][member]) revert NotMember(); - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); - uint256 currentIndex = circle.currentIndex; - return circle.members[currentIndex] == member; + /** + * @notice TODO + * @param _id TODO + * @param _member TODO + */ + function withdrawable(bytes32 _id, address _member) external view override returns (bool) { + Circle storage _circle = circles[_id]; + + if (!isMember[_id][_member]) revert NotMember(); + if (_circle.members.length == 0) revert CircleNotFound(); + + return _circle.members[_circle.currentIndex] == _member; } - function circleWithdrawable(bytes32 hashedName) public view override returns (bool) { - Circle storage circle = circles[hashedName]; - if (circle.members.length == 0) revert CircleNotFound(); + /** + * @notice TODO + * @param _id TODO + */ + function circleWithdrawable(bytes32 _id) external view override returns (bool) { + return _circleWithdrawable(_id); + } + + function _deposit(bytes32 _id, address _member, uint256 _value) internal { + Circle storage _circle = circles[_id]; + + if (_circle.members.length == 0) revert CircleNotFound(); + if (!isMember[_id][_member]) revert NotMember(); + + IERC20(_circle.token).transferFrom(msg.sender, address(this), _value); + + circleBalances[_id][_member] = circleBalances[_id][_member] + _value; + emit DepositMade(_id, _member, _value); + } + + function _circleWithdrawable(bytes32 _id) internal view returns (bool) { + Circle storage _circle = circles[_id]; + + if (_circle.members.length == 0) revert CircleNotFound(); // Check if enough time has passed since circle start for current withdrawal - if (block.timestamp < circle.circleStart + (circle.depositInterval * circle.currentIndex)) { + if (block.timestamp < _circle.circleStart + (_circle.depositInterval * _circle.currentIndex)) { return false; } // Check if all members have made their initial deposit - for (uint256 i = 0; i < circle.members.length; i++) { - if (circleBalances[hashedName][circle.members[i]] < circle.depositAmount) { + for (uint256 i = 0; i < _circle.members.length; i++) { + if (circleBalances[_id][_circle.members[i]] < _circle.depositAmount) { return false; } } return true; } - - function _deposit(bytes32 circleIdentifier, address member, uint256 value) internal { - Circle storage circle = circles[circleIdentifier]; - if (circle.members.length == 0) revert CircleNotFound(); - if (!isMember[circleIdentifier][member]) revert NotMember(); - - IERC20(circle.tokenAddress).transferFrom(msg.sender, address(this), value); - - circleBalances[circleIdentifier][member] = circleBalances[circleIdentifier][member] + value; - emit DepositMade(circleIdentifier, member, value); - } } diff --git a/src/interfaces/ISavingsCircle.sol b/src/interfaces/ISavingsCircle.sol index 6a7bb19..5631a43 100644 --- a/src/interfaces/ISavingsCircle.sol +++ b/src/interfaces/ISavingsCircle.sol @@ -8,66 +8,48 @@ interface ISavingsCircle { address[] members; uint256 currentIndex; uint256 depositAmount; - address tokenAddress; + address token; uint256 depositInterval; uint256 circleStart; uint256 maxDeposits; } event CircleCreated( - bytes32 indexed circleIdentifier, - string name, - address[] members, - address tokenAddress, - uint256 depositAmount, - uint256 depositInterval + bytes32 indexed id, string name, address[] members, address token, uint256 depositAmount, uint256 depositInterval ); - event DepositMade(bytes32 indexed circleIdentifier, address indexed contributor, uint256 amount); - event WithdrawalMade(bytes32 indexed circleIdentifier, address indexed withdrawer, uint256 amount); - event TokenAllowlisted(address indexed token); - event TokenDenylisted(address indexed token); - event CircleDecommissioned(bytes32 indexed circleIdentifier); + event CircleDecommissioned(bytes32 indexed id); + event DepositMade(bytes32 indexed id, address indexed contributor, uint256 amount); + event WithdrawalMade(bytes32 indexed id, address indexed withdrawer, uint256 amount); + event TokenAllowed(address indexed token, bool indexed allowed); - error InvalidToken(); - error InvalidInterval(); + error AlreadyDeposited(); + error CircleExists(); + error CircleNotFound(); error InvalidDeposit(); + error InvalidIndex(); + error InvalidInterval(); error InvalidMembers(); - error CircleNotFound(); + error InvalidStart(); + error InvalidToken(); error NotMember(); error NotOwner(); error NotWithdrawable(); - error CircleExists(); - error AlreadyDeposited(); - error InvalidStart(); - error InvalidIndex(); - // External functions (state-changing) + error TransferFailed(); - function initialize() external; - function allowlistToken(address token) external; - function denylistToken(address token) external; + // External functions (state-changing) + function initialize(address owner) external; + function setTokenAllowed(address token, bool allowed) external; function addCircle(Circle memory circle) external; - function deposit(bytes32 circleIdentifier, uint256 value) external; - function depositFor(bytes32 circleIdentifier, address member, uint256 value) external; - function withdraw(bytes32 circleIdentifier) external; - function decommissionCircle(bytes32 circleIdentifier) external; + function deposit(bytes32 id, uint256 value) external; + function depositFor(bytes32 id, address member, uint256 value) external; + function withdraw(bytes32 id) external; + function decommissionCircle(bytes32 id) external; // External view functions - function isTokenAllowlisted(address token) external view returns (bool); - function circleInfo(bytes32 circleIdentifier) - external - view - returns ( - string memory name, - address[] memory members, - address tokenAddress, - uint256 depositAmount, - uint256 depositInterval, - uint256 circleStart, - uint256 numWithdrawals, - uint256 currentIndex - ); - function balancesForCircle(bytes32 circleIdentifier) external view returns (address[] memory, uint256[] memory); - function circleWithdrawable(bytes32 circleIdentifier) external view returns (bool); - function circleMembers(bytes32 circleIdentifier) external view returns (address[] memory); - function withdrawable(bytes32 circleIdentifier, address member) external view returns (bool); + 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); } diff --git a/test/SavingsCircle.t.sol b/test/SavingsCircle.t.sol index 6ec53d3..0c29baa 100644 --- a/test/SavingsCircle.t.sol +++ b/test/SavingsCircle.t.sol @@ -1,20 +1,17 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; +import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; +import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; +import {Test} from 'forge-std/Test.sol'; + import {SavingsCircle} from '../src/contracts/SavingsCircle.sol'; import {ISavingsCircle} from '../src/interfaces/ISavingsCircle.sol'; import {MockERC20} from './mocks/MockERC20.sol'; -import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; -import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; -import {Test, console} from 'forge-std/Test.sol'; - contract SavingsCircleTest is Test { - SavingsCircle public implementation; SavingsCircle public circle; MockERC20 public token; - ProxyAdmin public proxyAdmin; - TransparentUpgradeableProxy public proxy; address public alice = makeAddr('alice'); address public bob = makeAddr('bob'); @@ -32,81 +29,91 @@ contract SavingsCircleTest is Test { function setUp() public { vm.startPrank(owner); - // Deploy implementation - implementation = new SavingsCircle(); - - // Deploy ProxyAdmin - proxyAdmin = new ProxyAdmin(owner); - - // Deploy proxy - bytes memory initData = abi.encodeWithSelector(SavingsCircle.initialize.selector); - - proxy = new TransparentUpgradeableProxy(address(implementation), address(proxyAdmin), initData); - - // Get interface for proxy - circle = SavingsCircle(address(proxy)); + circle = SavingsCircle( + address( + new TransparentUpgradeableProxy( + address(new SavingsCircle()), + address(new ProxyAdmin(owner)), + abi.encodeWithSelector(SavingsCircle.initialize.selector, owner) + ) + ) + ); token = new MockERC20('Test Token', 'TEST'); - circle.allowlistToken(address(token)); + circle.setTokenAllowed(address(token), true); vm.stopPrank(); // Setup test accounts vm.startPrank(alice); token.mint(alice, DEPOSIT_AMOUNT * 10); token.approve(address(circle), type(uint256).max); + members.push(alice); vm.stopPrank(); vm.startPrank(bob); token.mint(bob, DEPOSIT_AMOUNT * 10); token.approve(address(circle), type(uint256).max); + members.push(bob); vm.stopPrank(); vm.startPrank(carol); token.mint(carol, DEPOSIT_AMOUNT * 10); token.approve(address(circle), type(uint256).max); + members.push(carol); vm.stopPrank(); - // Setup base circle parameters - members[0] = alice; - members[1] = bob; - members[2] = carol; - baseCircle = ISavingsCircle.Circle({ owner: alice, name: 'Test Circle', members: members, currentIndex: BASE_CURRENT_INDEX, circleStart: block.timestamp, - tokenAddress: address(token), + token: address(token), depositAmount: DEPOSIT_AMOUNT, depositInterval: DEPOSIT_INTERVAL, maxDeposits: BASE_MAX_DEPOSITS }); } - function test_AllowlistToken() public { - address newToken = makeAddr('newToken'); + function test_SetTokenAllowed() public { + // Check initial state + assertFalse(circle.isTokenAllowed(address(token))); + + // Test enabling token vm.prank(owner); - circle.allowlistToken(newToken); - assertTrue(circle.isTokenAllowlisted(newToken)); - } + circle.setTokenAllowed(address(token), true); + assertTrue(circle.isTokenAllowed(address(token))); + + // Test disabling token + vm.prank(owner); + circle.setTokenAllowed(address(token), false); + assertFalse(circle.isTokenAllowed(address(token))); - function test_DenylistToken() public { + // Test enabling multiple tokens + address newToken = makeAddr('newToken'); vm.startPrank(owner); - circle.allowlistToken(address(token)); - circle.denylistToken(address(token)); + circle.setTokenAllowed(address(token), true); + circle.setTokenAllowed(newToken, true); vm.stopPrank(); - assertFalse(circle.isTokenAllowlisted(address(token))); + + assertTrue(circle.isTokenAllowed(address(token))); + assertTrue(circle.isTokenAllowed(newToken)); + + // Test emitted events + vm.prank(owner); + vm.expectEmit(true, true, false, true); + emit ISavingsCircle.TokenAllowed(address(token), false); + circle.setTokenAllowed(address(token), false); } function testFail_NonOwnerAllowlist() public { vm.prank(alice); - circle.allowlistToken(address(token)); + circle.setTokenAllowed(address(token), true); } function testFail_CreateCircleWithUnAllowlistedToken() public { address badToken = makeAddr('badToken'); - baseCircle.tokenAddress = badToken; + baseCircle.token = badToken; vm.prank(alice); circle.addCircle(baseCircle); } @@ -189,7 +196,7 @@ contract SavingsCircleTest is Test { // Check circle deleted vm.expectRevert(ISavingsCircle.CircleNotFound.selector); - circle.circleInfo(BASE_CIRCLE_ID); + circle.circle(BASE_CIRCLE_ID); } function testFail_NonOwnerDecommission() public { diff --git a/test/unit/Greeter.t.sol b/test/unit/Greeter.t.sol index 5a7b5a7..d8901b1 100644 --- a/test/unit/Greeter.t.sol +++ b/test/unit/Greeter.t.sol @@ -6,6 +6,7 @@ import {Greeter, IGreeter} from 'contracts/Greeter.sol'; import {Test} from 'forge-std/Test.sol'; contract UnitGreeter is Test { +/* address internal _owner = makeAddr('owner'); IERC20 internal _token = IERC20(makeAddr('token')); uint256 internal _initialBalance = 100; @@ -96,4 +97,5 @@ contract UnitGreeter is Test { vm.expectRevert(IGreeter.Greeter_OnlyOwner.selector); _greeter.setGreeting('new greeting'); } +*/ } diff --git a/test/unit/SavingCircleTest.t.sol b/test/unit/SavingCircleTest.t.sol index 3fc453d..882ced6 100644 --- a/test/unit/SavingCircleTest.t.sol +++ b/test/unit/SavingCircleTest.t.sol @@ -37,7 +37,7 @@ contract SavingsCircleTest is Test { token = IERC20(makeAddr('token')); // Setup test data - members = new address[](3); + members = new address[](4); members[0] = alice; members[1] = bob; members[2] = carol; @@ -51,7 +51,7 @@ contract SavingsCircleTest is Test { members: members, currentIndex: 0, circleStart: block.timestamp, - tokenAddress: address(token), + token: address(token), depositAmount: DEPOSIT_AMOUNT, depositInterval: DEPOSIT_INTERVAL, maxDeposits: 1000 @@ -61,11 +61,11 @@ contract SavingsCircleTest is Test { vm.startPrank(owner); SavingsCircle implementation = new SavingsCircle(); ProxyAdmin proxyAdmin = new ProxyAdmin(owner); - bytes memory initData = abi.encodeWithSelector(SavingsCircle.initialize.selector); + bytes memory initData = abi.encodeWithSelector(SavingsCircle.initialize.selector, owner); TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(address(implementation), address(proxyAdmin), initData); savingcircles = SavingsCircle(address(proxy)); - savingcircles.allowlistToken(address(token)); + savingcircles.setTokenAllowed(address(token), true); vm.stopPrank(); // Create initial test savingcircles @@ -73,36 +73,36 @@ contract SavingsCircleTest is Test { savingcircles.addCircle(baseCircle); } - function test_AllowlistTokenWhenCallerIsNotOwner() external { + function test_SetTokenAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingcircles.allowlistToken(address(0x1)); + savingcircles.setTokenAllowed(address(0x1), true); } - function test_AllowlistTokenWhenCallerIsOwner() external { + function test_SetTokenAllowedWhenCallerIsOwner() external { address newToken = makeAddr('newToken'); vm.prank(owner); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.TokenAllowlisted(newToken); - savingcircles.allowlistToken(newToken); + emit ISavingsCircle.TokenAllowed(newToken, true); + savingcircles.setTokenAllowed(newToken, true); - assertTrue(savingcircles.isTokenAllowlisted(newToken)); + assertTrue(savingcircles.isTokenAllowed(newToken)); } - function test_DenylistTokenWhenCallerIsNotOwner() external { + function test_SetTokenNotAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingcircles.denylistToken(address(token)); + savingcircles.setTokenAllowed(address(token), false); } - function test_DenylistTokenWhenCallerIsOwner() external { + function test_SetTokenNotAllowedWhenCallerIsOwner() external { vm.prank(owner); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.TokenDenylisted(address(token)); - savingcircles.denylistToken(address(token)); + emit ISavingsCircle.TokenAllowed(address(token), false); + savingcircles.setTokenAllowed(address(token), false); - assertFalse(savingcircles.isTokenAllowlisted(address(token))); + assertFalse(savingcircles.isTokenAllowed(address(token))); } function test_DepositWhenCircleDoesNotExist() external { @@ -225,18 +225,17 @@ contract SavingsCircleTest is Test { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingcircles.circleInfo(nonExistentCircleId); + savingcircles.circle(nonExistentCircleId); } function test_CircleInfoWhenCircleExists() external { - (string memory name, address[] memory circleMembers, address tokenAddr, uint256 deposit, uint256 interval,,,) = - savingcircles.circleInfo(baseCircleId); - - assertEq(name, 'Test Circle'); - assertEq(circleMembers.length, members.length); - assertEq(tokenAddr, address(token)); - assertEq(deposit, DEPOSIT_AMOUNT); - assertEq(interval, DEPOSIT_INTERVAL); + ISavingsCircle.Circle memory _circle = savingcircles.circle(baseCircleId); + + assertEq(_circle.name, 'Test Circle'); + assertEq(_circle.members.length, members.length); + assertEq(_circle.token, address(token)); + assertEq(_circle.depositAmount, DEPOSIT_AMOUNT); + assertEq(_circle.depositInterval, DEPOSIT_INTERVAL); } function test_DecommissionWhenCallerIsNotOwner() external { @@ -271,7 +270,7 @@ contract SavingsCircleTest is Test { function test_AddCircleWhenTokenIsNotWhitelisted() external { address nonWhitelistedToken = makeAddr('nonWhitelistedToken'); ISavingsCircle.Circle memory invalidParams = baseCircle; - invalidParams.tokenAddress = nonWhitelistedToken; + invalidParams.token = nonWhitelistedToken; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidToken.selector)); From a7f0a9c4ae8a8aad66b3678d4f8b4fd91c02e461 Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 23 Dec 2024 14:40:31 -0500 Subject: [PATCH 03/10] test: update tests to get them all passing --- script/DeploySavingsCircle.s.sol | 2 +- src/contracts/SavingsCircle.sol | 50 ++++--- src/interfaces/ISavingsCircle.sol | 2 +- test/SavingsCircle.t.sol | 51 +++++-- test/mocks/MockERC20.sol | 2 +- test/unit/SavingCircleTest.t.sol | 215 ++++++++++++++++++------------ 6 files changed, 207 insertions(+), 115 deletions(-) diff --git a/script/DeploySavingsCircle.s.sol b/script/DeploySavingsCircle.s.sol index 4032bde..71fd149 100644 --- a/script/DeploySavingsCircle.s.sol +++ b/script/DeploySavingsCircle.s.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.28; import {SavingsCircle} from '../src/contracts/SavingsCircle.sol'; diff --git a/src/contracts/SavingsCircle.sol b/src/contracts/SavingsCircle.sol index 160b637..21f4804 100644 --- a/src/contracts/SavingsCircle.sol +++ b/src/contracts/SavingsCircle.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.28; import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgradeable.sol'; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; @@ -16,16 +16,36 @@ import {ISavingsCircle} from '../interfaces/ISavingsCircle.sol'; */ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { mapping(bytes32 id => Circle circle) public circles; - mapping(bytes32 id => mapping(address => uint256)) public circleBalances; + mapping(bytes32 id => mapping(address => uint256)) public balances; mapping(address token => bool status) public allowedTokens; mapping(bytes32 id => mapping(address member => bool status)) public isMember; modifier validDeposit(bytes32 _id, uint256 _value, address _member) { - if ( - block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].currentIndex) - || block.timestamp >= circles[_id].circleStart + (circles[_id].depositInterval * circles[_id].maxDeposits) - || circleBalances[_id][_member] + _value >= circles[_id].depositAmount - ) revert InvalidDeposit(); + Circle memory _circle = circles[_id]; + + if (_circle.members.length == 0) revert CircleNotFound(); + + // Check if circle has started + 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(); + } + _; } @@ -97,13 +117,12 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { Circle storage _circle = circles[_id]; if (!_circleWithdrawable(_id)) revert NotWithdrawable(); - if (_circle.members.length == 0) revert CircleNotFound(); if (_circle.members[_circle.currentIndex] != msg.sender) revert NotWithdrawable(); uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); for (uint256 i = 0; i < _circle.members.length; i++) { - circleBalances[_id][_circle.members[i]] = 0; + balances[_id][_circle.members[i]] = 0; } _circle.currentIndex = (_circle.currentIndex + 1) % _circle.members.length; @@ -129,16 +148,17 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { * @dev Returns all deposits to members * @param _id Identifier of the circle */ - function decommissionCircle(bytes32 _id) external override onlyOwner { + function decommissionCircle(bytes32 _id) external override { Circle storage _circle = circles[_id]; + if (_circle.members.length == 0) revert CircleNotFound(); if (_circle.owner != msg.sender) revert NotOwner(); for (uint256 i = 0; i < _circle.members.length; i++) { address _member = _circle.members[i]; - uint256 _balance = circleBalances[_id][_member]; + uint256 _balance = balances[_id][_member]; if (_balance > 0) { - circleBalances[_id][_member] = 0; + balances[_id][_member] = 0; IERC20(_circle.token).transfer(_member, _balance); } } @@ -194,7 +214,7 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { _balances = new uint256[](_circle.members.length); for (uint256 i = 0; i < _circle.members.length; i++) { - _balances[i] = circleBalances[_id][_circle.members[i]]; + _balances[i] = balances[_id][_circle.members[i]]; } return (_circle.members, _balances); @@ -230,7 +250,7 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { IERC20(_circle.token).transferFrom(msg.sender, address(this), _value); - circleBalances[_id][_member] = circleBalances[_id][_member] + _value; + balances[_id][_member] = balances[_id][_member] + _value; emit DepositMade(_id, _member, _value); } @@ -246,7 +266,7 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { // Check if all members have made their initial deposit for (uint256 i = 0; i < _circle.members.length; i++) { - if (circleBalances[_id][_circle.members[i]] < _circle.depositAmount) { + if (balances[_id][_circle.members[i]] < _circle.depositAmount) { return false; } } diff --git a/src/interfaces/ISavingsCircle.sol b/src/interfaces/ISavingsCircle.sol index 5631a43..5ec7bea 100644 --- a/src/interfaces/ISavingsCircle.sol +++ b/src/interfaces/ISavingsCircle.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.28; interface ISavingsCircle { struct Circle { diff --git a/test/SavingsCircle.t.sol b/test/SavingsCircle.t.sol index 0c29baa..f849942 100644 --- a/test/SavingsCircle.t.sol +++ b/test/SavingsCircle.t.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.28; +import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgradeable.sol'; import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Test} from 'forge-std/Test.sol'; @@ -9,6 +10,8 @@ import {SavingsCircle} from '../src/contracts/SavingsCircle.sol'; import {ISavingsCircle} from '../src/interfaces/ISavingsCircle.sol'; import {MockERC20} from './mocks/MockERC20.sol'; +/* solhint-disable func-name-mixedcase */ + contract SavingsCircleTest is Test { SavingsCircle public circle; MockERC20 public token; @@ -19,11 +22,12 @@ contract SavingsCircleTest is Test { address public owner = makeAddr('owner'); address[] public members; + string public constant BASE_CIRCLE_NAME = 'Test Circle'; uint256 public constant DEPOSIT_AMOUNT = 1000e18; uint256 public constant DEPOSIT_INTERVAL = 7 days; uint256 public constant BASE_CURRENT_INDEX = 0; uint256 public constant BASE_MAX_DEPOSITS = 1000; - bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked('Test Circle')); + bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked(BASE_CIRCLE_NAME)); ISavingsCircle.Circle public baseCircle; @@ -40,7 +44,6 @@ contract SavingsCircleTest is Test { ); token = new MockERC20('Test Token', 'TEST'); - circle.setTokenAllowed(address(token), true); vm.stopPrank(); // Setup test accounts @@ -64,7 +67,7 @@ contract SavingsCircleTest is Test { baseCircle = ISavingsCircle.Circle({ owner: alice, - name: 'Test Circle', + name: BASE_CIRCLE_NAME, members: members, currentIndex: BASE_CURRENT_INDEX, circleStart: block.timestamp, @@ -75,6 +78,14 @@ contract SavingsCircleTest is Test { }); } + function createBaseCircle() public { + vm.prank(owner); + circle.setTokenAllowed(address(token), true); + + vm.prank(alice); + circle.addCircle(baseCircle); + } + function test_SetTokenAllowed() public { // Check initial state assertFalse(circle.isTokenAllowed(address(token))); @@ -106,20 +117,23 @@ contract SavingsCircleTest is Test { circle.setTokenAllowed(address(token), false); } - function testFail_NonOwnerAllowlist() public { - vm.prank(alice); + function test_RevertWhen_NonOwnerAllowlistsToken() public { + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, bob)); circle.setTokenAllowed(address(token), true); } - function testFail_CreateCircleWithUnAllowlistedToken() public { + function test_RevertWhen_CreatingCircleWithUnallowlistedToken() public { address badToken = makeAddr('badToken'); baseCircle.token = badToken; vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidToken.selector)); circle.addCircle(baseCircle); } - function test_deposit() public { - // deposit + function test_Deposit() public { + createBaseCircle(); + vm.prank(alice); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); @@ -128,6 +142,8 @@ contract SavingsCircleTest is Test { } function test_DepositFor() public { + createBaseCircle(); + // Bob deposits for Alice vm.prank(bob); circle.depositFor(BASE_CIRCLE_ID, alice, DEPOSIT_AMOUNT); @@ -137,6 +153,8 @@ contract SavingsCircleTest is Test { } function test_WithdrawWithInterval() public { + createBaseCircle(); + vm.prank(alice); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); @@ -175,6 +193,8 @@ contract SavingsCircleTest is Test { } function test_DecommissionCircle() public { + createBaseCircle(); + // Members deposit vm.prank(alice); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); @@ -199,22 +219,25 @@ contract SavingsCircleTest is Test { circle.circle(BASE_CIRCLE_ID); } - function testFail_NonOwnerDecommission() public { - // Try to decommission as non-owner + function test_RevertWhen_NonOwnerDecommissions() public { + createBaseCircle(); + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotOwner.selector)); circle.decommissionCircle(BASE_CIRCLE_ID); } - function testFail_WithdrawNotEnoughContributions() public { - // Only two members deposit + function test_RevertWhen_NotEnoughContributions() public { + createBaseCircle(); + vm.prank(alice); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); vm.prank(bob); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); - // Try to withdraw vm.prank(alice); + vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); circle.withdraw(BASE_CIRCLE_ID); } diff --git a/test/mocks/MockERC20.sol b/test/mocks/MockERC20.sol index 505e98a..c125d14 100644 --- a/test/mocks/MockERC20.sol +++ b/test/mocks/MockERC20.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.28; import {ERC20} from '@openzeppelin/token/ERC20/ERC20.sol'; diff --git a/test/unit/SavingCircleTest.t.sol b/test/unit/SavingCircleTest.t.sol index 882ced6..be084b4 100644 --- a/test/unit/SavingCircleTest.t.sol +++ b/test/unit/SavingCircleTest.t.sol @@ -5,16 +5,20 @@ import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgrad import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; -import {ISavingsCircle, SavingsCircle} from 'contracts/SavingsCircle.sol'; import {Test} from 'forge-std/Test.sol'; +import {MockERC20} from '../mocks/MockERC20.sol'; +import {ISavingsCircle, SavingsCircle} from 'contracts/SavingsCircle.sol'; + +/* solhint-disable func-name-mixedcase */ + contract SavingsCircleTest is Test { uint256 public constant DEPOSIT_AMOUNT = 1 ether; uint256 public constant DEPOSIT_INTERVAL = 1 days; uint256 public constant CIRCLE_DURATION = 30 days; - SavingsCircle public savingcircles; - IERC20 public token; + SavingsCircle public savingsCircle; + MockERC20 public token; // Test addresses address public owner; @@ -34,14 +38,28 @@ contract SavingsCircleTest is Test { alice = makeAddr('alice'); bob = makeAddr('bob'); carol = makeAddr('carol'); - token = IERC20(makeAddr('token')); + + // Deploy and initialize the contract + vm.startPrank(owner); + savingsCircle = SavingsCircle( + address( + new TransparentUpgradeableProxy( + address(new SavingsCircle()), + address(new ProxyAdmin(owner)), + abi.encodeWithSelector(SavingsCircle.initialize.selector, owner) + ) + ) + ); + + token = new MockERC20('Test Token', 'TEST'); + savingsCircle.setTokenAllowed(address(token), true); + vm.stopPrank(); // Setup test data - members = new address[](4); + members = new address[](3); members[0] = alice; members[1] = bob; members[2] = carol; - members[3] = owner; baseCircleId = keccak256(abi.encodePacked('Test Circle')); // Setup savingcircles parameters @@ -57,26 +75,15 @@ contract SavingsCircleTest is Test { maxDeposits: 1000 }); - // Deploy and initialize the contract - vm.startPrank(owner); - SavingsCircle implementation = new SavingsCircle(); - ProxyAdmin proxyAdmin = new ProxyAdmin(owner); - bytes memory initData = abi.encodeWithSelector(SavingsCircle.initialize.selector, owner); - TransparentUpgradeableProxy proxy = - new TransparentUpgradeableProxy(address(implementation), address(proxyAdmin), initData); - savingcircles = SavingsCircle(address(proxy)); - savingcircles.setTokenAllowed(address(token), true); - vm.stopPrank(); - - // Create initial test savingcircles + // Create an initial test circle vm.prank(alice); - savingcircles.addCircle(baseCircle); + savingsCircle.addCircle(baseCircle); } function test_SetTokenAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingcircles.setTokenAllowed(address(0x1), true); + savingsCircle.setTokenAllowed(address(0x1), true); } function test_SetTokenAllowedWhenCallerIsOwner() external { @@ -85,24 +92,24 @@ contract SavingsCircleTest is Test { vm.prank(owner); vm.expectEmit(true, true, true, true); emit ISavingsCircle.TokenAllowed(newToken, true); - savingcircles.setTokenAllowed(newToken, true); + savingsCircle.setTokenAllowed(newToken, true); - assertTrue(savingcircles.isTokenAllowed(newToken)); + assertTrue(savingsCircle.isTokenAllowed(newToken)); } function test_SetTokenNotAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingcircles.setTokenAllowed(address(token), false); + savingsCircle.setTokenAllowed(address(token), false); } function test_SetTokenNotAllowedWhenCallerIsOwner() external { vm.prank(owner); vm.expectEmit(true, true, true, true); emit ISavingsCircle.TokenAllowed(address(token), false); - savingcircles.setTokenAllowed(address(token), false); + savingsCircle.setTokenAllowed(address(token), false); - assertFalse(savingcircles.isTokenAllowed(address(token))); + assertFalse(savingsCircle.isTokenAllowed(address(token))); } function test_DepositWhenCircleDoesNotExist() external { @@ -110,24 +117,28 @@ contract SavingsCircleTest is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingcircles.deposit(nonExistentCircleId, DEPOSIT_AMOUNT); + savingsCircle.deposit(nonExistentCircleId, DEPOSIT_AMOUNT); } function test_DepositWhenMemberHasAlreadyDeposited() external { + // Mint tokens to alice for deposit + token.mint(alice, DEPOSIT_AMOUNT * 2); + + // Mock token approval + vm.startPrank(alice); + token.approve(address(savingsCircle), DEPOSIT_AMOUNT * 2); + // First deposit - vm.startPrank(STRANGER); - vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); - vm.stopPrank(); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); - // Second deposit attempt - vm.prank(bob); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.AlreadyDeposited.selector)); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + // Second deposit attempt should fail since member has already deposited max amount + vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); } function test_DepositWhenParametersAreValid() external { - vm.startPrank(STRANGER); + vm.startPrank(alice); // Mock token transfer vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); @@ -136,10 +147,10 @@ contract SavingsCircleTest is Test { vm.expectEmit(true, true, true, true); emit ISavingsCircle.DepositMade(baseCircleId, alice, DEPOSIT_AMOUNT); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); // Verify deposit was recorded - uint256 balance = savingcircles.circleBalances(baseCircleId, alice); + uint256 balance = savingsCircle.balances(baseCircleId, alice); assertEq(balance, DEPOSIT_AMOUNT); vm.stopPrank(); @@ -151,7 +162,7 @@ contract SavingsCircleTest is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); } function test_WithdrawWhenCircleDoesNotExist() external { @@ -159,21 +170,21 @@ contract SavingsCircleTest is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingcircles.withdraw(nonExistentCircleId); + savingsCircle.withdraw(nonExistentCircleId); } function test_WithdrawWhenUserIsNotACircleMember() external { address nonMember = makeAddr('nonMember'); vm.prank(nonMember); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotMember.selector)); - savingcircles.withdraw(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); + savingsCircle.withdraw(baseCircleId); } function test_WithdrawWhenPayoutRoundHasNotEnded() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); - savingcircles.withdraw(baseCircleId); + savingsCircle.withdraw(baseCircleId); } function test_WithdrawWhenUserHasAlreadyClaimed() external { @@ -181,55 +192,88 @@ contract SavingsCircleTest is Test { vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); vm.startPrank(alice); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + vm.startPrank(bob); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + vm.startPrank(carol); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); // Move time past round - vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1); + vm.warp(block.timestamp + DEPOSIT_INTERVAL); + + // Mock token transfer for withdrawal + vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transfer.selector), abi.encode(true)); // First withdrawal vm.prank(alice); - savingcircles.withdraw(baseCircleId); + savingsCircle.withdraw(baseCircleId); - // Second withdrawal attempt + // Second withdrawal attempt should fail since currentIndex has moved to next member vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); - savingcircles.withdraw(baseCircleId); + savingsCircle.withdraw(baseCircleId); } function test_WithdrawWhenParametersAreValid() external { - // Complete deposits - vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); - + // Complete deposits from all members vm.startPrank(alice); - savingcircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + token.mint(alice, DEPOSIT_AMOUNT); + token.approve(address(savingsCircle), DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); - // Move time past round - vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1); + vm.startPrank(bob); + token.mint(bob, DEPOSIT_AMOUNT); + token.approve(address(savingsCircle), DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); - // Mock token transfer for withdrawal - vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transfer.selector), abi.encode(true)); + vm.startPrank(carol); + token.mint(carol, DEPOSIT_AMOUNT); + token.approve(address(savingsCircle), DEPOSIT_AMOUNT); + savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + // Move time past first round + vm.warp(block.timestamp + DEPOSIT_INTERVAL); + // Mint tokens to contract to enable withdrawal + uint256 withdrawAmount = DEPOSIT_AMOUNT * members.length; + + // First member (alice) should be able to withdraw vm.prank(alice); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.WithdrawalMade(baseCircleId, alice, DEPOSIT_AMOUNT); - savingcircles.withdraw(baseCircleId); + emit ISavingsCircle.WithdrawalMade(baseCircleId, alice, withdrawAmount); + savingsCircle.withdraw(baseCircleId); + + // Verify alice received the tokens + assertEq(token.balanceOf(alice), withdrawAmount); + + // Verify all member balances were reset + (, uint256[] memory balances) = savingsCircle.balancesForCircle(baseCircleId); + for (uint256 i = 0; i < balances.length; i++) { + assertEq(balances[i], 0); + } - // Verify withdrawal was recorded - bool hasWithdrawn = savingcircles.circleWithdrawable(baseCircleId); - assertFalse(hasWithdrawn); + // Verify current index moved to next member + ISavingsCircle.Circle memory circle = savingsCircle.circle(baseCircleId); + assertEq(circle.currentIndex, 1); } function test_CircleInfoWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingcircles.circle(nonExistentCircleId); + savingsCircle.circle(nonExistentCircleId); } function test_CircleInfoWhenCircleExists() external { - ISavingsCircle.Circle memory _circle = savingcircles.circle(baseCircleId); + ISavingsCircle.Circle memory _circle = savingsCircle.circle(baseCircleId); assertEq(_circle.name, 'Test Circle'); assertEq(_circle.members.length, members.length); @@ -240,8 +284,8 @@ contract SavingsCircleTest is Test { function test_DecommissionWhenCallerIsNotOwner() external { vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingcircles.decommissionCircle(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotOwner.selector)); + savingsCircle.decommissionCircle(baseCircleId); } function test_DecommissionWhenCircleDoesNotExist() external { @@ -249,61 +293,66 @@ contract SavingsCircleTest is Test { vm.prank(owner); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingcircles.decommissionCircle(nonExistentCircleId); + savingsCircle.decommissionCircle(nonExistentCircleId); } function test_DecommissionWhenParametersAreValid() external { vm.prank(owner); vm.expectEmit(true, true, true, true); emit ISavingsCircle.CircleDecommissioned(baseCircleId); - savingcircles.decommissionCircle(baseCircleId); - address[] memory emptyMembers = savingcircles.circleMembers(baseCircleId); + savingsCircle.decommissionCircle(baseCircleId); + address[] memory emptyMembers = savingsCircle.circleMembers(baseCircleId); assertEq(emptyMembers.length, 0); } function test_AddCircleWhenCircleNameAlreadyExists() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleExists.selector)); - savingcircles.addCircle(baseCircle); + savingsCircle.addCircle(baseCircle); } function test_AddCircleWhenTokenIsNotWhitelisted() external { - address nonWhitelistedToken = makeAddr('nonWhitelistedToken'); - ISavingsCircle.Circle memory invalidParams = baseCircle; - invalidParams.token = nonWhitelistedToken; + address _notAllowedToken = makeAddr('notAllowedToken'); + + ISavingsCircle.Circle memory _invalidCircle = baseCircle; + _invalidCircle.name = 'Invalid Circle'; + _invalidCircle.token = _notAllowedToken; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidToken.selector)); - savingcircles.addCircle(invalidParams); + savingsCircle.addCircle(_invalidCircle); } function test_AddCircleWhenIntervalIsZero() external { - ISavingsCircle.Circle memory invalidParams = baseCircle; - invalidParams.depositInterval = 0; + ISavingsCircle.Circle memory _invalidCircle = baseCircle; + _invalidCircle.name = 'Invalid Circle'; + _invalidCircle.depositInterval = 0; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidInterval.selector)); - savingcircles.addCircle(invalidParams); + savingsCircle.addCircle(_invalidCircle); } function test_AddCircleWhenDepositAmountIsZero() external { - ISavingsCircle.Circle memory invalidParams = baseCircle; - invalidParams.depositAmount = 0; + ISavingsCircle.Circle memory _invalidCircle = baseCircle; + _invalidCircle.name = 'Invalid Circle'; + _invalidCircle.depositAmount = 0; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); - savingcircles.addCircle(invalidParams); + savingsCircle.addCircle(_invalidCircle); } function test_AddCircleWhenMembersCountIsLessThanTwo() external { - address[] memory singleMember = new address[](1); - singleMember[0] = alice; + address[] memory _oneMember = new address[](1); + _oneMember[0] = alice; - ISavingsCircle.Circle memory invalidParams = baseCircle; - invalidParams.members = singleMember; + ISavingsCircle.Circle memory _invalidCircle = baseCircle; + _invalidCircle.name = 'Invalid Circle'; + _invalidCircle.members = _oneMember; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidMembers.selector)); - savingcircles.addCircle(invalidParams); + savingsCircle.addCircle(_invalidCircle); } } From b2acdb068d21c8f0ed099c019743785a00ed7881 Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 23 Dec 2024 15:55:01 -0500 Subject: [PATCH 04/10] refactor: rename savings circles to saving circles --- script/DeploySavingsCircle.s.sol | 8 +- .../{SavingsCircle.sol => SavingCircles.sol} | 20 +-- ...{ISavingsCircle.sol => ISavingCircles.sol} | 2 +- test/integration/IntegrationBase.sol | 1 + .../SavingCircles.t.sol} | 44 ++--- ...rcleTest.t.sol => SavingCirclesUnit.t.sol} | 150 +++++++++--------- ...CircleTest.tree => SavingCirclesUnit.tree} | 14 +- 7 files changed, 120 insertions(+), 119 deletions(-) rename src/contracts/{SavingsCircle.sol => SavingCircles.sol} (94%) rename src/interfaces/{ISavingsCircle.sol => ISavingCircles.sol} (98%) rename test/{SavingsCircle.t.sol => integration/SavingCircles.t.sol} (87%) rename test/unit/{SavingCircleTest.t.sol => SavingCirclesUnit.t.sol} (65%) rename test/unit/{SavingCircleTest.tree => SavingCirclesUnit.tree} (92%) diff --git a/script/DeploySavingsCircle.s.sol b/script/DeploySavingsCircle.s.sol index 71fd149..890baaf 100644 --- a/script/DeploySavingsCircle.s.sol +++ b/script/DeploySavingsCircle.s.sol @@ -1,24 +1,24 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.28; -import {SavingsCircle} from '../src/contracts/SavingsCircle.sol'; +import {SavingCircles} from '../src/contracts/SavingCircles.sol'; import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Script} from 'forge-std/Script.sol'; -contract DeploySavingsCircle is Script { +contract DeploySavingCircles is Script { function run() external returns (address proxy, address admin) { vm.startBroadcast(); // Deploy implementation - SavingsCircle implementation = new SavingsCircle(); + SavingCircles implementation = new SavingCircles(); // Deploy ProxyAdmin ProxyAdmin proxyAdmin = new ProxyAdmin(msg.sender); // Encode initialization call - bytes memory initData = abi.encodeWithSelector(SavingsCircle.initialize.selector); + bytes memory initData = abi.encodeWithSelector(SavingCircles.initialize.selector); // Deploy proxy TransparentUpgradeableProxy transparentProxy = diff --git a/src/contracts/SavingsCircle.sol b/src/contracts/SavingCircles.sol similarity index 94% rename from src/contracts/SavingsCircle.sol rename to src/contracts/SavingCircles.sol index 21f4804..cbd1867 100644 --- a/src/contracts/SavingsCircle.sol +++ b/src/contracts/SavingCircles.sol @@ -5,16 +5,16 @@ import {OwnableUpgradeable} from '@openzeppelin-upgradeable/access/OwnableUpgrad import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {ReentrancyGuard} from '@openzeppelin/utils/ReentrancyGuard.sol'; -import {ISavingsCircle} from '../interfaces/ISavingsCircle.sol'; +import {ISavingCircles} from '../interfaces/ISavingCircles.sol'; /** - * @title Savings Circle + * @title Saving Circles * @notice TODO * @author Breadchain Collective * @author @RonTuretzky * @author @bagelface */ -contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { +contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { mapping(bytes32 id => Circle circle) public circles; mapping(bytes32 id => mapping(address => uint256)) public balances; mapping(address token => bool status) public allowedTokens; @@ -59,8 +59,8 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice Commission a new savings circle - * @param _circle A new savings circle + * @notice Commission a new saving circle + * @param _circle A new saving circle */ function addCircle(Circle memory _circle) external override { bytes32 _id = keccak256(abi.encodePacked(_circle.name)); @@ -133,9 +133,9 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice Set if a token can be used for savings circles + * @notice Set if a token can be used for saving circles * @param _token Token to update the status of - * @param _allowed Can be used for savings circles + * @param _allowed Can be used for saving circles */ function setTokenAllowed(address _token, bool _allowed) external override onlyOwner { allowedTokens[_token] = _allowed; @@ -144,7 +144,7 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice Decommission an existing savings circle + * @notice Decommission an existing saving circle * @dev Returns all deposits to members * @param _id Identifier of the circle */ @@ -187,9 +187,9 @@ contract SavingsCircle is ISavingsCircle, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice Return the info of a specified savings circle + * @notice Return the info of a specified saving circle * @param _id Identifier of the circle - * @return _circle Savings circle + * @return _circle Saving circle */ function circle(bytes32 _id) external view override returns (Circle memory _circle) { _circle = circles[_id]; diff --git a/src/interfaces/ISavingsCircle.sol b/src/interfaces/ISavingCircles.sol similarity index 98% rename from src/interfaces/ISavingsCircle.sol rename to src/interfaces/ISavingCircles.sol index 5ec7bea..5c78051 100644 --- a/src/interfaces/ISavingsCircle.sol +++ b/src/interfaces/ISavingCircles.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.28; -interface ISavingsCircle { +interface ISavingCircles { struct Circle { address owner; string name; diff --git a/test/integration/IntegrationBase.sol b/test/integration/IntegrationBase.sol index 1d71ce4..3b738cb 100644 --- a/test/integration/IntegrationBase.sol +++ b/test/integration/IntegrationBase.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.28; import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Test} from 'forge-std/Test.sol'; import {Common} from 'script/Common.sol'; + // solhint-disable-next-line import 'script/Registry.sol'; diff --git a/test/SavingsCircle.t.sol b/test/integration/SavingCircles.t.sol similarity index 87% rename from test/SavingsCircle.t.sol rename to test/integration/SavingCircles.t.sol index f849942..33c55e4 100644 --- a/test/SavingsCircle.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -6,14 +6,14 @@ import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Test} from 'forge-std/Test.sol'; -import {SavingsCircle} from '../src/contracts/SavingsCircle.sol'; -import {ISavingsCircle} from '../src/interfaces/ISavingsCircle.sol'; -import {MockERC20} from './mocks/MockERC20.sol'; +import {SavingCircles} from '../../src/contracts/SavingCircles.sol'; +import {ISavingCircles} from '../../src/interfaces/ISavingCircles.sol'; +import {MockERC20} from '../mocks/MockERC20.sol'; /* solhint-disable func-name-mixedcase */ -contract SavingsCircleTest is Test { - SavingsCircle public circle; +contract SavingCirclesIntegration is Test { + SavingCircles public circle; MockERC20 public token; address public alice = makeAddr('alice'); @@ -29,16 +29,16 @@ contract SavingsCircleTest is Test { uint256 public constant BASE_MAX_DEPOSITS = 1000; bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked(BASE_CIRCLE_NAME)); - ISavingsCircle.Circle public baseCircle; + ISavingCircles.Circle public baseCircle; function setUp() public { vm.startPrank(owner); - circle = SavingsCircle( + circle = SavingCircles( address( new TransparentUpgradeableProxy( - address(new SavingsCircle()), + address(new SavingCircles()), address(new ProxyAdmin(owner)), - abi.encodeWithSelector(SavingsCircle.initialize.selector, owner) + abi.encodeWithSelector(SavingCircles.initialize.selector, owner) ) ) ); @@ -65,7 +65,7 @@ contract SavingsCircleTest is Test { members.push(carol); vm.stopPrank(); - baseCircle = ISavingsCircle.Circle({ + baseCircle = ISavingCircles.Circle({ owner: alice, name: BASE_CIRCLE_NAME, members: members, @@ -113,7 +113,7 @@ contract SavingsCircleTest is Test { // Test emitted events vm.prank(owner); vm.expectEmit(true, true, false, true); - emit ISavingsCircle.TokenAllowed(address(token), false); + emit ISavingCircles.TokenAllowed(address(token), false); circle.setTokenAllowed(address(token), false); } @@ -127,7 +127,7 @@ contract SavingsCircleTest is Test { address badToken = makeAddr('badToken'); baseCircle.token = badToken; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidToken.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidToken.selector)); circle.addCircle(baseCircle); } @@ -175,7 +175,7 @@ contract SavingsCircleTest is Test { // Try to withdraw before interval vm.prank(bob); - vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + vm.expectRevert(ISavingCircles.NotWithdrawable.selector); circle.withdraw(BASE_CIRCLE_ID); // Wait for interval (need to wait for index 1's interval) @@ -215,7 +215,7 @@ contract SavingsCircleTest is Test { assertEq(token.balanceOf(bob) - bobBalanceBefore, DEPOSIT_AMOUNT); // Check circle deleted - vm.expectRevert(ISavingsCircle.CircleNotFound.selector); + vm.expectRevert(ISavingCircles.CircleNotFound.selector); circle.circle(BASE_CIRCLE_ID); } @@ -223,7 +223,7 @@ contract SavingsCircleTest is Test { createBaseCircle(); vm.prank(bob); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotOwner.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotOwner.selector)); circle.decommissionCircle(BASE_CIRCLE_ID); } @@ -237,7 +237,7 @@ contract SavingsCircleTest is Test { circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); vm.prank(alice); - vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + vm.expectRevert(ISavingCircles.NotWithdrawable.selector); circle.withdraw(BASE_CIRCLE_ID); } @@ -246,7 +246,7 @@ contract SavingsCircleTest is Test { // // Branch 1: Circle doesn't exist // bytes32 nonExistentCircle = keccak256(abi.encodePacked("Non Existent")); // vm.prank(alice); - // vm.expectRevert(ISavingsCircle.CircleNotFound.selector); + // vm.expectRevert(ISavingCircles.CircleNotFound.selector); // circle.withdraw(nonExistentCircle); // // Setup circle for remaining tests @@ -261,7 +261,7 @@ contract SavingsCircleTest is Test { // // Branch 2: Not enough time passed // vm.prank(alice); - // vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + // vm.expectRevert(ISavingCircles.NotWithdrawable.selector); // circle.withdraw(hashedName); // // Branch 3: Not all members contributed @@ -271,14 +271,14 @@ contract SavingsCircleTest is Test { // circle.deposit(hashedName, DEPOSIT_AMOUNT); // // Carol hasn't contributed // vm.prank(alice); - // vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + // vm.expectRevert(ISavingCircles.NotWithdrawable.selector); // circle.withdraw(hashedName); // // Branch 4: Wrong member trying to withdraw // vm.prank(carol); // circle.deposit(hashedName, DEPOSIT_AMOUNT); // vm.prank(bob); - // vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + // vm.expectRevert(ISavingCircles.NotWithdrawable.selector); // circle.withdraw(hashedName); // // Branch 5: Successful withdrawal @@ -287,7 +287,7 @@ contract SavingsCircleTest is Test { // // Branch 6: Second withdrawal before interval // vm.prank(bob); - // vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); + // vm.expectRevert(ISavingCircles.NotWithdrawable.selector); // circle.withdraw(hashedName); // // Branch 7: Second withdrawal after interval @@ -303,7 +303,7 @@ contract SavingsCircleTest is Test { // // Branch 9: Circle wraps around // vm.warp(block.timestamp + DEPOSIT_INTERVAL); // vm.prank(alice); - // vm.expectRevert(ISavingsCircle.NotWithdrawable.selector); // Should fail as no new deposits made + // vm.expectRevert(ISavingCircles.NotWithdrawable.selector); // Should fail as no new deposits made // circle.withdraw(hashedName); // } } diff --git a/test/unit/SavingCircleTest.t.sol b/test/unit/SavingCirclesUnit.t.sol similarity index 65% rename from test/unit/SavingCircleTest.t.sol rename to test/unit/SavingCirclesUnit.t.sol index be084b4..3636699 100644 --- a/test/unit/SavingCircleTest.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -8,16 +8,16 @@ import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Test} from 'forge-std/Test.sol'; import {MockERC20} from '../mocks/MockERC20.sol'; -import {ISavingsCircle, SavingsCircle} from 'contracts/SavingsCircle.sol'; +import {ISavingCircles, SavingCircles} from 'contracts/SavingCircles.sol'; /* solhint-disable func-name-mixedcase */ -contract SavingsCircleTest is Test { +contract SavingCirclesUnit is Test { uint256 public constant DEPOSIT_AMOUNT = 1 ether; uint256 public constant DEPOSIT_INTERVAL = 1 days; uint256 public constant CIRCLE_DURATION = 30 days; - SavingsCircle public savingsCircle; + SavingCircles public savingCircles; MockERC20 public token; // Test addresses @@ -30,7 +30,7 @@ contract SavingsCircleTest is Test { // Test data bytes32 public baseCircleId; address[] public members; - ISavingsCircle.Circle public baseCircle; + ISavingCircles.Circle public baseCircle; function setUp() external { // Setup test addresses @@ -41,18 +41,18 @@ contract SavingsCircleTest is Test { // Deploy and initialize the contract vm.startPrank(owner); - savingsCircle = SavingsCircle( + savingCircles = SavingCircles( address( new TransparentUpgradeableProxy( - address(new SavingsCircle()), + address(new SavingCircles()), address(new ProxyAdmin(owner)), - abi.encodeWithSelector(SavingsCircle.initialize.selector, owner) + abi.encodeWithSelector(SavingCircles.initialize.selector, owner) ) ) ); token = new MockERC20('Test Token', 'TEST'); - savingsCircle.setTokenAllowed(address(token), true); + savingCircles.setTokenAllowed(address(token), true); vm.stopPrank(); // Setup test data @@ -63,7 +63,7 @@ contract SavingsCircleTest is Test { baseCircleId = keccak256(abi.encodePacked('Test Circle')); // Setup savingcircles parameters - baseCircle = ISavingsCircle.Circle({ + baseCircle = ISavingCircles.Circle({ owner: owner, name: 'Test Circle', members: members, @@ -77,13 +77,13 @@ contract SavingsCircleTest is Test { // Create an initial test circle vm.prank(alice); - savingsCircle.addCircle(baseCircle); + savingCircles.addCircle(baseCircle); } function test_SetTokenAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingsCircle.setTokenAllowed(address(0x1), true); + savingCircles.setTokenAllowed(address(0x1), true); } function test_SetTokenAllowedWhenCallerIsOwner() external { @@ -91,33 +91,33 @@ contract SavingsCircleTest is Test { vm.prank(owner); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.TokenAllowed(newToken, true); - savingsCircle.setTokenAllowed(newToken, true); + emit ISavingCircles.TokenAllowed(newToken, true); + savingCircles.setTokenAllowed(newToken, true); - assertTrue(savingsCircle.isTokenAllowed(newToken)); + assertTrue(savingCircles.isTokenAllowed(newToken)); } function test_SetTokenNotAllowedWhenCallerIsNotOwner() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); - savingsCircle.setTokenAllowed(address(token), false); + savingCircles.setTokenAllowed(address(token), false); } function test_SetTokenNotAllowedWhenCallerIsOwner() external { vm.prank(owner); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.TokenAllowed(address(token), false); - savingsCircle.setTokenAllowed(address(token), false); + emit ISavingCircles.TokenAllowed(address(token), false); + savingCircles.setTokenAllowed(address(token), false); - assertFalse(savingsCircle.isTokenAllowed(address(token))); + assertFalse(savingCircles.isTokenAllowed(address(token))); } function test_DepositWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingsCircle.deposit(nonExistentCircleId, DEPOSIT_AMOUNT); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + savingCircles.deposit(nonExistentCircleId, DEPOSIT_AMOUNT); } function test_DepositWhenMemberHasAlreadyDeposited() external { @@ -126,14 +126,14 @@ contract SavingsCircleTest is Test { // Mock token approval vm.startPrank(alice); - token.approve(address(savingsCircle), DEPOSIT_AMOUNT * 2); + token.approve(address(savingCircles), DEPOSIT_AMOUNT * 2); // First deposit - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); // Second deposit attempt should fail since member has already deposited max amount - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector)); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); } @@ -145,12 +145,12 @@ contract SavingsCircleTest is Test { // Expect deposit event vm.expectEmit(true, true, true, true); - emit ISavingsCircle.DepositMade(baseCircleId, alice, DEPOSIT_AMOUNT); + emit ISavingCircles.DepositMade(baseCircleId, alice, DEPOSIT_AMOUNT); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); // Verify deposit was recorded - uint256 balance = savingsCircle.balances(baseCircleId, alice); + uint256 balance = savingCircles.balances(baseCircleId, alice); assertEq(balance, DEPOSIT_AMOUNT); vm.stopPrank(); @@ -161,30 +161,30 @@ contract SavingsCircleTest is Test { vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1); vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector)); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); } function test_WithdrawWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingsCircle.withdraw(nonExistentCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + savingCircles.withdraw(nonExistentCircleId); } function test_WithdrawWhenUserIsNotACircleMember() external { address nonMember = makeAddr('nonMember'); vm.prank(nonMember); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); - savingsCircle.withdraw(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotWithdrawable.selector)); + savingCircles.withdraw(baseCircleId); } function test_WithdrawWhenPayoutRoundHasNotEnded() external { vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); - savingsCircle.withdraw(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotWithdrawable.selector)); + savingCircles.withdraw(baseCircleId); } function test_WithdrawWhenUserHasAlreadyClaimed() external { @@ -192,15 +192,15 @@ contract SavingsCircleTest is Test { vm.mockCall(address(token), abi.encodeWithSelector(IERC20.transferFrom.selector), abi.encode(true)); vm.startPrank(alice); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); vm.startPrank(bob); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); vm.startPrank(carol); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); // Move time past round @@ -211,32 +211,32 @@ contract SavingsCircleTest is Test { // First withdrawal vm.prank(alice); - savingsCircle.withdraw(baseCircleId); + savingCircles.withdraw(baseCircleId); // Second withdrawal attempt should fail since currentIndex has moved to next member vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotWithdrawable.selector)); - savingsCircle.withdraw(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotWithdrawable.selector)); + savingCircles.withdraw(baseCircleId); } function test_WithdrawWhenParametersAreValid() external { // Complete deposits from all members vm.startPrank(alice); token.mint(alice, DEPOSIT_AMOUNT); - token.approve(address(savingsCircle), DEPOSIT_AMOUNT); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); vm.startPrank(bob); token.mint(bob, DEPOSIT_AMOUNT); - token.approve(address(savingsCircle), DEPOSIT_AMOUNT); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); vm.startPrank(carol); token.mint(carol, DEPOSIT_AMOUNT); - token.approve(address(savingsCircle), DEPOSIT_AMOUNT); - savingsCircle.deposit(baseCircleId, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.stopPrank(); // Move time past first round @@ -248,32 +248,32 @@ contract SavingsCircleTest is Test { // First member (alice) should be able to withdraw vm.prank(alice); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.WithdrawalMade(baseCircleId, alice, withdrawAmount); - savingsCircle.withdraw(baseCircleId); + emit ISavingCircles.WithdrawalMade(baseCircleId, alice, withdrawAmount); + savingCircles.withdraw(baseCircleId); // Verify alice received the tokens assertEq(token.balanceOf(alice), withdrawAmount); // Verify all member balances were reset - (, uint256[] memory balances) = savingsCircle.balancesForCircle(baseCircleId); + (, uint256[] memory balances) = savingCircles.balancesForCircle(baseCircleId); for (uint256 i = 0; i < balances.length; i++) { assertEq(balances[i], 0); } // Verify current index moved to next member - ISavingsCircle.Circle memory circle = savingsCircle.circle(baseCircleId); + ISavingCircles.Circle memory circle = savingCircles.circle(baseCircleId); assertEq(circle.currentIndex, 1); } function test_CircleInfoWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingsCircle.circle(nonExistentCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + savingCircles.circle(nonExistentCircleId); } function test_CircleInfoWhenCircleExists() external { - ISavingsCircle.Circle memory _circle = savingsCircle.circle(baseCircleId); + ISavingCircles.Circle memory _circle = savingCircles.circle(baseCircleId); assertEq(_circle.name, 'Test Circle'); assertEq(_circle.members.length, members.length); @@ -284,75 +284,75 @@ contract SavingsCircleTest is Test { function test_DecommissionWhenCallerIsNotOwner() external { vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.NotOwner.selector)); - savingsCircle.decommissionCircle(baseCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotOwner.selector)); + savingCircles.decommissionCircle(baseCircleId); } function test_DecommissionWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.prank(owner); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleNotFound.selector)); - savingsCircle.decommissionCircle(nonExistentCircleId); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + savingCircles.decommissionCircle(nonExistentCircleId); } function test_DecommissionWhenParametersAreValid() external { vm.prank(owner); vm.expectEmit(true, true, true, true); - emit ISavingsCircle.CircleDecommissioned(baseCircleId); - savingsCircle.decommissionCircle(baseCircleId); - address[] memory emptyMembers = savingsCircle.circleMembers(baseCircleId); + emit ISavingCircles.CircleDecommissioned(baseCircleId); + savingCircles.decommissionCircle(baseCircleId); + address[] memory emptyMembers = savingCircles.circleMembers(baseCircleId); assertEq(emptyMembers.length, 0); } function test_AddCircleWhenCircleNameAlreadyExists() external { vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.CircleExists.selector)); - savingsCircle.addCircle(baseCircle); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleExists.selector)); + savingCircles.addCircle(baseCircle); } function test_AddCircleWhenTokenIsNotWhitelisted() external { address _notAllowedToken = makeAddr('notAllowedToken'); - ISavingsCircle.Circle memory _invalidCircle = baseCircle; + ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.token = _notAllowedToken; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidToken.selector)); - savingsCircle.addCircle(_invalidCircle); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidToken.selector)); + savingCircles.addCircle(_invalidCircle); } function test_AddCircleWhenIntervalIsZero() external { - ISavingsCircle.Circle memory _invalidCircle = baseCircle; + ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositInterval = 0; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidInterval.selector)); - savingsCircle.addCircle(_invalidCircle); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidInterval.selector)); + savingCircles.addCircle(_invalidCircle); } function test_AddCircleWhenDepositAmountIsZero() external { - ISavingsCircle.Circle memory _invalidCircle = baseCircle; + ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositAmount = 0; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidDeposit.selector)); - savingsCircle.addCircle(_invalidCircle); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidDeposit.selector)); + savingCircles.addCircle(_invalidCircle); } function test_AddCircleWhenMembersCountIsLessThanTwo() external { address[] memory _oneMember = new address[](1); _oneMember[0] = alice; - ISavingsCircle.Circle memory _invalidCircle = baseCircle; + ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.members = _oneMember; vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingsCircle.InvalidMembers.selector)); - savingsCircle.addCircle(_invalidCircle); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidMembers.selector)); + savingCircles.addCircle(_invalidCircle); } } diff --git a/test/unit/SavingCircleTest.tree b/test/unit/SavingCirclesUnit.tree similarity index 92% rename from test/unit/SavingCircleTest.tree rename to test/unit/SavingCirclesUnit.tree index ed86b18..96bd838 100644 --- a/test/unit/SavingCircleTest.tree +++ b/test/unit/SavingCirclesUnit.tree @@ -1,18 +1,18 @@ -SavingsCircleTest::AllowlistToken +SavingCirclesUnit::AllowlistToken ├── when caller is not owner │ └── it reverts └── when caller is owner ├── it allowslists the token └── it emits token allowedlisted event -SavingsCircleTest::DenylistToken +SavingCirclesUnit::DenylistToken ├── when caller is not owner │ └── it reverts └── when caller is owner ├── it denylists the token └── it emits token deniedlisted event -SavingsCircleTest::addCircle +SavingCirclesUnit::addCircle ├── when circle name already exists │ └── it reverts ├── when token is not whitelisted @@ -28,7 +28,7 @@ SavingsCircleTest::addCircle ├── it updates memberships └── it emits circle created event -SavingsCircleTest::deposit +SavingCirclesUnit::deposit ├── when circle does not exist │ └── it reverts ├── when user is not a circle member @@ -45,7 +45,7 @@ SavingsCircleTest::deposit └── given member has already deposited └── it reverts -SavingsCircleTest::withdraw +SavingCirclesUnit::withdraw ├── when circle does not exist │ └── it reverts ├── when user is not a circle member @@ -61,13 +61,13 @@ SavingsCircleTest::withdraw ├── it marks payout as claimed └── it emits payout claimed event -SavingsCircleTest::circleInfo +SavingCirclesUnit::circleInfo ├── when circle does not exist │ └── it reverts └── when circle exists └── it returns correct circle information -SavingsCircleTest::decommission +SavingCirclesUnit::decommission ├── when caller is not owner │ └── it reverts ├── when circle does not exist From ec57cddf0c99eb478ca902c1501fee07152ff62e Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 23 Dec 2024 16:45:53 -0500 Subject: [PATCH 05/10] refactor: simplify business logic, rename errors, remove redundant checks --- src/contracts/SavingCircles.sol | 92 ++++++++++++---------------- src/interfaces/ISavingCircles.sol | 4 +- test/integration/SavingCircles.t.sol | 4 +- test/unit/SavingCirclesUnit.t.sol | 18 ++---- 4 files changed, 49 insertions(+), 69 deletions(-) diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index cbd1867..826a821 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -20,35 +20,6 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { mapping(address token => bool status) public allowedTokens; mapping(bytes32 id => mapping(address member => bool status)) public isMember; - modifier validDeposit(bytes32 _id, uint256 _value, address _member) { - Circle memory _circle = circles[_id]; - - if (_circle.members.length == 0) revert CircleNotFound(); - - // Check if circle has started - 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(); - } - - _; - } - /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -64,7 +35,8 @@ 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 CircleExists(); + + 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(); @@ -74,9 +46,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { if (_circle.currentIndex != 0) revert InvalidIndex(); circles[_id] = _circle; - Circle storage _newCircle = circles[_id]; - for (uint256 i = 0; i < _newCircle.members.length; i++) { - isMember[_id][_newCircle.members[i]] = true; + for (uint256 i = 0; i < _circle.members.length; i++) { + isMember[_id][_circle.members[i]] = true; } emit CircleCreated( @@ -89,9 +60,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle * @param _value Amount of the token to deposit */ - function deposit(bytes32 _id, uint256 _value) external override nonReentrant validDeposit(_id, _value, msg.sender) { - if (!isMember[_id][msg.sender]) revert NotMember(); - + function deposit(bytes32 _id, uint256 _value) external override nonReentrant { _deposit(_id, msg.sender, _value); } @@ -101,11 +70,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _member Address to make a deposit for * @param _value Amount of the token to deposit */ - function depositFor( - bytes32 _id, - address _member, - uint256 _value - ) external override nonReentrant validDeposit(_id, _value, _member) { + function depositFor(bytes32 _id, address _member, uint256 _value) external override nonReentrant { _deposit(_id, _member, _value); } @@ -151,7 +116,6 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { function decommissionCircle(bytes32 _id) external override { Circle storage _circle = circles[_id]; - if (_circle.members.length == 0) revert CircleNotFound(); if (_circle.owner != msg.sender) revert NotOwner(); for (uint256 i = 0; i < _circle.members.length; i++) { @@ -194,7 +158,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { function circle(bytes32 _id) external view override returns (Circle memory _circle) { _circle = circles[_id]; - if (_circle.members.length == 0) revert CircleNotFound(); + if (_isDecommissioned(_circle)) revert NotCommissioned(); return _circle; } @@ -209,8 +173,9 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { override returns (address[] memory _members, uint256[] memory _balances) { - Circle storage _circle = circles[_id]; - if (_circle.members.length == 0) revert CircleNotFound(); + Circle memory _circle = circles[_id]; + + if (_isDecommissioned(_circle)) revert NotCommissioned(); _balances = new uint256[](_circle.members.length); for (uint256 i = 0; i < _circle.members.length; i++) { @@ -226,10 +191,10 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _member TODO */ function withdrawable(bytes32 _id, address _member) external view override returns (bool) { - Circle storage _circle = circles[_id]; + Circle memory _circle = circles[_id]; + if (_isDecommissioned(_circle)) revert NotCommissioned(); if (!isMember[_id][_member]) revert NotMember(); - if (_circle.members.length == 0) revert CircleNotFound(); return _circle.members[_circle.currentIndex] == _member; } @@ -243,21 +208,40 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } function _deposit(bytes32 _id, address _member, uint256 _value) internal { - Circle storage _circle = circles[_id]; + Circle memory _circle = circles[_id]; - if (_circle.members.length == 0) revert CircleNotFound(); + 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(); + } - IERC20(_circle.token).transferFrom(msg.sender, address(this), _value); + // Check if deposit amount does not exceed allowed deposit amount for member + if (balances[_id][_member] + _value > circles[_id].depositAmount) { + revert InvalidDeposit(); + } balances[_id][_member] = balances[_id][_member] + _value; + + bool success = IERC20(_circle.token).transferFrom(msg.sender, address(this), _value); + if (!success) revert TransferFailed(); + emit DepositMade(_id, _member, _value); } function _circleWithdrawable(bytes32 _id) internal view returns (bool) { - Circle storage _circle = circles[_id]; + Circle memory _circle = circles[_id]; - if (_circle.members.length == 0) revert CircleNotFound(); + if (_isDecommissioned(_circle)) revert NotCommissioned(); // Check if enough time has passed since circle start for current withdrawal if (block.timestamp < _circle.circleStart + (_circle.depositInterval * _circle.currentIndex)) { @@ -273,4 +257,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { return true; } + + function _isDecommissioned(Circle memory _circle) internal pure returns (bool) { + return _circle.owner == address(0); + } } diff --git a/src/interfaces/ISavingCircles.sol b/src/interfaces/ISavingCircles.sol index 5c78051..878e4a3 100644 --- a/src/interfaces/ISavingCircles.sol +++ b/src/interfaces/ISavingCircles.sol @@ -23,14 +23,14 @@ interface ISavingCircles { event TokenAllowed(address indexed token, bool indexed allowed); error AlreadyDeposited(); - error CircleExists(); - error CircleNotFound(); + error AlreadyExists(); error InvalidDeposit(); error InvalidIndex(); error InvalidInterval(); error InvalidMembers(); error InvalidStart(); error InvalidToken(); + error NotCommissioned(); error NotMember(); error NotOwner(); error NotWithdrawable(); diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index 33c55e4..6c28b25 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -215,7 +215,7 @@ contract SavingCirclesIntegration is Test { assertEq(token.balanceOf(bob) - bobBalanceBefore, DEPOSIT_AMOUNT); // Check circle deleted - vm.expectRevert(ISavingCircles.CircleNotFound.selector); + vm.expectRevert(ISavingCircles.NotCommissioned.selector); circle.circle(BASE_CIRCLE_ID); } @@ -246,7 +246,7 @@ contract SavingCirclesIntegration is Test { // // Branch 1: Circle doesn't exist // bytes32 nonExistentCircle = keccak256(abi.encodePacked("Non Existent")); // vm.prank(alice); - // vm.expectRevert(ISavingCircles.CircleNotFound.selector); + // vm.expectRevert(ISavingCircles.NotCommissioned.selector); // circle.withdraw(nonExistentCircle); // // Setup circle for remaining tests diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol index 3636699..77d0a9e 100644 --- a/test/unit/SavingCirclesUnit.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -116,7 +116,7 @@ contract SavingCirclesUnit is Test { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); savingCircles.deposit(nonExistentCircleId, DEPOSIT_AMOUNT); } @@ -169,7 +169,7 @@ contract SavingCirclesUnit is Test { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); savingCircles.withdraw(nonExistentCircleId); } @@ -268,11 +268,11 @@ contract SavingCirclesUnit is Test { function test_CircleInfoWhenCircleDoesNotExist() external { bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); savingCircles.circle(nonExistentCircleId); } - function test_CircleInfoWhenCircleExists() external { + function test_CircleInfoWhenCircleAlreadyExists() external { ISavingCircles.Circle memory _circle = savingCircles.circle(baseCircleId); assertEq(_circle.name, 'Test Circle'); @@ -288,14 +288,6 @@ contract SavingCirclesUnit is Test { savingCircles.decommissionCircle(baseCircleId); } - function test_DecommissionWhenCircleDoesNotExist() external { - bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); - - vm.prank(owner); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleNotFound.selector)); - savingCircles.decommissionCircle(nonExistentCircleId); - } - function test_DecommissionWhenParametersAreValid() external { vm.prank(owner); vm.expectEmit(true, true, true, true); @@ -307,7 +299,7 @@ contract SavingCirclesUnit is Test { function test_AddCircleWhenCircleNameAlreadyExists() external { vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.CircleExists.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.AlreadyExists.selector)); savingCircles.addCircle(baseCircle); } From 8ebc0ea0fb193535ae220923f489ccb6a15ccaf3 Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 23 Dec 2024 17:42:01 -0500 Subject: [PATCH 06/10] refactor: generalize custom errors, improve circle creation checks, update tests --- src/contracts/SavingCircles.sol | 66 ++++++++++------------------ src/interfaces/ISavingCircles.sol | 11 ++--- test/integration/SavingCircles.t.sol | 2 +- test/unit/SavingCirclesUnit.t.sol | 13 +++--- 4 files changed, 35 insertions(+), 57 deletions(-) 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); } } From e5f942f541e3c277169ba5faaa4d3dab514a364b Mon Sep 17 00:00:00 2001 From: bagelface Date: Thu, 26 Dec 2024 12:41:13 -0500 Subject: [PATCH 07/10] feat: allow non-owner members to decommission if deposits are not received on time --- src/contracts/SavingCircles.sol | 82 +++++++++++++++++++--------- src/interfaces/ISavingCircles.sol | 16 +++--- test/integration/SavingCircles.t.sol | 49 +++++++++++++---- test/unit/SavingCirclesUnit.t.sol | 67 ++++++++++++++++------- test/unit/SavingCirclesUnit.tree | 2 +- 5 files changed, 149 insertions(+), 67 deletions(-) diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index 398fbc6..1f0c62a 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -9,7 +9,7 @@ import {ISavingCircles} from '../interfaces/ISavingCircles.sol'; /** * @title Saving Circles - * @notice TODO + * @notice Simple implementation of a rotating savings and credit association (ROSCA) for ERC20 tokens * @author Breadchain Collective * @author @RonTuretzky * @author @bagelface @@ -32,10 +32,10 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice Commission a new saving circle + * @notice Create a new saving circle * @param _circle A new saving circle */ - function addCircle(Circle memory _circle) external override { + function create(Circle memory _circle) external override { bytes32 _id = keccak256(abi.encodePacked(_circle.name)); if (circles[_id].owner != address(0)) revert AlreadyExists(); @@ -63,17 +63,17 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _value Amount of the token to deposit */ function deposit(bytes32 _id, uint256 _value) external override nonReentrant { - _deposit(_id, msg.sender, _value); + _deposit(_id, _value, msg.sender); } /** * @notice Make a deposit on behalf of another member * @param _id Identifier of the circle - * @param _member Address to make a deposit for * @param _value Amount of the token to deposit + * @param _member Address to make a deposit for */ - function depositFor(bytes32 _id, address _member, uint256 _value) external override nonReentrant { - _deposit(_id, _member, _value); + function depositFor(bytes32 _id, uint256 _value, address _member) external override nonReentrant { + _deposit(_id, _value, _member); } /** @@ -85,6 +85,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { if (!_withdrawable(_id)) revert NotWithdrawable(); if (_circle.members[_circle.currentIndex] != msg.sender) revert NotWithdrawable(); + if (_circle.currentIndex >= _circle.maxDeposits) revert NotWithdrawable(); uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); @@ -96,7 +97,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { bool success = IERC20(_circle.token).transfer(msg.sender, _withdrawAmount); if (!success) revert TransferFailed(); - emit WithdrawalMade(_id, msg.sender, _withdrawAmount); + emit FundsWithdrawn(_id, msg.sender, _withdrawAmount); } /** @@ -115,17 +116,33 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @dev Returns all deposits to members * @param _id Identifier of the circle */ - function decommissionCircle(bytes32 _id) external override { + function decommission(bytes32 _id) external override { Circle storage _circle = circles[_id]; - if (_circle.owner != msg.sender) revert NotOwner(); + if (_circle.owner != msg.sender) { + if (!isMember[_id][msg.sender]) revert NotMember(); + if (block.timestamp <= _circle.circleStart + (_circle.depositInterval * (_circle.currentIndex + 1))) { + revert NotDecommissionable(); + } + + bool hasIncompleteDeposits = false; + for (uint256 i = 0; i < _circle.members.length; i++) { + if (balances[_id][_circle.members[i]] < _circle.depositAmount) { + hasIncompleteDeposits = true; + break; + } + } + if (!hasIncompleteDeposits) revert NotDecommissionable(); + } + // Return deposits to members for (uint256 i = 0; i < _circle.members.length; i++) { address _member = _circle.members[i]; uint256 _balance = balances[_id][_member]; if (_balance > 0) { balances[_id][_member] = 0; - IERC20(_circle.token).transfer(_member, _balance); + bool success = IERC20(_circle.token).transfer(_member, _balance); + if (!success) revert TransferFailed(); } } @@ -157,10 +174,12 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice TODO - * @param _id TODO + * @notice Return the balances of the members of a specified saving circle + * @param _id Identifier of the circle + * @return _members Members of the specified saving circle + * @return _balances Corresponding balances of the members of the circle */ - function balancesForCircle(bytes32 _id) + function memberBalances(bytes32 _id) external view override @@ -179,28 +198,33 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } /** - * @notice TODO - * @param _id TODO - * @param _member TODO + * @notice Return the member address which is currently able to withdraw from a specified circle + * @param _id Identifier of the circle + * @return address Member that is currently able to withdraw from the circle */ - function withdrawableBy(bytes32 _id, address _member) external view override returns (bool) { + function withdrawableBy(bytes32 _id) external view override returns (address) { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); - if (!isMember[_id][_member]) revert NotMember(); - return _circle.members[_circle.currentIndex] == _member; + return _circle.members[_circle.currentIndex]; } /** - * @notice TODO - * @param _id TODO + * @notice Return if a circle can currently be withdrawn from + * @param _id Identifier of the circle + * @return bool If the circle is able to be withdrawn from */ function withdrawable(bytes32 _id) external view override returns (bool) { return _withdrawable(_id); } - function _deposit(bytes32 _id, address _member, uint256 _value) internal { + /** + * @dev Make a deposit into a specified circle + * A deposit must be made in specific time window and can be made partially so long as the final balance equals + * the specified deposit amount for the circle. + */ + function _deposit(bytes32 _id, uint256 _value, address _member) internal { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); @@ -217,20 +241,23 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { bool success = IERC20(_circle.token).transferFrom(msg.sender, address(this), _value); if (!success) revert TransferFailed(); - emit DepositMade(_id, _member, _value); + emit FundsDeposited(_id, _member, _value); } + /** + * @dev Return if a specified circle is withdrawable + * To be considered withdrawable, enough time must have passed since the deposit interval started + * and all members must have made a deposit. + */ function _withdrawable(bytes32 _id) internal view returns (bool) { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); - // Check if enough time has passed since circle start for current withdrawal if (block.timestamp < _circle.circleStart + (_circle.depositInterval * _circle.currentIndex)) { return false; } - // Check if all members have made their initial deposit for (uint256 i = 0; i < _circle.members.length; i++) { if (balances[_id][_circle.members[i]] < _circle.depositAmount) { return false; @@ -240,6 +267,9 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { return true; } + /** + * @dev Return if a specified circle is decommissioned by checking if an owner is set + */ function _isDecommissioned(Circle memory _circle) internal pure returns (bool) { return _circle.owner == address(0); } diff --git a/src/interfaces/ISavingCircles.sol b/src/interfaces/ISavingCircles.sol index db94a68..00c6b07 100644 --- a/src/interfaces/ISavingCircles.sol +++ b/src/interfaces/ISavingCircles.sol @@ -18,8 +18,8 @@ interface ISavingCircles { bytes32 indexed id, string name, address[] members, address token, uint256 depositAmount, uint256 depositInterval ); event CircleDecommissioned(bytes32 indexed id); - event DepositMade(bytes32 indexed id, address indexed contributor, uint256 amount); - event WithdrawalMade(bytes32 indexed id, address indexed withdrawer, uint256 amount); + event FundsDeposited(bytes32 indexed id, address indexed member, uint256 amount); + event FundsWithdrawn(bytes32 indexed id, address indexed member, uint256 amount); event TokenAllowed(address indexed token, bool indexed allowed); error AlreadyDeposited(); @@ -28,23 +28,23 @@ interface ISavingCircles { error InvalidCircle(); error NotCommissioned(); error NotMember(); - error NotOwner(); + error NotDecommissionable(); error NotWithdrawable(); error TransferFailed(); // External functions (state-changing) function initialize(address owner) external; function setTokenAllowed(address token, bool allowed) external; - function addCircle(Circle memory circle) external; + function create(Circle memory circle) external; function deposit(bytes32 id, uint256 value) external; - function depositFor(bytes32 id, address member, uint256 value) external; + function depositFor(bytes32 id, uint256 value, address member) external; function withdraw(bytes32 id) external; - function decommissionCircle(bytes32 id) external; + function decommission(bytes32 id) external; // External view functions 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 memberBalances(bytes32 id) external view returns (address[] memory, uint256[] memory); function withdrawable(bytes32 id) external view returns (bool); - function withdrawableBy(bytes32 id, address member) external view returns (bool); + function withdrawableBy(bytes32 id) external view returns (address); } diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index 90fcdcd..7f939d7 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -83,7 +83,7 @@ contract SavingCirclesIntegration is Test { circle.setTokenAllowed(address(token), true); vm.prank(alice); - circle.addCircle(baseCircle); + circle.create(baseCircle); } function test_SetTokenAllowed() public { @@ -128,7 +128,7 @@ contract SavingCirclesIntegration is Test { baseCircle.token = badToken; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); - circle.addCircle(baseCircle); + circle.create(baseCircle); } function test_Deposit() public { @@ -137,7 +137,7 @@ contract SavingCirclesIntegration is Test { vm.prank(alice); circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); - (, uint256[] memory balances) = circle.balancesForCircle(BASE_CIRCLE_ID); + (, uint256[] memory balances) = circle.memberBalances(BASE_CIRCLE_ID); assertEq(balances[0], DEPOSIT_AMOUNT); } @@ -146,9 +146,9 @@ contract SavingCirclesIntegration is Test { // Bob deposits for Alice vm.prank(bob); - circle.depositFor(BASE_CIRCLE_ID, alice, DEPOSIT_AMOUNT); + circle.depositFor(BASE_CIRCLE_ID, DEPOSIT_AMOUNT, alice); - (, uint256[] memory balances) = circle.balancesForCircle(BASE_CIRCLE_ID); + (, uint256[] memory balances) = circle.memberBalances(BASE_CIRCLE_ID); assertEq(balances[0], DEPOSIT_AMOUNT); } @@ -208,7 +208,7 @@ contract SavingCirclesIntegration is Test { // Decommission circle vm.prank(alice); - circle.decommissionCircle(BASE_CIRCLE_ID); + circle.decommission(BASE_CIRCLE_ID); // Check balances returned assertEq(token.balanceOf(alice) - aliceBalanceBefore, DEPOSIT_AMOUNT); @@ -219,12 +219,39 @@ contract SavingCirclesIntegration is Test { circle.circle(BASE_CIRCLE_ID); } - function test_RevertWhen_NonOwnerDecommissions() public { + function test_MemberDecommissionWhenIncompleteDeposits() public { createBaseCircle(); - vm.prank(bob); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotOwner.selector)); - circle.decommissionCircle(BASE_CIRCLE_ID); + // Only Alice deposits + vm.prank(alice); + circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + + // Get initial balance + uint256 aliceBalanceBefore = token.balanceOf(alice); + + // Wait until after deposit interval + vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1); + + // Alice should be able to decommission since not all members deposited + vm.prank(alice); + vm.expectEmit(true, true, true, true); + emit ISavingCircles.CircleDecommissioned(BASE_CIRCLE_ID); + circle.decommission(BASE_CIRCLE_ID); + + // Check Alice got her deposit back + assertEq(token.balanceOf(alice) - aliceBalanceBefore, DEPOSIT_AMOUNT); + + // Check circle was deleted + vm.expectRevert(ISavingCircles.NotCommissioned.selector); + circle.circle(BASE_CIRCLE_ID); + } + + function test_RevertWhen_NonMemberDecommissions() public { + createBaseCircle(); + + vm.prank(makeAddr('stranger')); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotMember.selector)); + circle.decommission(BASE_CIRCLE_ID); } function test_RevertWhen_NotEnoughContributions() public { @@ -256,7 +283,7 @@ contract SavingCirclesIntegration is Test { // members[2] = carol; // vm.prank(alice); - // circle.addCircle("Test Circle", members, address(token), DEPOSIT_AMOUNT, DEPOSIT_INTERVAL); + // circle.create("Test Circle", members, address(token), DEPOSIT_AMOUNT, DEPOSIT_INTERVAL); // bytes32 hashedName = keccak256(abi.encodePacked("Test Circle")); // // Branch 2: Not enough time passed diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol index 4b84654..d201ce8 100644 --- a/test/unit/SavingCirclesUnit.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -77,7 +77,7 @@ contract SavingCirclesUnit is Test { // Create an initial test circle vm.prank(alice); - savingCircles.addCircle(baseCircle); + savingCircles.create(baseCircle); } function test_SetTokenAllowedWhenCallerIsNotOwner() external { @@ -145,7 +145,7 @@ contract SavingCirclesUnit is Test { // Expect deposit event vm.expectEmit(true, true, true, true); - emit ISavingCircles.DepositMade(baseCircleId, alice, DEPOSIT_AMOUNT); + emit ISavingCircles.FundsDeposited(baseCircleId, alice, DEPOSIT_AMOUNT); savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); @@ -248,14 +248,14 @@ contract SavingCirclesUnit is Test { // First member (alice) should be able to withdraw vm.prank(alice); vm.expectEmit(true, true, true, true); - emit ISavingCircles.WithdrawalMade(baseCircleId, alice, withdrawAmount); + emit ISavingCircles.FundsWithdrawn(baseCircleId, alice, withdrawAmount); savingCircles.withdraw(baseCircleId); // Verify alice received the tokens assertEq(token.balanceOf(alice), withdrawAmount); // Verify all member balances were reset - (, uint256[] memory balances) = savingCircles.balancesForCircle(baseCircleId); + (, uint256[] memory balances) = savingCircles.memberBalances(baseCircleId); for (uint256 i = 0; i < balances.length; i++) { assertEq(balances[i], 0); } @@ -282,29 +282,54 @@ contract SavingCirclesUnit is Test { assertEq(_circle.depositInterval, DEPOSIT_INTERVAL); } - function test_DecommissionWhenCallerIsNotOwner() external { - vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotOwner.selector)); - savingCircles.decommissionCircle(baseCircleId); + function test_DecommissionWhenOwner() external { + vm.prank(owner); + vm.expectEmit(true, true, true, true); + emit ISavingCircles.CircleDecommissioned(baseCircleId); + savingCircles.decommission(baseCircleId); + + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); + savingCircles.circle(baseCircleId); } - function test_DecommissionWhenParametersAreValid() external { - vm.prank(owner); + function test_DecommissionWhenNotMember() external { + vm.prank(makeAddr('stranger')); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotMember.selector)); + savingCircles.decommission(baseCircleId); + } + + function test_DecommissionWhenMemberAndIncompleteDeposits() external { + // Have alice deposit but not bob or carol + token.mint(alice, DEPOSIT_AMOUNT); + vm.startPrank(alice); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + // Warp past deposit interval + vm.warp(block.timestamp + DEPOSIT_INTERVAL + 1); + + // Member should be able to decommission since not all deposits were made + vm.prank(alice); vm.expectEmit(true, true, true, true); emit ISavingCircles.CircleDecommissioned(baseCircleId); - savingCircles.decommissionCircle(baseCircleId); + savingCircles.decommission(baseCircleId); + // Verify circle was deleted vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); savingCircles.circle(baseCircleId); + + // Verify alice got her deposit back + assertEq(token.balanceOf(alice), DEPOSIT_AMOUNT); } - function test_AddCircleWhenCircleNameAlreadyExists() external { + function test_CreateWhenCircleNameAlreadyExists() external { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.AlreadyExists.selector)); - savingCircles.addCircle(baseCircle); + savingCircles.create(baseCircle); } - function test_AddCircleWhenTokenIsNotWhitelisted() external { + function test_CreateWhenTokenIsNotWhitelisted() external { address _notAllowedToken = makeAddr('notAllowedToken'); ISavingCircles.Circle memory _invalidCircle = baseCircle; @@ -313,30 +338,30 @@ contract SavingCirclesUnit is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); - savingCircles.addCircle(_invalidCircle); + savingCircles.create(_invalidCircle); } - function test_AddCircleWhenIntervalIsZero() external { + function test_CreateWhenIntervalIsZero() external { ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositInterval = 0; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); - savingCircles.addCircle(_invalidCircle); + savingCircles.create(_invalidCircle); } - function test_AddCircleWhenDepositAmountIsZero() external { + function test_CreateWhenDepositAmountIsZero() external { ISavingCircles.Circle memory _invalidCircle = baseCircle; _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositAmount = 0; vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); - savingCircles.addCircle(_invalidCircle); + savingCircles.create(_invalidCircle); } - function test_AddCircleWhenMembersCountIsLessThanTwo() external { + function test_CreateWhenMembersCountIsLessThanTwo() external { address[] memory _oneMember = new address[](1); _oneMember[0] = alice; @@ -346,6 +371,6 @@ contract SavingCirclesUnit is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.InvalidCircle.selector)); - savingCircles.addCircle(_invalidCircle); + savingCircles.create(_invalidCircle); } } diff --git a/test/unit/SavingCirclesUnit.tree b/test/unit/SavingCirclesUnit.tree index 96bd838..81be96c 100644 --- a/test/unit/SavingCirclesUnit.tree +++ b/test/unit/SavingCirclesUnit.tree @@ -12,7 +12,7 @@ SavingCirclesUnit::DenylistToken ├── it denylists the token └── it emits token deniedlisted event -SavingCirclesUnit::addCircle +SavingCirclesUnit::create ├── when circle name already exists │ └── it reverts ├── when token is not whitelisted From 69bacfd3a0437330e2a4ad4e49f9d03a1cb603f1 Mon Sep 17 00:00:00 2001 From: bagelface Date: Thu, 26 Dec 2024 13:31:52 -0500 Subject: [PATCH 08/10] refactor: update deploy scripts and integration tests to make better use of common scripts --- script/Common.sol | 46 ++++++------ script/Deploy.sol | 4 +- script/DeploySavingsCircle.s.sol | 30 -------- test/integration/Greeter.t.sol | 28 -------- test/integration/IntegrationBase.sol | 69 +++++++++++++++--- test/integration/SavingCircles.t.sol | 78 ++------------------- test/unit/Greeter.t.sol | 101 --------------------------- test/unit/Greeter.tree | 25 ------- 8 files changed, 91 insertions(+), 290 deletions(-) delete mode 100644 script/DeploySavingsCircle.s.sol delete mode 100644 test/integration/Greeter.t.sol delete mode 100644 test/unit/Greeter.t.sol delete mode 100644 test/unit/Greeter.tree diff --git a/script/Common.sol b/script/Common.sol index d48fccd..8cefddf 100644 --- a/script/Common.sol +++ b/script/Common.sol @@ -1,40 +1,42 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.28; -import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; -import {Greeter, IGreeter} from 'contracts/Greeter.sol'; +import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; +import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Script} from 'forge-std/Script.sol'; -// solhint-disable-next-line -import 'script/Registry.sol'; + +import {SavingCircles} from '../src/contracts/SavingCircles.sol'; /** * @title Common Contract * @author Breadchain - * @notice This contract is used to deploy the Greeter contract + * @notice This contract is used to deploy an upgradable Saving Circles contract * @dev This contract is intended for use in Scripts and Integration Tests */ contract Common is Script { - struct DeploymentParams { - string greeting; - IERC20 token; - } - - IGreeter public greeter; + function setUp() public virtual {} - /// @notice Deployment parameters for each chain - mapping(uint256 _chainId => DeploymentParams _params) internal _deploymentParams; - - function setUp() public virtual { - // Optimism - _deploymentParams[10] = DeploymentParams('Hello, Optimism!', IERC20(OPTIMISM_DAI)); + function _deploySavingCircles() internal returns (SavingCircles) { + return new SavingCircles(); + } - // Gnosis - _deploymentParams[100] = DeploymentParams('Hello, Gnosis!', IERC20(GNOSIS_BREAD)); + function _deployProxyAdmin(address _admin) internal returns (ProxyAdmin) { + return new ProxyAdmin(_admin); } - function _deployContracts() internal { - DeploymentParams memory _params = _deploymentParams[block.chainid]; + function _deployTransparentProxy( + address _implementation, + address _proxyAdmin, + bytes memory _initData + ) internal returns (TransparentUpgradeableProxy) { + return new TransparentUpgradeableProxy(_implementation, _proxyAdmin, _initData); + } - greeter = new Greeter(_params.greeting, _params.token); + function _deployContracts(address _admin) internal returns (TransparentUpgradeableProxy) { + return _deployTransparentProxy( + address(_deploySavingCircles()), + address(_deployProxyAdmin(_admin)), + abi.encodeWithSelector(SavingCircles.initialize.selector, _admin) + ); } } diff --git a/script/Deploy.sol b/script/Deploy.sol index 28bef9e..bb1fdfa 100644 --- a/script/Deploy.sol +++ b/script/Deploy.sol @@ -4,10 +4,10 @@ pragma solidity 0.8.28; import {Common} from 'script/Common.sol'; contract Deploy is Common { - function run() public { + function run(address _admin) public { vm.startBroadcast(); - _deployContracts(); + _deployContracts(_admin); vm.stopBroadcast(); } diff --git a/script/DeploySavingsCircle.s.sol b/script/DeploySavingsCircle.s.sol deleted file mode 100644 index 890baaf..0000000 --- a/script/DeploySavingsCircle.s.sol +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.28; - -import {SavingCircles} from '../src/contracts/SavingCircles.sol'; - -import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; -import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; -import {Script} from 'forge-std/Script.sol'; - -contract DeploySavingCircles is Script { - function run() external returns (address proxy, address admin) { - vm.startBroadcast(); - - // Deploy implementation - SavingCircles implementation = new SavingCircles(); - - // Deploy ProxyAdmin - ProxyAdmin proxyAdmin = new ProxyAdmin(msg.sender); - - // Encode initialization call - bytes memory initData = abi.encodeWithSelector(SavingCircles.initialize.selector); - - // Deploy proxy - TransparentUpgradeableProxy transparentProxy = - new TransparentUpgradeableProxy(address(implementation), address(proxyAdmin), initData); - - vm.stopBroadcast(); - return (address(transparentProxy), address(proxyAdmin)); - } -} diff --git a/test/integration/Greeter.t.sol b/test/integration/Greeter.t.sol deleted file mode 100644 index 6bd0bdc..0000000 --- a/test/integration/Greeter.t.sol +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.28; - -import {IntegrationBase} from 'test/integration/IntegrationBase.sol'; - -contract IntegrationGreeter is IntegrationBase { - string internal _newGreeting; - - function setUp() public override { - /// @dev override for more specific setup - super.setUp(); - _newGreeting = 'Hello, Breadchain!'; - } - - function test_Greet() public { - DeploymentParams memory _params = _deploymentParams[block.chainid]; - - (string memory _initialGreeting, uint256 _balance) = greeter.greet(); - assertEq(_params.greeting, _initialGreeting); - assertEq(INIT_BALANCE, _balance); - - vm.prank(owner); - greeter.setGreeting(_newGreeting); - - (string memory _currentGreeting,) = greeter.greet(); - assertEq(_currentGreeting, greeter.greeting()); - } -} diff --git a/test/integration/IntegrationBase.sol b/test/integration/IntegrationBase.sol index 3b738cb..afe4d48 100644 --- a/test/integration/IntegrationBase.sol +++ b/test/integration/IntegrationBase.sol @@ -1,30 +1,83 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.28; -import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; import {Test} from 'forge-std/Test.sol'; import {Common} from 'script/Common.sol'; +import {SavingCircles} from '../../src/contracts/SavingCircles.sol'; +import {ISavingCircles} from '../../src/interfaces/ISavingCircles.sol'; +import {MockERC20} from '../mocks/MockERC20.sol'; + // solhint-disable-next-line import 'script/Registry.sol'; contract IntegrationBase is Common, Test { - uint256 public constant INIT_BALANCE = 1 ether; + SavingCircles public circle; + MockERC20 public token; - address public user = makeAddr('user'); + address public alice = makeAddr('alice'); + address public bob = makeAddr('bob'); + address public carol = makeAddr('carol'); address public owner = makeAddr('owner'); + address[] public members; + + ISavingCircles.Circle public baseCircle; - IERC20 public bread = IERC20(GNOSIS_BREAD); + string public constant BASE_CIRCLE_NAME = 'Test Circle'; + uint256 public constant DEPOSIT_AMOUNT = 1000e18; + uint256 public constant DEPOSIT_INTERVAL = 7 days; + uint256 public constant BASE_CURRENT_INDEX = 0; + uint256 public constant BASE_MAX_DEPOSITS = 1000; + bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked(BASE_CIRCLE_NAME)); function setUp() public virtual override { super.setUp(); - vm.createSelectFork(vm.rpcUrl('gnosis')); + vm.startPrank(owner); + circle = SavingCircles(address(_deployContracts(owner))); + token = new MockERC20('Test Token', 'TEST'); + vm.stopPrank(); + + _setUpAccounts(); + + baseCircle = ISavingCircles.Circle({ + owner: alice, + name: BASE_CIRCLE_NAME, + members: members, + currentIndex: BASE_CURRENT_INDEX, + circleStart: block.timestamp, + token: address(token), + depositAmount: DEPOSIT_AMOUNT, + depositInterval: DEPOSIT_INTERVAL, + maxDeposits: BASE_MAX_DEPOSITS + }); + } - /// @dev deploy contracts methods are located in script/Common.sol - _deployContracts(); + function createBaseCircle() public { + vm.prank(owner); + circle.setTokenAllowed(address(token), true); + + vm.prank(alice); + circle.create(baseCircle); + } + + function _setUpAccounts() internal { + vm.startPrank(alice); + token.mint(alice, DEPOSIT_AMOUNT * 10); + token.approve(address(circle), type(uint256).max); + members.push(alice); + vm.stopPrank(); + + vm.startPrank(bob); + token.mint(bob, DEPOSIT_AMOUNT * 10); + token.approve(address(circle), type(uint256).max); + members.push(bob); + vm.stopPrank(); + vm.startPrank(carol); + token.mint(carol, DEPOSIT_AMOUNT * 10); + token.approve(address(circle), type(uint256).max); + members.push(carol); vm.stopPrank(); - deal(GNOSIS_BREAD, address(greeter), INIT_BALANCE); } } diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index 7f939d7..ca29249 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -6,84 +6,14 @@ import {ProxyAdmin} from '@openzeppelin/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from '@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol'; import {Test} from 'forge-std/Test.sol'; -import {SavingCircles} from '../../src/contracts/SavingCircles.sol'; import {ISavingCircles} from '../../src/interfaces/ISavingCircles.sol'; -import {MockERC20} from '../mocks/MockERC20.sol'; +import {IntegrationBase} from 'test/integration/IntegrationBase.sol'; /* solhint-disable func-name-mixedcase */ -contract SavingCirclesIntegration is Test { - SavingCircles public circle; - MockERC20 public token; - - address public alice = makeAddr('alice'); - address public bob = makeAddr('bob'); - address public carol = makeAddr('carol'); - address public owner = makeAddr('owner'); - address[] public members; - - string public constant BASE_CIRCLE_NAME = 'Test Circle'; - uint256 public constant DEPOSIT_AMOUNT = 1000e18; - uint256 public constant DEPOSIT_INTERVAL = 7 days; - uint256 public constant BASE_CURRENT_INDEX = 0; - uint256 public constant BASE_MAX_DEPOSITS = 1000; - bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked(BASE_CIRCLE_NAME)); - - ISavingCircles.Circle public baseCircle; - - function setUp() public { - vm.startPrank(owner); - circle = SavingCircles( - address( - new TransparentUpgradeableProxy( - address(new SavingCircles()), - address(new ProxyAdmin(owner)), - abi.encodeWithSelector(SavingCircles.initialize.selector, owner) - ) - ) - ); - - token = new MockERC20('Test Token', 'TEST'); - vm.stopPrank(); - - // Setup test accounts - vm.startPrank(alice); - token.mint(alice, DEPOSIT_AMOUNT * 10); - token.approve(address(circle), type(uint256).max); - members.push(alice); - vm.stopPrank(); - - vm.startPrank(bob); - token.mint(bob, DEPOSIT_AMOUNT * 10); - token.approve(address(circle), type(uint256).max); - members.push(bob); - vm.stopPrank(); - - vm.startPrank(carol); - token.mint(carol, DEPOSIT_AMOUNT * 10); - token.approve(address(circle), type(uint256).max); - members.push(carol); - vm.stopPrank(); - - baseCircle = ISavingCircles.Circle({ - owner: alice, - name: BASE_CIRCLE_NAME, - members: members, - currentIndex: BASE_CURRENT_INDEX, - circleStart: block.timestamp, - token: address(token), - depositAmount: DEPOSIT_AMOUNT, - depositInterval: DEPOSIT_INTERVAL, - maxDeposits: BASE_MAX_DEPOSITS - }); - } - - function createBaseCircle() public { - vm.prank(owner); - circle.setTokenAllowed(address(token), true); - - vm.prank(alice); - circle.create(baseCircle); +contract SavingCirclesIntegration is IntegrationBase { + function setUp() public override { + super.setUp(); } function test_SetTokenAllowed() public { diff --git a/test/unit/Greeter.t.sol b/test/unit/Greeter.t.sol deleted file mode 100644 index d8901b1..0000000 --- a/test/unit/Greeter.t.sol +++ /dev/null @@ -1,101 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.28; - -import {IERC20} from '@openzeppelin/token/ERC20/IERC20.sol'; -import {Greeter, IGreeter} from 'contracts/Greeter.sol'; -import {Test} from 'forge-std/Test.sol'; - -contract UnitGreeter is Test { -/* - address internal _owner = makeAddr('owner'); - IERC20 internal _token = IERC20(makeAddr('token')); - uint256 internal _initialBalance = 100; - string internal _initialGreeting = 'hola'; - - Greeter internal _greeter; - - event GreetingSet(string _greeting); - - function setUp() external { - vm.prank(_owner); - _greeter = new Greeter(_initialGreeting, _token); - - vm.etch(address(_token), new bytes(0x1)); - } - - function test_EmptyTestExample() external { - // it does nothing - vm.skip(true); - } - - function test_ConstructorWhenPassingValidGreetingString() external { - vm.prank(_owner); - - // it deploys - _greeter = new Greeter(_initialGreeting, _token); - - // it sets the greeting string - assertEq(_greeter.greeting(), _initialGreeting); - - // it sets the owner as sender - assertEq(_greeter.OWNER(), _owner); - - // it sets the token used - assertEq(address(_greeter.token()), address(_token)); - } - - function test_ConstructorWhenPassingAnEmptyGreetingString() external { - vm.prank(_owner); - - // it reverts - vm.expectRevert(IGreeter.Greeter_InvalidGreeting.selector); - _greeter = new Greeter('', _token); - } - - function test_GreetWhenCalled() external { - vm.mockCall(address(_token), abi.encodeWithSelector(IERC20.balanceOf.selector), abi.encode(_initialBalance)); - vm.expectCall(address(_token), abi.encodeWithSelector(IERC20.balanceOf.selector)); - (string memory _greet, uint256 _balance) = _greeter.greet(); - - // it returns the greeting string - assertEq(_greet, _initialGreeting); - - // it returns the token balance of the contract - assertEq(_balance, _initialBalance); - } - - modifier whenCalledByTheOwner() { - vm.startPrank(_owner); - _; - vm.stopPrank(); - } - - function test_SetGreetingWhenPassingAValidGreetingString() external whenCalledByTheOwner { - string memory _newGreeting = 'hello'; - - // it emit GreetingSet - vm.expectEmit(true, true, true, true, address(_greeter)); - emit GreetingSet(_newGreeting); - - _greeter.setGreeting(_newGreeting); - - // it sets the greeting string - assertEq(_greeter.greeting(), _newGreeting); - } - - function test_SetGreetingWhenPassingAnEmptyGreetingString() external whenCalledByTheOwner { - // it reverts - vm.expectRevert(IGreeter.Greeter_InvalidGreeting.selector); - _greeter.setGreeting(''); - } - - function test_SetGreetingWhenCalledByANon_owner(address _caller) external { - vm.assume(_caller != _owner); - vm.prank(_caller); - - // it reverts - vm.expectRevert(IGreeter.Greeter_OnlyOwner.selector); - _greeter.setGreeting('new greeting'); - } -*/ -} diff --git a/test/unit/Greeter.tree b/test/unit/Greeter.tree deleted file mode 100644 index bbc74ea..0000000 --- a/test/unit/Greeter.tree +++ /dev/null @@ -1,25 +0,0 @@ -Greeter::constructor -├── when passing valid greeting string -│ ├── it deploys -│ ├── it sets the greeting string -│ ├── it sets the owner as sender -│ └── it sets the token used -└── when passing an empty greeting string - └── it reverts - - -Greeter::greet -└── when called - ├── it returns the greeting string - └── it returns the token balance of the contract - - -Greeter::setGreeting -├── when called by the owner -│ ├── when passing a valid greeting string -│ │ ├── it sets the greeting string -│ │ └── it emit GreetingSet -│ └── when passing an empty greeting string -│ └── it reverts -└── when called by a non-owner - └── it reverts \ No newline at end of file From b395d7d1407cd920ba5b9e35e36c4398375772a5 Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 30 Dec 2024 14:40:33 -0500 Subject: [PATCH 09/10] fix: use serial ids instead of ones based on names --- script/Common.sol | 2 +- src/contracts/SavingCircles.sol | 46 ++++++++++++++---------- src/interfaces/ISavingCircles.sol | 52 +++++++++++++++++++--------- test/integration/IntegrationBase.sol | 6 ++-- test/integration/SavingCircles.t.sol | 50 +++++++++++++------------- test/unit/SavingCirclesUnit.t.sol | 23 +++--------- 6 files changed, 95 insertions(+), 84 deletions(-) diff --git a/script/Common.sol b/script/Common.sol index 8cefddf..b56fea6 100644 --- a/script/Common.sol +++ b/script/Common.sol @@ -10,7 +10,7 @@ import {SavingCircles} from '../src/contracts/SavingCircles.sol'; /** * @title Common Contract * @author Breadchain - * @notice This contract is used to deploy an upgradable Saving Circles contract + * @notice This contract is used to deploy an upgradeable Saving Circles contract * @dev This contract is intended for use in Scripts and Integration Tests */ contract Common is Script { diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index 1f0c62a..2f07981 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -12,15 +12,16 @@ import {ISavingCircles} from '../interfaces/ISavingCircles.sol'; * @notice Simple implementation of a rotating savings and credit association (ROSCA) for ERC20 tokens * @author Breadchain Collective * @author @RonTuretzky - * @author @bagelface + * @author bagelface.eth */ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { uint256 public constant MINIMUM_MEMBERS = 2; - mapping(bytes32 id => Circle circle) public circles; + uint256 public nextId; + mapping(uint256 id => Circle circle) public circles; 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; + mapping(uint256 id => mapping(address token => uint256 balance)) public balances; + mapping(uint256 id => mapping(address member => bool status)) public isMember; /// @custom:oz-upgrades-unsafe-allow constructor constructor() { @@ -35,8 +36,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @notice Create a new saving circle * @param _circle A new saving circle */ - function create(Circle memory _circle) external override { - bytes32 _id = keccak256(abi.encodePacked(_circle.name)); + function create(Circle memory _circle) external override returns (uint256 _id) { + _id = nextId++; if (circles[_id].owner != address(0)) revert AlreadyExists(); if ( @@ -52,9 +53,9 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } circles[_id] = _circle; - emit CircleCreated( - _id, _circle.name, _circle.members, _circle.token, _circle.depositAmount, _circle.depositInterval - ); + emit CircleCreated(_id, _circle.members, _circle.token, _circle.depositAmount, _circle.depositInterval); + + return _id; } /** @@ -62,7 +63,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle * @param _value Amount of the token to deposit */ - function deposit(bytes32 _id, uint256 _value) external override nonReentrant { + function deposit(uint256 _id, uint256 _value) external override nonReentrant { _deposit(_id, _value, msg.sender); } @@ -72,7 +73,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _value Amount of the token to deposit * @param _member Address to make a deposit for */ - function depositFor(bytes32 _id, uint256 _value, address _member) external override nonReentrant { + function depositFor(uint256 _id, uint256 _value, address _member) external override nonReentrant { _deposit(_id, _value, _member); } @@ -80,7 +81,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @notice Make a withdrawal from a specified circle * @param _id Identifier of the circle */ - function withdraw(bytes32 _id) external override nonReentrant { + function withdraw(uint256 _id) external override nonReentrant { Circle storage _circle = circles[_id]; if (!_withdrawable(_id)) revert NotWithdrawable(); @@ -100,6 +101,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { emit FundsWithdrawn(_id, msg.sender, _withdrawAmount); } + function withdrawFor(uint256 _id, address _member) external override nonReentrant {} + /** * @notice Set if a token can be used for saving circles * @param _token Token to update the status of @@ -116,7 +119,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @dev Returns all deposits to members * @param _id Identifier of the circle */ - function decommission(bytes32 _id) external override { + function decommission(uint256 _id) external override { Circle storage _circle = circles[_id]; if (_circle.owner != msg.sender) { @@ -165,7 +168,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle * @return _circle Saving circle */ - function circle(bytes32 _id) external view override returns (Circle memory _circle) { + function circle(uint256 _id) external view override returns (Circle memory _circle) { _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); @@ -179,7 +182,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @return _members Members of the specified saving circle * @return _balances Corresponding balances of the members of the circle */ - function memberBalances(bytes32 _id) + function memberBalances(uint256 _id) external view override @@ -202,7 +205,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle * @return address Member that is currently able to withdraw from the circle */ - function withdrawableBy(bytes32 _id) external view override returns (address) { + function withdrawableBy(uint256 _id) external view override returns (address) { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); @@ -215,16 +218,21 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle * @return bool If the circle is able to be withdrawn from */ - function withdrawable(bytes32 _id) external view override returns (bool) { + function withdrawable(uint256 _id) external view override returns (bool) { return _withdrawable(_id); } + /** + * @dev TODO + */ + function _withdraw() internal {} + /** * @dev Make a deposit into a specified circle * A deposit must be made in specific time window and can be made partially so long as the final balance equals * the specified deposit amount for the circle. */ - function _deposit(bytes32 _id, uint256 _value, address _member) internal { + function _deposit(uint256 _id, uint256 _value, address _member) internal { Circle memory _circle = circles[_id]; if (_isDecommissioned(_circle)) revert NotCommissioned(); @@ -249,7 +257,7 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * To be considered withdrawable, enough time must have passed since the deposit interval started * and all members must have made a deposit. */ - function _withdrawable(bytes32 _id) internal view returns (bool) { + function _withdrawable(uint256 _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 00c6b07..a91a84c 100644 --- a/src/interfaces/ISavingCircles.sol +++ b/src/interfaces/ISavingCircles.sol @@ -2,9 +2,12 @@ pragma solidity ^0.8.28; interface ISavingCircles { + /*/////////////////////////////////////////////////////////////// + STRUCTS + //////////////////////////////////////////////////////////////*/ + struct Circle { address owner; - string name; address[] members; uint256 currentIndex; uint256 depositAmount; @@ -14,14 +17,22 @@ interface ISavingCircles { uint256 maxDeposits; } + /*/////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + event CircleCreated( - bytes32 indexed id, string name, address[] members, address token, uint256 depositAmount, uint256 depositInterval + uint256 indexed id, address[] members, address token, uint256 depositAmount, uint256 depositInterval ); - event CircleDecommissioned(bytes32 indexed id); - event FundsDeposited(bytes32 indexed id, address indexed member, uint256 amount); - event FundsWithdrawn(bytes32 indexed id, address indexed member, uint256 amount); + event CircleDecommissioned(uint256 indexed id); + event FundsDeposited(uint256 indexed id, address indexed member, uint256 amount); + event FundsWithdrawn(uint256 indexed id, address indexed member, uint256 amount); event TokenAllowed(address indexed token, bool indexed allowed); + /*/////////////////////////////////////////////////////////////// + ERRORS + //////////////////////////////////////////////////////////////*/ + error AlreadyDeposited(); error AlreadyExists(); error InvalidDeposit(); @@ -32,19 +43,26 @@ interface ISavingCircles { error NotWithdrawable(); error TransferFailed(); - // External functions (state-changing) + /*/////////////////////////////////////////////////////////////// + VIEW + //////////////////////////////////////////////////////////////*/ + function initialize(address owner) external; function setTokenAllowed(address token, bool allowed) external; - function create(Circle memory circle) external; - function deposit(bytes32 id, uint256 value) external; - function depositFor(bytes32 id, uint256 value, address member) external; - function withdraw(bytes32 id) external; - function decommission(bytes32 id) external; - - // External view functions - function circle(bytes32 id) external view returns (Circle memory); + function create(Circle memory circle) external returns (uint256); + function deposit(uint256 id, uint256 value) external; + function depositFor(uint256 id, uint256 value, address member) external; + function withdraw(uint256 id) external; + function withdrawFor(uint256 id, address member) external; + function decommission(uint256 id) external; + + /*/////////////////////////////////////////////////////////////// + VIEW + //////////////////////////////////////////////////////////////*/ + + function circle(uint256 id) external view returns (Circle memory); function isTokenAllowed(address token) external view returns (bool); - function memberBalances(bytes32 id) external view returns (address[] memory, uint256[] memory); - function withdrawable(bytes32 id) external view returns (bool); - function withdrawableBy(bytes32 id) external view returns (address); + function memberBalances(uint256 id) external view returns (address[] memory, uint256[] memory); + function withdrawable(uint256 id) external view returns (bool); + function withdrawableBy(uint256 id) external view returns (address); } diff --git a/test/integration/IntegrationBase.sol b/test/integration/IntegrationBase.sol index afe4d48..d12a844 100644 --- a/test/integration/IntegrationBase.sol +++ b/test/integration/IntegrationBase.sol @@ -22,13 +22,12 @@ contract IntegrationBase is Common, Test { address[] public members; ISavingCircles.Circle public baseCircle; + uint256 public baseCircleId; - string public constant BASE_CIRCLE_NAME = 'Test Circle'; uint256 public constant DEPOSIT_AMOUNT = 1000e18; uint256 public constant DEPOSIT_INTERVAL = 7 days; uint256 public constant BASE_CURRENT_INDEX = 0; uint256 public constant BASE_MAX_DEPOSITS = 1000; - bytes32 public constant BASE_CIRCLE_ID = keccak256(abi.encodePacked(BASE_CIRCLE_NAME)); function setUp() public virtual override { super.setUp(); @@ -42,7 +41,6 @@ contract IntegrationBase is Common, Test { baseCircle = ISavingCircles.Circle({ owner: alice, - name: BASE_CIRCLE_NAME, members: members, currentIndex: BASE_CURRENT_INDEX, circleStart: block.timestamp, @@ -58,7 +56,7 @@ contract IntegrationBase is Common, Test { circle.setTokenAllowed(address(token), true); vm.prank(alice); - circle.create(baseCircle); + baseCircleId = circle.create(baseCircle); } function _setUpAccounts() internal { diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index ca29249..0d0c353 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -65,9 +65,9 @@ contract SavingCirclesIntegration is IntegrationBase { createBaseCircle(); vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); - (, uint256[] memory balances) = circle.memberBalances(BASE_CIRCLE_ID); + (, uint256[] memory balances) = circle.memberBalances(baseCircleId); assertEq(balances[0], DEPOSIT_AMOUNT); } @@ -76,9 +76,9 @@ contract SavingCirclesIntegration is IntegrationBase { // Bob deposits for Alice vm.prank(bob); - circle.depositFor(BASE_CIRCLE_ID, DEPOSIT_AMOUNT, alice); + circle.depositFor(baseCircleId, DEPOSIT_AMOUNT, alice); - (, uint256[] memory balances) = circle.memberBalances(BASE_CIRCLE_ID); + (, uint256[] memory balances) = circle.memberBalances(baseCircleId); assertEq(balances[0], DEPOSIT_AMOUNT); } @@ -86,18 +86,18 @@ contract SavingCirclesIntegration is IntegrationBase { createBaseCircle(); vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(bob); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(carol); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); // First member withdraws uint256 balanceBefore = token.balanceOf(alice); vm.prank(alice); - circle.withdraw(BASE_CIRCLE_ID); + circle.withdraw(baseCircleId); uint256 balanceAfter = token.balanceOf(alice); // Alice should receive DEPOSIT_AMOUNT * 3 (from Bob and Carol) @@ -106,20 +106,20 @@ contract SavingCirclesIntegration is IntegrationBase { // Try to withdraw before interval vm.prank(bob); vm.expectRevert(ISavingCircles.NotWithdrawable.selector); - circle.withdraw(BASE_CIRCLE_ID); + circle.withdraw(baseCircleId); // Wait for interval (need to wait for index 1's interval) vm.warp(block.timestamp + DEPOSIT_INTERVAL); vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(bob); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(carol); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); // Bob should be able to withdraw vm.prank(bob); - circle.withdraw(BASE_CIRCLE_ID); + circle.withdraw(baseCircleId); } function test_DecommissionCircle() public { @@ -127,10 +127,10 @@ contract SavingCirclesIntegration is IntegrationBase { // Members deposit vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(bob); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); // Get initial balances uint256 aliceBalanceBefore = token.balanceOf(alice); @@ -138,7 +138,7 @@ contract SavingCirclesIntegration is IntegrationBase { // Decommission circle vm.prank(alice); - circle.decommission(BASE_CIRCLE_ID); + circle.decommission(baseCircleId); // Check balances returned assertEq(token.balanceOf(alice) - aliceBalanceBefore, DEPOSIT_AMOUNT); @@ -146,7 +146,7 @@ contract SavingCirclesIntegration is IntegrationBase { // Check circle deleted vm.expectRevert(ISavingCircles.NotCommissioned.selector); - circle.circle(BASE_CIRCLE_ID); + circle.circle(baseCircleId); } function test_MemberDecommissionWhenIncompleteDeposits() public { @@ -154,7 +154,7 @@ contract SavingCirclesIntegration is IntegrationBase { // Only Alice deposits vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); // Get initial balance uint256 aliceBalanceBefore = token.balanceOf(alice); @@ -165,15 +165,15 @@ contract SavingCirclesIntegration is IntegrationBase { // Alice should be able to decommission since not all members deposited vm.prank(alice); vm.expectEmit(true, true, true, true); - emit ISavingCircles.CircleDecommissioned(BASE_CIRCLE_ID); - circle.decommission(BASE_CIRCLE_ID); + emit ISavingCircles.CircleDecommissioned(baseCircleId); + circle.decommission(baseCircleId); // Check Alice got her deposit back assertEq(token.balanceOf(alice) - aliceBalanceBefore, DEPOSIT_AMOUNT); // Check circle was deleted vm.expectRevert(ISavingCircles.NotCommissioned.selector); - circle.circle(BASE_CIRCLE_ID); + circle.circle(baseCircleId); } function test_RevertWhen_NonMemberDecommissions() public { @@ -181,21 +181,21 @@ contract SavingCirclesIntegration is IntegrationBase { vm.prank(makeAddr('stranger')); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotMember.selector)); - circle.decommission(BASE_CIRCLE_ID); + circle.decommission(baseCircleId); } function test_RevertWhen_NotEnoughContributions() public { createBaseCircle(); vm.prank(alice); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(bob); - circle.deposit(BASE_CIRCLE_ID, DEPOSIT_AMOUNT); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); vm.prank(alice); vm.expectRevert(ISavingCircles.NotWithdrawable.selector); - circle.withdraw(BASE_CIRCLE_ID); + circle.withdraw(baseCircleId); } // // Withdraw function branching tests diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol index d201ce8..366e156 100644 --- a/test/unit/SavingCirclesUnit.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -28,7 +28,7 @@ contract SavingCirclesUnit is Test { address public immutable STRANGER = makeAddr('stranger'); // Test data - bytes32 public baseCircleId; + uint256 public baseCircleId; address[] public members; ISavingCircles.Circle public baseCircle; @@ -60,12 +60,10 @@ contract SavingCirclesUnit is Test { members[0] = alice; members[1] = bob; members[2] = carol; - baseCircleId = keccak256(abi.encodePacked('Test Circle')); // Setup savingcircles parameters baseCircle = ISavingCircles.Circle({ owner: owner, - name: 'Test Circle', members: members, currentIndex: 0, circleStart: block.timestamp, @@ -77,7 +75,7 @@ contract SavingCirclesUnit is Test { // Create an initial test circle vm.prank(alice); - savingCircles.create(baseCircle); + baseCircleId = savingCircles.create(baseCircle); } function test_SetTokenAllowedWhenCallerIsNotOwner() external { @@ -113,7 +111,7 @@ contract SavingCirclesUnit is Test { } function test_DepositWhenCircleDoesNotExist() external { - bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); + uint256 nonExistentCircleId = uint256(keccak256(abi.encodePacked('Non Existent Circle'))); vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); @@ -166,7 +164,7 @@ contract SavingCirclesUnit is Test { } function test_WithdrawWhenCircleDoesNotExist() external { - bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); + uint256 nonExistentCircleId = uint256(keccak256(abi.encodePacked('Non Existent Circle'))); vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); @@ -266,7 +264,7 @@ contract SavingCirclesUnit is Test { } function test_CircleInfoWhenCircleDoesNotExist() external { - bytes32 nonExistentCircleId = keccak256(abi.encodePacked('Non Existent Circle')); + uint256 nonExistentCircleId = uint256(keccak256(abi.encodePacked('Non Existent Circle'))); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); savingCircles.circle(nonExistentCircleId); @@ -275,7 +273,6 @@ contract SavingCirclesUnit is Test { function test_CircleInfoWhenCircleAlreadyExists() external { ISavingCircles.Circle memory _circle = savingCircles.circle(baseCircleId); - assertEq(_circle.name, 'Test Circle'); assertEq(_circle.members.length, members.length); assertEq(_circle.token, address(token)); assertEq(_circle.depositAmount, DEPOSIT_AMOUNT); @@ -323,17 +320,10 @@ contract SavingCirclesUnit is Test { assertEq(token.balanceOf(alice), DEPOSIT_AMOUNT); } - function test_CreateWhenCircleNameAlreadyExists() external { - vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.AlreadyExists.selector)); - savingCircles.create(baseCircle); - } - function test_CreateWhenTokenIsNotWhitelisted() external { address _notAllowedToken = makeAddr('notAllowedToken'); ISavingCircles.Circle memory _invalidCircle = baseCircle; - _invalidCircle.name = 'Invalid Circle'; _invalidCircle.token = _notAllowedToken; vm.prank(alice); @@ -343,7 +333,6 @@ contract SavingCirclesUnit is Test { function test_CreateWhenIntervalIsZero() external { ISavingCircles.Circle memory _invalidCircle = baseCircle; - _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositInterval = 0; vm.prank(alice); @@ -353,7 +342,6 @@ contract SavingCirclesUnit is Test { function test_CreateWhenDepositAmountIsZero() external { ISavingCircles.Circle memory _invalidCircle = baseCircle; - _invalidCircle.name = 'Invalid Circle'; _invalidCircle.depositAmount = 0; vm.prank(alice); @@ -366,7 +354,6 @@ contract SavingCirclesUnit is Test { _oneMember[0] = alice; ISavingCircles.Circle memory _invalidCircle = baseCircle; - _invalidCircle.name = 'Invalid Circle'; _invalidCircle.members = _oneMember; vm.prank(alice); From ee54c692432bf95a2e7afd67c8bd9d38c69acdcf Mon Sep 17 00:00:00 2001 From: bagelface Date: Mon, 30 Dec 2024 14:55:19 -0500 Subject: [PATCH 10/10] feat: add a withdrawFor function and tests --- src/contracts/SavingCircles.sol | 51 +++++++++++++++++----------- test/integration/SavingCircles.t.sol | 48 ++++++++++++++++++++++++++ test/unit/SavingCirclesUnit.t.sol | 51 +++++++++++++++++++++++++++- 3 files changed, 129 insertions(+), 21 deletions(-) diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index 2f07981..6c065ba 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -82,26 +82,17 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { * @param _id Identifier of the circle */ function withdraw(uint256 _id) external override nonReentrant { - Circle storage _circle = circles[_id]; - - if (!_withdrawable(_id)) revert NotWithdrawable(); - if (_circle.members[_circle.currentIndex] != msg.sender) revert NotWithdrawable(); - if (_circle.currentIndex >= _circle.maxDeposits) revert NotWithdrawable(); - - uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); - - for (uint256 i = 0; i < _circle.members.length; i++) { - balances[_id][_circle.members[i]] = 0; - } - - _circle.currentIndex = (_circle.currentIndex + 1) % _circle.members.length; - bool success = IERC20(_circle.token).transfer(msg.sender, _withdrawAmount); - if (!success) revert TransferFailed(); - - emit FundsWithdrawn(_id, msg.sender, _withdrawAmount); + _withdraw(_id, msg.sender); } - function withdrawFor(uint256 _id, address _member) external override nonReentrant {} + /** + * @notice Make a withdrawal from a specified circle on behalf of another member + * @param _id Identifier of the circle + * @param _member Address of the member to make a withdrawal for + */ + function withdrawFor(uint256 _id, address _member) external override nonReentrant { + _withdraw(_id, _member); + } /** * @notice Set if a token can be used for saving circles @@ -223,9 +214,29 @@ contract SavingCircles is ISavingCircles, ReentrancyGuard, OwnableUpgradeable { } /** - * @dev TODO + * @dev Make a withdrawal from a specified circle + * A withdrawal must be made by a member of the circle, even if it is for another member. */ - function _withdraw() internal {} + function _withdraw(uint256 _id, address _member) internal { + Circle storage _circle = circles[_id]; + + if (!isMember[_id][msg.sender]) revert NotMember(); + if (!_withdrawable(_id)) revert NotWithdrawable(); + if (_circle.members[_circle.currentIndex] != _member) revert NotWithdrawable(); + if (_circle.currentIndex >= _circle.maxDeposits) revert NotWithdrawable(); + + uint256 _withdrawAmount = _circle.depositAmount * (_circle.members.length); + + for (uint256 i = 0; i < _circle.members.length; i++) { + balances[_id][_circle.members[i]] = 0; + } + + _circle.currentIndex = (_circle.currentIndex + 1) % _circle.members.length; + bool success = IERC20(_circle.token).transfer(_member, _withdrawAmount); + if (!success) revert TransferFailed(); + + emit FundsWithdrawn(_id, _member, _withdrawAmount); + } /** * @dev Make a deposit into a specified circle diff --git a/test/integration/SavingCircles.t.sol b/test/integration/SavingCircles.t.sol index 0d0c353..d0f1592 100644 --- a/test/integration/SavingCircles.t.sol +++ b/test/integration/SavingCircles.t.sol @@ -122,6 +122,54 @@ contract SavingCirclesIntegration is IntegrationBase { circle.withdraw(baseCircleId); } + function test_WithdrawForWithInterval() public { + createBaseCircle(); + + // Initial deposits from all members + vm.prank(alice); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + + vm.prank(bob); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + + vm.prank(carol); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + + // Bob tries to withdraw for Alice (who is first in line) + uint256 balanceBefore = token.balanceOf(alice); + vm.prank(bob); + circle.withdrawFor(baseCircleId, alice); + uint256 balanceAfter = token.balanceOf(alice); + + // Alice should receive DEPOSIT_AMOUNT * 3 (from all members) + assertEq(balanceAfter - balanceBefore, DEPOSIT_AMOUNT * 3); + + // Try to withdraw for Bob before interval + vm.prank(alice); + vm.expectRevert(ISavingCircles.NotWithdrawable.selector); + circle.withdrawFor(baseCircleId, bob); + + // Wait for interval (need to wait for index 1's interval) + vm.warp(block.timestamp + DEPOSIT_INTERVAL); + + // New round of deposits + vm.prank(alice); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.prank(bob); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.prank(carol); + circle.deposit(baseCircleId, DEPOSIT_AMOUNT); + + // Alice withdraws for Bob (who is now next in line) + balanceBefore = token.balanceOf(bob); + vm.prank(alice); + circle.withdrawFor(baseCircleId, bob); + balanceAfter = token.balanceOf(bob); + + // Bob should receive DEPOSIT_AMOUNT * 3 + assertEq(balanceAfter - balanceBefore, DEPOSIT_AMOUNT * 3); + } + function test_DecommissionCircle() public { createBaseCircle(); diff --git a/test/unit/SavingCirclesUnit.t.sol b/test/unit/SavingCirclesUnit.t.sol index 366e156..4ff4966 100644 --- a/test/unit/SavingCirclesUnit.t.sol +++ b/test/unit/SavingCirclesUnit.t.sol @@ -168,6 +168,10 @@ contract SavingCirclesUnit is Test { vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotCommissioned.selector)); + savingCircles.withdrawable(nonExistentCircleId); + + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotMember.selector)); savingCircles.withdraw(nonExistentCircleId); } @@ -175,7 +179,7 @@ contract SavingCirclesUnit is Test { address nonMember = makeAddr('nonMember'); vm.prank(nonMember); - vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotWithdrawable.selector)); + vm.expectRevert(abi.encodeWithSelector(ISavingCircles.NotMember.selector)); savingCircles.withdraw(baseCircleId); } @@ -263,6 +267,51 @@ contract SavingCirclesUnit is Test { assertEq(circle.currentIndex, 1); } + function test_WithdrawForWhenParametersAreValid() external { + // Complete deposits from all members + vm.startPrank(alice); + token.mint(alice, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + vm.startPrank(bob); + token.mint(bob, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + vm.startPrank(carol); + token.mint(carol, DEPOSIT_AMOUNT); + token.approve(address(savingCircles), DEPOSIT_AMOUNT); + savingCircles.deposit(baseCircleId, DEPOSIT_AMOUNT); + vm.stopPrank(); + + // Move time past first round + vm.warp(block.timestamp + DEPOSIT_INTERVAL); + + uint256 withdrawAmount = DEPOSIT_AMOUNT * members.length; + + // Bob should be able to withdraw for Alice (who is first in line) + vm.prank(bob); + vm.expectEmit(true, true, true, true); + emit ISavingCircles.FundsWithdrawn(baseCircleId, alice, withdrawAmount); + savingCircles.withdrawFor(baseCircleId, alice); + + // Verify alice received the tokens + assertEq(token.balanceOf(alice), withdrawAmount); + + // Verify all member balances were reset + (, uint256[] memory balances) = savingCircles.memberBalances(baseCircleId); + for (uint256 i = 0; i < balances.length; i++) { + assertEq(balances[i], 0); + } + + // Verify current index moved to next member + ISavingCircles.Circle memory circle = savingCircles.circle(baseCircleId); + assertEq(circle.currentIndex, 1); + } + function test_CircleInfoWhenCircleDoesNotExist() external { uint256 nonExistentCircleId = uint256(keccak256(abi.encodePacked('Non Existent Circle')));