-
Notifications
You must be signed in to change notification settings - Fork 6
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
Open
novaknole
wants to merge
15
commits into
develop
Choose a base branch
from
feature/targets-and-standardized-proposal
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
27e6088
proposal and plugins interfaces changes with backwards compatibility
novaknole f6f5f3d
add artifacts folder in gitignore
novaknole b233838
fix calldata to memory
novaknole decd4ff
add tests
novaknole 918763d
add operation type for target
novaknole 045a403
feat: add tests and refactor (#100)
clauBv23 f4dc570
remove previous target from targetset event
novaknole 5306a73
add get proposalid in the interface
novaknole 6def9f4
rename function
novaknole 9b799db
from calldata to memory
novaknole 23c73bd
improved target config and virtual functions
novaknole db446f5
add params byte approach (#102)
novaknole 21ccec8
executor added with reentrancy guard
novaknole d1feeeb
call initialization only
novaknole 55a0cc6
add comments
novaknole File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,10 @@ dist-ssr | |
.cache | ||
typechain | ||
|
||
contracts/artifacts | ||
contracts/cache | ||
|
||
|
||
# misc | ||
.DS_Store | ||
*.pem | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"; | ||
|
@@ -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; | ||
} | ||
|
||
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(); | ||
|
||
/// @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) {} | ||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be
|
||
_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); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?