Skip to content

Commit

Permalink
feat: GovernorExecutor as default inheritance for governor
Browse files Browse the repository at this point in the history 
removed duplicate code, downgraded abstraction
improvement of 1% DeployC in GovernorContract (5728315)
#11
  • Loading branch information
@DuBento
DuBento committed Jul 25, 2023
1 parent 211dec4 commit de7a38c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 95 deletions.
26 changes: 2 additions & 24 deletions blockchain/contracts/DAO/GovernorContract.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ 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
Governor,
GovernorSettings,
GovernorCountingSimple,
GovernorVotes,
GovernorVotesQuorumFraction,
GovernorExecutor
GovernorVotesQuorumFraction
{
constructor(
IVotes token_,
Expand All @@ -23,11 +21,10 @@ contract GovernorContract is
uint256 votingPeriod_,
uint256 quorumFraction_
)
Governor("GovernorContract")
Governor("GovernorContract", executor_)
GovernorSettings(votingDelay_, votingPeriod_, 0)
GovernorVotes(token_)
GovernorVotesQuorumFraction(quorumFraction_)
GovernorExecutor(executor_)
{}

// The following functions are overrides required by Solidity.
Expand Down Expand Up @@ -69,23 +66,4 @@ 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();
}
}
72 changes: 6 additions & 66 deletions blockchain/contracts/DAO/governance/Governor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import "../../OpenZeppelin/utils/introspection/ERC165.sol";
import "../../OpenZeppelin/utils/math/SafeCast.sol";
import "../../OpenZeppelin/utils/Address.sol";
import "../../OpenZeppelin/utils/Context.sol";
import "./extensions/GovernorExecutor.sol";
import "./IGovernor.sol";

/**
Expand All @@ -25,6 +26,7 @@ import "./IGovernor.sol";
* Modified version of governance contracts from OpenZeppelin v4.9.1
*/
abstract contract Governor is
GovernorExecutor,
Context,
ERC165,
EIP712,
Expand Down Expand Up @@ -81,7 +83,10 @@ abstract contract Governor is
/**
* @dev Sets the value for {name} and {version}
*/
constructor(string memory name_) EIP712(name_, version()) {
constructor(
string memory name_,
Executor executor_
) EIP712(name_, version()) GovernorExecutor(executor_) {
_name = name_;
}

Expand Down Expand Up @@ -440,62 +445,6 @@ abstract contract Governor is
return _cancel(targets, values, calldatas, descriptionHash);
}

/**
* @dev Internal execution mechanism. Can be overridden to implement different execution mechanism
*/
function _execute(
uint256 /* proposalId */,
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 /*descriptionHash*/
) internal virtual {
for (uint256 i = 0; i < targets.length; ++i) {
(bool success, bytes memory returndata) = targets[i].call{
value: values[i]
}(calldatas[i]);
Address.verifyCallResult(success, returndata);
}
}

// 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
* canceled to allow distinguishing it from executed proposals.
Expand Down Expand Up @@ -719,15 +668,6 @@ 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.
*/
function _executor() internal view virtual returns (address) {
return address(this);
}

/**
* @dev See {IERC721Receiver-onERC721Received}.
* Receiving tokens is disabled if the governance executor is other than the governor itself (eg. when using with a timelock).
Expand Down
11 changes: 6 additions & 5 deletions blockchain/contracts/DAO/governance/extensions/GovernorExecutor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,15 @@

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 {
abstract contract GovernorExecutor {
Executor private _executorContract;

// Type declarations
Expand Down Expand Up @@ -45,7 +43,7 @@ abstract contract GovernorExecutor is Governor, Ownable {
/**
* @dev Address through which the governor executes action. In this case, the timelock.
*/
function _executor() internal view virtual override returns (address) {
function _executor() internal view virtual returns (address) {
return address(_executorContract);
}

Expand All @@ -58,7 +56,7 @@ abstract contract GovernorExecutor is Governor, Ownable {
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) internal virtual override {
) internal virtual {
// execute
_executorContract.executeBatch{value: msg.value}(
targets,
Expand All @@ -69,6 +67,9 @@ abstract contract GovernorExecutor is Governor, Ownable {
);
}

/**
* @dev Update executor contract address.
*/
function _updateExecutor(Executor newExecutor) internal {
emit ExecutorChange(address(_executorContract), address(newExecutor));
_executorContract = newExecutor;
Expand Down

0 comments on commit de7a38c

Please sign in to comment.