Skip to content

Commit

Permalink
Adapting tests WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
brickpop committed Jan 29, 2024
1 parent cf83166 commit b0ec215
Show file tree
Hide file tree
Showing 12 changed files with 135 additions and 555 deletions.
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -609,3 +609,26 @@ When a zero address is passed, only a passed proposal can make the DAO call `PSP
Every new version needs to be published to the plugin's repository.

[Learn more about plugin upgrades](https://devs.aragon.org/docs/osx/how-to-guides/plugin-development/upgradeable-plugin/updating-versions).

## Dependencies forked from Aragon

The plugins from this repo are built on top of many contract primitives from Aragon. In some cases, certain parameters are not required or data types need to differ. For this reason, the `packages/contracts/src/governance/base` folder contains 5 forks of existing Aragon primitives.

- `Addresslist.sol`
- Functions `addMembers` and `removeMembers` accepted an `address[] calldata` parameter
- However, the plugin needed to pass an `address[] memory`
- `IEditors.sol` and `IMembers.sol`
- Originally from `IMembership.sol`
- Geo defines a concept of members with conflicted with how the address list interprets its "members" (which are editors)
- Using separate, explicit interfaces to clarify the difference between members and editors
- `IMultisig.sol`
- The `accept()` function required 2 parameters, of which the second always has to be `true`.
- Changing the signature of `accept()` to only use relevant parameters
- `MajorityVotingBase.sol`
- `createProposal()` originally had two parameters that didn't apply to the current specs.
- The forked version Omits these 2 parameters (start date and end date) and instead:
- Starts immediately
- Ends after the predefined duration
- `minDuration()` was confusing given that the setting is used as the final duration, so `duration()` is used instead

The rest of dependencies are imported directly from Aragon or from OpenZeppelin.
48 changes: 8 additions & 40 deletions packages/contracts/src/conditions/MemberAccessExecuteCondition.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
import {PermissionCondition} from "@aragon/osx/core/permission/PermissionCondition.sol";
import {PermissionManager} from "@aragon/osx/core/permission/PermissionManager.sol";
import {MEMBER_PERMISSION_ID} from "../constants.sol";
import {MainVotingPlugin} from "../governance/MainVotingPlugin.sol";

/// @notice The condition associated with `TestSharedPlugin`
contract MemberAccessExecuteCondition is PermissionCondition {
Expand All @@ -18,7 +19,7 @@ contract MemberAccessExecuteCondition is PermissionCondition {
targetContract = _targetContract;
}

/// @notice Checks whether the current action wants to grant membership on the predefined address
/// @notice Checks whether the current action attempts to add or remove members
function isGranted(
address _where,
address _who,
Expand All @@ -27,35 +28,15 @@ contract MemberAccessExecuteCondition is PermissionCondition {
) external view returns (bool) {
(_where, _who, _permissionId);

// Is execute()?
if (getSelector(_data) != IDAO.execute.selector) {
if (
getSelector(_data) != MainVotingPlugin.addMember.selector &&
getSelector(_data) != MainVotingPlugin.removeMember.selector
) {
return false;
} else if (_where != targetContract) {
return false;
}

(, IDAO.Action[] memory _actions, ) = abi.decode(
_data[4:],
(bytes32, IDAO.Action[], uint256)
);

// Check actions
if (_actions.length != 1) return false;

// Decode the call being requested (both have the same parameters)
(
bytes4 _requestedSelector,
address _requestedWhere,
,
bytes32 _requestedPermission
) = decodeGrantRevokeCalldata(_actions[0].data);

if (
_requestedSelector != PermissionManager.grant.selector &&
_requestedSelector != PermissionManager.revoke.selector
) return false;

if (_requestedWhere != targetContract) return false;
else if (_requestedPermission != MEMBER_PERMISSION_ID) return false;

return true;
}

Expand All @@ -66,17 +47,4 @@ contract MemberAccessExecuteCondition is PermissionCondition {
selector := mload(add(_data, 0x20)) // 32
}
}

function decodeGrantRevokeCalldata(
bytes memory _data
) public pure returns (bytes4 sig, address where, address who, bytes32 permissionId) {
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
sig := mload(add(_data, 0x20)) // 32
where := mload(add(_data, 0x24)) // 32 + 4
who := mload(add(_data, 0x44)) // 32 + 4 + 32
permissionId := mload(add(_data, 0x64)) // 32 + 4 + 32 + 32
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
},
{
"internalType": "uint64",
"name": "minDuration",
"name": "duration",
"type": "uint64"
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ import {ethers} from 'hardhat';
const release = 1;
const hardhatForkNetwork = process.env.NETWORK_NAME ?? 'mainnet';
const pluginSettings: MajorityVotingBase.VotingSettingsStruct = {
minDuration: 60 * 60 * 24,
duration: 60 * 60 * 24,
minParticipation: 1,
supportThreshold: 1,
minProposerVotingPower: 0,
votingMode: 0,
};
const minMemberAccessProposalDuration = 60 * 60 * 24;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ import {ethers, network} from 'hardhat';
const release = 1;
const hardhatForkNetwork = process.env.NETWORK_NAME ?? 'mainnet';
const pluginSettings: MajorityVotingBase.VotingSettingsStruct = {
minDuration: 60 * 60 * 24,
duration: 60 * 60 * 24,
minParticipation: 1,
supportThreshold: 1,
minProposerVotingPower: 0,
votingMode: 0,
};
const minMemberAccessProposalDuration = 60 * 60 * 24;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ import {ethers, network} from 'hardhat';
const release = 1;
const hardhatForkNetwork = process.env.NETWORK_NAME ?? 'mainnet';
const pluginSettings: MajorityVotingBase.VotingSettingsStruct = {
minDuration: 60 * 60 * 24,
duration: 60 * 60 * 24,
minParticipation: 1,
supportThreshold: 1,
minProposerVotingPower: 0,
votingMode: 0,
};
const minMemberAccessProposalDuration = 60 * 60 * 24;
Expand Down Expand Up @@ -269,8 +268,8 @@ describe('Plugin upgrader', () => {
to: mainVotingPlugin.address,
value: 0,
data: MainVotingPlugin__factory.createInterface().encodeFunctionData(
'addAddresses',
[[pluginUpgrader.address]]
'addEditor',
[pluginUpgrader.address]
),
},
],
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts/test/unit-testing/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ export type VotingSettings = {
votingMode: number;
supportThreshold: BigNumber;
minParticipation: BigNumber;
minDuration: number;
duration: number;
minProposerVotingPower: number;
};

export const defaultMainVotingSettings: VotingSettings = {
minDuration: 60 * 60, // 1 second
duration: 60 * 60, // 1 second
minParticipation: pctToRatio(30), // 30%
supportThreshold: pctToRatio(50), // 50% + 1
minProposerVotingPower: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Governance Plugins Setup', function () {
votingMode: VotingMode.EarlyExecution,
supportThreshold: pctToRatio(25),
minParticipation: pctToRatio(50),
minDuration: 60 * 60 * 24 * 5,
duration: 60 * 60 * 24 * 5,
minProposerVotingPower: 0,
},
[alice.address],
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('Governance Plugins Setup', function () {
votingMode: VotingMode.EarlyExecution,
supportThreshold: pctToRatio(25),
minParticipation: pctToRatio(50),
minDuration: 60 * 60 * 24 * 5,
duration: 60 * 60 * 24 * 5,
minProposerVotingPower: 0,
},
[alice.address],
Expand Down
Loading

0 comments on commit b0ec215

Please sign in to comment.