diff --git a/contracts/DiamondDao.sol b/contracts/DiamondDao.sol index 6832da2..3cc2040 100644 --- a/contracts/DiamondDao.sol +++ b/contracts/DiamondDao.sol @@ -362,6 +362,7 @@ contract DiamondDao is IDiamondDao, Initializable, ReentrancyGuardUpgradeable, V function execute(uint256 proposalId) external nonReentrant exists(proposalId) { _requireState(proposalId, ProposalState.Accepted); + _requireIsExecutable(proposalId); Proposal storage proposal = proposals[proposalId]; @@ -553,6 +554,20 @@ contract DiamondDao is IDiamondDao, Initializable, ReentrancyGuardUpgradeable, V } } + function _requireIsExecutable(uint256 _proposalId) private view { + Proposal memory proposal = getProposal(_proposalId); + + if (proposal.daoPhaseCount + 1 != daoPhaseCount) { + revert OutsideExecutionWindow(_proposalId); + } + + if ( + proposal.proposalType == ProposalType.ContractUpgrade && proposal.proposer != msg.sender + ) { + revert NotProposer(_proposalId, msg.sender); + } + } + function _isValidator(address stakingAddress) private view returns (bool) { address miningAddress = validatorSet.miningByStakingAddress(stakingAddress); diff --git a/contracts/interfaces/IDiamondDao.sol b/contracts/interfaces/IDiamondDao.sol index f67a9f2..4a5b75e 100644 --- a/contracts/interfaces/IDiamondDao.sol +++ b/contracts/interfaces/IDiamondDao.sol @@ -54,6 +54,8 @@ interface IDiamondDao { error FunctionUpgradeNotAllowed(bytes4 funcSelector, address targetContract); error InvalidUpgradeValue(uint256 currentVal, uint256 newVal); error UnfinalizedProposalsExist(); + error OutsideExecutionWindow(uint256 proposalId); + error NotProposer(uint256 proposalId, address caller); function propose( address[] memory targets, diff --git a/test/DiamondDao.spec.ts b/test/DiamondDao.spec.ts index 2dfc3e3..cb148f9 100644 --- a/test/DiamondDao.spec.ts +++ b/test/DiamondDao.spec.ts @@ -482,6 +482,7 @@ describe("DiamondDao contract", function () { }); it("should revert propose if limit was reached", async function () { + const proposer = users[2]; const { dao } = await loadFixture(deployFixture); for (let i = 0; i < 1000; ++i) { @@ -489,7 +490,7 @@ describe("DiamondDao contract", function () { } await expect( - dao.connect(users[2]).propose( + dao.connect(proposer).propose( [users[3].address], [ethers.parseEther('10')], [EmptyBytes], @@ -1270,6 +1271,47 @@ describe("DiamondDao contract", function () { .withArgs(proposalId, ProposalState.Created); }); + it("should revert execute of proposals that are outside execution window", async function () { + const voters = users.slice(10, 25); + + const { dao, mockValidatorSet, mockStaking } = await loadFixture(deployFixture); + + const proposer = users[4]; + const userToFund = users[5]; + const fundAmount = ethers.parseEther('1'); + + const { proposalId } = await createProposal( + dao, + users[1], + "fund user 5", + [userToFund.address], + [fundAmount], + [EmptyBytes] + ); + + await proposer.sendTransaction({ + to: await dao.getAddress(), + value: fundAmount + }); + + await addValidatorsStake(mockValidatorSet, mockStaking, voters); + + await swithPhase(dao); // switches to: voting phase 1 + await vote(dao, proposalId, voters, Vote.Yes); + await swithPhase(dao); // switches to: proposal phase 2 (executable window) + await swithPhase(dao); // switches to: voting phase 2 (executable window) + + expect(await dao.finalize(proposalId)); + + await swithPhase(dao); // switches to: proposal phase 3 (outside executable window) + + const tx = dao.connect(proposer).execute(proposalId); + + await expect(tx) + .to.revertedWithCustomError(dao, "OutsideExecutionWindow") + .withArgs(proposalId) + }); + it("should revert execute of declined proposal", async function () { const voters = users.slice(10, 25); diff --git a/test/ProposalExecution.spec.ts b/test/ProposalExecution.spec.ts index 86614dd..d6006c4 100644 --- a/test/ProposalExecution.spec.ts +++ b/test/ProposalExecution.spec.ts @@ -168,6 +168,7 @@ describe("DAO proposal execution", function () { describe("self function calls", async function () { it("should not allow to set createProposalFee = 0", async function () { + const proposer = users[2]; const { dao, mockValidatorSet, mockStaking } = await loadFixture(deployFixture); const calldata = dao.interface.encodeFunctionData("setCreateProposalFee", [0n]); @@ -182,10 +183,11 @@ describe("DAO proposal execution", function () { [calldata] ); - await expect(dao.execute(proposalId)).to.be.revertedWithCustomError(dao, "NewValueOutOfRange"); + await expect(dao.connect(proposer).execute(proposalId)).to.be.revertedWithCustomError(dao, "NewValueOutOfRange"); }); it("should update createProposalFee using proposal", async function () { + const proposer = users[2]; const { dao, mockValidatorSet, mockStaking } = await loadFixture(deployFixture); const newFeeValue = ethers.parseEther('20'); const calldata = dao.interface.encodeFunctionData("setCreateProposalFee", [newFeeValue]); @@ -200,7 +202,7 @@ describe("DAO proposal execution", function () { [calldata] ); - await expect(dao.execute(proposalId)) + await expect(dao.connect(proposer).execute(proposalId)) .to.emit(dao, "SetCreateProposalFee") .withArgs(newFeeValue); @@ -210,6 +212,7 @@ describe("DAO proposal execution", function () { describe("self upgrade", async function () { it("should perform DAO self upgrade", async function () { + const proposer = users[2]; const { dao, mockValidatorSet, mockStaking } = await loadFixture(deployFixture); const daoAddress = await dao.getAddress(); @@ -244,9 +247,9 @@ describe("DAO proposal execution", function () { [calldata] ); - await expect(dao.execute(proposalId)) + await expect(dao.connect(proposer).execute(proposalId)) .to.emit(dao, "ProposalExecuted") - .withArgs(users[0].address, proposalId); + .withArgs(proposer.address, proposalId); expect(await upgrades.erc1967.getImplementationAddress(daoAddress)).to.equal(newImplementation); }); diff --git a/test/ProposalValueGuards.spec.ts b/test/ProposalValueGuards.spec.ts index 29f71d0..e9b3ba3 100644 --- a/test/ProposalValueGuards.spec.ts +++ b/test/ProposalValueGuards.spec.ts @@ -173,6 +173,7 @@ describe("DAO Ecosystem Paramater Change Value Guards Test", function () { describe("proposal Value Guards", async function () { it("should set staking contract as isCoreContract", async function () { + const proposer = users[2]; const calldata = dao.interface.encodeFunctionData("setIsCoreContract", [await mockStaking.getAddress(), true]); const { proposalId } = await finalizedProposal( @@ -185,7 +186,7 @@ describe("DAO Ecosystem Paramater Change Value Guards Test", function () { [calldata] ); - await expect(dao.execute(proposalId)).to.emit(dao, "SetIsCoreContract").withArgs(await mockStaking.getAddress(), true); + await expect(dao.connect(proposer).execute(proposalId)).to.emit(dao, "SetIsCoreContract").withArgs(await mockStaking.getAddress(), true); }); it("should fail to propose ecosystem parameter change as not allowed", async function () { @@ -334,5 +335,30 @@ describe("DAO Ecosystem Paramater Change Value Guards Test", function () { expect((await dao.getProposal(proposalId)).proposalType).to.equal(1); }); + + it("should propose a ecosystem parameter change and execute it", async function () { + const proposer = users[2]; + const calldata = mockStaking.interface.encodeFunctionData("setDelegatorMinStake", ['50000000000000000000']); + + const targets = [await mockStaking.getAddress()]; + const values = [0n]; + const calldatas = [calldata]; + + const { proposalId } = await finalizedProposal( + dao, + mockValidatorSet, + mockStaking, + Vote.Yes, + targets, + values, + calldatas + ); + + await expect(dao.connect(proposer).execute(proposalId)) + .to.emit(dao, "ProposalExecuted") + .withArgs(proposer.address, proposalId); + + expect(await mockStaking.delegatorMinStake()).to.equal('50000000000000000000'); + }); }); }); \ No newline at end of file