Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix contract and test #19

Merged
merged 4 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 2 additions & 22 deletions packages/contracts/src/Admin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,8 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
});
}

/// @notice Hashing function used to (re)build the proposal id from the proposal details..
/// @dev The proposal id is produced by hashing the ABI encoded `targets` array, the `values` array, the `calldatas` array
/// and the descriptionHash (bytes32 which itself is the keccak256 hash of the description string). This proposal id
/// can be produced from the proposal data which is part of the {ProposalCreated} event. It can even be computed in
/// advance, before the proposal is submitted.
/// The chainId and the governor address are not part of the proposal id computation. Consequently, the
/// same proposal (with same operation and same description) will have the same id if submitted on multiple governors
/// across multiple networks. This also means that in order to execute the same operation twice (on the same
/// governor) the proposer will have to change the description in order to avoid proposal id conflicts.
/// @param _actions The actions that will be executed after the proposal passes.
/// @param _metadata The metadata of the proposal.
/// @return proposalId The ID of the proposal.
function createProposalId(
Action[] calldata _actions,
bytes memory _metadata
) public pure override returns (uint256) {
return uint256(keccak256(abi.encode(_actions, _metadata)));
}

/// @inheritdoc IProposal
/// @dev Admin doesn't allow creating a proposal, so we return empty string.
function createProposalParamsABI() external pure override returns (string memory) {
function customProposalParamsABI() external pure override returns (string memory) {
return "(uint256 allowFailureMap)";
}

Expand Down Expand Up @@ -123,7 +103,7 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
) public auth(EXECUTE_PROPOSAL_PERMISSION_ID) returns (uint256 proposalId) {
uint64 currentTimestamp = block.timestamp.toUint64();

proposalId = createProposalId(_actions, _metadata);
proposalId = _createProposalId(keccak256(abi.encode(_actions, _metadata)));

TargetConfig memory targetConfig = getTargetConfig();

Expand Down
6 changes: 3 additions & 3 deletions packages/contracts/src/AdminSetup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {PluginSetup} from "@aragon/osx-commons-contracts/src/plugin/setup/Plugin
import {PermissionLib} from "@aragon/osx-commons-contracts/src/permission/PermissionLib.sol";
import {ProxyLib} from "@aragon/osx-commons-contracts/src/utils/deployment/ProxyLib.sol";
import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol";
import {PluginCloneable} from "@aragon/osx-commons-contracts/src/plugin/PluginCloneable.sol";
import {IPlugin} from "@aragon/osx-commons-contracts/src/plugin/IPlugin.sol";

import {Admin} from "./Admin.sol";

Expand Down Expand Up @@ -39,9 +39,9 @@ contract AdminSetup is PluginSetup {
bytes calldata _data
) external returns (address plugin, PreparedSetupData memory preparedSetupData) {
// Decode `_data` to extract the params needed for cloning and initializing the `Admin` plugin.
(address admin, PluginCloneable.TargetConfig memory targetConfig) = abi.decode(
(address admin, IPlugin.TargetConfig memory targetConfig) = abi.decode(
_data,
(address, PluginCloneable.TargetConfig)
(address, IPlugin.TargetConfig)
);

if (admin == address(0)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/src/mocks/CustomExecutorMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pragma solidity ^0.8.8;

import {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";
import {Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";

/// @dev DO NOT USE IN PRODUCTION!
contract CustomExecutorMock {
Expand Down
43 changes: 38 additions & 5 deletions packages/contracts/test/10_unit-testing/11_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,46 @@
import {
findEvent,
findEventTopicLog,
proposalIdToBytes32,
getInterfaceId,
DAO_PERMISSIONS,
} from '@aragon/osx-commons-sdk';
import {DAO, DAOEvents, DAOStructs} from '@aragon/osx-ethers';
import {loadFixture} from '@nomicfoundation/hardhat-network-helpers';
import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers';
import {expect} from 'chai';
import {BigNumber} from 'ethers';
import {defaultAbiCoder, keccak256} from 'ethers/lib/utils';
import {ethers} from 'hardhat';

let chainId: number;

async function createProposalId(
pluginAddress: string,
actions: DAOStructs.ActionStruct[],
metadata: string
): Promise<BigNumber> {
const blockNumber = (await ethers.provider.getBlock('latest')).number;
const salt = keccak256(
defaultAbiCoder.encode(
['tuple(address to,uint256 value,bytes data)[]', 'bytes'],
[actions, metadata]
)
);
return BigNumber.from(
keccak256(
defaultAbiCoder.encode(
['uint256', 'uint256', 'address', 'bytes32'],
[chainId, blockNumber + 1, pluginAddress, salt]
)
)
);
}

describe(PLUGIN_CONTRACT_NAME, function () {
before(async () => {
chainId = (await ethers.provider.getNetwork()).chainId;
});

describe('initialize', async () => {
it('reverts if trying to re-initialize', async () => {
const {
Expand Down Expand Up @@ -217,7 +246,8 @@
DAO_PERMISSIONS.EXECUTE_PERMISSION_ID
);

const currentExpectedProposalId = await plugin.createProposalId(
const currentExpectedProposalId = await createProposalId(
plugin.address,
dummyActions,
dummyMetadata
);
Expand Down Expand Up @@ -263,7 +293,8 @@
DAO_PERMISSIONS.EXECUTE_PERMISSION_ID
);

const currentExpectedProposalId = await plugin.createProposalId(
const currentExpectedProposalId = await createProposalId(
plugin.address,
dummyActions,
dummyMetadata
);
Expand Down Expand Up @@ -299,7 +330,8 @@

const newPlugin = plugin.connect(alice);
{
const proposalId = await plugin.createProposalId(
const proposalId = await createProposalId(
plugin.address,
dummyActions,
dummyMetadata
);
Expand Down Expand Up @@ -329,7 +361,8 @@
{
const newMetadata = dummyMetadata + '11';

const proposalId = await plugin.createProposalId(
const proposalId = await createProposalId(
plugin.address,
dummyActions,
newMetadata
);
Expand Down Expand Up @@ -380,7 +413,7 @@
it('executes target with delegate call', async () => {
const {
alice,
bob,

Check failure on line 416 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / checks

'bob' is assigned a value but never used. Allowed unused vars must match /_/u

Check failure on line 416 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / formatting-linting / checks

'bob' is assigned a value but never used. Allowed unused vars must match /_/u

Check failure on line 416 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / formatting-linting / checks

'bob' is assigned a value but never used. Allowed unused vars must match /_/u
dummyMetadata,
dummyActions,
deployer,
Expand All @@ -393,7 +426,7 @@

const abiA = CustomExecutorMock__factory.abi;
const abiB = Admin__factory.abi;
// @ts-ignore

Check failure on line 429 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / checks

Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free

Check failure on line 429 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / formatting-linting / checks

Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free

Check failure on line 429 in packages/contracts/test/10_unit-testing/11_plugin.ts

View workflow job for this annotation

GitHub Actions / formatting-linting / checks

Use "@ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free
const mergedABI = abiA.concat(abiB);

await dao.grant(
Expand Down
Loading