From 974ec940d1b2acfa36e7964de622835c7f9eadaf Mon Sep 17 00:00:00 2001
From: r4bbit <445106+0x-r4bbit@users.noreply.github.com>
Date: Tue, 16 May 2023 22:22:50 +0200
Subject: [PATCH] fix: ensure cooldownPeriod check is done correctly

The `FeaturedVotingContract` comes with a check in `initializeVoting()`
that aims to ensure that the community that the voting is being
initialized for, has not been featured previously for #n votings.

This is denoted as the `cooldownPeriod`. If `cooldownPeriod = 1`, this
means there needs to be at least one voting which doesn't include the
community in question, that came *after* the voting that did.

The internal `_isInCooldownPeriod()` check has a bug which will return
false positives for any communiy that has been featured before,
regardless of `cooldownPeriod`s value.

When iterating previous votings, the contract actually needs to start
with the last one and iterate downwards, however it does the opposite so
it will never reach the correct votings to check.

This commit fixes the check and adds two tests to cover the case
accordingly.
---
 .../contracts/FeaturedVotingContract.sol      |  2 +-
 .../test/3.featuredVotingContract.test.ts     | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/packages/contracts/contracts/FeaturedVotingContract.sol b/packages/contracts/contracts/FeaturedVotingContract.sol
index e41f0c9..bde90d0 100644
--- a/packages/contracts/contracts/FeaturedVotingContract.sol
+++ b/packages/contracts/contracts/FeaturedVotingContract.sol
@@ -185,7 +185,7 @@ contract FeaturedVotingContract {
     function _isInCooldownPeriod(bytes calldata publicKey) private view returns (bool) {
         uint256 votingsCount = _min(votings.length, cooldownPeriod);
         for (uint256 i = 0; i < votingsCount; i++) {
-            bytes[] storage featured = featuredByVotingID[votings[i].id];
+            bytes[] storage featured = featuredByVotingID[votings[votings.length - i - 1].id];
             for (uint256 j = 0; j < featured.length; j++) {
                 if (_compareBytes(featured[j], publicKey)) {
                     return true;
diff --git a/packages/contracts/test/3.featuredVotingContract.test.ts b/packages/contracts/test/3.featuredVotingContract.test.ts
index c255e49..19db3f3 100644
--- a/packages/contracts/test/3.featuredVotingContract.test.ts
+++ b/packages/contracts/test/3.featuredVotingContract.test.ts
@@ -150,6 +150,56 @@ describe('FeaturedVotingContract', () => {
       const secondSender = featuredVotingContract.connect(secondSigner)
       await expect(secondSender.initializeVoting(publicKeys[0], 1000000)).to.be.revertedWith('not enough token')
     })
+
+    it('should revert when cooldown period for community has not passed yet', async () => {
+      const { featuredVotingContract } = await loadFixture(fixture)
+
+      // first initialize voting with first pubkey
+      await expect(featuredVotingContract.initializeVoting(publicKeys[0], 100)).to.emit(
+        featuredVotingContract,
+        'VotingStarted'
+      )
+
+      await time.increase(votingWithVerificationLength + 1)
+      await expect(featuredVotingContract.finalizeVoting()).to.emit(featuredVotingContract, 'VotingFinalized')
+      await expect((await featuredVotingContract.votings(0)).finalized).to.eq(true)
+
+      await expect(featuredVotingContract.initializeVoting(publicKeys[0], 100)).to.be.revertedWith(
+        'community has been featured recently'
+      )
+    })
+
+    it('should initialize voting for community when cooldown period has passed', async () => {
+      const { featuredVotingContract } = await loadFixture(fixture)
+
+      // first initialize voting with first pubkey
+      await expect(featuredVotingContract.initializeVoting(publicKeys[0], 100)).to.emit(
+        featuredVotingContract,
+        'VotingStarted'
+      )
+
+      // fast forward to finalize first voting
+      await time.increase(votingWithVerificationLength + 1)
+      await expect(featuredVotingContract.finalizeVoting()).to.emit(featuredVotingContract, 'VotingFinalized')
+      await expect((await featuredVotingContract.votings(0)).finalized).to.eq(true)
+
+      // the `cooldownPeriod` is `1` so we need at least one voting
+      // that doesn't feature `publicKeys[0]` to start a new
+      await expect(featuredVotingContract.initializeVoting(publicKeys[1], 100)).to.emit(
+        featuredVotingContract,
+        'VotingStarted'
+      )
+
+      // fast forward again to finalize second voting (which does not include `publicKeys[0]`)
+      await time.increase(votingWithVerificationLength + 1)
+      await expect(featuredVotingContract.finalizeVoting()).to.emit(featuredVotingContract, 'VotingFinalized')
+
+      // initializing vote for `publicKey[0]` should be fine now
+      await expect(featuredVotingContract.initializeVoting(publicKeys[0], 100)).to.emit(
+        featuredVotingContract,
+        'VotingStarted'
+      )
+    })
   })
 
   describe('#castVotes()', () => {