Skip to content

Commit

Permalink
roll back to instead of
Browse files Browse the repository at this point in the history
  • Loading branch information
novaknole committed Oct 1, 2024
1 parent d86d779 commit 23998dc
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 23 deletions.
22 changes: 10 additions & 12 deletions packages/contracts/src/Admin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {ProposalUpgradeable} from "@aragon/osx-commons-contracts/src/plugin/exte
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 {IExecutor, Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";
import {Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";

/// @title Admin
/// @author Aragon X - 2022-2023
Expand All @@ -22,14 +22,12 @@ 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.execute.selector;
bytes4 internal constant ADMIN_INTERFACE_ID = this.executeProposal.selector;

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

error NotAllowedOperation();

/// @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).
Expand Down Expand Up @@ -96,15 +94,15 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
uint64,
uint64,
bytes memory _data
) public override returns (uint256) {
) public override returns (uint256 proposalId) {
uint256 allowFailureMap;

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

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

/// @inheritdoc IProposal
Expand All @@ -118,14 +116,14 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
/// @param _allowFailureMap A bitmap allowing the proposal to succeed, even if individual actions might revert.
/// If the bit at index `i` is 1, the proposal succeeds even if the `i`th action reverts. A failure map value
// of 0 requires every action to not revert.
function execute(
function executeProposal(
bytes calldata _metadata,
Action[] calldata _actions,
uint256 _allowFailureMap
) public auth(EXECUTE_PROPOSAL_PERMISSION_ID) {
uint64 currentTimestamp64 = block.timestamp.toUint64();
) public auth(EXECUTE_PROPOSAL_PERMISSION_ID) returns (uint256 proposalId) {
uint64 currentTimestamp = block.timestamp.toUint64();

uint256 proposalId = createProposalId(_actions, _metadata);
proposalId = createProposalId(_actions, _metadata);

TargetConfig memory targetConfig = getTargetConfig();

Expand All @@ -140,8 +138,8 @@ contract Admin is IMembership, PluginCloneable, ProposalUpgradeable {
emit ProposalCreated(
proposalId,
_msgSender(),
currentTimestamp64,
currentTimestamp64,
currentTimestamp,
currentTimestamp,
_metadata,
_actions,
_allowFailureMap
Expand Down
7 changes: 5 additions & 2 deletions packages/contracts/src/AdminSetup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,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 Down Expand Up @@ -59,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
14 changes: 7 additions & 7 deletions packages/contracts/test/10_unit-testing/11_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {

// Expect Alice's `execute` call to be reverted because she hasn't `EXECUTE_PROPOSAL_PERMISSION_ID` on the Admin plugin
await expect(
plugin.connect(alice).execute(dummyMetadata, dummyActions, 0)
plugin.connect(alice).executeProposal(dummyMetadata, dummyActions, 0)
)
.to.be.revertedWithCustomError(plugin, 'DaoUnauthorized')
.withArgs(
Expand Down Expand Up @@ -185,7 +185,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {

// Expect Alice's the `execute` call to be reverted because the Admin plugin hasn't `EXECUTE_PERMISSION_ID` on the DAO
await expect(
plugin.connect(alice).execute(dummyMetadata, dummyActions, 0)
plugin.connect(alice).executeProposal(dummyMetadata, dummyActions, 0)
)
.to.be.revertedWithCustomError(dao, 'Unauthorized')
.withArgs(
Expand Down Expand Up @@ -226,7 +226,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {

const tx = await plugin
.connect(alice)
.execute(dummyMetadata, dummyActions, allowFailureMap);
.executeProposal(dummyMetadata, dummyActions, allowFailureMap);

const eventName = plugin.interface.getEvent('ProposalCreated').name;
await expect(tx).to.emit(plugin, eventName);
Expand Down Expand Up @@ -269,7 +269,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {
);

await expect(
plugin.connect(alice).execute(dummyMetadata, dummyActions, 0)
plugin.connect(alice).executeProposal(dummyMetadata, dummyActions, 0)
)
.to.emit(plugin, plugin.interface.getEvent('ProposalExecuted').name)
.withArgs(currentExpectedProposalId);
Expand Down Expand Up @@ -308,7 +308,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {

const tx = await newPlugin
.connect(alice)
.execute(dummyMetadata, dummyActions, allowFailureMap);
.executeProposal(dummyMetadata, dummyActions, allowFailureMap);

const event = findEventTopicLog<DAOEvents.ExecutedEvent>(
await tx.wait(),
Expand Down Expand Up @@ -336,7 +336,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {

const tx = await newPlugin
.connect(alice)
.execute(newMetadata, dummyActions, 0);
.executeProposal(newMetadata, dummyActions, 0);

const event = findEventTopicLog<DAOEvents.ExecutedEvent>(
await tx.wait(),
Expand Down Expand Up @@ -420,7 +420,7 @@ describe(PLUGIN_CONTRACT_NAME, function () {
);

await expect(
plugin.connect(alice).execute(dummyMetadata, dummyActions, 1)
plugin.connect(alice).executeProposal(dummyMetadata, dummyActions, 1)
)
.to.emit(pluginMerged, 'ExecutedCustom')
.to.emit(pluginMerged, 'ProposalExecuted');
Expand Down
3 changes: 1 addition & 2 deletions packages/contracts/test/admin-constants.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {ethers} from 'hardhat';

export const ADMIN_INTERFACE = new ethers.utils.Interface([
'function initialize(address,tuple(address,uint8))',
'function execute(bytes,tuple(address,uint256,bytes)[],uint256)',
'function executeProposal(bytes,tuple(address,uint256,bytes)[],uint256)',
]);

// Permissions
Expand Down

0 comments on commit 23998dc

Please sign in to comment.