From ce07cd30fbdebc092785d6497bc21ce84c959347 Mon Sep 17 00:00:00 2001 From: Szymon Szlachtowicz <38212223+Szymx95@users.noreply.github.com> Date: Mon, 6 Sep 2021 15:19:30 +0200 Subject: [PATCH] Improve smart contract (#54) --- packages/contracts/README.md | 34 +++++----- .../contracts/contracts/VotingContract.sol | 38 ++++++----- .../contracts/test/1.votingContract.test.ts | 64 +++++++++++++------ 3 files changed, 82 insertions(+), 54 deletions(-) diff --git a/packages/contracts/README.md b/packages/contracts/README.md index 2ac4b8d..0165378 100644 --- a/packages/contracts/README.md +++ b/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` - Constant describing length of voting room in seconds - TODO: - - maybe set voting length per voting room ? +- `votingLength` + Variable describing length of voting room in seconds + Voting length it is assigned at contract creation. - `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. - - 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 ? \ No newline at end of file + 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. \ No newline at end of file diff --git a/packages/contracts/contracts/VotingContract.sol b/packages/contracts/contracts/VotingContract.sol index 258e8ed..2884d4c 100644 --- a/packages/contracts/contracts/VotingContract.sol +++ b/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)) { - if (voted[roomId][vote.voter] == false) { - if (token.balanceOf(vote.voter) >= vote.tokenAmount) { - updateRoomVotes(vote, roomId); - emit VoteCast(roomId, vote.voter); - } else { - emit NotEnoughToken(roomId, vote.voter); - } - } - } + require(votingRooms[roomId].endAt > block.timestamp, 'vote closed'); + require(verify(vote, vote.r, vote.vs), 'vote has wrong signature'); + require(voted[roomId][vote.voter] == false, 'voter already voted'); + require(token.balanceOf(vote.voter) >= vote.tokenAmount, 'voter doesnt have enough tokens'); + updateRoomVotes(vote, roomId); + emit VoteCast(roomId, vote.voter); } } } diff --git a/packages/contracts/test/1.votingContract.test.ts b/packages/contracts/test/1.votingContract.test.ts index 91f2667..af73b6a 100644 --- a/packages/contracts/test/1.votingContract.test.ts +++ b/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]) - await provider.send('evm_mine', [Math.floor(Date.now() / 1000)]) + const contract = await deployContract(alice, VotingContract, [erc20.address, 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])) - .to.emit(contract, 'NotEnoughToken') - .withArgs(0, firstAddress.address) + await expect(contract.castVotes([signedMessage])).to.be.revertedWith('voter doesnt have enough tokens') 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 expect(contract.castVotes(signedMessages)).to.be.reverted + await provider.send('evm_increaseTime', [10000]) + 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') }) }) })