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

Build 1.2 new changes #18

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
77 changes: 61 additions & 16 deletions packages/contracts/src/Admin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {IMembership} from "@aragon/osx-commons-contracts/src/plugin/extensions/m
import {ProposalUpgradeable} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol";
import {PluginCloneable} from "@aragon/osx-commons-contracts/src/plugin/PluginCloneable.sol";
import {IDAO} from "@aragon/osx-commons-contracts/src/dao/IDAO.sol";
import {IProposal} from "@aragon/osx-commons-contracts/src/plugin/extensions/proposal/IProposal.sol";
import {Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";

/// @title Admin
/// @author Aragon X - 2022-2023
Expand All @@ -20,8 +22,7 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
using SafeCastUpgradeable for uint256;

/// @notice The [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID of the contract.
bytes4 internal constant ADMIN_INTERFACE_ID =
this.initialize.selector ^ this.executeProposal.selector;
bytes4 internal constant ADMIN_INTERFACE_ID = this.executeProposal.selector;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for multisig
aragon#19 (review)

This interface is now deprecated


/// @notice The ID of the permission required to call the `executeProposal` function.
bytes32 public constant EXECUTE_PROPOSAL_PERMISSION_ID =
Expand All @@ -30,9 +31,11 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
/// @notice Initializes the contract.
/// @param _dao The associated DAO.
/// @dev This method is required to support [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167).
function initialize(IDAO _dao) external initializer {
function initialize(IDAO _dao, TargetConfig calldata _targetConfig) external initializer {
__PluginCloneable_init(_dao);

_setTargetConfig(_targetConfig);

Comment on lines 33 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use reintializer(n) here, and then create another function for initializing from other versions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admin is non-upgradeable but cloneable, so no..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

emit MembershipContractAnnounced({definingContract: address(_dao)});
}

Expand All @@ -59,6 +62,34 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
});
}

/// @inheritdoc IProposal
function customProposalParamsABI() external pure override returns (string memory) {
return "(uint256 allowFailureMap)";
}

/// @inheritdoc IProposal
function createProposal(
bytes calldata _metadata,
Action[] calldata _actions,
uint64,
uint64,
bytes memory _data
) public override returns (uint256 proposalId) {
uint256 allowFailureMap;

if (_data.length > 0) {
allowFailureMap = abi.decode(_data, (uint256));
}

// Uses public function for permission check.
proposalId = executeProposal(_metadata, _actions, allowFailureMap);
}

/// @inheritdoc IProposal
function canExecute(uint256) public view virtual override returns (bool) {
return true;
}

/// @notice Creates and executes a new proposal.
/// @param _metadata The metadata of the proposal.
/// @param _actions The actions to be executed.
Expand All @@ -67,19 +98,33 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
// of 0 requires every action to not revert.
function executeProposal(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function executeProposal(
/// @dev This function is deprecated, as createProposal achieves the same effect
function executeProposal(

bytes calldata _metadata,
IDAO.Action[] calldata _actions,
Action[] calldata _actions,
uint256 _allowFailureMap
) external auth(EXECUTE_PROPOSAL_PERMISSION_ID) {
uint64 currentTimestamp64 = block.timestamp.toUint64();

uint256 proposalId = _createProposal({
_creator: _msgSender(),
_metadata: _metadata,
_startDate: currentTimestamp64,
_endDate: currentTimestamp64,
_actions: _actions,
_allowFailureMap: _allowFailureMap
});
_executeProposal(dao(), proposalId, _actions, _allowFailureMap);
) public auth(EXECUTE_PROPOSAL_PERMISSION_ID) returns (uint256 proposalId) {
uint64 currentTimestamp = block.timestamp.toUint64();

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

TargetConfig memory targetConfig = getTargetConfig();

_execute(
targetConfig.target,
bytes32(proposalId),
_actions,
_allowFailureMap,
targetConfig.operation
);

emit ProposalCreated(
proposalId,
_msgSender(),
currentTimestamp,
currentTimestamp,
_metadata,
_actions,
_allowFailureMap
);

emit ProposalExecuted(proposalId);
}
}
15 changes: 11 additions & 4 deletions packages/contracts/src/AdminSetup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +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 {IPlugin} from "@aragon/osx-commons-contracts/src/plugin/IPlugin.sol";

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

Expand All @@ -18,10 +19,13 @@ import {Admin} from "./Admin.sol";
contract AdminSetup is PluginSetup {
using ProxyLib for address;

// TODO This permission identifier has to be moved inside `PermissionLib` as per task OS-954.
/// @notice The ID of the permission required to call the `execute` function.
bytes32 internal constant EXECUTE_PERMISSION_ID = keccak256("EXECUTE_PERMISSION");

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

/// @notice Thrown if the admin address is zero.
/// @param admin The admin address.
error AdminAddressInvalid(address admin);
Expand All @@ -35,14 +39,17 @@ 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 = abi.decode(_data, (address));
(address admin, IPlugin.TargetConfig memory targetConfig) = abi.decode(
_data,
(address, IPlugin.TargetConfig)
);

if (admin == address(0)) {
revert AdminAddressInvalid({admin: admin});
}

// Clone and initialize the plugin contract.
bytes memory initData = abi.encodeCall(Admin.initialize, (IDAO(_dao)));
bytes memory initData = abi.encodeCall(Admin.initialize, (IDAO(_dao), targetConfig));
plugin = IMPLEMENTATION.deployMinimalProxy(initData);

// Prepare permissions
Expand All @@ -55,7 +62,7 @@ contract AdminSetup is PluginSetup {
where: plugin,
who: admin,
condition: PermissionLib.NO_CONDITION,
permissionId: Admin(plugin).EXECUTE_PROPOSAL_PERMISSION_ID()
permissionId: EXECUTE_PROPOSAL_PERMISSION_ID
});

// Grant `EXECUTE_PERMISSION` on the DAO to the plugin.
Expand Down
20 changes: 20 additions & 0 deletions packages/contracts/src/build-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,26 @@
"name": "admin",
"type": "address",
"description": "The address of the admin account receiving the `EXECUTE_PERMISSION_ID` permission."
},
{
"components": [
{
"internalType": "address",
"name": "target",
"type": "address",
"description": "The target contract to which actions will be forwarded to for execution."
},
{
"internalType": "uint8",
"name": "operation",
"type": "uint8",
"description": "The operation type(either `call` or `delegatecall`) that will be used for execution forwarding."
}
],
"internalType": "struct TokenVoting.TargetConfig",
"name": "TargetConfig",
"type": "tuple",
"description": "The initial target config"
}
]
},
Expand Down
26 changes: 26 additions & 0 deletions packages/contracts/src/mocks/CustomExecutorMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity ^0.8.8;

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

/// @dev DO NOT USE IN PRODUCTION!
contract CustomExecutorMock {
error FailedCustom();

event ExecutedCustom();

function execute(
bytes32 callId,
Action[] memory,
uint256
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap);

if (callId == bytes32(0)) {
revert FailedCustom();
} else {
emit ExecutedCustom();
}
}
}
Loading
Loading