Improve smart contract (#54)

packages/contracts/README.md

@@ -24,9 +24,10 @@ Main types used for voting are:
 
     #### Fields
     ```solidity
-    //block at which room was created
+    // block at which room was created
     uint256 startBlock; 
-    //timestamp after which new votes won't be accepted
+    // timestamp in seconds after which new votes won't be accepted
+    // when casting votes endAt is compared to block.timestamp
     uint256 endAt; 
     // question of a proposal which voting room describes
     string question;
@@ -36,7 +37,7 @@ Main types used for voting are:
     uint256 totalVotesFor;
     // amount of summed votes against
     uint256 totalVotesAgainst;
-    //list of addresses that already voted
+    // list of addresses that already voted
     address[] voters;
     ```
 
@@ -67,10 +68,9 @@ Main types used for voting are:
 - `token`
     Variable that holds address of token used for vote verification. It is assigned at contract creation.
 
-- `VOTING_LENGTH`
+- `votingLength`
-    Constant describing length of voting room in seconds
+    Variable describing length of voting room in seconds
-    TODO:
+    Voting length it is assigned at contract creation.
-        - maybe set voting length per voting room ?
 
 - `EIP712DOMAIN_TYPEHASH`
     Constant holding type hash of EIP712 domain as per EIP712 specification
@@ -126,20 +126,24 @@ For more information about EIP-712 go to [docs](https://eips.ethereum.org/EIPS/e
 ### Functions
 
 - `constructor(IERC20 _address)`
-    assigns `_address` to `token` and generates `DOMAIN_SEPARATOR`
+    Assigns `_address` to `token` and generates `DOMAIN_SEPARATOR`
 
 - `getVotingRooms()`
-    returns votingRooms
+    Returns votingRooms
+
+- `getOngoingVotingRooms()`
+    Returns votingRooms in which `room.endAt > block.timestamp` which means the rooms are still accepting votes.
+    Since `votingLength` is set at contract creation and never changed, `room.endAt` is never decreasing with increasing index of votingRoom. Therefore it is enough to check from votingRooms.length up to first element which `endAt < block.timestamp`
 
 - `listRoomVoters(uint256 roomId)`
-    returns a list of voters for a given voting room. Reverts if roomId doesn't exist.
+    Returns a list of voters for a given voting room. Reverts if roomId doesn't exist.
 
 - `initializeVotingRoom(string calldata question,string calldata description,uint256 voteAmount)`
     Creates a new voting room with vote for set to voteAmount.
     First checks if voter has enough tokens to set vote for.
     Then creates a new voting room.
     `startBlock` is set as current block number.
-    `endAt` is set a current block timestamp plus.`VOTING_LENGTH`.
+    `endAt` is set a current block timestamp plus.`votingLength`.
     `question` is set as argument `question`.
     `description` is set as argument `description`.
     `totalVotesFor` is set as argument `voteAmount`.
@@ -177,10 +181,4 @@ For more information about EIP-712 go to [docs](https://eips.ethereum.org/EIPS/e
 
     Then it is checked that `vote.voter` didn't vote in this vote room before if he did function goes to another voter (IDEA: emit alreadyVoted ?).
 
-    Last check is whether `vote.voter` has enough tokens to vote. If he does not `NotEnoughToken` is emitted and function goes to another voter. If he does voting room is updated with `updateRoomVotes` and `VoteCast` is emitted.
+    Last check is whether `vote.voter` has enough tokens to vote. If he does not `NotEnoughToken` is emitted and function goes to another voter. If he does voting room is updated with `updateRoomVotes` and `VoteCast` is emitted.
-
-    TODO:
-    - not emit on Not enough tokens ?
-    - emit on wrong signature ?
-    - if instead of require for voting room not found ?
-    - if instead of require for vote closed ?

packages/contracts/contracts/VotingContract.sol

@@ -8,7 +8,7 @@ contract VotingContract {
     using SafeMath for uint256;
 
     IERC20 public token;
-    uint256 private constant VOTING_LENGTH = 1000;
+    uint256 private votingLength;
 
     bytes32 private constant EIP712DOMAIN_TYPEHASH =
         keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)');
@@ -36,11 +36,11 @@ contract VotingContract {
     }
 
     event VoteCast(uint256 roomId, address voter);
-    event NotEnoughToken(uint256 roomId, address voter);
     event VotingRoomStarted(uint256 roomId, string question);
 
-    constructor(IERC20 _address) {
+    constructor(IERC20 _address, uint256 _votingLength) {
         token = _address;
+        votingLength = _votingLength;
         DOMAIN_SEPARATOR = keccak256(
             abi.encode(
                 EIP712DOMAIN_TYPEHASH,
@@ -56,6 +56,19 @@ contract VotingContract {
         return votingRooms;
     }
 
+    function getOngoingVotingRooms() public view returns (VotingRoom[] memory result) {
+        uint256 votingRoomsLen = votingRooms.length;
+        uint256 i = votingRoomsLen;
+        result = new VotingRoom[](votingRoomsLen);
+        while (i > 0 && votingRooms[--i].endAt > block.timestamp) {
+            result[votingRooms.length - 1 - i] = votingRooms[i];
+            votingRoomsLen--;
+        }
+        assembly {
+            mstore(result, sub(mload(result), votingRoomsLen))
+        }
+    }
+
     function listRoomVoters(uint256 roomId) public view returns (address[] memory) {
         require(roomId < votingRooms.length, 'No room');
         return votingRooms[roomId].voters;
@@ -69,7 +82,7 @@ contract VotingContract {
         require(token.balanceOf(msg.sender) >= voteAmount, 'not enough token');
         VotingRoom memory newVotingRoom;
         newVotingRoom.startBlock = block.number;
-        newVotingRoom.endAt = block.timestamp.add(VOTING_LENGTH);
+        newVotingRoom.endAt = block.timestamp.add(votingLength);
         newVotingRoom.question = question;
         newVotingRoom.description = description;
         newVotingRoom.totalVotesFor = voteAmount;
@@ -105,18 +118,13 @@ contract VotingContract {
         for (uint256 i = 0; i < votes.length; i++) {
             Vote calldata vote = votes[i];
             uint256 roomId = vote.roomIdAndType >> 1;
-            require(votingRooms[roomId].endAt > block.timestamp, 'vote closed');
             require(roomId < votingRooms.length, 'vote not found');
-            if (verify(vote, vote.r, vote.vs)) {
+            require(votingRooms[roomId].endAt > block.timestamp, 'vote closed');
-                if (voted[roomId][vote.voter] == false) {
+            require(verify(vote, vote.r, vote.vs), 'vote has wrong signature');
-                    if (token.balanceOf(vote.voter) >= vote.tokenAmount) {
+            require(voted[roomId][vote.voter] == false, 'voter already voted');
-                        updateRoomVotes(vote, roomId);
+            require(token.balanceOf(vote.voter) >= vote.tokenAmount, 'voter doesnt have enough tokens');
-                        emit VoteCast(roomId, vote.voter);
+            updateRoomVotes(vote, roomId);
-                    } else {
+            emit VoteCast(roomId, vote.voter);
-                        emit NotEnoughToken(roomId, vote.voter);
-                    }
-                }
-            }
         }
     }
 }

packages/contracts/test/1.votingContract.test.ts

@@ -4,6 +4,7 @@ import { VotingContract, ERC20Mock } from '../abi'
 import { utils, Wallet, Contract } from 'ethers'
 import { signTypedMessage, TypedMessage } from 'eth-sig-util'
 import { BigNumber } from '@ethersproject/bignumber'
+import { JsonRpcProvider } from '@ethersproject/providers'
 
 use(solidity)
 
@@ -45,12 +46,6 @@ const getSignedMessages = async (
   secondAddress: Wallet
 ): Promise<{ messages: any[]; signedMessages: any[] }> => {
   const votes = [
-    {
-      voter: alice,
-      vote: 1,
-      tokenAmount: BigNumber.from(100),
-      sessionID: 0,
-    },
     {
       voter: firstAddress,
       vote: 0,
@@ -86,12 +81,11 @@ const getSignedMessages = async (
   return { messages, signedMessages }
 }
 
-async function fixture([alice, firstAddress, secondAddress]: any[], provider: any) {
+async function fixture([alice, firstAddress, secondAddress]: any[], provider: JsonRpcProvider) {
   const erc20 = await deployContract(alice, ERC20Mock, ['MSNT', 'Mock SNT', alice.address, 100000])
   await erc20.transfer(firstAddress.address, 10000)
   await erc20.transfer(secondAddress.address, 10000)
-  const contract = await deployContract(alice, VotingContract, [erc20.address])
+  const contract = await deployContract(alice, VotingContract, [erc20.address, 1000])
-  await provider.send('evm_mine', [Math.floor(Date.now() / 1000)])
   return { contract, alice, firstAddress, secondAddress, provider }
 }
 
@@ -147,10 +141,41 @@ describe('Contract', () => {
       await contract.initializeVotingRoom('T2', '', BigNumber.from(200))
       await expect(contract.votingRooms(1)).to.be.reverted
     })
+
+    it('getOngoingVotingRooms', async () => {
+      const { contract, provider } = await loadFixture(fixture)
+
+      await expect((await contract.getOngoingVotingRooms()).length).to.eq(0)
+      await contract.initializeVotingRoom('test1', 'short desc', BigNumber.from(100))
+
+      let rooms
+      rooms = await contract.getOngoingVotingRooms()
+      await expect(rooms.length).to.eq(1)
+      await expect(rooms[0][2]).to.eq('test1')
+      await provider.send('evm_increaseTime', [500])
+      await provider.send('evm_mine', [])
+
+      await contract.initializeVotingRoom('test2', 'short desc', BigNumber.from(100))
+      rooms = await contract.getOngoingVotingRooms()
+      await expect(rooms.length).to.eq(2)
+      await expect(rooms[0][2]).to.eq('test2')
+      await expect(rooms[1][2]).to.eq('test1')
+      await provider.send('evm_increaseTime', [600])
+      await provider.send('evm_mine', [])
+
+      rooms = await contract.getOngoingVotingRooms()
+
+      await expect(rooms.length).to.eq(1)
+      await expect(rooms[0][2]).to.eq('test2')
+      await provider.send('evm_increaseTime', [600])
+      await provider.send('evm_mine', [])
+      rooms = await contract.getOngoingVotingRooms()
+      await expect(rooms.length).to.eq(0)
+    })
   })
   describe('helpers', () => {
     it('get voting rooms', async () => {
-      const { contract, firstAddress, secondAddress, provider } = await loadFixture(fixture)
+      const { contract } = await loadFixture(fixture)
       await contract.initializeVotingRoom('T1', 't1', BigNumber.from(100))
 
       await contract.initializeVotingRoom('T2', 't2', BigNumber.from(200))
@@ -177,7 +202,7 @@ describe('Contract', () => {
       await contract.initializeVotingRoom('0xabA1eF51ef4aE360a9e8C9aD2d787330B602eb24', '', BigNumber.from(100))
 
       expect(await contract.listRoomVoters(0)).to.deep.eq([alice.address])
-      await contract.castVotes(signedMessages.slice(2))
+      await contract.castVotes(signedMessages.slice(1))
 
       expect(await contract.listRoomVoters(0)).to.deep.eq([alice.address, secondAddress.address])
     })
@@ -196,9 +221,7 @@ describe('Contract', () => {
       )
       const signedMessage = [...msg, sig.r, sig._vs]
 
-      await expect(await contract.castVotes([signedMessage]))
+      await expect(contract.castVotes([signedMessage])).to.be.revertedWith('voter doesnt have enough tokens')
-        .to.emit(contract, 'NotEnoughToken')
-        .withArgs(0, firstAddress.address)
 
       const votingRoom = await contract.votingRooms(0)
       expect(votingRoom[2]).to.eq('test')
@@ -225,7 +248,7 @@ describe('Contract', () => {
       const { signedMessages } = await getSignedMessages(alice, firstAddress, secondAddress)
       await contract.initializeVotingRoom('test', '', BigNumber.from(100))
       await contract.castVotes(signedMessages)
-      await contract.castVotes(signedMessages)
+      await expect(contract.castVotes(signedMessages)).to.be.revertedWith('voter already voted')
 
       const votingRoom = await contract.votingRooms(0)
       expect(votingRoom[2]).to.eq('test')
@@ -243,23 +266,22 @@ describe('Contract', () => {
     it('none existent room', async () => {
       const { contract, alice, firstAddress, secondAddress } = await loadFixture(fixture)
       const { signedMessages } = await getSignedMessages(alice, firstAddress, secondAddress)
-      await expect(contract.castVotes(signedMessages)).to.be.reverted
+      await expect(contract.castVotes(signedMessages)).to.be.revertedWith('vote not found')
     })
 
     it('old room', async () => {
       const { contract, alice, firstAddress, secondAddress, provider } = await loadFixture(fixture)
       const { signedMessages } = await getSignedMessages(alice, firstAddress, secondAddress)
       await contract.initializeVotingRoom('test', '', BigNumber.from(100))
-      await provider.send('evm_mine', [Math.floor(Date.now() / 1000 + 2000)])
+      await provider.send('evm_increaseTime', [10000])
-      await expect(contract.castVotes(signedMessages)).to.be.reverted
+      await expect(contract.castVotes(signedMessages)).to.be.revertedWith('vote closed')
     })
 
     it('wrong signature', async () => {
-      const { contract, alice, firstAddress, secondAddress, provider } = await loadFixture(fixture)
+      const { contract, alice, firstAddress, secondAddress } = await loadFixture(fixture)
       const { messages } = await getSignedMessages(alice, firstAddress, secondAddress)
 
       await contract.initializeVotingRoom('test', '', BigNumber.from(100))
-      await provider.send('evm_mine', [Math.floor(Date.now() / 1000 + 2000)])
 
       const signedMessages = await Promise.all(
         messages.map(async (msg) => {
@@ -275,7 +297,7 @@ describe('Contract', () => {
         })
       )
 
-      await expect(contract.castVotes(signedMessages)).to.be.reverted
+      await expect(contract.castVotes(signedMessages)).to.be.revertedWith('vote has wrong signature')
     })
   })
 })