test(challengeBadFirstStep): added tests

contracts/adapters/voting/OffchainVoting.sol

@@ -283,6 +283,44 @@ contract OffchainVotingContract is
             );
     }
 
+    // slither-disable-next-line reentrancy-benign
+    function startNewVotingForProposal(
+        DaoRegistry dao,
+        bytes32 proposalId,
+        bytes memory data
+    ) external override onlyAdapter(dao) {
+        SnapshotProposalContract.ProposalMessage memory proposal = abi.decode(
+            data,
+            (SnapshotProposalContract.ProposalMessage)
+        );
+        (bool success, uint256 blockNumber) = ovHash.stringToUint(
+            proposal.payload.snapshot
+        );
+        require(success, "snapshot conversion error");
+        require(blockNumber <= block.number, "snapshot block in future");
+        require(blockNumber > 0, "block number cannot be 0");
+
+        votes[address(dao)][proposalId].startingTime = uint64(block.timestamp);
+        votes[address(dao)][proposalId].snapshot = blockNumber;
+
+        require(
+            _getBank(dao).balanceOf(
+                dao.getAddressIfDelegated(proposal.submitter),
+                DaoHelper.UNITS
+            ) > 0,
+            "not active member"
+        );
+
+        require(
+            SignatureChecker.isValidSignatureNow(
+                proposal.submitter,
+                _snapshotContract.hashMessage(dao, msg.sender, proposal),
+                proposal.sig
+            ),
+            "invalid proposal signature"
+        );
+    }
+
     /*
      * @notice Saves the vote result to the storage if resultNode (vote) is valid
      * @notice A valid vote result node must satisfy all the conditions in the function, so it can be stored
@@ -478,54 +516,23 @@ contract OffchainVotingContract is
         _challengeResult(dao, proposalId);
     }
 
-    // slither-disable-next-line reentrancy-benign
-    function startNewVotingForProposal(
-        DaoRegistry dao,
-        bytes32 proposalId,
-        bytes memory data
-    ) external override onlyAdapter(dao) {
-        SnapshotProposalContract.ProposalMessage memory proposal = abi.decode(
-            data,
-            (SnapshotProposalContract.ProposalMessage)
-        );
-        (bool success, uint256 blockNumber) = ovHash.stringToUint(
-            proposal.payload.snapshot
-        );
-        require(success, "snapshot conversion error");
-        require(blockNumber <= block.number, "snapshot block in future");
-        require(blockNumber > 0, "block number cannot be 0");
-
-        votes[address(dao)][proposalId].startingTime = uint64(block.timestamp);
-        votes[address(dao)][proposalId].snapshot = blockNumber;
-
-        require(
-            _getBank(dao).balanceOf(
-                dao.getAddressIfDelegated(proposal.submitter),
-                DaoHelper.UNITS
-            ) > 0,
-            "not active member"
-        );
-
-        require(
-            SignatureChecker.isValidSignatureNow(
-                proposal.submitter,
-                _snapshotContract.hashMessage(dao, msg.sender, proposal),
-                proposal.sig
-            ),
-            "invalid proposal signature"
-        );
-    }
-
+    /**
+     * @notice Allows anyone to challenge the first vote step in the result tree.
+     * @param dao The DAO address.
+     * @param proposalId The proposal id associated with the vote step.
+     * @param node The actual vote step that is being challenged.
+     */
     // slither-disable-next-line reentrancy-benign,reentrancy-events
-    function challengeBadFirstNode(
+    function challengeBadFirstStep(
         DaoRegistry dao,
         bytes32 proposalId,
         OffchainVotingHashContract.VoteResultNode memory node
     ) external reimbursable(dao) {
-        require(node.index == 1, "only first node");
+        require(node.index == 1, "only first step");
 
         Voting storage vote = votes[address(dao)][proposalId];
-        require(vote.resultRoot != bytes32(0), "no result available yet!");
+        require(vote.resultRoot != bytes32(0), "vote result not");
+
         (address actionId, ) = dao.proposals(proposalId);
         _verifyNode(dao, actionId, node, vote.resultRoot);
 

test/adapters/offchain.voting.test.js

@@ -63,11 +63,13 @@ const {
   prepareProposalPayload,
   prepareVoteProposalData,
   prepareProposalMessage,
+  prepareVoteResult,
   getVoteStepDomainDefinition,
   BadNodeError,
 } = require("../../utils/offchain-voting-util");
 
 const {
+  vote,
   buildVoteTree,
   buildProposal,
   buildVotes,
@@ -263,7 +265,8 @@ describe("Adapter - Offchain Voting", () => {
     configuration,
     voting,
     submitter,
-    moveBlockTime = true
+    moveBlockTime = true,
+    submitVoteResult = true
   ) => {
     const proposalId = getProposalCounter();
     const actionId = configuration.address;
@@ -316,14 +319,16 @@ describe("Adapter - Offchain Voting", () => {
     );
     await advanceTime(10); // ends the voting period
     // Submit a valid result
-    await voting.submitVoteResult(
-      dao.address,
-      proposalId,
-      resultTree.resultHash,
-      submitter.address,
-      resultTree.lastVoteResult,
-      resultTree.rootSig
-    );
+    if (submitVoteResult)
+      await voting.submitVoteResult(
+        dao.address,
+        proposalId,
+        resultTree.resultHash,
+        submitter.address,
+        resultTree.lastVoteResult,
+        resultTree.rootSig
+      );
+
     return { proposalId, resultTree };
   };
 
@@ -2210,6 +2215,186 @@ describe("Adapter - Offchain Voting", () => {
     });
   });
 
+  describe("Challenge Bad First Step", async () => {
+    it.skip("should be possible to challenge a bad first step", async () => {
+      //FIXME how to submit a modified first step but still valid?
+      const dao = this.dao;
+      const bank = this.extensions.bankExt;
+      const voting = this.adapters.voting;
+      const configuration = this.adapters.configuration;
+      const submitter = members[0];
+
+      const result = await submitValidVoteResult(
+        dao,
+        bank,
+        configuration,
+        voting,
+        submitter,
+        false
+      );
+
+      const badVote = await vote(
+        dao,
+        bank,
+        result.proposalId,
+        submitter,
+        0,
+        configuration.address,
+        chainId,
+        "2000"
+      );
+
+      const validStep1 = result.resultTree.votesResults[0];
+      const badVoteStep1 = {
+        nbNo: "0",
+        nbYes: "2000",
+        index: 1,
+        choice: 0,
+        sig: badVote.sig,
+        timestamp: badVote.timestamp,
+        proposalId: result.proposalId,
+        proof: validStep1.proof,
+      };
+
+      const call = voting.challengeBadFirstStep(
+        dao.address,
+        result.proposalId,
+        badVoteStep1
+      );
+
+      const challengeProposalId = soliditySha3(
+        result.proposalId,
+        result.resultTree.resultHash
+      );
+
+      await expectEvent(
+        call,
+        "ResultChallenged",
+        dao.address,
+        result.proposalId,
+        result.resultTree.resultHash,
+        challengeProposalId
+      );
+
+      // The vote result should be set to challenged
+      const storedVote = await voting.getVote(dao.address, result.proposalId);
+      expect(storedVote.isChallenged).to.be.true;
+
+      // submitter should be in jail after challenging a missing step
+      const notJailed = await dao.notJailed(submitter.address);
+      expect(notJailed).to.be.false;
+
+      // A challenge proposal must be created if the challenge call was successful
+      const challengeProposal = await dao.proposals(challengeProposalId);
+      expect(challengeProposal.flags.toString()).to.be.equal("1");
+      expect(challengeProposal.adapterAddress).to.be.equal(voting.address);
+    });
+
+    it("should not be possible to challenge a step that is not the first one", async () => {
+      const dao = this.dao;
+      const bank = this.extensions.bankExt;
+      const voting = this.adapters.voting;
+      const configuration = this.adapters.configuration;
+      const submitter = members[0];
+
+      const result = await submitValidVoteResult(
+        dao,
+        bank,
+        configuration,
+        voting,
+        submitter,
+        false
+      );
+
+      const voteResults = result.resultTree.votesResults;
+
+      for (let i = 1; i < voteResults.length; i++) {
+        await expect(
+          voting.challengeBadFirstStep(
+            dao.address,
+            result.proposalId,
+            voteResults[i]
+          )
+        ).to.be.revertedWith("only first step");
+      }
+    });
+
+    it("should not be possible to challenge a step that does not have a vote result", async () => {
+      const dao = this.dao;
+      const bank = this.extensions.bankExt;
+      const voting = this.adapters.voting;
+      const configuration = this.adapters.configuration;
+      const submitter = members[0];
+
+      const result = await submitValidVoteResult(
+        dao,
+        bank,
+        configuration,
+        voting,
+        submitter,
+        false,
+        false // do not submit the vote result, but return the result tree
+      );
+
+      const voteResults = result.resultTree.votesResults;
+      const voteStep1 = voteResults[0];
+      await expect(
+        voting.challengeBadFirstStep(dao.address, result.proposalId, voteStep1)
+      ).to.be.revertedWith("vote result not found");
+    });
+
+    it("should not be possible to challenge a step that has an invalid proof", async () => {
+      const dao = this.dao;
+      const bank = this.extensions.bankExt;
+      const voting = this.adapters.voting;
+      const configuration = this.adapters.configuration;
+      const submitter = members[0];
+
+      const result = await submitValidVoteResult(
+        dao,
+        bank,
+        configuration,
+        voting,
+        submitter,
+        false
+      );
+
+      const voteResults = result.resultTree.votesResults;
+      const voteStep1 = voteResults[0];
+      // Set an invalid proof to mimic the behavior of a invalid step
+      voteStep1.proof[0] =
+        "0x1d2c3a91bdb8c7ccbd7cf5ea1df6c9408f9678deef9bfc27639e8ea9429a3572";
+
+      await expect(
+        voting.challengeBadFirstStep(dao.address, result.proposalId, voteStep1)
+      ).to.be.revertedWith("invalid step proof");
+    });
+
+    it("should revert if there is nothing to be challenged when the first step is correct", async () => {
+      const dao = this.dao;
+      const bank = this.extensions.bankExt;
+      const voting = this.adapters.voting;
+      const configuration = this.adapters.configuration;
+      const submitter = members[0];
+
+      const result = await submitValidVoteResult(
+        dao,
+        bank,
+        configuration,
+        voting,
+        submitter,
+        false
+      );
+
+      const voteResults = result.resultTree.votesResults;
+      const voteStep1 = voteResults[0];
+
+      await expect(
+        voting.challengeBadFirstStep(dao.address, result.proposalId, voteStep1)
+      ).to.be.revertedWith("nothing to challenge");
+    });
+  });
+
   describe("Governance Token", async () => {
     it("should be possible to update a DAO configuration if you are a member and a maintainer that holds an external governance token", async () => {
       const accountIndex = 0;