Fix DMDcoin#53: Implicit double-voting prevention and separate change…

…Vote functionality
MSalman6 · Jan 24, 2025 · 118b942 · 118b942
1 parent a532195
commit 118b942
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 1 deletion.
diff --git a/contracts/DiamondDao.sol b/contracts/DiamondDao.sol
@@ -339,6 +339,36 @@ contract DiamondDao is IDiamondDao, Initializable, ReentrancyGuardUpgradeable, V
         emit SubmitVoteWithReason(voter, proposalId, _vote, reason);
     }
 
+    function changeVote(
+        uint256 proposalId,
+        Vote _vote,
+        string calldata reason
+    ) external exists(proposalId) onlyPhase(Phase.Voting) onlyValidator {
+        if (!_proposalVoters[proposalId].contains(msg.sender)) {
+            revert NoVoteFound(proposalId, msg.sender);
+        }
+
+        address voter = msg.sender;
+
+        Proposal storage proposal = proposals[proposalId];
+
+        if (proposal.state != ProposalState.Active) {
+            revert UnexpectedProposalState(proposalId, proposal.state);
+        }
+
+        VoteRecord storage voteRecord = votes[proposalId][voter];
+
+        if (voteRecord.vote == _vote && keccak256(bytes(voteRecord.reason)) == keccak256(bytes(reason))) {
+            revert SameVote(proposalId, voter, _vote);
+        }
+
+        voteRecord.vote = _vote;
+        voteRecord.reason = reason;
+        voteRecord.timestamp = uint64(block.timestamp);
+
+        emit ChangeVote(voter, proposalId, _vote, reason);
+    }
+
     function finalize(uint256 proposalId) external nonReentrant exists(proposalId) {
         _requireState(proposalId, ProposalState.VotingFinished);
 
@@ -513,6 +543,10 @@ contract DiamondDao is IDiamondDao, Initializable, ReentrancyGuardUpgradeable, V
     ) private {
         _requireState(proposalId, ProposalState.Active);
 
+        if (_proposalVoters[proposalId].contains(voter)) {
+            revert AlreadyVoted(proposalId, voter, _vote);
+        }
+
         _daoEpochVoters[daoPhase.daoEpoch].add(voter);
         _proposalVoters[proposalId].add(voter);
 

diff --git a/contracts/interfaces/IDiamondDao.sol b/contracts/interfaces/IDiamondDao.sol
@@ -31,6 +31,8 @@ interface IDiamondDao {
         string reason
     );
 
+    event ChangeVote(address indexed voter, uint256 indexed proposalId, Vote indexed vote, string reason);
+
     event SwitchDaoPhase(Phase indexed phase, uint256 indexed start, uint256 indexed end);
 
     event SetCreateProposalFee(uint256 indexed fee);
@@ -55,6 +57,9 @@ interface IDiamondDao {
     error UnfinalizedProposalsExist();
     error OutsideExecutionWindow(uint256 proposalId);
     error NotProposer(uint256 proposalId, address caller);
+    error SameVote(uint256 proposalId, address vote, Vote _vote);
+    error AlreadyVoted(uint256 proposalId, address voter, Vote vote);
+    error NoVoteFound(uint256 proposalId, address voter);
 
     function propose(
         address[] memory targets,

diff --git a/test/DiamondDao.spec.ts b/test/DiamondDao.spec.ts
@@ -142,6 +142,17 @@ describe("DiamondDao contract", function () {
     }
   }
 
+  async function changeVote(
+    dao: DiamondDao,
+    proposalId: bigint,
+    voters: HardhatEthersSigner[],
+    vote: Vote
+  ) {
+    for (const voter of voters) {
+      await dao.connect(voter).changeVote(proposalId, vote, "");
+    }
+  }
+
   describe("initializer", async function () {
     it("should not deploy contract with invalid ValidatorSet address", async function () {
       const daoFactory = await ethers.getContractFactory("DiamondDao");
@@ -903,6 +914,55 @@ describe("DiamondDao contract", function () {
     });
   });
 
+  describe("changeVote", async function () {
+    it("should revert change vote if voter has not voted yet", async function () {
+      const { dao, mockValidatorSet } = await loadFixture(deployFixture);
+
+      const proposer = users[10];
+      const voter = users[9];
+
+      const { proposalId } = await createProposal(dao, proposer, "a");
+
+      await mockValidatorSet.add(voter.address, voter.address, true);
+      await swithPhase(dao);
+
+      await expect(
+        dao.connect(voter).changeVote(proposalId, Vote.Yes, "reason")
+      ).to.be.revertedWithCustomError(dao, "VoteNotExist")
+        .withArgs(proposalId, voter.address);
+    });
+
+    it("should allow user to change vote", async function () {
+      const { dao, mockValidatorSet } = await loadFixture(deployFixture);
+
+      const proposer = users[10];
+      const voter = users[9];
+
+      const { proposalId } = await createProposal(dao, proposer, "a");
+
+      await mockValidatorSet.add(voter.address, voter.address, true);
+      await swithPhase(dao);
+
+      await dao.connect(voter).vote(proposalId, Vote.Yes);
+      await expect(dao.connect(voter).changeVote(proposalId, Vote.No, "reason")).to.not.be.reverted;
+    });
+
+    it("should allow user to change vote reason", async function () {
+      const { dao, mockValidatorSet } = await loadFixture(deployFixture);
+
+      const proposer = users[10];
+      const voter = users[9];
+
+      const { proposalId } = await createProposal(dao, proposer, "a");
+
+      await mockValidatorSet.add(voter.address, voter.address, true);
+      await swithPhase(dao);
+
+      await dao.connect(voter).voteWithReason(proposalId, Vote.Yes, "reason");
+      await expect(dao.connect(voter).changeVote(proposalId, Vote.No, "new reason")).to.not.be.reverted;
+    });
+  });
+
   describe("countVotes", async function () {
     it("should revert count votes for non-existing proposal", async function () {
       const { dao } = await loadFixture(deployFixture);
@@ -962,7 +1022,7 @@ describe("DiamondDao contract", function () {
         0n
       ]);
 
-      await vote(dao, proposalId, voters, Vote.No);
+      await changeVote(dao, proposalId, voters, Vote.No);
 
       expect(Object.values(await dao.countVotes(proposalId))).to.deep.equal([
         0n,