diff --git a/blockchain/contracts/DAO/Executor.sol b/blockchain/contracts/DAO/Executor.sol new file mode 100644 index 0000000..66ed271 --- /dev/null +++ b/blockchain/contracts/DAO/Executor.sol @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 +// Based on Openzeppelin Contracts (last updated v4.9.0) + +pragma solidity ^0.8.19; + +import "../custom/Ownable.sol"; +import "../OpenZeppelin/utils/Address.sol"; + +contract Executor is Ownable { + // Type declarations + // State variables + bool private locked; + + // Events + + /** + * @dev Emitted when a call is performed as part of operation `id`. + */ + event CallExecuted( + bytes32 indexed id, + uint256 indexed index, + address target, + uint256 value, + bytes data + ); + + // Errors + /** + * @dev Prevent reentrant calls. + */ + error ReentrantCall(); + + /** + * @dev Mismatch between the parameters length for an operation call. + */ + error ExecutorInvalidOperationLength( + uint256 targets, + uint256 payloads, + uint256 values + ); + + // Modifiers + modifier nonReentrant() { + if (locked) revert ReentrantCall(); + locked = true; + _; + locked = false; + } + + // Functions + //* constructor + //* receive function + //* fallback function (if exists) + //* external + //* public + /** + * @dev Execute an (ready) operation containing a single transaction. + * + * Emits a {CallExecuted} event. + * + * Only owner can execute, usually Governor Contract. + */ + function execute( + address target, + uint256 value, + bytes calldata payload, + bytes32 predecessor, + bytes32 salt + ) public payable virtual nonReentrant onlyOwner { + bytes32 id = hashOperation(target, value, payload, predecessor, salt); + + _execute(target, value, payload); + emit CallExecuted(id, 0, target, value, payload); + } + + /** + * @dev Execute an (ready) operation containing a batch of transactions. + * + * Emits one {CallExecuted} event per transaction in the batch. + * + * Only owner can execute, usually Governor Contract. + */ + function executeBatch( + address[] calldata targets, + uint256[] calldata values, + bytes[] calldata payloads, + bytes32 predecessor, + bytes32 salt + ) public payable virtual nonReentrant onlyOwner { + if ( + targets.length != values.length || targets.length != payloads.length + ) { + revert ExecutorInvalidOperationLength( + targets.length, + payloads.length, + values.length + ); + } + + bytes32 id = hashOperationBatch( + targets, + values, + payloads, + predecessor, + salt + ); + + for (uint256 i = 0; i < targets.length; ++i) { + address target = targets[i]; + uint256 value = values[i]; + bytes calldata payload = payloads[i]; + _execute(target, value, payload); + emit CallExecuted(id, i, target, value, payload); + } + } + + /** + * @dev Returns the identifier of an operation containing a single + * transaction. + */ + function hashOperation( + address target, + uint256 value, + bytes calldata data, + bytes32 predecessor, + bytes32 salt + ) public pure virtual returns (bytes32) { + return keccak256(abi.encode(target, value, data, predecessor, salt)); + } + + /** + * @dev Returns the identifier of an operation containing a batch of + * transactions. + */ + function hashOperationBatch( + address[] calldata targets, + uint256[] calldata values, + bytes[] calldata payloads, + bytes32 predecessor, + bytes32 salt + ) public pure virtual returns (bytes32) { + return + keccak256(abi.encode(targets, values, payloads, predecessor, salt)); + } + + //* internal + /** + * @dev Execute an operation's call. + */ + function _execute( + address target, + uint256 value, + bytes calldata data + ) internal virtual { + (bool success, bytes memory returndata) = target.call{value: value}( + data + ); + Address.verifyCallResult(success, returndata); + } + //* private + //* asserts +} diff --git a/blockchain/contracts/DAO/GovernorContract.sol b/blockchain/contracts/DAO/GovernorContract.sol index 3d2185c..32fa02f 100644 --- a/blockchain/contracts/DAO/GovernorContract.sol +++ b/blockchain/contracts/DAO/GovernorContract.sol @@ -5,6 +5,7 @@ import "./governance/extensions/GovernorCountingSimple.sol"; import "./governance/extensions/GovernorVotes.sol"; import "./governance/extensions/GovernorVotesQuorumFraction.sol"; import "./governance/extensions/GovernorSettings.sol"; +import "./governance/extensions/GovernorExecutor.sol"; import "./governance/Governor.sol"; contract GovernorContract is @@ -13,17 +14,20 @@ contract GovernorContract is GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, + GovernorExecutor { constructor( - IVotes _token, - uint256 _votingDelay, - uint256 _votingPeriod, - uint256 _quorumFraction + IVotes token_, + Executor executor_, + uint256 votingDelay_, + uint256 votingPeriod_, + uint256 quorumFraction_ ) Governor("GovernorContract") - GovernorSettings(_votingDelay, _votingPeriod, 0) - GovernorVotes(_token) - GovernorVotesQuorumFraction(_quorumFraction) + GovernorSettings(votingDelay_, votingPeriod_, 0) + GovernorVotes(token_) + GovernorVotesQuorumFraction(quorumFraction_) + GovernorExecutor(executor_) {} // The following functions are overrides required by Solidity. @@ -65,4 +69,23 @@ contract GovernorContract is { return super.proposalThreshold(); } + + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorExecutor) { + super._execute(proposalId, targets, values, calldatas, descriptionHash); + } + + function _executor() + internal + view + override(Governor, GovernorExecutor) + returns (address) + { + return super._executor(); + } } diff --git a/blockchain/contracts/DAO/governance/Governor.sol b/blockchain/contracts/DAO/governance/Governor.sol index 7338926..5ab3f79 100644 --- a/blockchain/contracts/DAO/governance/Governor.sol +++ b/blockchain/contracts/DAO/governance/Governor.sol @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v4.9.1) (governance/Governor.sol) +// SPDX-License-Identifier: Apache-2.0 +// Based on Openzeppelin Contracts (last updated v4.9.1) pragma solidity ^0.8.19; @@ -12,6 +12,7 @@ import "../../OpenZeppelin/utils/math/SafeCast.sol"; import "../../OpenZeppelin/utils/Address.sol"; import "../../OpenZeppelin/utils/Context.sol"; import "./IGovernor.sol"; + /** * @dev Core of the governance system, designed to be extended though various modules. * @@ -21,7 +22,7 @@ import "./IGovernor.sol"; * - A voting module must implement {_getVotes} * - Additionally, {votingPeriod} must also be implemented * - * _Available since v4.3._ + * Modified version of governance contracts from OpenZeppelin v4.9.1 */ abstract contract Governor is Context, @@ -31,8 +32,6 @@ abstract contract Governor is IERC721Receiver, IERC1155Receiver { - using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque; - bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)"); bytes32 public constant EXTENDED_BALLOT_TYPEHASH = @@ -62,12 +61,6 @@ abstract contract Governor is /// @custom:oz-retyped-from mapping(uint256 => Governor.ProposalCore) mapping(uint256 => ProposalCore) private _proposals; - // This queue keeps track of the governor operating on itself. Calls to functions protected by the - // {onlyGovernance} modifier needs to be whitelisted in this queue. Whitelisting is set in {_beforeExecute}, - // consumed by the {onlyGovernance} modifier and eventually reset in {_afterExecute}. This ensures that the - // execution of {onlyGovernance} protected calls can only be achieved through successful proposals. - DoubleEndedQueue.Bytes32Deque private _governanceCall; - /** * @dev Restricts a function so it can only be executed through governance proposals. For example, governance * parameter setters in {GovernorSettings} are protected using this modifier. @@ -82,11 +75,6 @@ abstract contract Governor is if (_executor() != _msgSender()) { revert GovernorOnlyExecutor(_msgSender()); } - if (_executor() != address(this)) { - bytes32 msgDataHash = keccak256(_msgData()); - // loop until popping the expected operation - throw if deque is empty (operation not authorized) - while (_governanceCall.popFront() != msgDataHash) {} - } _; } @@ -418,9 +406,7 @@ abstract contract Governor is emit ProposalExecuted(proposalId); - _beforeExecute(proposalId, targets, values, calldatas, descriptionHash); _execute(proposalId, targets, values, calldatas, descriptionHash); - _afterExecute(proposalId, targets, values, calldatas, descriptionHash); return proposalId; } @@ -472,41 +458,43 @@ abstract contract Governor is } } - /** - * @dev Hook before execution is triggered. - */ - function _beforeExecute( - uint256 /* proposalId */, - address[] memory targets, - uint256[] memory /* values */, - bytes[] memory calldatas, - bytes32 /*descriptionHash*/ - ) internal virtual { - if (_executor() != address(this)) { - for (uint256 i = 0; i < targets.length; ++i) { - if (targets[i] == address(this)) { - _governanceCall.pushBack(keccak256(calldatas[i])); - } - } - } - } - - /** - * @dev Hook after execution is triggered. - */ - function _afterExecute( - uint256 /* proposalId */, - address[] memory /* targets */, - uint256[] memory /* values */, - bytes[] memory /* calldatas */, - bytes32 /*descriptionHash*/ - ) internal virtual { - if (_executor() != address(this)) { - if (!_governanceCall.empty()) { - _governanceCall.clear(); - } - } - } + // TODO remove + + // /** + // * @dev Hook before execution is triggered. + // */ + // function _beforeExecute( + // uint256 /* proposalId */, + // address[] memory targets, + // uint256[] memory /* values */, + // bytes[] memory calldatas, + // bytes32 /*descriptionHash*/ + // ) internal virtual { + // if (_executor() != address(this)) { + // for (uint256 i = 0; i < targets.length; ++i) { + // if (targets[i] == address(this)) { + // _governanceCall.pushBack(keccak256(calldatas[i])); + // } + // } + // } + // } + + // /** + // * @dev Hook after execution is triggered. + // */ + // function _afterExecute( + // uint256 /* proposalId */, + // address[] memory /* targets */, + // uint256[] memory /* values */, + // bytes[] memory /* calldatas */, + // bytes32 /*descriptionHash*/ + // ) internal virtual { + // if (_executor() != address(this)) { + // if (!_governanceCall.empty()) { + // _governanceCall.clear(); + // } + // } + // } /** * @dev Internal cancel mechanism: locks up the proposal timer, preventing it from being re-submitted. Marks it as @@ -731,6 +719,7 @@ abstract contract Governor is Address.verifyCallResult(success, returndata); } + // TODO remove /** * @dev Address through which the governor executes action. Will be overloaded by module that execute actions * through another contract such as a timelock. diff --git a/blockchain/contracts/DAO/governance/extensions/GovernorExecutor.sol b/blockchain/contracts/DAO/governance/extensions/GovernorExecutor.sol new file mode 100644 index 0000000..dd6d362 --- /dev/null +++ b/blockchain/contracts/DAO/governance/extensions/GovernorExecutor.sol @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 +// Based on Openzeppelin Contracts (last updated v4.9.0) + +pragma solidity ^0.8.19; + +import "../../../custom/Ownable.sol"; +import "../../Executor.sol"; +import "../Governor.sol"; + +/** + * @dev Extension of {Governor} that binds the execution process to an instance of {Executer}. + * + * Using this model means the proposal will be operated by the {Executor} and not by the {Governor}. Thus, + * the assets and permissions must be attached to the {Executor} + */ +abstract contract GovernorExecutor is Governor, Ownable { + Executor private _executorContract; + + // Type declarations + // State variables + // Events + /** + * @dev Emitted when the executor used for proposal execution is modified. + */ + event ExecutorChange(address oldExecutor, address newExecutor); + + // Errors + // Modifiers + // Functions + //* constructor + + /** + * @dev Set the executor. + */ + constructor(Executor executorAddress) { + _executorContract = executorAddress; + } + + //* receive function + //* fallback function (if exists) + //* external + //* public + //* internal + + /** + * @dev Address through which the governor executes action. In this case, the timelock. + */ + function _executor() internal view virtual override returns (address) { + return address(_executorContract); + } + + /** + * @dev Execute function that run the ready proposal through the executor. + */ + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal virtual override { + // execute + _executorContract.executeBatch{value: msg.value}( + targets, + values, + calldatas, + 0, + descriptionHash + ); + } + + function _updateExecutor(Executor newExecutor) internal { + emit ExecutorChange(address(_executorContract), address(newExecutor)); + _executorContract = newExecutor; + } + + //* private + //* asserts +} diff --git a/blockchain/deploy/02_executor.ts b/blockchain/deploy/02_executor.ts new file mode 100644 index 0000000..bbc1bdd --- /dev/null +++ b/blockchain/deploy/02_executor.ts @@ -0,0 +1,25 @@ +import { DeployFunction } from "hardhat-deploy/types"; +import { HardhatRuntimeEnvironment } from "hardhat/types"; + +import { padCenter, scriptName } from "../lib/utils"; + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { getNamedAccounts, deployments } = hre; + const { deploy, log, get } = deployments; + const { deployer } = await getNamedAccounts(); + + log(padCenter(scriptName(__filename), 50)); + log("Deploying Executor..."); + + const executor = await deploy("Executor", { + from: deployer, + args: [], + log: true, + // TODO verify if live on network + }); + + log(`Executor at ${executor.address}`); +}; + +module.exports = func; +module.exports.tags = ["all", "dao", "executor"]; diff --git a/blockchain/deploy/02_governor.ts b/blockchain/deploy/03_governor.ts similarity index 83% rename from blockchain/deploy/02_governor.ts rename to blockchain/deploy/03_governor.ts index 872f3a3..a8ced9f 100644 --- a/blockchain/deploy/02_governor.ts +++ b/blockchain/deploy/03_governor.ts @@ -10,13 +10,20 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { const { deployer } = await getNamedAccounts(); const governorToken = await get("GovernorToken"); + const executor = await get("Executor"); log(padCenter(scriptName(__filename), 50)); log("Deploying GovernorContract..."); const governorContract = await deploy("GovernorContract", { from: deployer, - args: [governorToken.address, VOTING_DELAY, VOTING_PERIOD, VOTING_QUORUM], + args: [ + governorToken.address, + executor.address, + VOTING_DELAY, + VOTING_PERIOD, + VOTING_QUORUM, + ], log: true, // TODO verify if live on network }); diff --git a/blockchain/deploy/91_setup_governance.ts b/blockchain/deploy/91_setup_governance.ts index 7bffe19..834b35f 100644 --- a/blockchain/deploy/91_setup_governance.ts +++ b/blockchain/deploy/91_setup_governance.ts @@ -1,6 +1,7 @@ import { DeployFunction } from "hardhat-deploy/types"; import { HardhatRuntimeEnvironment } from "hardhat/types"; import { + Executor, GovernorContract, GovernorToken, SupplychainFactory, @@ -17,22 +18,36 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { const { deployer } = await getNamedAccounts(); log = deployments.log; - const governorContractDeployment = await get("GovernorContract"); + const executorContract = await utils.getContract("Executor", { + signerAddress: deployer, + }); + + log(padCenter(scriptName(__filename), 50)); + + await Promise.all([ + setupExecutor(executorContract, deployer), + setupGovernorToken(executorContract, deployer), + setupUserRegistry(executorContract, deployer), + setupSupplychainFactory(executorContract, deployer), + ]); +}; + +const setupExecutor = async function ( + executorContract: Executor, + deployer: string +) { + log("Setting up Executor..."); const governorContract = await utils.getContract( "GovernorContract", { signerAddress: deployer } ); - log(padCenter(scriptName(__filename), 50)); - - await setupGovernorToken(governorContract, deployer); - await setupUserRegistry(governorContract, deployer); - await setupSupplychainFactory(governorContract, deployer); + executorContract.transferOwnership(await governorContract.getAddress()); }; const setupGovernorToken = async function ( - governorContract: GovernorContract, + executorContract: Executor, deployer: string ) { log("Setting up GovernorToken..."); @@ -47,7 +62,7 @@ const setupGovernorToken = async function ( // Transfer owner to timelock const transferOwnershipTx = await governorTokenContract.transferOwnership( - await governorContract.getAddress() + await executorContract.getAddress() ); await transferOwnershipTx.wait(); }; @@ -67,7 +82,7 @@ const delegateVotingPower = async function ( }; const setupUserRegistry = async function ( - governorContract: GovernorContract, + executorContract: Executor, deployer: string ) { log("Setting up UserRegistry..."); @@ -79,13 +94,13 @@ const setupUserRegistry = async function ( ); const transferOwnershipTx = await userRegistryContract.transferOwnership( - await governorContract.getAddress() + await executorContract.getAddress() ); await transferOwnershipTx.wait(); }; const setupSupplychainFactory = async function ( - governorContract: GovernorContract, + executorContract: Executor, deployer: string ) { log("Setting up SupplychainFactory..."); @@ -98,7 +113,7 @@ const setupSupplychainFactory = async function ( const transferOwnershipTx = await supplychainFactoryContract.transferOwnership( - await governorContract.getAddress() + await executorContract.getAddress() ); await transferOwnershipTx.wait(); }; diff --git a/blockchain/lib/execute.ts b/blockchain/lib/execute.ts index 817be90..3d67fdc 100644 --- a/blockchain/lib/execute.ts +++ b/blockchain/lib/execute.ts @@ -16,7 +16,6 @@ export async function execute( ); console.log("Executing..."); - // this will fail on a testnet because you need to wait for the MIN_DELAY! const executeTx = await governor.execute( [proposalTarget], [0], diff --git a/blockchain/lib/vote.ts b/blockchain/lib/vote.ts index f2b034c..342ecf5 100644 --- a/blockchain/lib/vote.ts +++ b/blockchain/lib/vote.ts @@ -1,3 +1,4 @@ +import { EventLog } from "ethers"; import { getChainId, network } from "hardhat"; import { GovernorContract } from "../artifacts-frontend/typechain"; import * as utils from "../lib/utils"; @@ -15,14 +16,28 @@ export async function vote( { signerAddress } ); + let proposalState = await governor.state(proposalId); + console.log(`Before - Proposal State: ${proposalState}`); + const voteTx = await governor.castVoteWithReason( proposalId, decision, reason ); - await voteTx.wait(); + const receipt = await voteTx.wait(); + + const events = receipt!.logs.filter( + (log) => log instanceof EventLog + ) as EventLog[]; - console.log(`Proposed with proposal ID:\n ${proposalId}`); + console.log( + `Events reveived: ${JSON.stringify(events.map((event) => event.eventName))}` + ); + console.log( + `Events args: ${JSON.stringify( + events.map((event) => event.args.toString()) + )}` + ); // Moving forward to the end of the voting period if (DEVELOPMENT_CHAINS.includes(network.name)) { @@ -30,8 +45,8 @@ export async function vote( } // Check the proposal state - const proposalState = await governor.state(proposalId); - console.log(`Current Proposal State: ${proposalState}`); + proposalState = await governor.state(proposalId); + console.log(`After - Proposal State: ${proposalState}`); return proposalState; } diff --git a/blockchain/properties.ts b/blockchain/properties.ts index 3fa4655..ce04fa6 100644 --- a/blockchain/properties.ts +++ b/blockchain/properties.ts @@ -19,7 +19,6 @@ export const CONTRACT_ADDRESS_FILE = path.join( export const PROPOSALS_FILE = "proposals.json"; // governor contract -export const MIN_DELAY = 3600; // 1 hour export const VOTING_DELAY = 1; // 1 block export const VOTING_PERIOD = 5; // 5 blocks export const VOTING_QUORUM = 4; // porcentage diff --git a/blockchain/test/GovernorTest.ts b/blockchain/test/GovernorTest.ts index 5127ba1..f3212e6 100644 --- a/blockchain/test/GovernorTest.ts +++ b/blockchain/test/GovernorTest.ts @@ -1,6 +1,7 @@ import { expect } from "chai"; import { deployments, ethers, getNamedAccounts } from "hardhat"; import { + Executor, GovernorContract, GovernorToken, Supplychain, @@ -34,6 +35,7 @@ import { describe("Governor", function () { let governorContract: GovernorContract; let governorToken: GovernorToken; + let executor: Executor; let supplychainFactory: SupplychainFactory; let userRegistry: UserRegistry; @@ -42,6 +44,7 @@ describe("Governor", function () { governorContract = await utils.getContract( "GovernorContract" ); + executor = await utils.getContract("Executor"); governorToken = await utils.getContract("GovernorToken"); supplychainFactory = await utils.getContract( "SupplychainFactory" @@ -50,16 +53,17 @@ describe("Governor", function () { console.log(`##### GovernorContract: ${await governorContract.getAddress()} + Executor: ${await executor.getAddress()} GovernorToken: ${await governorToken.getAddress()} SupplychainFactory: ${await supplychainFactory.getAddress()} UserRegistry: ${await userRegistry.getAddress()} #####`); }); - it("Succesfully deploys and Timelock is the correct owner", async function () { + it("Succesfully deploys and Executor contract is the correct owner", async function () { // used to show gas report for deplpoyment expect(await supplychainFactory.owner()).to.equal( - await governorContract.getAddress() + await executor.getAddress() ); }); @@ -261,7 +265,7 @@ describe("Governor", function () { it("Propose, vote and execute a new supplychain contract", async function () { expect(await supplychainContractAsManager.owner()).to.equal( - await governorContract.getAddress() + await executor.getAddress() ); });