-
Notifications
You must be signed in to change notification settings - Fork 8
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
Changes from all 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 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 2476aa2
fallback to dao as a target if no target set (#104)
novaknole db06fde
metadata contracts (#105)
novaknole 409c90c
Update contracts/src/executors/IExecutor.sol
novaknole 2f31a61
review changes
novaknole 8916046
Merge branch 'feature/targets-and-standardized-proposal' of https://g…
novaknole cf6cdc5
unused import remove
novaknole 2cc8ea5
remove unusued import
novaknole 8a23aea
from memory to calldata in metadataextension
novaknole ef42b24
create proposal id not enforced (#106)
novaknole 2016e23
remove unusued function
novaknole 3d484e5
add condition (#108)
novaknole d4cfd50
remove always true condition
novaknole 461a75d
add hasSucceeded instead of (#112)
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
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 |
---|---|---|
|
@@ -7,16 +7,6 @@ pragma solidity ^0.8.8; | |
/// @notice The interface required for DAOs within the Aragon App DAO framework. | ||
/// @custom:security-contact [email protected] | ||
interface IDAO { | ||
/// @notice The action struct to be consumed by the DAO's `execute` function resulting in an external call. | ||
/// @param to The address to call. | ||
/// @param value The native token value to be sent with the call. | ||
/// @param data The bytes-encoded function selector and calldata for the call. | ||
struct Action { | ||
address to; | ||
uint256 value; | ||
bytes data; | ||
} | ||
|
||
/// @notice Checks if an address has permission on a contract via a permission identifier and considers if `ANY_ADDRESS` was used in the granting process. | ||
/// @param _where The address of the contract. | ||
/// @param _who The address of a EOA or contract to give the permissions. | ||
|
@@ -38,35 +28,6 @@ interface IDAO { | |
/// @param metadata The IPFS hash of the new metadata object. | ||
event MetadataSet(bytes metadata); | ||
|
||
/// @notice Executes a list of actions. If a zero allow-failure map is provided, a failing action reverts the entire execution. If a non-zero allow-failure map is provided, allowed actions can fail without the entire call being reverted. | ||
/// @param _callId The ID of the call. The definition of the value of `callId` is up to the calling contract and can be used, e.g., as a nonce. | ||
/// @param _actions The array of actions. | ||
/// @param _allowFailureMap A bitmap allowing execution to succeed, even if individual actions might revert. If the bit at index `i` is 1, the execution succeeds even if the `i`th action reverts. A failure map value of 0 requires every action to not revert. | ||
/// @return The array of results obtained from the executed actions in `bytes`. | ||
/// @return The resulting failure map containing the actions have actually failed. | ||
function execute( | ||
bytes32 _callId, | ||
Action[] memory _actions, | ||
uint256 _allowFailureMap | ||
) external returns (bytes[] memory, uint256); | ||
|
||
/// @notice Emitted when a proposal is executed. | ||
/// @param actor The address of the caller. | ||
/// @param callId The ID of the call. | ||
/// @param actions The array of actions executed. | ||
/// @param allowFailureMap The allow failure map encoding which actions are allowed to fail. | ||
/// @param failureMap The failure map encoding which actions have failed. | ||
/// @param execResults The array with the results of the executed actions. | ||
/// @dev The value of `callId` is defined by the component/contract calling the execute function. A `Plugin` implementation can use it, for example, as a nonce. | ||
event Executed( | ||
address indexed actor, | ||
bytes32 callId, | ||
Action[] actions, | ||
uint256 allowFailureMap, | ||
uint256 failureMap, | ||
bytes[] execResults | ||
); | ||
|
||
/// @notice Emitted when a standard callback is registered. | ||
/// @param interfaceId The ID of the interface. | ||
/// @param callbackSelector The selector of the callback function. | ||
|
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,145 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
|
||
import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; | ||
|
||
import {IExecutor, Action} from "./IExecutor.sol"; | ||
import {flipBit, hasBit} from "../utils/math/BitMap.sol"; | ||
|
||
/// @notice Simple Executor that loops through the actions and executes them. | ||
/// @dev This doesn't use any type of permission for execution and can be called by anyone. | ||
/// Most useful use-case is to deploy as non-upgradeable and call from another contract via delegatecall. | ||
/// If used with delegatecall, DO NOT add state variables in sequential slots, otherwise this will overwrite | ||
/// the storage of the calling contract. | ||
contract Executor is IExecutor, ERC165 { | ||
/// @notice The internal constant storing the maximal action array length. | ||
uint256 internal constant MAX_ACTIONS = 256; | ||
|
||
// keccak256("osx-commons.storage.Executor") | ||
bytes32 private constant REENTRANCY_GUARD_STORAGE_LOCATION = | ||
0x4d6542319dfb3f7c8adbb488d7b4d7cf849381f14faf4b64de3ac05d08c0bdec; | ||
|
||
/// @notice The first out of two values to which the `_reentrancyStatus` state variable (used by the `nonReentrant` modifier) can be set indicating that a function was not entered. | ||
uint256 private constant _NOT_ENTERED = 1; | ||
|
||
/// @notice The second out of two values to which the `_reentrancyStatus` state variable (used by the `nonReentrant` modifier) can be set indicating that a function was entered. | ||
uint256 private constant _ENTERED = 2; | ||
|
||
/// @notice Thrown if the action array length is larger than `MAX_ACTIONS`. | ||
error TooManyActions(); | ||
|
||
/// @notice Thrown if an action has insufficient gas left. | ||
error InsufficientGas(); | ||
|
||
/// @notice Thrown if action execution has failed. | ||
/// @param index The index of the action in the action array that failed. | ||
error ActionFailed(uint256 index); | ||
|
||
/// @notice Thrown if a call is reentrant. | ||
error ReentrantCall(); | ||
|
||
constructor() { | ||
_storeReentrancyStatus(_NOT_ENTERED); | ||
} | ||
|
||
modifier nonReentrant() { | ||
if (_getReentrancyStatus() == _ENTERED) { | ||
revert ReentrantCall(); | ||
} | ||
|
||
_storeReentrancyStatus(_ENTERED); | ||
|
||
_; | ||
|
||
_storeReentrancyStatus(_NOT_ENTERED); | ||
} | ||
|
||
/// @notice Checks if this or the parent contract supports an interface by its ID. | ||
/// @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(IExecutor).interfaceId || super.supportsInterface(_interfaceId); | ||
} | ||
|
||
/// @inheritdoc IExecutor | ||
function execute( | ||
bytes32 _callId, | ||
Action[] memory _actions, | ||
uint256 _allowFailureMap | ||
) | ||
public | ||
virtual | ||
override | ||
nonReentrant | ||
returns (bytes[] memory execResults, uint256 failureMap) | ||
{ | ||
// Check that the action array length is within bounds. | ||
if (_actions.length > MAX_ACTIONS) { | ||
revert TooManyActions(); | ||
} | ||
|
||
execResults = new bytes[](_actions.length); | ||
|
||
uint256 gasBefore; | ||
uint256 gasAfter; | ||
|
||
for (uint256 i = 0; i < _actions.length; ) { | ||
gasBefore = gasleft(); | ||
|
||
(bool success, bytes memory data) = _actions[i].to.call{value: _actions[i].value}( | ||
_actions[i].data | ||
); | ||
|
||
gasAfter = gasleft(); | ||
|
||
// Check if failure is allowed | ||
if (!success) { | ||
if (!hasBit(_allowFailureMap, uint8(i))) { | ||
revert ActionFailed(i); | ||
} | ||
|
||
// Make sure that the action call did not fail because 63/64 of `gasleft()` was insufficient to execute the external call `.to.call` (see [ERC-150](https://eips.ethereum.org/EIPS/eip-150)). | ||
// In specific scenarios, i.e. proposal execution where the last action in the action array is allowed to fail, the account calling `execute` could force-fail this action by setting a gas limit | ||
// where 63/64 is insufficient causing the `.to.call` to fail, but where the remaining 1/64 gas are sufficient to successfully finish the `execute` call. | ||
if (gasAfter < gasBefore / 64) { | ||
revert InsufficientGas(); | ||
} | ||
// Store that this action failed. | ||
failureMap = flipBit(failureMap, uint8(i)); | ||
} | ||
|
||
execResults[i] = data; | ||
|
||
unchecked { | ||
++i; | ||
} | ||
} | ||
|
||
emit Executed({ | ||
actor: msg.sender, | ||
callId: _callId, | ||
actions: _actions, | ||
allowFailureMap: _allowFailureMap, | ||
failureMap: failureMap, | ||
execResults: execResults | ||
}); | ||
} | ||
|
||
/// @notice Gets the current reentrancy status. | ||
/// @return status This returns the current reentrancy status. | ||
function _getReentrancyStatus() private view returns (uint256 status) { | ||
// solhint-disable-next-line no-inline-assembly | ||
assembly { | ||
status := sload(REENTRANCY_GUARD_STORAGE_LOCATION) | ||
} | ||
} | ||
|
||
/// @notice Stores the reentrancy status on a specific slot. | ||
function _storeReentrancyStatus(uint256 _status) private { | ||
// solhint-disable-next-line no-inline-assembly | ||
assembly { | ||
sstore(REENTRANCY_GUARD_STORAGE_LOCATION, _status) | ||
} | ||
} | ||
} |
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,48 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
|
||
/// @notice The action struct to be consumed by the DAO's `execute` function resulting in an external call. | ||
/// @param to The address to call. | ||
/// @param value The native token value to be sent with the call. | ||
/// @param data The bytes-encoded function selector and calldata for the call. | ||
struct Action { | ||
address to; | ||
uint256 value; | ||
bytes data; | ||
} | ||
|
||
/// @title IExecutor | ||
/// @author Aragon X - 2022-2024 | ||
/// @notice The interface required for Executors within the Aragon App DAO framework. | ||
/// @custom:security-contact [email protected] | ||
interface IExecutor { | ||
/// @notice Emitted when a proposal is executed. | ||
/// @param actor The address of the caller. | ||
/// @param callId The ID of the call. | ||
/// @param actions The array of actions executed. | ||
/// @param allowFailureMap The allow failure map encoding which actions are allowed to fail. | ||
/// @param failureMap The failure map encoding which actions have failed. | ||
/// @param execResults The array with the results of the executed actions. | ||
/// @dev The value of `callId` is defined by the component/contract calling the execute function. A `Plugin` implementation can use it, for example, as a nonce. | ||
event Executed( | ||
address indexed actor, | ||
bytes32 callId, | ||
Action[] actions, | ||
uint256 allowFailureMap, | ||
uint256 failureMap, | ||
bytes[] execResults | ||
); | ||
|
||
/// @notice Executes a list of actions. If a zero allow-failure map is provided, a failing action reverts the entire execution. If a non-zero allow-failure map is provided, allowed actions can fail without the entire call being reverted. | ||
/// @param _callId The ID of the call. The definition of the value of `callId` is up to the calling contract and can be used, e.g., as a nonce. | ||
/// @param _actions The array of actions. | ||
/// @param _allowFailureMap A bitmap allowing execution to succeed, even if individual actions might revert. If the bit at index `i` is 1, the execution succeeds even if the `i`th action reverts. A failure map value of 0 requires every action to not revert. | ||
/// @return The array of results obtained from the executed actions in `bytes`. | ||
/// @return The resulting failure map containing the actions have actually failed. | ||
function execute( | ||
bytes32 _callId, | ||
Action[] memory _actions, | ||
uint256 _allowFailureMap | ||
) external returns (bytes[] memory, uint256); | ||
} |
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 |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
|
||
import {IExecutor, Action} from "../../executors/Executor.sol"; | ||
|
||
/// @notice A dummy contract to test if Executor can successfully execute an action. | ||
contract ActionExecute { | ||
uint256 internal _num = 10; | ||
|
||
function setTest(uint256 newNum) public returns (uint256) { | ||
_num = newNum; | ||
return _num; | ||
} | ||
|
||
function fail() public pure { | ||
// solhint-disable-next-line reason-string, custom-errors | ||
revert("ActionExecute:Revert"); | ||
} | ||
|
||
// Used to test custom reentrancy guard | ||
// that is implemented on Executor contract's | ||
// execute function. | ||
function callBackCaller() public { | ||
IExecutor(msg.sender).execute(bytes32(0), new Action[](0), 0); | ||
} | ||
} |
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,15 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
|
||
/// @notice This contract is used for testing to consume gas. | ||
contract GasConsumer { | ||
// solhint-disable-next-line named-parameters-mapping | ||
mapping(uint256 => uint256) public store; | ||
|
||
function consumeGas(uint256 count) external { | ||
for (uint256 i = 0; i < count; i++) { | ||
store[i] = 1; | ||
} | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
contracts/src/mocks/permission/condition/extensions/RuledConditionMock.sol
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,25 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
import {RuledCondition} from "../../../../permission/condition/extensions/RuledCondition.sol"; | ||
import {DaoAuthorizableUpgradeable} from "../../../../permission/auth/DaoAuthorizableUpgradeable.sol"; | ||
|
||
/// @notice A mock powerful condition to expose internal functions | ||
/// @dev DO NOT USE IN PRODUCTION! | ||
contract RuledConditionMock is DaoAuthorizableUpgradeable, RuledCondition { | ||
function updateRules(Rule[] memory _rules) public virtual { | ||
_updateRules(_rules); | ||
} | ||
|
||
function isGranted( | ||
address _where, | ||
address _who, | ||
bytes32 _permissionId, | ||
bytes calldata data | ||
) external view override returns (bool isPermitted) { | ||
uint256[] memory _compareList = data.length == 0 | ||
? new uint256[](0) | ||
: abi.decode(data, (uint256[])); | ||
return _evalRule(0, _where, _who, _permissionId, _compareList); | ||
} | ||
} |
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,26 @@ | ||
// SPDX-License-Identifier: AGPL-3.0-or-later | ||
|
||
pragma solidity ^0.8.8; | ||
|
||
import {IExecutor, Action} from "../../executors/IExecutor.sol"; | ||
|
||
/// @notice A mock DAO that anyone can set permissions in. | ||
/// @dev DO NOT USE IN PRODUCTION! | ||
contract CustomExecutorMock is IExecutor { | ||
error Failed(); | ||
|
||
function execute( | ||
bytes32 callId, | ||
Action[] memory _actions, | ||
uint256 allowFailureMap | ||
) external override 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); | ||
} | ||
} | ||
} |
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.
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.
We're already using
InsufficientGas
name in DAO.sol so better to stick with this name.