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

feature: targets and standardized proposal #99

Merged
merged 28 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
27e6088
proposal and plugins interfaces changes with backwards compatibility
novaknole Aug 14, 2024
f6f5f3d
add artifacts folder in gitignore
novaknole Aug 14, 2024
b233838
fix calldata to memory
novaknole Aug 14, 2024
decd4ff
add tests
novaknole Aug 14, 2024
918763d
add operation type for target
novaknole Aug 20, 2024
045a403
feat: add tests and refactor (#100)
clauBv23 Aug 22, 2024
f4dc570
remove previous target from targetset event
novaknole Aug 26, 2024
5306a73
add get proposalid in the interface
novaknole Aug 27, 2024
6def9f4
rename function
novaknole Aug 27, 2024
9b799db
from calldata to memory
novaknole Sep 10, 2024
23c73bd
improved target config and virtual functions
novaknole Sep 19, 2024
db446f5
add params byte approach (#102)
novaknole Sep 27, 2024
21ccec8
executor added with reentrancy guard
novaknole Sep 27, 2024
d1feeeb
call initialization only
novaknole Sep 28, 2024
55a0cc6
add comments
novaknole Oct 1, 2024
2476aa2
fallback to dao as a target if no target set (#104)
novaknole Oct 7, 2024
db06fde
metadata contracts (#105)
novaknole Oct 8, 2024
409c90c
Update contracts/src/executors/IExecutor.sol
novaknole Oct 9, 2024
2f31a61
review changes
novaknole Oct 9, 2024
8916046
Merge branch 'feature/targets-and-standardized-proposal' of https://g…
novaknole Oct 9, 2024
cf6cdc5
unused import remove
novaknole Oct 9, 2024
2cc8ea5
remove unusued import
novaknole Oct 9, 2024
8a23aea
from memory to calldata in metadataextension
novaknole Oct 9, 2024
ef42b24
create proposal id not enforced (#106)
novaknole Oct 11, 2024
2016e23
remove unusued function
novaknole Oct 17, 2024
3d484e5
add condition (#108)
novaknole Oct 30, 2024
d4cfd50
remove always true condition
novaknole Oct 30, 2024
461a75d
add hasSucceeded instead of (#112)
novaknole Nov 4, 2024
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ dist-ssr
.cache
typechain

contracts/artifacts
contracts/cache


# misc
.DS_Store
*.pem
Expand Down
35 changes: 35 additions & 0 deletions contracts/src/mocks/plugin/CustomExecutorMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity ^0.8.8;

import {IDAO} from "../../dao/IDAO.sol";

/// @notice A mock DAO that anyone can set permissions in.
/// @dev DO NOT USE IN PRODUCTION!
contract CustomExecutorMock {
error Failed();

event Executed(
address indexed actor,
bytes32 callId,
IDAO.Action[] actions,
uint256 allowFailureMap,
uint256 failureMap,
bytes[] execResults
);

function execute(
bytes32 callId,
IDAO.Action[] memory _actions,
uint256 allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
if (callId == bytes32(0)) {
revert Failed();
} else if (callId == bytes32(uint256(123))) {
// solhint-disable-next-line reason-string, custom-errors
revert();
} else {
emit Executed(msg.sender, callId, _actions, allowFailureMap, failureMap, execResults);
}
}
}
24 changes: 24 additions & 0 deletions contracts/src/mocks/plugin/PluginCloneableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ contract PluginCloneableMockBuild1 is PluginCloneable {
__PluginCloneable_init(_dao);
state1 = 1;
}

function execute(
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(bytes32(_callId), _actions, _allowFailureMap);
}

function execute(
address _target,
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap,
Operation _op
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(
_target,
bytes32(_callId),
_actions,
_allowFailureMap,
_op
);
}
}

/// @notice A mock cloneable plugin to be deployed via the minimal proxy pattern.
Expand Down
24 changes: 24 additions & 0 deletions contracts/src/mocks/plugin/PluginMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,28 @@ contract PluginMockBuild1 is Plugin {
constructor(IDAO _dao) Plugin(_dao) {
state1 = 1;
}

function execute(
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(bytes32(_callId), _actions, _allowFailureMap);
}

function execute(
address _target,
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap,
Operation _op
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(
_target,
bytes32(_callId),
_actions,
_allowFailureMap,
_op
);
}
}
24 changes: 24 additions & 0 deletions contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ contract PluginUUPSUpgradeableMockBuild1 is PluginUUPSUpgradeable {
__PluginUUPSUpgradeable_init(_dao);
state1 = 1;
}

function execute(
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(bytes32(_callId), _actions, _allowFailureMap);
}

function execute(
address _target,
uint256 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap,
Operation _op
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _execute(
_target,
bytes32(_callId),
_actions,
_allowFailureMap,
_op
);
}
}

/// @notice A mock upgradeable plugin to be deployed via the UUPS proxy pattern.
Expand Down
38 changes: 10 additions & 28 deletions contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,19 @@ pragma solidity ^0.8.8;
import {Proposal} from "../../../../plugin/extensions/proposal/Proposal.sol";
import {IDAO} from "../../../../dao/IDAO.sol";

/// @notice A mock contract containing functions to create and execute proposals.
/// @notice A mock contract.
/// @dev DO NOT USE IN PRODUCTION!
contract ProposalMock is Proposal {
function createProposalId() external returns (uint256 proposalId) {
proposalId = _createProposalId();
}
// We don't need to test these below functions as they will be tested in the actual plugins.
// This mock contract is only used to test `supportsInterface` function.

// solhint-disable no-empty-blocks
function createProposal(
address _creator,
bytes calldata _metadata,
uint64 _startDate,
uint64 _endDate,
IDAO.Action[] calldata _actions,
uint256 _allowFailureMap
) external returns (uint256 proposalId) {
proposalId = _createProposal(
_creator,
_metadata,
_startDate,
_endDate,
_actions,
_allowFailureMap
);
}
bytes memory data,
IDAO.Action[] memory actions,
uint64 startDate,
uint64 endDate
) external returns (uint256 proposalId) {}

function executeProposal(
IDAO _dao,
uint256 _proposalId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _executeProposal(_dao, _proposalId, _actions, _allowFailureMap);
}
function canExecute(uint256 proposalId) external returns (bool) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,19 @@ pragma solidity ^0.8.8;
import {ProposalUpgradeable} from "../../../../plugin/extensions/proposal/ProposalUpgradeable.sol";
import {IDAO} from "../../../../dao/IDAO.sol";

/// @notice A mock contract containing functions to create and execute proposals as well as storage gaps in inherited contracts to be used in UUPS upgradeable contracts.
/// @notice A mock contract.
/// @dev DO NOT USE IN PRODUCTION!
contract ProposalUpgradeableMock is ProposalUpgradeable {
function createProposalId() external returns (uint256 proposalId) {
proposalId = _createProposalId();
}
// We don't need to test these below functions as they will be tested in the actual plugins.
// This mock contract is only used to test `supportsInterface` function.

// solhint-disable no-empty-blocks
function createProposal(
address _creator,
bytes calldata _metadata,
uint64 _startDate,
uint64 _endDate,
IDAO.Action[] calldata _actions,
uint256 _allowFailureMap
) external returns (uint256 proposalId) {
proposalId = _createProposal(
_creator,
_metadata,
_startDate,
_endDate,
_actions,
_allowFailureMap
);
}
bytes memory data,
IDAO.Action[] memory actions,
uint64 startDate,
uint64 endDate
) external returns (uint256 proposalId) {}

function executeProposal(
IDAO _dao,
uint256 _proposalId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) external returns (bytes[] memory execResults, uint256 failureMap) {
(execResults, failureMap) = _executeProposal(_dao, _proposalId, _actions, _allowFailureMap);
}
function canExecute(uint256 proposalId) external returns (bool) {}
}
124 changes: 123 additions & 1 deletion contracts/src/plugin/Plugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
pragma solidity ^0.8.8;

import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";

import {IProtocolVersion} from "../utils/versioning/IProtocolVersion.sol";
import {ProtocolVersion} from "../utils/versioning/ProtocolVersion.sol";
Expand All @@ -15,6 +16,33 @@ import {IPlugin} from "./IPlugin.sol";
/// @notice An abstract, non-upgradeable contract to inherit from when creating a plugin being deployed via the `new` keyword.
/// @custom:security-contact [email protected]
abstract contract Plugin is IPlugin, ERC165, DaoAuthorizable, ProtocolVersion {
using ERC165Checker for address;

enum Operation {
Call,
DelegateCall
}

struct TargetConfig {
address target;
Operation operation;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to IPlugin. They produce no code when not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 2f31a61


TargetConfig private currentTargetConfig;

/// @notice Thrown when target is of type 'IDAO', but operation is `delegateCall`.
/// @param targetConfig The target config to update it to.
error InvalidTargetConfig(TargetConfig targetConfig);

/// @dev Emitted each time the TargetConfig is set.
event TargetSet(TargetConfig previousTargetConfig, TargetConfig newTargetConfig);

/// @notice Thrown when `delegatecall` fails.
error ExecuteFailed();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
error ExecuteFailed();
error DelegateCallFailed();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 2f31a61


/// @notice The ID of the permission required to call the `setTarget` function.
bytes32 public constant SET_TARGET_PERMISSION_ID = keccak256("SET_TARGET_PERMISSION");

/// @notice Constructs the plugin by storing the associated DAO.
/// @param _dao The DAO contract.
constructor(IDAO _dao) DaoAuthorizable(_dao) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the constructor define an initial target?

Expand All @@ -24,13 +52,107 @@ abstract contract Plugin is IPlugin, ERC165, DaoAuthorizable, ProtocolVersion {
return PluginType.Constructable;
}

/// @notice Checks if this or the parent contract supports an interface by its ID.
/// @notice Returns the currently set target contract.
function getTargetConfig() public view returns (TargetConfig memory) {
return currentTargetConfig;
}

/// @dev Sets the target to a new target (`newTarget`).
/// @param _targetConfig The target Config containing the address and operation type.
function setTargetConfig(
TargetConfig calldata _targetConfig
) public auth(SET_TARGET_PERMISSION_ID) {
_setTargetConfig(_targetConfig);
}

/// @notice Checks if an interface is supported by this or its parent contract.
/// @param _interfaceId The ID of the interface.
/// @return Returns `true` if the interface is supported.
function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
return
_interfaceId == type(IPlugin).interfaceId ||
_interfaceId == type(IProtocolVersion).interfaceId ||
_interfaceId == this.setTargetConfig.selector ^ this.getTargetConfig.selector ||
super.supportsInterface(_interfaceId);
}

/// @notice Sets the target to a new target (`newTarget`).
/// @param _targetConfig The target Config containing the address and operation type.
function _setTargetConfig(TargetConfig calldata _targetConfig) internal virtual {
TargetConfig memory previousTargetConfig = currentTargetConfig;

currentTargetConfig = _targetConfig;

// safety check to avoid setting dao as `target` with `delegatecall` operation
// as this would not work and cause the plugin to be bricked.
if (
_targetConfig.target.supportsInterface(type(IDAO).interfaceId) &&
_targetConfig.operation == Operation.DelegateCall
) {
revert InvalidTargetConfig(_targetConfig);
}

emit TargetSet(previousTargetConfig, _targetConfig);
}

/// @notice Forwards the actions to the currently set `target` for the execution.
/// @param _callId Identifier for this execution.
/// @param _actions actions that will be eventually called.
/// @param _allowFailureMap Bitmap-encoded number. TODO:
/// @return execResults address of the implementation contract.
/// @return failureMap address of the implementation contract.
function _execute(
bytes32 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap
) internal virtual returns (bytes[] memory execResults, uint256 failureMap) {
return
_execute(
currentTargetConfig.target,
novaknole marked this conversation as resolved.
Show resolved Hide resolved
_callId,
_actions,
_allowFailureMap,
currentTargetConfig.operation
);
}

/// @notice Forwards the actions to the `target` for the execution.
/// @param _target Forwards the actions to the specific target.
/// @param _callId Identifier for this execution.
/// @param _actions actions that will be eventually called.
/// @param _allowFailureMap Bitmap-encoded number. TODO:
/// @return execResults address of the implementation contract.
/// @return failureMap address of the implementation contract.
function _execute(
address _target,
bytes32 _callId,
IDAO.Action[] memory _actions,
uint256 _allowFailureMap,
Operation _op
) internal virtual returns (bytes[] memory execResults, uint256 failureMap) {
if (_op == Operation.DelegateCall) {
bool success;
bytes memory data;

// solhint-disable-next-line avoid-low-level-calls
(success, data) = _target.delegatecall(
abi.encodeCall(IDAO.execute, (_callId, _actions, _allowFailureMap))
);

if (!success) {
if (data.length > 0) {
// solhint-disable-next-line no-inline-assembly
assembly {
let returndata_size := mload(data)
revert(add(32, data), returndata_size)
}
} else {
revert ExecuteFailed();
}
}
(execResults, failureMap) = abi.decode(data, (bytes[], uint256));
} else {
(execResults, failureMap) = IDAO(_target).execute(_callId, _actions, _allowFailureMap);
}
}
}
Loading
Loading