Skip to content

Commit

Permalink
Member access condition tests working
Browse files Browse the repository at this point in the history
  • Loading branch information
brickpop committed Jan 30, 2024
1 parent bb6dab0 commit 6953dda
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 271 deletions.
36 changes: 30 additions & 6 deletions packages/contracts/src/conditions/MemberAccessExecuteCondition.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,28 @@ contract MemberAccessExecuteCondition is PermissionCondition {
) external view returns (bool) {
(_where, _who, _permissionId);

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

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

// Check actions
if (_actions.length != 1) return false;
else if (_actions[0].to != targetContract) return false;

// Decode the call being requested (both have the same parameters)
(bytes4 _requestedSelector, ) = decodeAddRemoveMemberCalldata(_actions[0].data);

if (
_requestedSelector != MainVotingPlugin.addMember.selector &&
_requestedSelector != MainVotingPlugin.removeMember.selector
) return false;

return true;
}

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

function decodeAddRemoveMemberCalldata(
bytes memory _data
) public pure returns (bytes4 sig, address account) {
// Slicing is only supported for bytes calldata, not bytes memory
// Bytes memory requires an assembly block
assembly {
sig := mload(add(_data, 0x20)) // 32
account := mload(add(_data, 0x24)) // 32 + 4
}
}
}
122 changes: 36 additions & 86 deletions packages/contracts/test/integration-testing/member-access-condition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
PluginSetupProcessor__factory,
PluginRepoFactory__factory,
PluginRepoRegistry__factory,
IDAO,
DAO__factory,
} from '@aragon/osx-ethers';
import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers';
Expand All @@ -46,8 +45,9 @@ const pluginSettings: MajorityVotingBase.VotingSettingsStruct = {
supportThreshold: 1,
votingMode: 0,
};
const minMemberAccessProposalDuration = 60 * 60 * 24;
const memberAccessProposalDuration = 60 * 60 * 24;
const daoInterface = DAO__factory.createInterface();
const mainVotingInterface = MainVotingPlugin__factory.createInterface();

describe('Member Access Condition E2E', () => {
let deployer: SignerWithAddress;
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('Member Access Condition E2E', () => {
const data = await pluginSetup.encodeInstallationParams(
pluginSettings,
[deployer.address],
minMemberAccessProposalDuration,
memberAccessProposalDuration,
pluginUpgrader.address
);
const installation = await installPlugin(psp, dao, pluginSetupRef, data);
Expand All @@ -165,133 +165,83 @@ describe('Member Access Condition E2E', () => {
});

it('Executing a proposal to add membership works', async () => {
expect(
await dao.isGranted(
mainVotingPlugin.address, // where
alice.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(false);
expect(await mainVotingPlugin.isMember(alice.address)).to.eq(false);

await memberAccessPlugin.proposeNewMember('0x', alice.address);

await expect(memberAccessPlugin.proposeNewMember('0x', alice.address)).to
.not.be.reverted;

expect(
await dao.isGranted(
mainVotingPlugin.address, // where
alice.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(true);

// Valid grant
const actions: IDAO.ActionStruct[] = [
{
to: dao.address,
value: 0,
data: daoInterface.encodeFunctionData('grant', [
mainVotingPlugin.address,
bob.address,
MEMBER_PERMISSION_ID,
]),
},
];
expect(await mainVotingPlugin.isMember(alice.address)).to.eq(true);

// Via direct create proposal
expect(
await dao.isGranted(
mainVotingPlugin.address, // where
bob.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(false);

await expect(memberAccessPlugin.createArbitraryProposal('0x', actions)).to
.not.be.reverted;
// // Valid addition
// const actions: IDAO.ActionStruct[] = [
// {
// to: mainVotingPlugin.address,
// value: 0,
// data: mainVotingInterface.encodeFunctionData('addMember', [
// bob.address,
// ]),
// },
// ];

expect(
await dao.isGranted(
mainVotingPlugin.address, // where
bob.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(true);
// // Via direct create proposal
// expect(await mainVotingPlugin.isMember(bob.address)).to.eq(false);

// await expect(memberAccessPlugin.createArbitraryProposal('0x', actions)).to
// .not.be.reverted;

// expect(await mainVotingPlugin.isMember(bob.address)).to.eq(true);
});

it('Executing a proposal to remove membership works', async () => {
await expect(memberAccessPlugin.proposeNewMember('0x', alice.address)).to
.not.be.reverted;
await expect(memberAccessPlugin.proposeRemoveMember('0x', alice.address)).to
.not.be.reverted;
expect(
await dao.isGranted(
mainVotingPlugin.address, // where
alice.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(false);
expect(await mainVotingPlugin.isMember(alice.address)).to.eq(false);

// Valid revoke
const grantAction = {
to: dao.address,
to: mainVotingPlugin.address,
value: 0,
data: daoInterface.encodeFunctionData('grant', [
mainVotingPlugin.address,
bob.address,
MEMBER_PERMISSION_ID,
]),
data: mainVotingInterface.encodeFunctionData('addMember', [bob.address]),
};
const revokeAction = {
to: dao.address,
to: mainVotingPlugin.address,
value: 0,
data: daoInterface.encodeFunctionData('revoke', [
mainVotingPlugin.address,
data: mainVotingInterface.encodeFunctionData('removeMember', [
bob.address,
MEMBER_PERMISSION_ID,
]),
};

// Via direct create proposal
await expect(
memberAccessPlugin.createArbitraryProposal('0x', [grantAction])
).to.not.be.reverted;
expect(await mainVotingPlugin.isMember(bob.address)).to.eq(true);

await expect(
memberAccessPlugin.createArbitraryProposal('0x', [revokeAction])
).to.not.be.reverted;

expect(
await dao.isGranted(
mainVotingPlugin.address, // where
bob.address, // who
MEMBER_PERMISSION_ID, // permission
'0x'
)
).to.eq(false);
expect(await mainVotingPlugin.isMember(bob.address)).to.eq(false);
});

it('Executing a proposal to do something else reverts', async () => {
const validActions = [
{
to: dao.address,
to: mainVotingPlugin.address,
value: 0,
data: daoInterface.encodeFunctionData('grant', [
mainVotingPlugin.address,
data: mainVotingInterface.encodeFunctionData('addMember', [
bob.address,
MEMBER_PERMISSION_ID,
]),
},
{
to: dao.address,
to: mainVotingPlugin.address,
value: 0,
data: daoInterface.encodeFunctionData('revoke', [
mainVotingPlugin.address,
data: mainVotingInterface.encodeFunctionData('removeMember', [
bob.address,
MEMBER_PERMISSION_ID,
]),
},
];
Expand Down
Loading

0 comments on commit 6953dda

Please sign in to comment.