From 96bf27bb963dc39906961c319c101b91877c8756 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Thu, 19 Sep 2024 17:44:12 +0400 Subject: [PATCH 1/9] bytes param added --- packages/contracts/src/MajorityVotingBase.sol | 4 +- packages/contracts/src/TokenVoting.sol | 51 +++- packages/contracts/src/TokenVotingSetup.sol | 7 +- .../contracts/src/VotingPowerCondition.sol | 2 +- .../src/mocks/MajorityVotingMock.sol | 19 +- .../test/10_unit-testing/11_plugin.ts | 249 +++++++++++++----- .../test/10_unit-testing/12_plugin-setup.ts | 99 ++++--- .../base/11_majority-voting.ts | 55 +++- .../22_setup-processing.ts | 8 +- .../31_upgradeability.ts | 19 +- .../test/test-utils/token-voting-constants.ts | 11 +- 11 files changed, 375 insertions(+), 149 deletions(-) diff --git a/packages/contracts/src/MajorityVotingBase.sol b/packages/contracts/src/MajorityVotingBase.sol index 80e79890..c570bf63 100644 --- a/packages/contracts/src/MajorityVotingBase.sol +++ b/packages/contracts/src/MajorityVotingBase.sol @@ -377,7 +377,9 @@ abstract contract MajorityVotingBase is } /// @inheritdoc IMajorityVoting - function canExecute(uint256 _proposalId) public view virtual override(IMajorityVoting, IProposal) returns (bool) { + function canExecute( + uint256 _proposalId + ) public view virtual override(IMajorityVoting, IProposal) returns (bool) { return _canExecute(_proposalId); } diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index ec92ffdb..df1f2440 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -11,6 +11,7 @@ import {_applyRatioCeiled} from "@aragon/osx-commons-contracts/src/utils/math/Ra import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {MajorityVotingBase} from "./MajorityVotingBase.sol"; +import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol"; /// @title TokenVoting /// @author Aragon X - 2021-2023 @@ -68,12 +69,12 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @dev WARNING: The contract should only be upgradeable through PSP to ensure that _fromBuild is not incorrectly passed, and that the appropriate permissions for the upgrade are properly configured. /// @param _fromBuild The build version number of the previous implementation contract this upgrade is transitioning from. /// @param _initData The initialization data to be passed to via `upgradeToAndCall` (see [ERC-1967](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Upgrade)). - function initializeFrom( - uint16 _fromBuild, - bytes calldata _initData - ) external reinitializer(2) { - if(_fromBuild < 3) { - (uint256 minApprovals, TargetConfig memory targetConfig) = abi.decode(_initData, (uint256, TargetConfig)); + function initializeFrom(uint16 _fromBuild, bytes calldata _initData) external reinitializer(2) { + if (_fromBuild < 3) { + (uint256 minApprovals, TargetConfig memory targetConfig) = abi.decode( + _initData, + (uint256, TargetConfig) + ); _updateMinApprovals(minApprovals); _setTargetConfig(targetConfig); @@ -178,14 +179,46 @@ contract TokenVoting is IMembership, MajorityVotingBase { ); } + /// @inheritdoc IProposal function createProposal( bytes calldata _metadata, IDAO.Action[] calldata _actions, uint64 _startDate, - uint64 _endDate + uint64 _endDate, + bytes memory _data ) external override returns (uint256 proposalId) { - // Calls public function for permission check. - proposalId = createProposal(_metadata, _actions, 0, _startDate, _endDate, VoteOption.None, false); + // Note that this calls public function for permission check. + if (_data.length == 0) { + // Proposal can still be created with default values. + proposalId = createProposal( + _metadata, + _actions, + 0, + _startDate, + _endDate, + VoteOption.None, + false + ); + } else { + (uint256 allowFailureMap, VoteOption _voteOption, bool tryEarlyExecution) = abi.decode( + _data, + (uint256, VoteOption, bool) + ); + proposalId = createProposal( + _metadata, + _actions, + allowFailureMap, + _startDate, + _endDate, + _voteOption, + tryEarlyExecution + ); + } + } + + /// @inheritdoc IProposal + function createProposalParams() external pure override returns (string memory) { + return "[uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution]"; } /// @inheritdoc IMembership diff --git a/packages/contracts/src/TokenVotingSetup.sol b/packages/contracts/src/TokenVotingSetup.sol index 5feb1c4a..4df253e7 100644 --- a/packages/contracts/src/TokenVotingSetup.sol +++ b/packages/contracts/src/TokenVotingSetup.sol @@ -40,7 +40,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup { /// @dev TODO: Migrate this constant to a common library that can be shared across plugins. bytes32 public constant EXECUTE_PERMISSION_ID = keccak256("EXECUTE_PERMISSION"); - /// @notice The ID of the permission required to call the `setTargetConfig` function. + /// @notice The ID of the permission required to call the `setTargetConfig` function. bytes32 public constant SET_TARGET_CONFIG_PERMISSION_ID = keccak256("SET_TARGET_CONFIG_PERMISSION"); @@ -283,10 +283,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup { preparedSetupData.helpers = new address[](1); preparedSetupData.helpers[0] = votingPowerCondition; - initData = abi.encodeCall( - TokenVoting.initializeFrom, - (_fromBuild, _payload.data) - ); + initData = abi.encodeCall(TokenVoting.initializeFrom, (_fromBuild, _payload.data)); } } diff --git a/packages/contracts/src/VotingPowerCondition.sol b/packages/contracts/src/VotingPowerCondition.sol index 9011477c..7f23c387 100644 --- a/packages/contracts/src/VotingPowerCondition.sol +++ b/packages/contracts/src/VotingPowerCondition.sol @@ -27,7 +27,7 @@ contract VotingPowerCondition is PermissionCondition { (_where, _data, _permissionId); uint256 minProposerVotingPower_ = TOKEN_VOTING.minProposerVotingPower(); - + if (minProposerVotingPower_ != 0) { if ( VOTING_TOKEN.getVotes(_who) < minProposerVotingPower_ && diff --git a/packages/contracts/src/mocks/MajorityVotingMock.sol b/packages/contracts/src/mocks/MajorityVotingMock.sol index 2e78f33c..081cace3 100644 --- a/packages/contracts/src/mocks/MajorityVotingMock.sol +++ b/packages/contracts/src/mocks/MajorityVotingMock.sol @@ -33,14 +33,27 @@ contract MajorityVotingMock is MajorityVotingBase { return 0; } - function createProposal( + function createProposal( bytes calldata _metadata, IDAO.Action[] calldata _actions, uint64 _startDate, - uint64 _endDate + uint64 _endDate, + bytes memory ) external pure override returns (uint256 proposalId) { // Calls public function for permission check. - proposalId = createProposal(_metadata, _actions, 0, _startDate, _endDate, VoteOption.None, false); + proposalId = createProposal( + _metadata, + _actions, + 0, + _startDate, + _endDate, + VoteOption.None, + false + ); + } + + function createProposalParams() external pure override returns (string memory) { + return "[uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution]"; } function totalVotingPower(uint256 /* _blockNumber */) public pure override returns (uint256) { diff --git a/packages/contracts/test/10_unit-testing/11_plugin.ts b/packages/contracts/test/10_unit-testing/11_plugin.ts index afa1be6b..afc9a19a 100644 --- a/packages/contracts/test/10_unit-testing/11_plugin.ts +++ b/packages/contracts/test/10_unit-testing/11_plugin.ts @@ -33,6 +33,7 @@ import { CREATE_PROPOSAL_SIGNATURE, CREATE_PROPOSAL_PERMISSION_ID, ANY_ADDR, + CREATE_PROPOSAL_SIGNATURE_IProposal, } from '../test-utils/token-voting-constants'; import { TokenVoting__factory, @@ -82,7 +83,7 @@ type GlobalFixtureResult = { defaultMinApproval: BigNumber; token: TestGovernanceERC20; dao: DAO; - defaultTargetConfig: TargetConfig + defaultTargetConfig: TargetConfig; dummyActions: DAOStructs.ActionStruct[]; dummyMetadata: string; }; @@ -140,11 +141,17 @@ async function globalFixture(): Promise { const defaultTargetConfig: TargetConfig = { target: dao.address, operation: Operation.call, - } + }; const pluginInitData = pluginImplementation.interface.encodeFunctionData( INITIALIZE_SIGNATURE, - [dao.address, defaultVotingSettings, token.address, defaultTargetConfig, defaultMinApproval] + [ + dao.address, + defaultVotingSettings, + token.address, + defaultTargetConfig, + defaultMinApproval, + ] ); const deploymentTx1 = await proxyFactory.deployUUPSProxy(pluginInitData); const proxyCreatedEvent1 = findEvent( @@ -194,7 +201,12 @@ async function globalFixture(): Promise { }, ]; - await grantCreateProposalPermissions(deployer, dao, initializedPlugin, uninitializedPlugin) + await grantCreateProposalPermissions( + deployer, + dao, + initializedPlugin, + uninitializedPlugin + ); return { deployer, @@ -222,9 +234,9 @@ async function globalFixture(): Promise { } async function grantCreateProposalPermissions( - deployer: SignerWithAddress, - dao: DAO, - initializedPlugin: TokenVoting, + deployer: SignerWithAddress, + dao: DAO, + initializedPlugin: TokenVoting, uninitializedPlugin: TokenVoting ) { const condition = await new VotingPowerCondition__factory(deployer).deploy( @@ -468,6 +480,110 @@ describe('TokenVoting', function () { }); }); + // These tests ensure that overriden `createProposal` function from `IProposal` + // successfully creates a proposal with default values(when `data` is not passed) + // and with custom values when it's passed. + describe('Proposal creation: IProposal Interface Function', async () => { + let voteSettingsWithMinProposerVotingPower: MajorityVotingBase.VotingSettingsStruct; + + before(async () => { + voteSettingsWithMinProposerVotingPower = { + votingMode: VotingMode.EarlyExecution, + supportThreshold: pctToRatio(0), + minParticipation: pctToRatio(0), + minDuration: TIME.HOUR, + minProposerVotingPower: 0, + }; + }); + + it('creates proposal with default values if `data` param is encoded with custom values', async () => { + const { + deployer, + initializedPlugin: plugin, + token, + dummyActions, + dummyMetadata, + } = await loadFixture(globalFixture); + + await plugin.updateVotingSettings(voteSettingsWithMinProposerVotingPower); + + // Make sure the supply is not zero. + await setBalances(token, [ + { + receiver: deployer.address, + amount: 1, + }, + ]); + + await setTotalSupply(token, 5); + + const data = ethers.utils.defaultAbiCoder.encode( + ['uint256', 'uint256', 'bool'], + [1, 2, true] + ); + + await plugin[CREATE_PROPOSAL_SIGNATURE_IProposal]( + dummyMetadata, + dummyActions, + 0, + 0, + data + ); + + const proposalId = await plugin.createProposalId( + dummyActions, + dummyMetadata + ); + const proposal = await plugin.getProposal(proposalId); + expect(proposal.allowFailureMap).to.equal(1); + expect(await plugin.getVoteOption(proposalId, deployer.address)).to.equal( + 2 + ); + expect(proposal.executed).to.be.true; + }); + + it('creates proposal with default values if `data` param is passed as empty', async () => { + const { + deployer, + initializedPlugin: plugin, + token, + dummyActions, + dummyMetadata, + } = await loadFixture(globalFixture); + + await plugin.updateVotingSettings(voteSettingsWithMinProposerVotingPower); + + // Make sure the supply is not zero. + await setBalances(token, [ + { + receiver: deployer.address, + amount: 1, + }, + ]); + + await setTotalSupply(token, 5); + + await plugin[CREATE_PROPOSAL_SIGNATURE_IProposal]( + dummyMetadata, + dummyActions, + 0, + 0, + '0x' + ); + + const proposalId = await plugin.createProposalId( + dummyActions, + dummyMetadata + ); + const proposal = await plugin.getProposal(proposalId); + expect(proposal.allowFailureMap).to.equal(0); + expect(await plugin.getVoteOption(proposalId, deployer.address)).to.equal( + 0 + ); + expect(proposal.executed).to.be.false; + }); + }); + describe('Proposal creation', async () => { let voteSettingsWithMinProposerVotingPower: MajorityVotingBase.VotingSettingsStruct; @@ -478,7 +594,7 @@ describe('TokenVoting', function () { minParticipation: pctToRatio(20), minDuration: TIME.HOUR, minProposerVotingPower: 123, - } + }; }); describe('minProposerVotingPower == 0', async () => { @@ -489,7 +605,7 @@ describe('TokenVoting', function () { token, dummyActions, dummyMetadata, - } = await loadFixture(globalFixture) + } = await loadFixture(globalFixture); await setTotalSupply(token, 1); @@ -506,7 +622,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - + const expectedProposalId = await plugin.createProposalId( dummyActions, dummyMetadata @@ -560,13 +676,13 @@ describe('TokenVoting', function () { false ) ) - .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') - .withArgs( - dao.address, - plugin.address, - alice.address, - CREATE_PROPOSAL_PERMISSION_ID - ); + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + CREATE_PROPOSAL_PERMISSION_ID + ); // Create a proposal as Bob. await expect( @@ -580,7 +696,8 @@ describe('TokenVoting', function () { endDate, VoteOption.None, false - )).not.to.be.reverted; + ) + ).not.to.be.reverted; }); it('reverts if `_msgSender` owns no tokens and has no tokens delegated to her/him in the current block although having them in the last block', async () => { @@ -636,13 +753,13 @@ describe('TokenVoting', function () { false ) ) - .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') - .withArgs( - dao.address, - plugin.address, - alice.address, - CREATE_PROPOSAL_PERMISSION_ID - ); + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + CREATE_PROPOSAL_PERMISSION_ID + ); // Transaction 3: Create the proposal as Bob. const tx3 = await plugin @@ -656,7 +773,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Check the balances before the block is mined. Note that `balanceOf` is a view function, // whose result will be immediately available and does not rely on the block to be mined. @@ -739,13 +856,13 @@ describe('TokenVoting', function () { false ) ) - .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') - .withArgs( - dao.address, - plugin.address, - bob.address, - CREATE_PROPOSAL_PERMISSION_ID - ); + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + bob.address, + CREATE_PROPOSAL_PERMISSION_ID + ); // Check that Alice who has enough balance can create a proposal. await expect( @@ -805,7 +922,9 @@ describe('TokenVoting', function () { await tx.wait(), 'ProposalCreated' ); - expect(event.args.proposalId).to.equal(await plugin.createProposalId(dummyActions, dummyMetadata)); + expect(event.args.proposalId).to.equal( + await plugin.createProposalId(dummyActions, dummyMetadata) + ); }); it('creates a proposal if `_msgSender` owns no tokens but has enough tokens delegated to her/him in the current block', async () => { @@ -897,13 +1016,13 @@ describe('TokenVoting', function () { false ) ) - .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') - .withArgs( - dao.address, - plugin.address, - alice.address, - CREATE_PROPOSAL_PERMISSION_ID - ); + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + CREATE_PROPOSAL_PERMISSION_ID + ); // As Alice delegate all votes to Bob. await token.connect(alice).delegate(bob.address); @@ -922,13 +1041,13 @@ describe('TokenVoting', function () { false ) ) - .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') - .withArgs( - dao.address, - plugin.address, - alice.address, - CREATE_PROPOSAL_PERMISSION_ID - ); + .to.be.revertedWithCustomError(plugin, 'DaoUnauthorized') + .withArgs( + dao.address, + plugin.address, + alice.address, + CREATE_PROPOSAL_PERMISSION_ID + ); }); }); @@ -1095,7 +1214,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId([], dummyMetadata) + const id = await plugin.createProposalId([], dummyMetadata); const expectedStartDate = BigNumber.from(await time.latest()); const expectedEndDate = expectedStartDate.add( @@ -1154,7 +1273,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); const event = findEvent( await tx.wait(), 'ProposalCreated' @@ -1197,7 +1316,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); const event = findEvent( await tx.wait(), 'ProposalCreated' @@ -1234,7 +1353,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Check that the `ProposalCreated` event is emitted and `VoteCast` is not. await expect(tx) @@ -1321,7 +1440,7 @@ describe('TokenVoting', function () { VoteOption.Yes, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Check that the `ProposalCreated` and `VoteCast` events are emitted with the expected data. await expect(tx) @@ -1383,7 +1502,7 @@ describe('TokenVoting', function () { const startDate = (await time.latest()) + TIME.HOUR; const endDate = startDate + TIME.DAY; expect(await time.latest()).to.be.lessThan(startDate); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); await expect( plugin .connect(alice) @@ -1461,7 +1580,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); await expect(plugin.connect(alice).vote(id, VoteOption.Yes, false)) .to.be.revertedWithCustomError(plugin, 'VoteCastForbidden') @@ -1488,7 +1607,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // check the mallory has 0 token expect(await token.balanceOf(mallory.address)).to.equal(0); @@ -1522,7 +1641,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Vote with Alice. await expect(plugin.connect(alice).vote(id, VoteOption.Yes, false)) @@ -1584,7 +1703,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Check that voting is possible but don't vote using `callStatic` await expect( @@ -1716,7 +1835,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Vote as Alice. await plugin.connect(alice).vote(id, VoteOption.Yes, false); @@ -1761,7 +1880,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Vote with enough voters so that the execution criteria are met. // Vote with enough votes so that the execution criteria and the vote outcome cannot change anymore, @@ -1806,7 +1925,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Vote with enough voters so that the execution criteria are met. await voteWithSigners(plugin, id, { @@ -1903,7 +2022,7 @@ describe('TokenVoting', function () { VoteOption.None, false ); - const id = await plugin.createProposalId(dummyActions, dummyMetadata) + const id = await plugin.createProposalId(dummyActions, dummyMetadata); // Try to execute it while the vote is not decided yet. await expect(plugin.execute(id)) @@ -1984,7 +2103,7 @@ describe('TokenVoting', function () { await initializedPlugin .connect(deployer) .updateVotingSettings(newVotingSettings); - + return { deployer, alice, @@ -2248,7 +2367,9 @@ describe('TokenVoting', function () { ); expect(event.args.actor).to.equal(plugin.address); - expect(event.args.callId).to.equal(ethers.utils.hexZeroPad(id.toHexString(), 32)); + expect(event.args.callId).to.equal( + ethers.utils.hexZeroPad(id.toHexString(), 32) + ); expect(event.args.actions.length).to.equal(1); expect(event.args.actions[0].to).to.equal(dummyActions[0].to); expect(event.args.actions[0].value).to.equal(dummyActions[0].value); diff --git a/packages/contracts/test/10_unit-testing/12_plugin-setup.ts b/packages/contracts/test/10_unit-testing/12_plugin-setup.ts index 3b6effe8..37d2cd47 100644 --- a/packages/contracts/test/10_unit-testing/12_plugin-setup.ts +++ b/packages/contracts/test/10_unit-testing/12_plugin-setup.ts @@ -18,7 +18,6 @@ import { UPDATE_VOTING_SETTINGS_PERMISSION_ID, } from '../test-utils/token-voting-constants'; import {Operation as Op} from '../test-utils/token-voting-constants'; - import { TokenVoting__factory, TokenVotingSetup, @@ -103,8 +102,8 @@ async function fixture(): Promise { const defaultTargetConfig: TargetConfig = { target: dao.address, - operation: Op.call - } + operation: Op.call, + }; const defaultVotingSettings: MajorityVotingBase.VotingSettingsStruct = { votingMode: VotingMode.EarlyExecution, @@ -126,7 +125,7 @@ async function fixture(): Promise { Object.values(defaultTokenSettings), Object.values(defaultMintSettings), Object.values(defaultTargetConfig), - defaultMinApproval + defaultMinApproval, ] ); @@ -139,10 +138,10 @@ async function fixture(): Promise { ); const updateMinApproval = pctToRatio(35); - const updateTargetConfig : TargetConfig = { + const updateTargetConfig: TargetConfig = { target: pluginSetup.address, - operation: Op.call - } + operation: Op.call, + }; // Provide update inputs const prepareUpdateBuild3Inputs = ethers.utils.defaultAbiCoder.encode( @@ -225,7 +224,7 @@ describe('TokenVotingSetup', function () { defaultVotingSettings, defaultTokenSettings, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const receivers: string[] = [AddressZero]; @@ -271,7 +270,7 @@ describe('TokenVotingSetup', function () { defaultVotingSettings, defaultMintSettings, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const data = abiCoder.encode( @@ -299,7 +298,7 @@ describe('TokenVotingSetup', function () { defaultVotingSettings, defaultMintSettings, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const data = abiCoder.encode( @@ -329,7 +328,7 @@ describe('TokenVotingSetup', function () { defaultMintSettings, erc20, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const nonce = await ethers.provider.getTransactionCount( @@ -373,7 +372,10 @@ describe('TokenVotingSetup', function () { expect(plugin).to.be.equal(anticipatedPluginAddress); expect(helpers.length).to.be.equal(2); - expect(helpers).to.be.deep.equal([anticipatedWrappedTokenAddress, anticipatedCondition]); + expect(helpers).to.be.deep.equal([ + anticipatedWrappedTokenAddress, + anticipatedCondition, + ]); expect(permissions.length).to.be.equal(4); expect(permissions).to.deep.equal([ [ @@ -436,7 +438,7 @@ describe('TokenVotingSetup', function () { [erc20.address, 'myName', 'mySymb'], Object.values(defaultMintSettings), defaultTargetConfig, - defaultMinApproval + defaultMinApproval, ] ); @@ -465,7 +467,7 @@ describe('TokenVotingSetup', function () { defaultVotingSettings, defaultMintSettings, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const governanceERC20 = await new GovernanceERC20__factory( @@ -506,7 +508,10 @@ describe('TokenVotingSetup', function () { expect(plugin).to.be.equal(anticipatedPluginAddress); expect(helpers.length).to.be.equal(2); - expect(helpers).to.be.deep.equal([governanceERC20.address, anticipatedCondition]); + expect(helpers).to.be.deep.equal([ + governanceERC20.address, + anticipatedCondition, + ]); expect(permissions.length).to.be.equal(4); expect(permissions).to.deep.equal([ [ @@ -536,7 +541,7 @@ describe('TokenVotingSetup', function () { dao.address, AddressZero, SET_TARGET_CONFIG_PERMISSION_ID, - ] + ], ]); }); @@ -573,7 +578,10 @@ describe('TokenVotingSetup', function () { expect(plugin).to.be.equal(anticipatedPluginAddress); expect(helpers.length).to.be.equal(2); - expect(helpers).to.be.deep.equal([anticipatedTokenAddress, anticipatedCondition]); + expect(helpers).to.be.deep.equal([ + anticipatedTokenAddress, + anticipatedCondition, + ]); expect(permissions.length).to.be.equal(5); expect(permissions).to.deep.equal([ [ @@ -623,7 +631,7 @@ describe('TokenVotingSetup', function () { defaultTokenSettings, defaultMintSettings, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); const daoAddress = dao.address; @@ -680,7 +688,7 @@ describe('TokenVotingSetup', function () { expect(await tokenVoting.getTargetConfig()).to.deep.equal([ defaultTargetConfig.target, - defaultTargetConfig.operation + defaultTargetConfig.operation, ]); // check helpers @@ -696,20 +704,28 @@ describe('TokenVotingSetup', function () { describe('prepareUpdate', async () => { it('returns the permissions expected for the update from build 1', async () => { - const {pluginSetup, dao, prepareInstallationInputs, prepareUpdateBuild3Inputs, updateMinApproval, updateTargetConfig } = await loadFixture( - fixture - ); + const { + pluginSetup, + dao, + prepareInstallationInputs, + prepareUpdateBuild3Inputs, + updateMinApproval, + updateTargetConfig, + } = await loadFixture(fixture); const nonce = await ethers.provider.getTransactionCount( pluginSetup.address ); - + const plugin = ethers.utils.getContractAddress({ from: pluginSetup.address, nonce: nonce + 1, }); - await pluginSetup.prepareInstallation(dao.address, prepareInstallationInputs) + await pluginSetup.prepareInstallation( + dao.address, + prepareInstallationInputs + ); // Make a static call to check that the plugin update data being returned is correct. const { @@ -721,9 +737,8 @@ describe('TokenVotingSetup', function () { ethers.Wallet.createRandom().address, ], data: prepareUpdateBuild3Inputs, - plugin + plugin, }); - // Check the return data. expect(initData).to.be.eq( @@ -736,7 +751,7 @@ describe('TokenVotingSetup', function () { const currentNonce = await ethers.provider.getTransactionCount( pluginSetup.address ); - + const anticipatedCondition = ethers.utils.getContractAddress({ from: pluginSetup.address, nonce: currentNonce, @@ -757,33 +772,41 @@ describe('TokenVotingSetup', function () { plugin, ANY_ADDR, anticipatedCondition, - CREATE_PROPOSAL_PERMISSION_ID + CREATE_PROPOSAL_PERMISSION_ID, ], [ Operation.Grant, plugin, dao.address, AddressZero, - SET_TARGET_CONFIG_PERMISSION_ID + SET_TARGET_CONFIG_PERMISSION_ID, ], ]); }); it('returns the permissions expected for the update from build 2', async () => { - const {pluginSetup, dao, prepareInstallationInputs, prepareUpdateBuild3Inputs, updateMinApproval, updateTargetConfig} = await loadFixture( - fixture - ); + const { + pluginSetup, + dao, + prepareInstallationInputs, + prepareUpdateBuild3Inputs, + updateMinApproval, + updateTargetConfig, + } = await loadFixture(fixture); const nonce = await ethers.provider.getTransactionCount( pluginSetup.address ); - + const plugin = ethers.utils.getContractAddress({ from: pluginSetup.address, nonce: nonce + 1, }); - await pluginSetup.prepareInstallation(dao.address, prepareInstallationInputs) + await pluginSetup.prepareInstallation( + dao.address, + prepareInstallationInputs + ); // Make a static call to check that the plugin update data being returned is correct. const { @@ -801,7 +824,7 @@ describe('TokenVotingSetup', function () { const currentNonce = await ethers.provider.getTransactionCount( pluginSetup.address ); - + const anticipatedCondition = ethers.utils.getContractAddress({ from: pluginSetup.address, nonce: currentNonce, @@ -829,14 +852,14 @@ describe('TokenVotingSetup', function () { plugin, ANY_ADDR, anticipatedCondition, - CREATE_PROPOSAL_PERMISSION_ID + CREATE_PROPOSAL_PERMISSION_ID, ], [ Operation.Grant, plugin, dao.address, AddressZero, - SET_TARGET_CONFIG_PERMISSION_ID + SET_TARGET_CONFIG_PERMISSION_ID, ], ]); }); @@ -917,7 +940,7 @@ describe('TokenVotingSetup', function () { plugin, ANY_ADDR, AddressZero, - CREATE_PROPOSAL_PERMISSION_ID + CREATE_PROPOSAL_PERMISSION_ID, ], ]; diff --git a/packages/contracts/test/10_unit-testing/base/11_majority-voting.ts b/packages/contracts/test/10_unit-testing/base/11_majority-voting.ts index ddf4a89d..b5508bee 100644 --- a/packages/contracts/test/10_unit-testing/base/11_majority-voting.ts +++ b/packages/contracts/test/10_unit-testing/base/11_majority-voting.ts @@ -15,6 +15,11 @@ import { MAJORITY_VOTING_BASE_INTERFACE, MAJORITY_VOTING_BASE_OLD_INTERFACE, } from '../../test-utils/majority-voting-constants'; +import { + Operation, + SET_TARGET_CONFIG_PERMISSION_ID, + TargetConfig, +} from '../../test-utils/token-voting-constants'; import {IMajorityVoting_V1_3_0__factory} from '../../test-utils/typechain-versions'; import {VotingMode} from '../../test-utils/voting-helpers'; import {TIME, findEvent} from '@aragon/osx-commons-sdk'; @@ -25,7 +30,6 @@ import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {BigNumber} from 'ethers'; import {ethers} from 'hardhat'; -import { Operation, SET_TARGET_CONFIG_PERMISSION_ID, TargetConfig } from '../../test-utils/token-voting-constants'; describe('MajorityVotingMock', function () { let signers: SignerWithAddress[]; @@ -35,7 +39,7 @@ describe('MajorityVotingMock', function () { let votingSettings: MajorityVotingBase.VotingSettingsStruct; let minApproval: BigNumber; - let targetConfig: TargetConfig + let targetConfig: TargetConfig; before(async () => { signers = await ethers.getSigners(); @@ -56,8 +60,8 @@ describe('MajorityVotingMock', function () { targetConfig = { target: dao.address, - operation: Operation.call - } + operation: Operation.call, + }; const pluginImplementation = await new MajorityVotingMock__factory( signers[0] @@ -84,10 +88,20 @@ describe('MajorityVotingMock', function () { describe('initialize', async () => { it('reverts if trying to re-initialize', async () => { - await votingBase.initializeMock(dao.address, votingSettings, targetConfig, minApproval); + await votingBase.initializeMock( + dao.address, + votingSettings, + targetConfig, + minApproval + ); await expect( - votingBase.initializeMock(dao.address, votingSettings, targetConfig, minApproval) + votingBase.initializeMock( + dao.address, + votingSettings, + targetConfig, + minApproval + ) ).to.be.revertedWith('Initializable: contract is already initialized'); }); }); @@ -152,7 +166,12 @@ describe('MajorityVotingMock', function () { describe('updateVotingSettings', async () => { beforeEach(async () => { - await votingBase.initializeMock(dao.address, votingSettings, targetConfig, minApproval); + await votingBase.initializeMock( + dao.address, + votingSettings, + targetConfig, + minApproval + ); }); it('reverts if the support threshold specified equals 100%', async () => { @@ -213,7 +232,12 @@ describe('MajorityVotingMock', function () { describe('updateMinApprovals', async () => { beforeEach(async () => { - await votingBase.initializeMock(dao.address, votingSettings, targetConfig, minApproval); + await votingBase.initializeMock( + dao.address, + votingSettings, + targetConfig, + minApproval + ); }); it('reverts if the minimum approval specified exceeds 100%', async () => { @@ -233,7 +257,12 @@ describe('MajorityVotingMock', function () { describe('updateTargetConfig', async () => { beforeEach(async () => { - await votingBase.initializeMock(dao.address, votingSettings, targetConfig, minApproval); + await votingBase.initializeMock( + dao.address, + votingSettings, + targetConfig, + minApproval + ); await dao.grant( votingBase.address, @@ -245,14 +274,14 @@ describe('MajorityVotingMock', function () { it('should change the minimum approval successfully', async () => { const newTargetConfig = { target: votingBase.address, - operation: Operation.delegatecall - } + operation: Operation.delegatecall, + }; - await votingBase.setTargetConfig(newTargetConfig) + await votingBase.setTargetConfig(newTargetConfig); expect(await votingBase.getTargetConfig()).to.deep.equal([ newTargetConfig.target, - Operation.delegatecall + Operation.delegatecall, ]); }); }); diff --git a/packages/contracts/test/20_integration-testing/22_setup-processing.ts b/packages/contracts/test/20_integration-testing/22_setup-processing.ts index 54943472..3c9b87b7 100644 --- a/packages/contracts/test/20_integration-testing/22_setup-processing.ts +++ b/packages/contracts/test/20_integration-testing/22_setup-processing.ts @@ -2,6 +2,7 @@ import {METADATA, VERSION} from '../../plugin-settings'; import {GovernanceERC20} from '../../typechain'; import {MajorityVotingBase} from '../../typechain/src/MajorityVotingBase'; import {getProductionNetworkName, findPluginRepo} from '../../utils/helpers'; +import {Operation, TargetConfig} from '../test-utils/token-voting-constants'; import { GovernanceERC20__factory, TokenVotingSetup, @@ -39,7 +40,6 @@ import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import env, {deployments, ethers} from 'hardhat'; -import { Operation, TargetConfig } from '../test-utils/token-voting-constants'; const productionNetworkName = getProductionNetworkName(env); @@ -134,8 +134,8 @@ async function fixture(): Promise { const defaultTargetConfig = { target: dao.address, - operation: Operation.call - } + operation: Operation.call, + }; const defaultTokenSettings = { addr: token.address, @@ -271,7 +271,7 @@ describe(`PluginSetup processing on network '${productionNetworkName}'`, functio defaultVotingSettings, pluginSetupRefLatestBuild, defaultMinApproval, - defaultTargetConfig + defaultTargetConfig, } = await loadFixture(fixture); // Grant deployer all required permissions diff --git a/packages/contracts/test/30_regression-testing/31_upgradeability.ts b/packages/contracts/test/30_regression-testing/31_upgradeability.ts index 44e8cc53..e09bff39 100644 --- a/packages/contracts/test/30_regression-testing/31_upgradeability.ts +++ b/packages/contracts/test/30_regression-testing/31_upgradeability.ts @@ -1,7 +1,12 @@ import {createDaoProxy} from '../20_integration-testing/test-helpers'; import {TestGovernanceERC20} from '../../typechain'; import {MajorityVotingBase} from '../../typechain/src'; -import {INITIALIZE_SIGNATURE, INITIALIZE_SIGNATURE_OLD, Operation, TargetConfig} from '../test-utils/token-voting-constants'; +import { + INITIALIZE_SIGNATURE, + INITIALIZE_SIGNATURE_OLD, + Operation, + TargetConfig, +} from '../test-utils/token-voting-constants'; import { TokenVoting_V1_0_0__factory, TokenVoting_V1_3_0__factory, @@ -38,7 +43,7 @@ describe('Upgrades', () => { defaultInitData.votingSettings, defaultInitData.token.address, defaultInitData.targetConfig, - defaultInitData.minApproval + defaultInitData.minApproval, ], INITIALIZE_SIGNATURE, currentContractFactory, @@ -82,7 +87,7 @@ describe('Upgrades', () => { expect(toProtocolVersion).to.deep.equal([1, 4, 0]); // TODO Check this automatically }); - /// TODO: why is this saying from 1.3.0 ? + /// TODO: why is this saying from 1.3.0 ? it('from v1.3.0', async () => { const {deployer, alice, dao, defaultInitData} = await loadFixture(fixture); const currentContractFactory = new TokenVoting__factory(deployer); @@ -129,7 +134,7 @@ type FixtureResult = { votingSettings: MajorityVotingBase.VotingSettingsStruct; token: TestGovernanceERC20; minApproval: BigNumber; - targetConfig: TargetConfig + targetConfig: TargetConfig; }; }; @@ -157,7 +162,7 @@ async function fixture(): Promise { minDuration: TIME.HOUR, minProposerVotingPower: 0, }; - + // Create an initialized plugin clone const defaultInitData = { votingSettings, @@ -165,8 +170,8 @@ async function fixture(): Promise { minApproval: pctToRatio(10), targetConfig: { target: dao.address, - operation: Operation.call - } + operation: Operation.call, + }, }; return { diff --git a/packages/contracts/test/test-utils/token-voting-constants.ts b/packages/contracts/test/test-utils/token-voting-constants.ts index 3f30d6a0..e77b2fd4 100644 --- a/packages/contracts/test/test-utils/token-voting-constants.ts +++ b/packages/contracts/test/test-utils/token-voting-constants.ts @@ -35,11 +35,14 @@ export const VOTING_EVENTS = { export const ANY_ADDR = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF'; - export const INITIALIZE_SIGNATURE = 'initialize(address,(uint8,uint32,uint32,uint64,uint256),address,(address,uint8),uint256)'; - -export const CREATE_PROPOSAL_SIGNATURE = 'createProposal(bytes,(address,uint256,bytes)[],uint256,uint64,uint64,uint8,bool)' + +export const CREATE_PROPOSAL_SIGNATURE = + 'createProposal(bytes,(address,uint256,bytes)[],uint256,uint64,uint64,uint8,bool)'; + +export const CREATE_PROPOSAL_SIGNATURE_IProposal = + 'createProposal(bytes,(address,uint256,bytes)[],uint64,uint64,bytes)'; export enum Operation { call, @@ -49,4 +52,4 @@ export enum Operation { export type TargetConfig = { target: string; operation: number; -}; \ No newline at end of file +}; From cd5543e26c0ead819e5ea20e5ce1744af3031431 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Thu, 19 Sep 2024 18:03:58 +0400 Subject: [PATCH 2/9] rename param function --- packages/contracts/src/TokenVoting.sol | 2 +- packages/contracts/src/mocks/MajorityVotingMock.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index df1f2440..6d0315a1 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -217,7 +217,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { } /// @inheritdoc IProposal - function createProposalParams() external pure override returns (string memory) { + function createProposalParamsABI() external pure override returns (string memory) { return "[uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution]"; } diff --git a/packages/contracts/src/mocks/MajorityVotingMock.sol b/packages/contracts/src/mocks/MajorityVotingMock.sol index 081cace3..77b2c3ce 100644 --- a/packages/contracts/src/mocks/MajorityVotingMock.sol +++ b/packages/contracts/src/mocks/MajorityVotingMock.sol @@ -52,7 +52,7 @@ contract MajorityVotingMock is MajorityVotingBase { ); } - function createProposalParams() external pure override returns (string memory) { + function createProposalParamsABI() external pure override returns (string memory) { return "[uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution]"; } From 42073e3f8d36e5655f160ce7bf0660d5f5aae8aa Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Wed, 25 Sep 2024 10:51:11 +0400 Subject: [PATCH 3/9] Update packages/contracts/src/TokenVoting.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jør∂¡ <4456749+brickpop@users.noreply.github.com> --- packages/contracts/src/TokenVoting.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index 6d0315a1..196ff4c2 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -218,7 +218,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @inheritdoc IProposal function createProposalParamsABI() external pure override returns (string memory) { - return "[uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution]"; + return "(uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution)"; } /// @inheritdoc IMembership From 2051e2798394447d92d16186a04218175d3bf205 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Wed, 25 Sep 2024 10:53:52 +0400 Subject: [PATCH 4/9] Update packages/contracts/src/TokenVoting.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jør∂¡ <4456749+brickpop@users.noreply.github.com> --- packages/contracts/src/TokenVoting.sol | 38 +++++++++++--------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index 196ff4c2..bf8a4ed1 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -188,34 +188,28 @@ contract TokenVoting is IMembership, MajorityVotingBase { bytes memory _data ) external override returns (uint256 proposalId) { // Note that this calls public function for permission check. - if (_data.length == 0) { - // Proposal can still be created with default values. - proposalId = createProposal( - _metadata, - _actions, - 0, - _startDate, - _endDate, - VoteOption.None, - false - ); - } else { - (uint256 allowFailureMap, VoteOption _voteOption, bool tryEarlyExecution) = abi.decode( + uint256 allowFailureMap; + VoteOption _voteOption = VoteOption.None; + bool tryEarlyExecution; + + if (_data.length != 0) { + (allowFailureMap, _voteOption, tryEarlyExecution) = abi.decode( _data, (uint256, VoteOption, bool) ); - proposalId = createProposal( - _metadata, - _actions, - allowFailureMap, - _startDate, - _endDate, - _voteOption, - tryEarlyExecution - ); } } + proposalId = createProposal( + _metadata, + _actions, + allowFailureMap, + _startDate, + _endDate, + _voteOption, + tryEarlyExecution + ); + /// @inheritdoc IProposal function createProposalParamsABI() external pure override returns (string memory) { return "(uint256 allowFailureMap, uint8 voteOption, bool tryEarlyExecution)"; From 70f28e6471fd0d9406aef06c4c376bcc6a0a53e0 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Wed, 25 Sep 2024 10:58:36 +0400 Subject: [PATCH 5/9] fix typo --- packages/contracts/src/TokenVoting.sol | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index bf8a4ed1..0a879beb 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -198,17 +198,17 @@ contract TokenVoting is IMembership, MajorityVotingBase { (uint256, VoteOption, bool) ); } - } - proposalId = createProposal( - _metadata, - _actions, - allowFailureMap, - _startDate, - _endDate, - _voteOption, - tryEarlyExecution - ); + proposalId = createProposal( + _metadata, + _actions, + allowFailureMap, + _startDate, + _endDate, + _voteOption, + tryEarlyExecution + ); + } /// @inheritdoc IProposal function createProposalParamsABI() external pure override returns (string memory) { From ccfe8ec0c8db05f90f25d6bfdd1419b734434eea Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Thu, 26 Sep 2024 14:28:50 +0400 Subject: [PATCH 6/9] add delegate call --- .../src/mocks/CustomExecutorMock.sol | 24 ++++++ .../test/10_unit-testing/11_plugin.ts | 83 ++++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 packages/contracts/src/mocks/CustomExecutorMock.sol diff --git a/packages/contracts/src/mocks/CustomExecutorMock.sol b/packages/contracts/src/mocks/CustomExecutorMock.sol new file mode 100644 index 00000000..31ce3025 --- /dev/null +++ b/packages/contracts/src/mocks/CustomExecutorMock.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.8; + +import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; + +/// @dev DO NOT USE IN PRODUCTION! +contract CustomExecutorMock { + error FailedCustom(); + + event ExecutedCustom(); + + function execute( + bytes32 callId, + IDAO.Action[] memory _actions, + uint256 allowFailureMap + ) external returns (bytes[] memory execResults, uint256 failureMap) { + if (callId == bytes32(0)) { + revert FailedCustom(); + } else { + emit ExecutedCustom(); + } + } +} diff --git a/packages/contracts/test/10_unit-testing/11_plugin.ts b/packages/contracts/test/10_unit-testing/11_plugin.ts index afc9a19a..20030465 100644 --- a/packages/contracts/test/10_unit-testing/11_plugin.ts +++ b/packages/contracts/test/10_unit-testing/11_plugin.ts @@ -10,6 +10,7 @@ import { IProtocolVersion__factory, ProxyFactory__factory, VotingPowerCondition__factory, + CustomExecutorMock__factory, } from '../../typechain'; import {ProxyCreatedEvent} from '../../typechain/@aragon/osx-commons-contracts/src/utils/deployment/ProxyFactory'; import {MajorityVotingBase} from '../../typechain/src/MajorityVotingBase'; @@ -34,6 +35,7 @@ import { CREATE_PROPOSAL_PERMISSION_ID, ANY_ADDR, CREATE_PROPOSAL_SIGNATURE_IProposal, + SET_TARGET_CONFIG_PERMISSION_ID, } from '../test-utils/token-voting-constants'; import { TokenVoting__factory, @@ -57,7 +59,12 @@ import { RATIO_BASE, DAO_PERMISSIONS, } from '@aragon/osx-commons-sdk'; -import {DAO, DAOStructs, DAO__factory} from '@aragon/osx-ethers'; +import { + DAO, + DAOStructs, + DAO__factory, + MajorityVotingBase__factory, +} from '@aragon/osx-ethers'; import {loadFixture, time} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; @@ -2310,6 +2317,80 @@ describe('TokenVoting', function () { expect(await plugin.canExecute(id)).to.equal(false); }); + it('executes target with delegate call', async () => { + let { + alice, + bob, + carol, + dave, + eve, + frank, + grace, + harold, + ivan, + dummyMetadata, + dummyActions, + deployer, + dao, + initializedPlugin: plugin, + } = await loadFixture(localFixture); + + const executorFactory = new CustomExecutorMock__factory(deployer); + const executor = await executorFactory.deploy(); + + const abiA = CustomExecutorMock__factory.abi; + const abiB = TokenVoting__factory.abi; + + // @ts-ignore + const mergedABI = abiA.concat(abiB); + + await dao.grant( + plugin.address, + deployer.address, + SET_TARGET_CONFIG_PERMISSION_ID + ); + + await plugin.connect(deployer).setTargetConfig({ + target: executor.address, + operation: Operation.delegatecall, + }); + + // @ts-ignore + const pluginMerged = (await ethers.getContractAt( + mergedABI, + plugin.address + )) as TokenVoting; + + const endDate = (await time.latest()) + TIME.DAY; + await plugin[CREATE_PROPOSAL_SIGNATURE]( + dummyMetadata, + dummyActions, + 0, + 0, + endDate, + VoteOption.None, + false + ); + const id = await plugin.createProposalId(dummyActions, dummyMetadata); + + // Vote with enough people so that execution criteria are met. + await voteWithSigners(plugin, id, { + yes: [alice, bob, carol, dave, eve], // 50 yes + no: [frank, grace, harold], // 30 votes + abstain: [ivan], // 10 votes + }); + + // Advance after the end date. + await time.increaseTo(endDate); + + // Check that the vote is executable because support > 50%, participation > 20%, and the voting period is over. + expect(await plugin.canExecute(id)).to.equal(true); + + await expect(plugin.execute(id)) + .to.emit(pluginMerged, 'ExecutedCustom') + .to.emit(pluginMerged, 'ProposalExecuted'); + }); + it('executes the vote immediately when the vote is decided early and the tryEarlyExecution options is selected', async () => { const { alice, From f3cb27c86f2ea885a66b9ca337ac9ba90382c155 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Thu, 26 Sep 2024 14:40:29 +0400 Subject: [PATCH 7/9] silence warnings --- packages/contracts/src/mocks/CustomExecutorMock.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/mocks/CustomExecutorMock.sol b/packages/contracts/src/mocks/CustomExecutorMock.sol index 31ce3025..1c53739f 100644 --- a/packages/contracts/src/mocks/CustomExecutorMock.sol +++ b/packages/contracts/src/mocks/CustomExecutorMock.sol @@ -12,9 +12,11 @@ contract CustomExecutorMock { function execute( bytes32 callId, - IDAO.Action[] memory _actions, - uint256 allowFailureMap + IDAO.Action[] memory, + uint256 ) external returns (bytes[] memory execResults, uint256 failureMap) { + (execResults, failureMap); + if (callId == bytes32(0)) { revert FailedCustom(); } else { From 386d417c15ae1b8a59e3f18a9b215e94d2ed208a Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Thu, 26 Sep 2024 16:18:00 +0400 Subject: [PATCH 8/9] add executor interface --- packages/contracts/src/MajorityVotingBase.sol | 7 ++++--- packages/contracts/src/TokenVoting.sol | 10 ++++++---- packages/contracts/src/mocks/CustomExecutorMock.sol | 4 ++-- packages/contracts/src/mocks/DAOMock.sol | 5 +++-- packages/contracts/src/mocks/MajorityVotingMock.sol | 8 +++++--- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/contracts/src/MajorityVotingBase.sol b/packages/contracts/src/MajorityVotingBase.sol index c570bf63..a8b18319 100644 --- a/packages/contracts/src/MajorityVotingBase.sol +++ b/packages/contracts/src/MajorityVotingBase.sol @@ -13,6 +13,7 @@ import {RATIO_BASE, RatioOutOfBounds} from "@aragon/osx-commons-contracts/src/ut import {PluginUUPSUpgradeable} from "@aragon/osx-commons-contracts/src/plugin/PluginUUPSUpgradeable.sol"; import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol"; +import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; import {IMajorityVoting} from "./IMajorityVoting.sol"; @@ -174,7 +175,7 @@ abstract contract MajorityVotingBase is ProposalParameters parameters; Tally tally; mapping(address => IMajorityVoting.VoteOption) voters; - IDAO.Action[] actions; + IExecutor.Action[] actions; uint256 allowFailureMap; uint256 minApprovalPower; TargetConfig targetConfig; // added in v1.3 @@ -486,7 +487,7 @@ abstract contract MajorityVotingBase is bool executed, ProposalParameters memory parameters, Tally memory tally, - IDAO.Action[] memory actions, + IExecutor.Action[] memory actions, uint256 allowFailureMap ) { @@ -533,7 +534,7 @@ abstract contract MajorityVotingBase is /// @return proposalId The ID of the proposal. function createProposal( bytes calldata _metadata, - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, uint256 _allowFailureMap, uint64 _startDate, uint64 _endDate, diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index 0a879beb..25750774 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -10,8 +10,10 @@ import {IMembership} from "@aragon/osx-commons-contracts/src/plugin/extensions/m import {_applyRatioCeiled} from "@aragon/osx-commons-contracts/src/utils/math/Ratio.sol"; import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; -import {MajorityVotingBase} from "./MajorityVotingBase.sol"; import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol"; +import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; + +import {MajorityVotingBase} from "./MajorityVotingBase.sol"; /// @title TokenVoting /// @author Aragon X - 2021-2023 @@ -107,7 +109,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @inheritdoc MajorityVotingBase function createProposal( bytes calldata _metadata, - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, uint256 _allowFailureMap, uint64 _startDate, uint64 _endDate, @@ -182,7 +184,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @inheritdoc IProposal function createProposal( bytes calldata _metadata, - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, uint64 _startDate, uint64 _endDate, bytes memory _data @@ -236,7 +238,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @param _metadata The metadata of the proposal. /// @return proposalId The ID of the proposal. function createProposalId( - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, bytes memory _metadata ) public pure override returns (uint256) { return uint256(keccak256(abi.encode(_actions, _metadata))); diff --git a/packages/contracts/src/mocks/CustomExecutorMock.sol b/packages/contracts/src/mocks/CustomExecutorMock.sol index 1c53739f..1cb6ba04 100644 --- a/packages/contracts/src/mocks/CustomExecutorMock.sol +++ b/packages/contracts/src/mocks/CustomExecutorMock.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.8; -import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; +import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; /// @dev DO NOT USE IN PRODUCTION! contract CustomExecutorMock { @@ -12,7 +12,7 @@ contract CustomExecutorMock { function execute( bytes32 callId, - IDAO.Action[] memory, + IExecutor.Action[] memory, uint256 ) external returns (bytes[] memory execResults, uint256 failureMap) { (execResults, failureMap); diff --git a/packages/contracts/src/mocks/DAOMock.sol b/packages/contracts/src/mocks/DAOMock.sol index 008fcecf..20fb6c41 100644 --- a/packages/contracts/src/mocks/DAOMock.sol +++ b/packages/contracts/src/mocks/DAOMock.sol @@ -5,8 +5,9 @@ pragma solidity ^0.8.8; import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {IPermissionCondition} from "@aragon/osx-commons-contracts/src/permission/condition/IPermissionCondition.sol"; import {PermissionLib} from "@aragon/osx-commons-contracts/src/permission/PermissionLib.sol"; +import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; -contract DAOMock is IDAO { +contract DAOMock is IDAO, IExecutor { address internal constant NO_CONDITION = address(0); event Granted( @@ -112,7 +113,7 @@ contract DAOMock is IDAO { function execute( bytes32 callId, - Action[] memory _actions, + IExecutor.Action[] memory _actions, uint256 allowFailureMap ) external override returns (bytes[] memory execResults, uint256 failureMap) { emit Executed(msg.sender, callId, _actions, allowFailureMap, failureMap, execResults); diff --git a/packages/contracts/src/mocks/MajorityVotingMock.sol b/packages/contracts/src/mocks/MajorityVotingMock.sol index 77b2c3ce..707e6d55 100644 --- a/packages/contracts/src/mocks/MajorityVotingMock.sol +++ b/packages/contracts/src/mocks/MajorityVotingMock.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.8; +import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; + import {MajorityVotingBase, IDAO} from "../MajorityVotingBase.sol"; contract MajorityVotingMock is MajorityVotingBase { @@ -15,7 +17,7 @@ contract MajorityVotingMock is MajorityVotingBase { } function createProposalId( - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, bytes memory _metadata ) public pure override returns (uint256) { return uint256(keccak256(abi.encode(_actions, _metadata))); @@ -23,7 +25,7 @@ contract MajorityVotingMock is MajorityVotingBase { function createProposal( bytes calldata /* _metadata */, - IDAO.Action[] calldata /* _actions */, + IExecutor.Action[] calldata /* _actions */, uint256 /* _allowFailureMap */, uint64 /* _startDate */, uint64 /* _endDate */, @@ -35,7 +37,7 @@ contract MajorityVotingMock is MajorityVotingBase { function createProposal( bytes calldata _metadata, - IDAO.Action[] calldata _actions, + IExecutor.Action[] calldata _actions, uint64 _startDate, uint64 _endDate, bytes memory From 329e76b9ee8ed3653072ca9392039ae02351fa07 Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Fri, 27 Sep 2024 08:33:03 +0400 Subject: [PATCH 9/9] action as free level --- packages/contracts/src/MajorityVotingBase.sol | 8 ++++---- packages/contracts/src/TokenVoting.sol | 8 ++++---- packages/contracts/src/mocks/CustomExecutorMock.sol | 4 ++-- packages/contracts/src/mocks/DAOMock.sol | 4 ++-- packages/contracts/src/mocks/MajorityVotingMock.sol | 8 ++++---- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/contracts/src/MajorityVotingBase.sol b/packages/contracts/src/MajorityVotingBase.sol index a8b18319..c551e145 100644 --- a/packages/contracts/src/MajorityVotingBase.sol +++ b/packages/contracts/src/MajorityVotingBase.sol @@ -13,7 +13,7 @@ import {RATIO_BASE, RatioOutOfBounds} from "@aragon/osx-commons-contracts/src/ut import {PluginUUPSUpgradeable} from "@aragon/osx-commons-contracts/src/plugin/PluginUUPSUpgradeable.sol"; import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol"; -import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; +import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; import {IMajorityVoting} from "./IMajorityVoting.sol"; @@ -175,7 +175,7 @@ abstract contract MajorityVotingBase is ProposalParameters parameters; Tally tally; mapping(address => IMajorityVoting.VoteOption) voters; - IExecutor.Action[] actions; + Action[] actions; uint256 allowFailureMap; uint256 minApprovalPower; TargetConfig targetConfig; // added in v1.3 @@ -487,7 +487,7 @@ abstract contract MajorityVotingBase is bool executed, ProposalParameters memory parameters, Tally memory tally, - IExecutor.Action[] memory actions, + Action[] memory actions, uint256 allowFailureMap ) { @@ -534,7 +534,7 @@ abstract contract MajorityVotingBase is /// @return proposalId The ID of the proposal. function createProposal( bytes calldata _metadata, - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, uint256 _allowFailureMap, uint64 _startDate, uint64 _endDate, diff --git a/packages/contracts/src/TokenVoting.sol b/packages/contracts/src/TokenVoting.sol index 25750774..f309d5f6 100644 --- a/packages/contracts/src/TokenVoting.sol +++ b/packages/contracts/src/TokenVoting.sol @@ -11,7 +11,7 @@ import {_applyRatioCeiled} from "@aragon/osx-commons-contracts/src/utils/math/Ra import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol"; -import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; +import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; import {MajorityVotingBase} from "./MajorityVotingBase.sol"; @@ -109,7 +109,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @inheritdoc MajorityVotingBase function createProposal( bytes calldata _metadata, - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, uint256 _allowFailureMap, uint64 _startDate, uint64 _endDate, @@ -184,7 +184,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @inheritdoc IProposal function createProposal( bytes calldata _metadata, - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, uint64 _startDate, uint64 _endDate, bytes memory _data @@ -238,7 +238,7 @@ contract TokenVoting is IMembership, MajorityVotingBase { /// @param _metadata The metadata of the proposal. /// @return proposalId The ID of the proposal. function createProposalId( - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, bytes memory _metadata ) public pure override returns (uint256) { return uint256(keccak256(abi.encode(_actions, _metadata))); diff --git a/packages/contracts/src/mocks/CustomExecutorMock.sol b/packages/contracts/src/mocks/CustomExecutorMock.sol index 1cb6ba04..e59fa539 100644 --- a/packages/contracts/src/mocks/CustomExecutorMock.sol +++ b/packages/contracts/src/mocks/CustomExecutorMock.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.8; -import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; +import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; /// @dev DO NOT USE IN PRODUCTION! contract CustomExecutorMock { @@ -12,7 +12,7 @@ contract CustomExecutorMock { function execute( bytes32 callId, - IExecutor.Action[] memory, + Action[] memory, uint256 ) external returns (bytes[] memory execResults, uint256 failureMap) { (execResults, failureMap); diff --git a/packages/contracts/src/mocks/DAOMock.sol b/packages/contracts/src/mocks/DAOMock.sol index 20fb6c41..8a0eedb7 100644 --- a/packages/contracts/src/mocks/DAOMock.sol +++ b/packages/contracts/src/mocks/DAOMock.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.8; import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol"; import {IPermissionCondition} from "@aragon/osx-commons-contracts/src/permission/condition/IPermissionCondition.sol"; import {PermissionLib} from "@aragon/osx-commons-contracts/src/permission/PermissionLib.sol"; -import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; +import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; contract DAOMock is IDAO, IExecutor { address internal constant NO_CONDITION = address(0); @@ -113,7 +113,7 @@ contract DAOMock is IDAO, IExecutor { function execute( bytes32 callId, - IExecutor.Action[] memory _actions, + Action[] memory _actions, uint256 allowFailureMap ) external override returns (bytes[] memory execResults, uint256 failureMap) { emit Executed(msg.sender, callId, _actions, allowFailureMap, failureMap, execResults); diff --git a/packages/contracts/src/mocks/MajorityVotingMock.sol b/packages/contracts/src/mocks/MajorityVotingMock.sol index 707e6d55..fc279576 100644 --- a/packages/contracts/src/mocks/MajorityVotingMock.sol +++ b/packages/contracts/src/mocks/MajorityVotingMock.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.8; -import {IExecutor} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; +import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol"; import {MajorityVotingBase, IDAO} from "../MajorityVotingBase.sol"; @@ -17,7 +17,7 @@ contract MajorityVotingMock is MajorityVotingBase { } function createProposalId( - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, bytes memory _metadata ) public pure override returns (uint256) { return uint256(keccak256(abi.encode(_actions, _metadata))); @@ -25,7 +25,7 @@ contract MajorityVotingMock is MajorityVotingBase { function createProposal( bytes calldata /* _metadata */, - IExecutor.Action[] calldata /* _actions */, + Action[] calldata /* _actions */, uint256 /* _allowFailureMap */, uint64 /* _startDate */, uint64 /* _endDate */, @@ -37,7 +37,7 @@ contract MajorityVotingMock is MajorityVotingBase { function createProposal( bytes calldata _metadata, - IExecutor.Action[] calldata _actions, + Action[] calldata _actions, uint64 _startDate, uint64 _endDate, bytes memory