Feature/improve contract

contracts/IPoolFactory.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.0;
+
+interface IPoolFactory{
+    function enroll(address participant) external returns (uint256);
+    function deposit() external payable returns (uint256);
+    function deposit_and_invest_compound(address payable _cEtherContract) external payable returns (uint256);
+    function withdraw(uint256 withdrawAmount) external payable returns (uint256 remainingBal);
+    function withdraw_and_redeem(uint256 withdrawAmount, bool redeemType,address _cEtherContract) external returns (uint256 remainingBal);
+    function balance() external view returns (uint256);
+    function depositsBalance() external view returns (uint256);
+    function is_owner() external view returns (bool);
+    function get_owner() external view returns (address);
+    function is_public() external view returns(bool);
+    function balanceParticipant(address participant) external view returns (uint256);
+    function is_allowed(address participant) external view returns (bool);
+    function getParticipantList() external view returns (address[] memory);
+    function getPoolInfo() external view returns(string memory, string memory, address, bool, uint);
+}

contracts/PoolFactory.sol

@@ -4,33 +4,70 @@ pragma solidity >=0.8.0;
 import "./compound/Compound.sol";
 
 contract PoolFactory is Compound {
-    bool private isPublic;
+    bool public isPublic;
     address public owner;
-    uint8 private participantCount;
+    string public title;
+    string public description;
     address[] public participantsList;
     mapping(address => uint256) public balances;
     mapping(address => bool) public exists;
 
     // Log the event about a deposit being made by an address and its amount
     event LogDepositMade(address indexed accountAddress, uint256 amount);
 
-    constructor(bool _isPublic, address _owner) {
+    constructor(
+        bool _isPublic,
+        address _owner,
+        string memory _title,
+        string memory _description
+    ) {
         /* Set the owner to the creator of this contract */
         isPublic = _isPublic;
         owner = _owner;
+        title = _title;
+        description = _description;
         balances[owner] = 0;
         exists[owner] = true;
-        participantCount = 0;
         participantsList.push(owner);
     }
 
+    modifier onlyOwnerOrPublic() {
+        require(msg.sender == owner, "Only owner can call this function.");
+        _;
+    }
+
+    modifier onlyOwner() {
+        require(msg.sender == owner, "Not authorized");
+        _;
+    }
+
+    modifier autoEnroll() {
+        if (is_allowed(msg.sender) == false) {
+            participantsList.push(msg.sender);
+            balances[msg.sender] = 0;
+            exists[msg.sender] = true;
+        }
+        _;
+    }
+
+    modifier onlyEnrolled() {
+        require(exists[msg.sender] == true, "Not allowed");
+        _;
+    }
+
+    modifier sufficentBalanceCheck(uint256 withdrawAmount) {
+        require(
+            withdrawAmount <= balances[msg.sender],
+            "Error amount, can't withdraw more than deposit"
+        );
+        _;
+    }
+
     /// @notice Enroll a customer with the bank,
     /// Only the owner can enroll a participant
     /// @return The balance of the user after enrolling
-    function enroll(address participant) public returns (uint256) {
-        require(msg.sender == owner, "Not authorized");
+    function enroll(address participant) public onlyOwner returns (uint256) {
         require(exists[participant] == false, "Already enrolled");
-        participantCount++;
         participantsList.push(participant);
         balances[participant] = 0;
         exists[participant] = true;
@@ -39,14 +76,13 @@ contract PoolFactory is Compound {
 
     /// @notice Deposit ether into bank, requires method is "payable"
     /// @return The balance of the user after the deposit is made
-    function deposit() public payable returns (uint256) {
-        require(exists[msg.sender] == true || isPublic == true, "Not allowed");
-        if (is_allowed(msg.sender) == false) {
-            participantCount++;
-            participantsList.push(msg.sender);
-            balances[msg.sender] = 0;
-            exists[msg.sender] = true;
-        }
+    function deposit()
+        public
+        payable
+        onlyOwnerOrPublic
+        autoEnroll
+        returns (uint256)
+    {
         balances[msg.sender] += msg.value;
         emit LogDepositMade(msg.sender, msg.value);
         return balances[msg.sender];
@@ -55,15 +91,10 @@ contract PoolFactory is Compound {
     function deposit_and_invest_compound(address payable _cEtherContract)
         public
         payable
+        onlyOwnerOrPublic
+        autoEnroll
         returns (uint256)
     {
-        require(exists[msg.sender] == true || isPublic == true, "Not allowed");
-        if (is_allowed(msg.sender) == false) {
-            participantCount++;
-            participantsList.push(msg.sender);
-            balances[msg.sender] = 0;
-            exists[msg.sender] = true;
-        }
         balances[msg.sender] += msg.value;
         supplyEthToCompound(_cEtherContract);
         emit LogDepositMade(msg.sender, msg.value);
@@ -74,31 +105,29 @@ contract PoolFactory is Compound {
     /// @return remainingBal : the balance remaining for the user
     function withdraw(uint256 withdrawAmount)
         public
+        onlyEnrolled
+        sufficentBalanceCheck(withdrawAmount)
         returns (uint256 remainingBal)
     {
-        require(exists[msg.sender] == true, "Not allowed");
-        require(withdrawAmount <= balances[msg.sender], "Error amount, can't withdraw more than deposit");
-        // Check enough balance available, otherwise just return balance
-        if (withdrawAmount <= balances[msg.sender]) {
-            balances[msg.sender] -= withdrawAmount;
-            payable(msg.sender).transfer(withdrawAmount);
-        }
+        balances[msg.sender] -= withdrawAmount;
+        payable(msg.sender).transfer(withdrawAmount);
         return balances[msg.sender];
     }
 
-    function withdraw_and_redeem(uint256 withdrawAmount, bool redeemType,
-        address _cEtherContract)
+    function withdraw_and_redeem(
+        uint256 withdrawAmount,
+        bool redeemType,
+        address _cEtherContract
+    )
         public
+        onlyEnrolled
+        sufficentBalanceCheck(withdrawAmount)
         returns (uint256 remainingBal)
     {
-        require(exists[msg.sender] == true, "Not allowed");
-        require(withdrawAmount <= balances[msg.sender], "Error amount, can't withdraw more than deposit");
         // Check enough balance available, otherwise just return balance
         redeemCEth(withdrawAmount, redeemType, _cEtherContract);
-        if (withdrawAmount <= balances[msg.sender]) {
-            balances[msg.sender] -= withdrawAmount;
-            payable(msg.sender).transfer(withdrawAmount);
-        }
+        balances[msg.sender] -= withdrawAmount;
+        payable(msg.sender).transfer(withdrawAmount);
         return balances[msg.sender];
     }
 
@@ -128,7 +157,7 @@ contract PoolFactory is Compound {
         return owner;
     }
 
-    function is_public() public view returns(bool) {
+    function is_public() public view returns (bool) {
         return isPublic;
     }
 
@@ -148,4 +177,18 @@ contract PoolFactory is Compound {
     function getParticipantList() public view returns (address[] memory) {
         return participantsList;
     }
+
+    function getPoolInfo()
+        public
+        view
+        returns (
+            string memory,
+            string memory,
+            address,
+            bool,
+            uint256
+        )
+    {
+        return (title, description, owner, isPublic, participantsList.length);
+    }
 }

contracts/PoolRecorder.sol

@@ -2,19 +2,10 @@
 pragma solidity >=0.8.0;
 
 import "./PoolFactory.sol";
+import "./IPoolFactory.sol";
 
 contract PoolRecorder {
-    struct Pool {
-        string name;
-        string description;
-        address owner;
-        address PoolAddress;
-        bool visible;
-    }
-
     address[] poolList;
-    mapping(address => Pool) public poolRecorded;
-
     event PoolAdded(address poolAddress);
 
     function createPool(
@@ -23,35 +14,11 @@ contract PoolRecorder {
         bool _visible,
         address _owner
     ) public returns (address) {
-        PoolFactory newPoolBank = new PoolFactory(_visible, _owner);
-        addPool(
-            address(newPoolBank),
-            _owner,
-            _name,
-            _description,
-            _visible
-        );
+        PoolFactory newPoolBank = new PoolFactory(_visible, _owner, _name, _description);
+        poolList.push(address(newPoolBank));
         return address(newPoolBank);
     }
 
-    function addPool(
-        address poolAddress,
-        address _owner,
-        string memory _name,
-        string memory _description,
-        bool _visible
-    ) private {
-        poolList.push(poolAddress);
-        poolRecorded[poolAddress] = Pool(
-            _name,
-            _description,
-            _owner,
-            poolAddress,
-            _visible
-        );
-        emit PoolAdded(poolAddress);
-    }
-
     function removePool(address poolAddress) public {
         for (uint256 index = 0; index < poolList.length; index++) {
             if (poolList[index] == poolAddress) {
@@ -69,8 +36,9 @@ contract PoolRecorder {
     function getPoolInfo(address poolAddress)
         public
         view
-        returns (Pool memory)
+        returns (string memory, string memory, address,bool, uint)
     {
-        return poolRecorded[poolAddress];
+        IPoolFactory pool = IPoolFactory(poolAddress);
+        return pool.getPoolInfo();
     }
 }

test/PoolRecorder.test.js

@@ -15,26 +15,26 @@ describe('PoolRecorder', function () {
 
     it('should create new pools from PoolRecorder smarcontract', async () => {
         addressAlicePool = await this.poolRecorder.createPool("alice's pool", "pool for alice and friends", true, alice, { from: alice })
-        addressMyDefiPool = await this.poolRecorder.createPool("MyDefi's pool", "MyDefi is a new defi project that have a great impact", true, charlie, { from: charlie })
+        addressMyDefiPool = await this.poolRecorder.createPool("MyDefi pool", "MyDefi is a new defi project that have a great impact", true, charlie, { from: charlie })
         const listPool = await this.poolRecorder.getListPools()
         assert.equal(listPool.length, 2)
-        getPoolInfoAlice = await this.poolRecorder.getPoolInfo(listPool[0])
-        assert.equal(getPoolInfoAlice.name, "alice\'s pool")
-        assert.equal(getPoolInfoAlice.description, "pool for alice and friends")
-        assert.equal(getPoolInfoAlice.visible, true)
-        assert.equal(getPoolInfoAlice.owner, alice)
+        getPoolInfoAlice = await this.poolRecorder.getPoolInfo(listPool[0], {from: owner})
+        assert.equal(getPoolInfoAlice[0], "alice's pool")
+        assert.equal(getPoolInfoAlice[1], "pool for alice and friends")
+        assert.equal(getPoolInfoAlice[3], true)
+        assert.equal(getPoolInfoAlice[2], alice)
     });
 
     it('should remove a pool from PoolRecorder smarcontract', async () => {
         addressAlicePool = await this.poolRecorder.createPool("alice's pool", "pool for alice and friends", true, alice, { from: alice })
         // addressMyDefiPool = await this.poolRecorder.createPool("MyDefi's pool", "MyDefi is a new defi project that have a great impact", true, { from: charlie })
         const listPool = await this.poolRecorder.getListPools()
         assert.equal(listPool.length, 1)
-        getPoolInfoAlice = await this.poolRecorder.getPoolInfo(listPool[0])
-        assert.equal(getPoolInfoAlice.name, "alice\'s pool")
-        assert.equal(getPoolInfoAlice.description, "pool for alice and friends")
-        assert.equal(getPoolInfoAlice.visible, true)
-        assert.equal(getPoolInfoAlice.owner, alice)
+        getPoolInfoAlice = await this.poolRecorder.getPoolInfo(listPool[0], {from: owner})
+        assert.equal(getPoolInfoAlice[0], "alice\'s pool")
+        assert.equal(getPoolInfoAlice[1], "pool for alice and friends")
+        assert.equal(getPoolInfoAlice[3], true)
+        assert.equal(getPoolInfoAlice[2], alice)
     });
 
     it('should remove a pool from PoolRecorder smarcontract', async () => {

test/poolFactory.test.js

@@ -11,7 +11,7 @@ const [chairperson, alice, bob, charlie, danny] = accounts;
 describe("PoolFactory", () => {
 
   it("enroll everyone", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "title", "description", { from: chairperson });
     assert.isTrue(await pool.is_owner({ from: chairperson }))
 
     await pool.enroll(alice, { from: chairperson });
@@ -29,10 +29,18 @@ describe("PoolFactory", () => {
     await pool.enroll(danny, { from: chairperson });
     const dannyBalance = await pool.balance({ from: danny });
     assert.equal(dannyBalance, 0, "initial balance is incorrect");
+
+    const poolInfo = await pool.getPoolInfo({from: chairperson});
+    assert.equal(poolInfo[0], "title")
+    assert.equal(poolInfo[1], "description")
+    assert.equal(poolInfo[2], chairperson)
+    assert.equal(poolInfo[3], false)
+    assert.equal(poolInfo[4].toNumber(), 5)
+
   });
 
   it("should deposit correct amount", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "0x"+"title", "description", { from: chairperson });
     const deposit = 1.5 * ether;
     await pool.enroll(alice, { from: chairperson });
     const receipt = await pool.deposit({ from: alice, value: Web3.utils.toBN(deposit) });
@@ -52,7 +60,7 @@ describe("PoolFactory", () => {
   });
 
   it("should not deposit if not enrolled", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "0x"+"title", "description", { from: chairperson });
     const deposit = 1.5 * ether;
 
     await expectRevert(
@@ -62,7 +70,7 @@ describe("PoolFactory", () => {
   });
 
   it("should withdraw correct amount", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "0x"+"title", "description", { from: chairperson });
     const deposit = 5 * ether;
 
     await pool.enroll(alice, { from: chairperson });
@@ -78,7 +86,7 @@ describe("PoolFactory", () => {
   });
 
   it("should keep balance unchanged if withdraw greater than balance", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "0x"+"title", "description", { from: chairperson });
     const deposit = 3 * ether;
 
     await pool.enroll(alice, { from: chairperson });
@@ -93,7 +101,7 @@ describe("PoolFactory", () => {
   });
 
   it("should revert ether sent to this contract through fallback", async () => {
-    pool = await PoolFactory.new(false, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(false, chairperson, "0x"+"title", "description", { from: chairperson });
     const deposit = 3 * ether;
 
     const first_balance = await balance.current(alice);
@@ -109,7 +117,7 @@ describe("PoolFactory", () => {
   });
 
   it("should allow depost if pool is public", async () => {
-    pool = await PoolFactory.new(true, chairperson, { from: chairperson });
+    pool = await PoolFactory.new(true, chairperson, "title", "description", { from: chairperson });
     const deposit = 3 * ether;
     await pool.deposit({ from: alice, value: Web3.utils.toBN(deposit) });
     await pool.deposit({ from: bob, value: Web3.utils.toBN(deposit) });