Skip to content

Commit

Permalink
feat: add auth to execute() and update Setup & testing
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rekard0
Rekard0 committed Oct 31, 2024
1 parent 8e9b423 commit f365fb5
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 17 deletions.
6 changes: 5 additions & 1 deletion packages/contracts/src/MajorityVotingBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ abstract contract MajorityVotingBase is
/// @notice The ID of the permission required to call the `addAddresses` and `removeAddresses` functions.
bytes32 public constant CREATE_PROPOSAL_PERMISSION_ID = keccak256("CREATE_PROPOSAL_PERMISSION");

/// @notice The ID of the permission required to call the `addAddresses` and `removeAddresses` functions.
bytes32 public constant EXECUTE_PROPOSAL_PERMISSION_ID =
keccak256("EXECUTE_PROPOSAL_PERMISSION");

/// @notice A mapping between proposal IDs and proposal information.
// solhint-disable-next-line named-parameters-mapping
mapping(uint256 => Proposal) internal proposals;
Expand Down Expand Up @@ -362,7 +366,7 @@ abstract contract MajorityVotingBase is
}

/// @inheritdoc IMajorityVoting
function execute(uint256 _proposalId) public virtual {
function execute(uint256 _proposalId) public virtual auth(EXECUTE_PROPOSAL_PERMISSION_ID) {
if (!_canExecute(_proposalId)) {
revert ProposalExecutionForbidden(_proposalId);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/contracts/src/TokenVoting.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,14 @@ contract TokenVoting is IMembership, MajorityVotingBase {
votingPower: votingPower
});

if (_tryEarlyExecution && _canExecute(_proposalId)) {
if (!_tryEarlyExecution) {
return;
}

if (
_canExecute(_proposalId) &&
dao().hasPermission(address(this), _voter, EXECUTE_PROPOSAL_PERMISSION_ID, msg.data)
) {
_execute(_proposalId);
}
}
Expand Down
41 changes: 34 additions & 7 deletions packages/contracts/src/TokenVotingSetup.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
/// @notice The ID of the permission required to call the `upgradeToAndCall` function.
bytes32 internal constant UPGRADE_PLUGIN_PERMISSION_ID = keccak256("UPGRADE_PLUGIN_PERMISSION");

/// @notice A special address encoding permissions that are valid for any address `who` or `where`.
address internal constant ANY_ADDR = address(type(uint160).max);

/// @notice The address of the `TokenVoting` base contract.
// solhint-disable-next-line immutable-vars-naming
TokenVoting private immutable tokenVotingBase;
Expand Down Expand Up @@ -175,7 +178,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
// Prepare permissions
PermissionLib.MultiTargetPermission[]
memory permissions = new PermissionLib.MultiTargetPermission[](
tokenSettings.addr != address(0) ? 5 : 6
tokenSettings.addr != address(0) ? 6 : 7
);

// Set plugin permissions to be granted.
Expand All @@ -200,7 +203,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
permissions[2] = PermissionLib.MultiTargetPermission(
PermissionLib.Operation.GrantWithCondition,
plugin,
address(type(uint160).max), // ANY_ADDR
ANY_ADDR, // ANY_ADDR
preparedSetupData.helpers[0], // VotingPowerCondition
TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
);
Expand All @@ -221,10 +224,18 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
permissionId: SET_METADATA_PERMISSION_ID
});

permissions[5] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Grant,
where: plugin,
who: ANY_ADDR,
condition: PermissionLib.NO_CONDITION,
permissionId: TokenVoting(IMPLEMENTATION).EXECUTE_PROPOSAL_PERMISSION_ID()
});

if (tokenSettings.addr == address(0)) {
bytes32 tokenMintPermission = GovernanceERC20(token).MINT_PERMISSION_ID();

permissions[5] = PermissionLib.MultiTargetPermission({
permissions[6] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Grant,
where: token,
who: _dao,
Expand All @@ -251,7 +262,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
address votingPowerCondition = address(new VotingPowerCondition(_payload.plugin));

PermissionLib.MultiTargetPermission[]
memory permissions = new PermissionLib.MultiTargetPermission[](4);
memory permissions = new PermissionLib.MultiTargetPermission[](5);

permissions[0] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Revoke,
Expand All @@ -264,7 +275,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
permissions[1] = PermissionLib.MultiTargetPermission(
PermissionLib.Operation.GrantWithCondition,
_payload.plugin,
address(type(uint160).max), // ANY_ADDR
ANY_ADDR, // ANY_ADDR
votingPowerCondition,
TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
);
Expand All @@ -285,6 +296,14 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
permissionId: SET_METADATA_PERMISSION_ID
});

permissions[4] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Grant,
where: _payload.plugin,
who: ANY_ADDR,
condition: PermissionLib.NO_CONDITION,
permissionId: TokenVoting(IMPLEMENTATION).EXECUTE_PROPOSAL_PERMISSION_ID()
});

preparedSetupData.permissions = permissions;
preparedSetupData.helpers = new address[](1);
preparedSetupData.helpers[0] = votingPowerCondition;
Expand All @@ -299,7 +318,7 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
SetupPayload calldata _payload
) external view returns (PermissionLib.MultiTargetPermission[] memory permissions) {
// Prepare permissions.
permissions = new PermissionLib.MultiTargetPermission[](5);
permissions = new PermissionLib.MultiTargetPermission[](6);

// Set permissions to be Revoked.
permissions[0] = PermissionLib.MultiTargetPermission({
Expand Down Expand Up @@ -337,10 +356,18 @@ contract TokenVotingSetup is PluginUpgradeableSetup {
permissions[4] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Revoke,
where: _payload.plugin,
who: address(type(uint160).max), // ANY_ADDR
who: ANY_ADDR, // ANY_ADDR
condition: PermissionLib.NO_CONDITION,
permissionId: TokenVoting(IMPLEMENTATION).CREATE_PROPOSAL_PERMISSION_ID()
});

permissions[5] = PermissionLib.MultiTargetPermission({
operation: PermissionLib.Operation.Revoke,
where: _payload.plugin,
who: ANY_ADDR,
condition: PermissionLib.NO_CONDITION,
permissionId: TokenVoting(IMPLEMENTATION).EXECUTE_PROPOSAL_PERMISSION_ID()
});
}

/// @notice Unsatisfiably determines if the token is an IVotes interface.
Expand Down
19 changes: 19 additions & 0 deletions packages/contracts/test/10_unit-testing/11_plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
import {
TOKEN_VOTING_INTERFACE,
UPDATE_VOTING_SETTINGS_PERMISSION_ID,
EXECUTE_PROPOSAL_PERMISSION_ID,
INITIALIZE_SIGNATURE,
INITIALIZE_SIGNATURE_OLD,
Operation,
Expand Down Expand Up @@ -198,6 +199,24 @@ async function globalFixture(): Promise<GlobalFixtureResult> {
deployer
);

// Grant deployer the permission to execute proposals
await dao
.connect(deployer)
.grant(
initializedPlugin.address,
deployer.address,
EXECUTE_PROPOSAL_PERMISSION_ID
);

// Grant grace the permission to execute proposals
await dao
.connect(deployer)
.grant(
initializedPlugin.address,
grace.address,
EXECUTE_PROPOSAL_PERMISSION_ID
);

// Grant deployer the permission to update the voting settings
await dao
.connect(deployer)
Expand Down
58 changes: 50 additions & 8 deletions packages/contracts/test/10_unit-testing/12_plugin-setup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
TargetConfig,
SET_METADATA_PERMISSION_ID,
UPDATE_VOTING_SETTINGS_PERMISSION_ID,
EXECUTE_PROPOSAL_PERMISSION_ID,
} from '../test-utils/token-voting-constants';
import {Operation as Op} from '../test-utils/token-voting-constants';
import {
Expand Down Expand Up @@ -400,7 +401,7 @@ describe('TokenVotingSetup', function () {
anticipatedCondition,
anticipatedWrappedTokenAddress,
]);
expect(permissions.length).to.be.equal(5);
expect(permissions.length).to.be.equal(6);
expect(permissions).to.deep.equal([
[
Operation.Grant,
Expand Down Expand Up @@ -437,6 +438,13 @@ describe('TokenVotingSetup', function () {
AddressZero,
SET_METADATA_PERMISSION_ID,
],
[
Operation.Grant,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
]);
});

Expand Down Expand Up @@ -580,7 +588,7 @@ describe('TokenVotingSetup', function () {
anticipatedCondition,
governanceERC20.address,
]);
expect(permissions.length).to.be.equal(5);
expect(permissions.length).to.be.equal(6);
expect(permissions).to.deep.equal([
[
Operation.Grant,
Expand Down Expand Up @@ -617,6 +625,13 @@ describe('TokenVotingSetup', function () {
AddressZero,
SET_METADATA_PERMISSION_ID,
],
[
Operation.Grant,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
]);
});

Expand Down Expand Up @@ -664,7 +679,7 @@ describe('TokenVotingSetup', function () {
anticipatedCondition,
anticipatedTokenAddress,
]);
expect(permissions.length).to.be.equal(6);
expect(permissions.length).to.be.equal(7);
expect(permissions).to.deep.equal([
[
Operation.Grant,
Expand Down Expand Up @@ -701,6 +716,13 @@ describe('TokenVotingSetup', function () {
AddressZero,
SET_METADATA_PERMISSION_ID,
],
[
Operation.Grant,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
[
Operation.Grant,
anticipatedTokenAddress,
Expand Down Expand Up @@ -858,7 +880,7 @@ describe('TokenVotingSetup', function () {
});

expect(helpers).to.deep.equal([anticipatedCondition]);
expect(permissions.length).to.be.eql(4);
expect(permissions.length).to.be.eql(5);
expect(permissions).to.deep.equal([
[
Operation.Revoke,
Expand All @@ -881,14 +903,20 @@ describe('TokenVotingSetup', function () {
AddressZero,
SET_TARGET_CONFIG_PERMISSION_ID,
],

[
Operation.Grant,
plugin,
dao.address,
AddressZero,
SET_METADATA_PERMISSION_ID,
],
[
Operation.Grant,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
]);
});

Expand Down Expand Up @@ -944,7 +972,7 @@ describe('TokenVotingSetup', function () {
)
);
expect(helpers).to.be.eql([anticipatedCondition]);
expect(permissions.length).to.be.eql(4);
expect(permissions.length).to.be.eql(5);
expect(permissions).to.deep.equal([
[
Operation.Revoke,
Expand Down Expand Up @@ -974,6 +1002,13 @@ describe('TokenVotingSetup', function () {
AddressZero,
SET_METADATA_PERMISSION_ID,
],
[
Operation.Grant,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
]);
});

Expand Down Expand Up @@ -1080,9 +1115,16 @@ describe('TokenVotingSetup', function () {
AddressZero,
CREATE_PROPOSAL_PERMISSION_ID,
],
[
Operation.Revoke,
plugin,
ANY_ADDR,
AddressZero,
EXECUTE_PROPOSAL_PERMISSION_ID,
],
];

expect(permissions1.length).to.be.equal(5);
expect(permissions1.length).to.be.equal(6);
expect(permissions1).to.deep.equal(essentialPermissions);

const permissions2 = await pluginSetup.callStatic.prepareUninstallation(
Expand All @@ -1094,7 +1136,7 @@ describe('TokenVotingSetup', function () {
}
);

expect(permissions2.length).to.be.equal(5);
expect(permissions2.length).to.be.equal(6);
expect(permissions2).to.deep.equal(essentialPermissions);
});
});
Expand Down
4 changes: 4 additions & 0 deletions packages/contracts/test/test-utils/token-voting-constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export const MAJORITY_VOTING_BASE_INTERFACE = new ethers.utils.Interface([
'function createProposal(bytes,tuple(address,uint256,bytes)[],uint256,uint64,uint64,uint8,bool)',
]);

export const EXECUTE_PROPOSAL_PERMISSION_ID = ethers.utils.id(
'EXECUTE_PROPOSAL_PERMISSION'
);

export const UPDATE_VOTING_SETTINGS_PERMISSION_ID = ethers.utils.id(
'UPDATE_VOTING_SETTINGS_PERMISSION'
);
Expand Down

0 comments on commit f365fb5

Please sign in to comment.