Fix ProposalCreated event dates (#369)

* feat: protect the DAO executor against reentrancy * fix: remove console.log * fix: prevent storage corruption * feat: refactored reentrancy guard into a modifier * feat: added tests and preliminary versioning * chore: cleaning * fix: remove empty comment * fix: refactor findEventTopicLog function * fix: moved implementation slot and renamed file * fix: remove redundant includes * prepare package * remember the branch * only artifact src * correct the paths * correct root path * update hashes * add logs * only copy typechain * generate typechain * setup rollup * update npm packages * update rollup index * add esm cjs * add readme * remove gitIgnore * update typechain * update typescript * convert script to typescript * change commit hash json * rename types to typechain * add test * add json to rollup and export types * update usage test * generate index.ts dynamically * update readme * add empty line * update commit hashes * fix index.ts * chore: update test * fix: removed redundant imports * chore: remove legacy version files of DAO * fix: adapt contracts-version tests * fix: remove version in title as contracts will get a constant * fix: compile legacy contracts for regression testing * Revert "fix: compile legacy contracts for regression testing " This reverts commit 403436c. * revert versioning changes * adapt tests * test with creating symbolic link * correct path * feat: added tests for proposal start and end date handling * fix: corrected wrong validation order * Revert "chore: remove legacy version files of DAO" This reverts commit 57f755c. * add legacy versions * fix: renaming of the versions * chore: remove old CI task * chore: maintained changelog * fix: version numbers * fix: imports * WIP * doc: clarified the origin of the implementation slot * fix: versioning * fix: move files * Apply suggestions from code review Co-authored-by: Jør∂¡ <[email protected]> * chore: formatting * chore: remove NatSpec version number from osx files * feat: added more tests * Applied review suggestion Co-authored-by: Rekard0 <[email protected]> * feat: added test checking that execution is still possible after the upgrade * Improved comments * feat: initialize _rentrancyStatus * doc: clarify comments * fix: change version number * fix: use upgradeToAndCall and correct versions * feat: added initializeUpgradeFrom function and tests * feat: improved function and added more tests * WIP * fix: tests * fix: finalize tests * fix typechain and add a script * fix: typo * feat: improve initializeUpgradeFrom * feat: added _initData parameter * feat: added test * fix: wrong version number * WIP * fix remaining tests * fix subgraph * add ref depth * fix: replacing getContractFactory and getContractAt by typechain factory calls and added missing awaits * fix: test timings * fix: re-introduced yarn postinstall * fix: remove redundant includes * fix: adapt upgrade tests * feat: optimize if statements * fix: tests * feat: rename initializeUpgradeFrom to initializeFrom * fix: correct test description * fix: removed duplicate file * fix: typo * fix: delete legacy files * revert: metadata changes as they will happen in #375 * fix: merge conflicts * fix: re-order changelog entries * fix: remove wrong/redundant import originating from merge * feat: improve upgrade version checks * fix: remove redundant ROOT_PERMISSION grant call * feat: added more checks to the upgrade tests * feat: more explicit variable naming * fix: reworked initializeFrom logic * feat: improved changelog entry * feat: improved comment * chore: remove deploy scripts to be handled in a separate task --------- Co-authored-by: Rekard0 <[email protected]> Co-authored-by: Jør∂¡ <[email protected]>
aragon · May 19, 2023 · 0ddff1b · 0ddff1b
1 parent 4c10d1f
commit 0ddff1b
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 9 deletions.
diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Fixed logic bug in the `TokenVoting` and `AddresslistVoting` implementations that caused the `createProposal` function to emit the unvalidated `_startDate` and `_endDate` input arguments (that both can be zero) in the `ProposalCreated` event instead of the validated ones.
 - Changed the `createProposal` functions in `Multisig` to allow creating proposals when the `_msgSender()` is listed in the current block.
 - Changed the `createProposal` functions in `AddresslistVoting` to allow creating proposals when the `_msgSender()` is listed in the current block.
 - Changed the `createProposal` functions in `TokenVoting` to allow creating proposals when the `_msgSender()` owns more tokens at least `minProposerVotingPower()` tokens in the current block.

diff --git a/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol b/packages/contracts/src/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol
@@ -98,6 +98,8 @@ contract AddresslistVoting is IMembership, Addresslist, MajorityVotingBase {
             snapshotBlock = block.number.toUint64() - 1; // The snapshot block must be mined already to protect the transaction against backrunning transactions causing census changes.
         }
 
+        (_startDate, _endDate) = _validateProposalDates(_startDate, _endDate);
+
         proposalId = _createProposal({
             _creator: _msgSender(),
             _metadata: _metadata,
@@ -110,10 +112,8 @@ contract AddresslistVoting is IMembership, Addresslist, MajorityVotingBase {
         // Store proposal related information
         Proposal storage proposal_ = proposals[proposalId];
 
-        (proposal_.parameters.startDate, proposal_.parameters.endDate) = _validateProposalDates({
-            _start: _startDate,
-            _end: _endDate
-        });
+        proposal_.parameters.startDate = _startDate;
+        proposal_.parameters.endDate = _endDate;
         proposal_.parameters.snapshotBlock = snapshotBlock;
         proposal_.parameters.votingMode = votingMode();
         proposal_.parameters.supportThreshold = supportThreshold();

diff --git a/packages/contracts/src/plugins/governance/majority-voting/token/TokenVoting.sol b/packages/contracts/src/plugins/governance/majority-voting/token/TokenVoting.sol
@@ -105,6 +105,8 @@ contract TokenVoting is IMembership, MajorityVotingBase {
             revert NoVotingPower();
         }
 
+        (_startDate, _endDate) = _validateProposalDates(_startDate, _endDate);
+
         proposalId = _createProposal({
             _creator: _msgSender(),
             _metadata: _metadata,
@@ -117,10 +119,8 @@ contract TokenVoting is IMembership, MajorityVotingBase {
         // Store proposal related information
         Proposal storage proposal_ = proposals[proposalId];
 
-        (proposal_.parameters.startDate, proposal_.parameters.endDate) = _validateProposalDates(
-            _startDate,
-            _endDate
-        );
+        proposal_.parameters.startDate = _startDate;
+        proposal_.parameters.endDate = _endDate;
         proposal_.parameters.snapshotBlock = snapshotBlock.toUint64();
         proposal_.parameters.votingMode = votingMode();
         proposal_.parameters.supportThreshold = supportThreshold();
@@ -148,7 +148,7 @@ contract TokenVoting is IMembership, MajorityVotingBase {
 
     /// @inheritdoc IMembership
     function isMember(address _account) external view returns (bool) {
-        // A member must own or least one token or have at least one token delegated to her/him.
+        // A member must own at least one token or have at least one token delegated to her/him.
         return
             votingToken.getVotes(_account) > 0 ||
             IERC20Upgradeable(address(votingToken)).balanceOf(_account) > 0;

diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts
@@ -478,6 +478,52 @@ describe('AddresslistVoting', function () {
         .withArgs(earliestEndDate, tooEarlyEndDate);
     });
 
+    it('sets the startDate to now and endDate to startDate + minDuration, if 0 is provided as an input', async () => {
+      await voting.initialize(dao.address, votingSettings, [
+        signers[0].address,
+      ]);
+
+      // Create a proposal with zero as an input for `_startDate` and `_endDate`
+      const startDate = 0; // now
+      const endDate = 0; // startDate + minDuration
+
+      const creationTx = await voting.createProposal(
+        dummyMetadata,
+        [],
+        0,
+        startDate,
+        endDate,
+        VoteOption.None,
+        false
+      );
+
+      const currentTime = (
+        await ethers.provider.getBlock((await creationTx.wait()).blockNumber)
+      ).timestamp;
+
+      const expectedStartDate = currentTime;
+      const expectedEndDate = expectedStartDate + votingSettings.minDuration;
+
+      // Check the state
+      const proposal = await voting.getProposal(id);
+      expect(proposal.parameters.startDate).to.eq(expectedStartDate);
+      expect(proposal.parameters.endDate).to.eq(expectedEndDate);
+
+      // Check the event
+      const event = await findEvent<ProposalCreatedEvent>(
+        creationTx,
+        'ProposalCreated'
+      );
+
+      expect(event.args.proposalId).to.equal(id);
+      expect(event.args.creator).to.equal(signers[0].address);
+      expect(event.args.startDate).to.equal(expectedStartDate);
+      expect(event.args.endDate).to.equal(expectedEndDate);
+      expect(event.args.metadata).to.equal(dummyMetadata);
+      expect(event.args.actions).to.deep.equal([]);
+      expect(event.args.allowFailureMap).to.equal(0);
+    });
+
     it('ceils the `minVotingPower` value if it has a remainder', async () => {
       votingSettings.minParticipation = pctToRatio(30).add(1); // 30.0001 %
 

diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts
@@ -783,6 +783,54 @@ describe('TokenVoting', function () {
         .withArgs(earliestEndDate, tooEarlyEndDate);
     });
 
+    it('sets the startDate to now and endDate to startDate + minDuration, if 0 is provided as an input', async () => {
+      await voting.initialize(
+        dao.address,
+        votingSettings,
+        governanceErc20Mock.address
+      );
+
+      // Create a proposal with zero as an input for `_startDate` and `_endDate`
+      const startDate = 0; // now
+      const endDate = 0; // startDate + minDuration
+
+      const creationTx = await voting.createProposal(
+        dummyMetadata,
+        [],
+        0,
+        startDate,
+        endDate,
+        VoteOption.None,
+        false
+      );
+
+      const currentTime = (
+        await ethers.provider.getBlock((await creationTx.wait()).blockNumber)
+      ).timestamp;
+
+      const expectedStartDate = currentTime;
+      const expectedEndDate = expectedStartDate + votingSettings.minDuration;
+
+      // Check the state
+      const proposal = await voting.getProposal(id);
+      expect(proposal.parameters.startDate).to.eq(expectedStartDate);
+      expect(proposal.parameters.endDate).to.eq(expectedEndDate);
+
+      // Check the event
+      const event = await findEvent<ProposalCreatedEvent>(
+        creationTx,
+        'ProposalCreated'
+      );
+
+      expect(event.args.proposalId).to.equal(id);
+      expect(event.args.creator).to.equal(signers[0].address);
+      expect(event.args.startDate).to.equal(expectedStartDate);
+      expect(event.args.endDate).to.equal(expectedEndDate);
+      expect(event.args.metadata).to.equal(dummyMetadata);
+      expect(event.args.actions).to.deep.equal([]);
+      expect(event.args.allowFailureMap).to.equal(0);
+    });
+
     it('ceils the `minVotingPower` value if it has a remainder', async () => {
       votingSettings.minParticipation = pctToRatio(30).add(1); // 30.0001 %