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

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 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
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
39 changes: 0 additions & 39 deletions contracts/src/dao/IDAO.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
134 changes: 134 additions & 0 deletions contracts/src/executors/Executor.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// SPDX-License-Identifier: AGPL-3.0-or-later

pragma solidity ^0.8.8;

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 {
Copy link

Choose a reason for hiding this comment

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

Shouldn't this contract be abstract?

/// @notice The internal constant storing the maximal action array length.
uint256 internal constant MAX_ACTIONS = 256;

// keccak256("osx-commons.storage.Executor")
bytes32 private constant ReentrancyGuardStorageLocation =
Copy link

Choose a reason for hiding this comment

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

Could this value be computed? We are doing this for permission ID's

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can but then assembly's sstore fails with:

TypeError: Only direct number constants and references to such constants are supported by inline assembly.

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();
Copy link

Choose a reason for hiding this comment

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

Suggested change
error InsufficientGas();
error InsufficientGasLeft();

Copy link
Contributor Author

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.


/// @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);
}

/// @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) {
assembly {
status := sload(ReentrancyGuardStorageLocation)
}
}

/// @notice Stores the reentrancy status on a specific slot.
function _storeReentrancyStatus(uint256 _status) private {
assembly {
sstore(ReentrancyGuardStorageLocation, _status)
}
}
}
48 changes: 48 additions & 0 deletions contracts/src/executors/IExecutor.sol
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);
}
3 changes: 2 additions & 1 deletion contracts/src/mocks/dao/DAOMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
pragma solidity ^0.8.8;

import {IDAO} from "../../dao/IDAO.sol";
import {IExecutor, Action} from "../../executors/IExecutor.sol";

/// @notice A mock DAO that anyone can set permissions in.
/// @dev DO NOT USE IN PRODUCTION!
contract DAOMock is IDAO {
contract DAOMock is IDAO, IExecutor {
bool public hasPermissionReturnValueMock;

function setHasPermissionReturnValueMock(bool _hasPermissionReturnValueMock) external {
Expand Down
27 changes: 27 additions & 0 deletions contracts/src/mocks/executors/ActionExecute.sol
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";
import "hardhat/console.sol";

/// @notice A dummy contract to test if Executor can successfully execute an action.
contract ActionExecute {
uint num = 10;

function setTest(uint newNum) public returns (uint) {
num = newNum;
return num;
}

function fail() public pure {
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);
}
}
14 changes: 14 additions & 0 deletions contracts/src/mocks/executors/GasConsumer.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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 {
mapping(uint256 => uint256) public store;

function consumeGas(uint256 count) external {
for (uint256 i = 0; i < count; i++) {
store[i] = 1;
}
}
}
27 changes: 27 additions & 0 deletions contracts/src/mocks/plugin/CustomExecutorMock.sol
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 {IDAO} from "../../dao/IDAO.sol";
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);
}
}
}
25 changes: 25 additions & 0 deletions contracts/src/mocks/plugin/PluginCloneableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity ^0.8.8;

import {PluginCloneable} from "../../plugin/PluginCloneable.sol";
import {IDAO} from "../../dao/IDAO.sol";
import {IExecutor, Action} from "../../executors/IExecutor.sol";

/// @notice A mock cloneable plugin to be deployed via the minimal proxy pattern.
/// v1.1 (Release 1, Build 1)
Expand All @@ -16,6 +17,30 @@ contract PluginCloneableMockBuild1 is PluginCloneable {
__PluginCloneable_init(_dao);
state1 = 1;
}

function execute(
uint256 _callId,
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,
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
Loading
Loading