Skip to content

Commit

Permalink
Merge branch 'master' into add-erc20-pausable-warning
Browse files Browse the repository at this point in the history
  • Loading branch information
ernestognw committed Jan 28, 2023
2 parents f1626de + bc6de21 commit 3ec0716
Show file tree
Hide file tree
Showing 21 changed files with 227 additions and 72 deletions.
5 changes: 5 additions & 0 deletions .changeset/five-ducks-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`UUPSUpgradeable.sol`: Change visibility to the functions `upgradeTo ` and `upgradeToAndCall ` from `external` to `public`.
5 changes: 5 additions & 0 deletions .changeset/five-poets-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`TimelockController`: Add the `CallSalt` event to emit on operation schedule.
5 changes: 5 additions & 0 deletions .changeset/flat-deers-end.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Governor`: add a public `cancel(uint256)` function.
14 changes: 14 additions & 0 deletions GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ Some other examples of automation are:
- Looking for common security vulnerabilities or errors in our code (eg. reentrancy analysis).
- Keeping dependencies up to date and monitoring for vulnerable dependencies.

## Pull requests

Pull requests are squash-merged to keep the `master` branch history clean. The title of the pull request becomes the commit message, so it should be written in a consistent format:

1) Begin with a capital letter.
2) Do not end with a period.
3) Write in the imperative: "Add feature X" and not "Adds feature X" or "Added feature X".

This repository does not follow conventional commits, so do not prefix the title with "fix:" or "feat:".

Work in progress pull requests should be submitted as Drafts and should not be prefixed with "WIP:".

Branch names don't matter, and commit messages within a pull request mostly don't matter either, although they can help the review process.

# Solidity Conventions

In addition to the official Solidity Style Guide we have a number of other conventions that must be followed.
Expand Down
30 changes: 27 additions & 3 deletions contracts/governance/Governor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
Timers.BlockNumber voteEnd;
bool executed;
bool canceled;
address proposer;
}

string private _name;
Expand Down Expand Up @@ -95,9 +96,11 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
return
interfaceId ==
(type(IGovernor).interfaceId ^
this.cancel.selector ^
this.castVoteWithReasonAndParams.selector ^
this.castVoteWithReasonAndParamsBySig.selector ^
this.getVotesWithParams.selector) ||
interfaceId == (type(IGovernor).interfaceId ^ this.cancel.selector) ||
interfaceId == type(IGovernor).interfaceId ||
interfaceId == type(IERC1155Receiver).interfaceId ||
super.supportsInterface(interfaceId);
Expand Down Expand Up @@ -248,8 +251,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
bytes[] memory calldatas,
string memory description
) public virtual override returns (uint256) {
address proposer = _msgSender();

require(
getVotes(_msgSender(), block.number - 1) >= proposalThreshold(),
getVotes(proposer, block.number - 1) >= proposalThreshold(),
"Governor: proposer votes below proposal threshold"
);

Expand All @@ -267,10 +272,11 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive

proposal.voteStart.setDeadline(snapshot);
proposal.voteEnd.setDeadline(deadline);
proposal.proposer = proposer;

emit ProposalCreated(
proposalId,
_msgSender(),
proposer,
targets,
values,
new string[](targets.length),
Expand Down Expand Up @@ -310,6 +316,15 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
return proposalId;
}

/**
* @dev See {IGovernor-cancel}.
*/
function cancel(uint256 proposalId) public virtual override {
require(state(proposalId) == ProposalState.Pending, "Governor: too late to cancel");
require(_msgSender() == _proposals[proposalId].proposer, "Governor: only proposer can cancel");
_cancel(proposalId);
}

/**
* @dev Internal execution mechanism. Can be overridden to implement different execution mechanism
*/
Expand Down Expand Up @@ -375,7 +390,16 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
bytes[] memory calldatas,
bytes32 descriptionHash
) internal virtual returns (uint256) {
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
return _cancel(hashProposal(targets, values, calldatas, descriptionHash));
}

/**
* @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.
*
* Emits a {IGovernor-ProposalCanceled} event.
*/
function _cancel(uint256 proposalId) internal virtual returns (uint256) {
ProposalState status = state(proposalId);

require(
Expand Down
8 changes: 8 additions & 0 deletions contracts/governance/IGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ abstract contract IGovernor is IERC165 {
bytes32 descriptionHash
) public payable virtual returns (uint256 proposalId);

/**
* @dev Cancel a proposal. This is restricted to Pending proposal (before the vote starts) and is restricted to
* the proposal's proposer.
*
* Emits a {ProposalCanceled} event.
*/
function cancel(uint256 proposalId) public virtual;

/**
* @dev Cast a vote
*
Expand Down
15 changes: 13 additions & 2 deletions contracts/governance/TimelockController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
*/
event CallExecuted(bytes32 indexed id, uint256 indexed index, address target, uint256 value, bytes data);

/**
* @dev Emitted when new proposal is scheduled with non-zero salt.
*/
event CallSalt(bytes32 indexed id, bytes32 salt);

/**
* @dev Emitted when operation `id` is cancelled.
*/
Expand Down Expand Up @@ -206,7 +211,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
/**
* @dev Schedule an operation containing a single transaction.
*
* Emits a {CallScheduled} event.
* Emits events {CallScheduled} and {CallSalt}.
*
* Requirements:
*
Expand All @@ -223,12 +228,15 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
bytes32 id = hashOperation(target, value, data, predecessor, salt);
_schedule(id, delay);
emit CallScheduled(id, 0, target, value, data, predecessor, delay);
if (salt != bytes32(0)) {
emit CallSalt(id, salt);
}
}

/**
* @dev Schedule an operation containing a batch of transactions.
*
* Emits one {CallScheduled} event per transaction in the batch.
* Emits a {CallSalt} event and one {CallScheduled} event per transaction in the batch.
*
* Requirements:
*
Expand All @@ -250,6 +258,9 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
for (uint256 i = 0; i < targets.length; ++i) {
emit CallScheduled(id, i, targets[i], values[i], payloads[i], predecessor, delay);
}
if (salt != bytes32(0)) {
emit CallSalt(id, salt);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,15 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
);
}

function cancel(uint256 proposalId) public virtual override {
function cancel(uint256 proposalId) public virtual override(IGovernor, Governor) {
ProposalDetails storage details = _proposalDetails[proposalId];

require(
_msgSender() == details.proposer || getVotes(details.proposer, block.number - 1) < proposalThreshold(),
"GovernorBravo: proposer above threshold"
);

_cancel(
details.targets,
details.values,
_encodeCalldata(details.signatures, details.calldatas),
details.descriptionHash
);
_cancel(proposalId);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ abstract contract IGovernorCompatibilityBravo is IGovernor {
*/
function execute(uint256 proposalId) public payable virtual;

/**
* @dev Cancels a proposal only if sender is the proposer, or proposer delegates dropped below proposal threshold.
*/
function cancel(uint256 proposalId) public virtual;

/**
* @dev Part of the Governor Bravo's interface: _"Gets actions of a proposal"_.
*/
Expand Down
4 changes: 4 additions & 0 deletions contracts/mocks/governance/GovernorCompatibilityBravoMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ abstract contract GovernorCompatibilityBravoMock is
return super.execute(targets, values, calldatas, salt);
}

function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) {
super.cancel(proposalId);
}

function _execute(
uint256 proposalId,
address[] memory targets,
Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/proxy/UUPSLegacy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock {
}

// hooking into the old mechanism
function upgradeTo(address newImplementation) external override {
function upgradeTo(address newImplementation) public override {
_upgradeToAndCallSecureLegacyV1(newImplementation, bytes(""), false);
}

function upgradeToAndCall(address newImplementation, bytes memory data) external payable override {
function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
_upgradeToAndCallSecureLegacyV1(newImplementation, data, false);
}
}
4 changes: 2 additions & 2 deletions contracts/mocks/proxy/UUPSUpgradeableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable {
}

contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
function upgradeTo(address newImplementation) external override {
function upgradeTo(address newImplementation) public override {
ERC1967Upgrade._upgradeToAndCall(newImplementation, bytes(""), false);
}

function upgradeToAndCall(address newImplementation, bytes memory data) external payable override {
function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
ERC1967Upgrade._upgradeToAndCall(newImplementation, data, false);
}
}
4 changes: 4 additions & 0 deletions contracts/mocks/wizard/MyGovernor3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ contract MyGovernor is
return super.propose(targets, values, calldatas, description);
}

function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) {
super.cancel(proposalId);
}

function _execute(
uint256 proposalId,
address[] memory targets,
Expand Down
4 changes: 2 additions & 2 deletions contracts/proxy/utils/UUPSUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade {
*
* @custom:oz-upgrades-unsafe-allow-reachable delegatecall
*/
function upgradeTo(address newImplementation) external virtual onlyProxy {
function upgradeTo(address newImplementation) public virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
}
Expand All @@ -80,7 +80,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade {
*
* @custom:oz-upgrades-unsafe-allow-reachable delegatecall
*/
function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual onlyProxy {
function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, data, true);
}
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/governance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ votingDelay: How long after a proposal is created should voting power be fixed.

votingPeriod: How long does a proposal remain open to votes.

These parameters are specified in number of blocks. Assuming block time of around 13.14 seconds, we will set votingDelay = 1 day = 6570 blocks, and votingPeriod = 1 week = 45992 blocks.
These parameters are specified in number of blocks. Assuming block time of around 12 seconds, we will set votingDelay = 1 day = 7200 blocks, and votingPeriod = 1 week = 50400 blocks.

We can optionally set a proposal threshold as well. This restricts proposal creation to accounts who have enough voting power.

Expand Down
Loading

0 comments on commit 3ec0716

Please sign in to comment.