Skip to content

Commit

Permalink
Updating the tests WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
@brickpop
brickpop committed Nov 30, 2023
1 parent 805bf2f commit c7cff26
Show file tree
Hide file tree
Showing 8 changed files with 583 additions and 104 deletions.
10 changes: 2 additions & 8 deletions packages/contracts/deploy/02_setup/10_setup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
PersonalSpaceAdminPluginSetupParams,
SpacePluginSetupParams,
} from '../../plugin-setup-params';
import {activeContractsList} from '@aragon/osx-ethers';
import {getPluginSetupProcessorAddress} from '../../utils/helpers';
import {DeployFunction} from 'hardhat-deploy/types';
import {HardhatRuntimeEnvironment} from 'hardhat/types';

Expand All @@ -12,13 +12,7 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
const {deploy} = deployments;
const {deployer} = await getNamedAccounts();

const pspAddress =
activeContractsList[network.name as keyof typeof activeContractsList]
.PluginSetupProcessor;

console.log(
`\nUsing the PluginSetupProcessor address ${pspAddress} on ${network.name}`
);
const pspAddress = getPluginSetupProcessorAddress(network.name);

// Space Setup
console.log(
Expand Down
42 changes: 21 additions & 21 deletions packages/contracts/src/MemberAccessExecuteCondition.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,6 @@ contract MemberAccessExecuteCondition is PermissionCondition {
targetContract = _targetContract;
}

function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
// Slices are only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
}
}

function decodeGrantRevokeCalldata(
bytes memory _data
) public pure returns (bytes4 sig, address who, address where, bytes32 permissionId) {
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
sig := mload(add(_data, 32))
who := mload(add(_data, 36))
where := mload(add(_data, 68))
permissionId := mload(add(_data, 100))
}
}

/// @notice Checks whether the current action wants to grant membership on the predefined address
function isGranted(
address _where,
Expand Down Expand Up @@ -79,4 +58,25 @@ contract MemberAccessExecuteCondition is PermissionCondition {

return true;
}

function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
// Slices are only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
}
}

function decodeGrantRevokeCalldata(
bytes memory _data
) public pure returns (bytes4 sig, address who, address where, bytes32 permissionId) {
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
sig := mload(add(_data, 32))
who := mload(add(_data, 36))
where := mload(add(_data, 68))
permissionId := mload(add(_data, 100))
}
}
}
160 changes: 99 additions & 61 deletions packages/contracts/src/OnlyPluginUpgraderCondition.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {PermissionManager} from "@aragon/osx/core/permission/PermissionManager.s
import {PluginSetupProcessor} from "@aragon/osx/framework/plugin/setup/PluginSetupProcessor.sol";
import {DAO} from "@aragon/osx/core/dao/DAO.sol";
import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
import {MEMBER_PERMISSION_ID} from "./constants.sol";

/// @notice The condition associated with the `pluginUpgrader`
contract OnlyPluginUpgraderCondition is PermissionCondition {
Expand All @@ -17,8 +16,8 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
address private dao;
/// @notice The address of the PluginSetupProcessor contract
address private psp;
/// @notice The address of the contract where the permission can be granted
address[] private targetPluginAddresses;
/// @notice Contracts where the permission can be granted
mapping(address => bool) private allowedPluginAddresses;

/// @notice Thrown when the constructor receives empty parameters
error InvalidParameters();
Expand All @@ -33,20 +32,19 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
) {
revert InvalidParameters();
}
targetPluginAddresses = _targetPluginAddresses;
psp = address(_psp);
dao = address(_dao);
}

function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
// Slices are only supported for bytes calldata
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
for (uint256 i = 0; i < _targetPluginAddresses.length; ) {
allowedPluginAddresses[_targetPluginAddresses[i]] = true;

unchecked {
i++;
}
}
psp = address(_psp);
dao = address(_dao);
}

/// @notice Checks whether the current action wants to grant membership on the predefined address
/// @notice Checks whether the current action grants update to the PSP, updates a predefined plugin and revokes the update permission
function isGranted(
address _where,
address _who,
Expand All @@ -60,6 +58,7 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
return false;
}

// Unwrap the execute actions
(, IDAO.Action[] memory _actions, ) = abi.decode(
_data[4:],
(bytes32, IDAO.Action[], uint256)
Expand All @@ -68,68 +67,107 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
// Check all actions
if (_actions.length != 3) return false;

// Action 1: GRANT UPGRADE_PLUGIN_PERMISSION_ID to the PSP on targetPlugis[]
_requestedFuncSig = getSelector(_actions[0].data);
if (_requestedFuncSig != PermissionManager.grant.selector) return false;
else if (_actions[0].to != dao) return false;
else if (!isValidExecuteGrantRevokeCalldata(_actions[0].data)) return false;
// Action 1/3: GRANT/REVOKE UPGRADE_PLUGIN_PERMISSION_ID to the PSP on targetPlugis[]
if (_actions[0].to != dao || _actions[2].to != dao) return false;
else if (!isValidGrantRevokeCalldata(_actions[0].data, _actions[2].data)) return false;

// Action 2: CALL PSP.applyUpdate() onto targetPlugins[]
_requestedFuncSig = getSelector(_actions[1].data);
if (_requestedFuncSig != PluginSetupProcessor.applyUpdate.selector) return false;
else if (_actions[1].to != psp) return false;
else if (!isValidExecuteApplyUpdateCalldata(_actions[1].data)) return false;

// Action 3: REVOKE UPGRADE_PLUGIN_PERMISSION_ID to the PSP on targetPlugis[]
_requestedFuncSig = getSelector(_actions[2].data);
if (_requestedFuncSig != PermissionManager.revoke.selector) return false;
else if (_actions[2].to != dao) return false;
else if (!isValidExecuteGrantRevokeCalldata(_actions[2].data)) return false;
if (_actions[1].to != psp) return false;
else if (!isValidApplyUpdateCalldata(_actions[1].data)) return false;

return true;
}

// Internal helpers

function isValidExecuteGrantRevokeCalldata(bytes memory _data) private view returns (bool) {
// Decode the call being requested
(, address _requestedWhere, address _requestedWho, bytes32 _requestedPermission) = abi
.decode(
_data,
(bytes4 /*funcSig*/, address /*where*/, address /*who*/, bytes32 /*perm*/)
);

if (_requestedPermission != UPGRADE_PLUGIN_PERMISSION_ID) return false;
else if (_requestedWho != psp) return false;
function getSelector(bytes memory _data) public pure returns (bytes4 selector) {
// Slices are only supported for bytes calldata
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
}
}

// Search the first match
for (uint256 j = 0; j < targetPluginAddresses.length; ) {
if (_requestedWhere == targetPluginAddresses[j]) return true;
function decodeGrantRevokeCalldata(
bytes memory _data
) public pure returns (bytes4 selector, address who, address where, bytes32 permissionId) {
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
who := mload(add(_data, 36))
where := mload(add(_data, 68))
permissionId := mload(add(_data, 100))
}
}

unchecked {
j++;
}
function decodeApplyUpdateCalldata(
bytes memory _data
)
public
pure
returns (
bytes4 selector,
address daoAddress,
PluginSetupProcessor.ApplyUpdateParams memory applyUpdateParams
)
{
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
selector := mload(add(_data, 32))
daoAddress := mload(add(_data, 36))
applyUpdateParams := mload(add(_data, 68))
}
return false;
}

function isValidExecuteApplyUpdateCalldata(bytes memory _data) private view returns (bool) {
//
(, address _dao, PluginSetupProcessor.ApplyUpdateParams memory _applyParams) = abi.decode(
_data,
(bytes4, address, PluginSetupProcessor.ApplyUpdateParams)
);
// Internal helpers

function isValidGrantRevokeCalldata(
bytes memory _grantData,
bytes memory _revokeData
) private view returns (bool) {
// Grant checks
(
bytes4 _grantSelector,
address _grantWhere,
address _grantWho,
bytes32 _grantPermission
) = decodeGrantRevokeCalldata(_grantData);

if (_grantSelector != PermissionManager.grant.selector) return false;
else if (_grantWho != psp) return false;
else if (_grantPermission != UPGRADE_PLUGIN_PERMISSION_ID) return false;
else if (!allowedPluginAddresses[_grantWhere]) return false;

// Revoke checks
(
bytes4 _revokeSelector,
address _revokeWhere,
address _revokeWho,
bytes32 _revokePermission
) = decodeGrantRevokeCalldata(_revokeData);

if (_revokeSelector != PermissionManager.revoke.selector) return false;
else if (_revokeWho != psp) return false;
else if (_revokePermission != UPGRADE_PLUGIN_PERMISSION_ID) return false;
else if (!allowedPluginAddresses[_revokeWhere]) return false;

// Combined checks
if (_grantWhere != _revokeWhere) return false;

return true;
}

if (_dao != dao) return false;
function isValidApplyUpdateCalldata(bytes memory _data) private view returns (bool) {
(
bytes4 _selector,
address _dao,
PluginSetupProcessor.ApplyUpdateParams memory _applyParams
) = decodeApplyUpdateCalldata(_data);

// Search the first match
for (uint256 j = 0; j < targetPluginAddresses.length; ) {
if (_applyParams.plugin != targetPluginAddresses[j]) return false;
if (_selector != PluginSetupProcessor.applyUpdate.selector) return false;
else if (_dao != dao) return false;
else if (!allowedPluginAddresses[_applyParams.plugin]) return false;

unchecked {
j++;
}
}
return true;
}
}
1 change: 1 addition & 0 deletions packages/contracts/test/unit-testing/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const MAX_UINT64 = ethers.BigNumber.from(2).pow(64).sub(1);
export const ADDRESS_ZERO = ethers.constants.AddressZero;
export const ADDRESS_ONE = `0x${'0'.repeat(39)}1`;
export const ADDRESS_TWO = `0x${'0'.repeat(39)}2`;
export const ADDRESS_THREE = `0x${'0'.repeat(39)}3`;
export const NO_CONDITION = ADDRESS_ZERO;

export async function getTime(): Promise<number> {
Expand Down
17 changes: 11 additions & 6 deletions packages/contracts/test/unit-testing/member-access-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
MainVotingPlugin__factory,
MemberAccessPlugin,
MemberAccessPlugin__factory,
MemberAccessExecuteCondition,
MemberAccessExecuteCondition__factory,
SpacePlugin,
SpacePlugin__factory,
} from '../../typechain';
Expand Down Expand Up @@ -62,6 +64,7 @@ describe('Member Access Plugin', function () {
let dave: SignerWithAddress;
let dao: DAO;
let memberAccessPlugin: MemberAccessPlugin;
let memberAccessExecuteCondition: MemberAccessExecuteCondition;
let mainVotingPlugin: MainVotingPlugin;
let spacePlugin: SpacePlugin;
let defaultInput: InitData;
Expand All @@ -85,6 +88,11 @@ describe('Member Access Plugin', function () {
new SpacePlugin__factory(alice)
);

memberAccessExecuteCondition =
await new MemberAccessExecuteCondition__factory(alice).deploy(
mainVotingPlugin.address
);

// inits
await memberAccessPlugin.initialize(dao.address, {
proposalDuration: 60 * 60 * 24 * 5,
Expand All @@ -108,10 +116,11 @@ describe('Member Access Plugin', function () {
MEMBER_PERMISSION_ID
);
// The plugin can execute on the DAO
await dao.grant(
await dao.grantWithCondition(
dao.address,
memberAccessPlugin.address,
EXECUTE_PERMISSION_ID
EXECUTE_PERMISSION_ID,
memberAccessExecuteCondition.address
);
// The main voting plugin can also execute on the DAO
await dao.grant(
Expand Down Expand Up @@ -1143,20 +1152,16 @@ describe('Member Access Plugin', function () {

await expect(dao.execute(ZERO_BYTES32, actionsWith(ADDRESS_ZERO), 0)).to
.be.reverted;

await expect(dao.execute(ZERO_BYTES32, actionsWith(ADDRESS_ONE), 0)).to.be
.reverted;

await expect(dao.execute(ZERO_BYTES32, actionsWith(ADDRESS_TWO), 0)).to.be
.reverted;

await expect(dao.execute(ZERO_BYTES32, actionsWith(bob.address), 0)).to.be
.reverted;

await expect(
dao.execute(ZERO_BYTES32, actionsWith(memberAccessPlugin.address), 0)
).to.be.reverted;

await expect(
dao.execute(ZERO_BYTES32, actionsWith(mainVotingPlugin.address), 0)
).to.not.be.reverted;
Expand Down
Loading

0 comments on commit c7cff26

Please sign in to comment.