Skip to content

Commit b2b9e81

Browse files
ernestognwernestognw
authored
Merge branch 'master' into lib-618-accesscontrol-admin-rules
2 parents b4ee08a + 7b3e7b7 commit b2b9e81

File tree

11 files changed

+141
-37
lines changed

11 files changed

+141
-37
lines changed

‎.changeset/small-cars-appear.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': patch
3+
---
4+
5+
`ECDSA`: Add a function `toDataWithIntendedValidatorHash` that encodes data with version 0x00 following EIP-191.

‎contracts/governance/Governor.sol

+9-11
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,16 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
336336
/**
337337
* @dev See {IGovernor-cancel}.
338338
*/
339-
function cancel(uint256 proposalId) public virtual override {
339+
function cancel(
340+
address[] memory targets,
341+
uint256[] memory values,
342+
bytes[] memory calldatas,
343+
bytes32 descriptionHash
344+
) public virtual override returns (uint256) {
345+
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
340346
require(state(proposalId) == ProposalState.Pending, "Governor: too late to cancel");
341347
require(_msgSender() == _proposals[proposalId].proposer, "Governor: only proposer can cancel");
342-
_cancel(proposalId);
348+
return _cancel(targets, values, calldatas, descriptionHash);
343349
}
344350

345351
/**
@@ -407,16 +413,8 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
407413
bytes[] memory calldatas,
408414
bytes32 descriptionHash
409415
) internal virtual returns (uint256) {
410-
return _cancel(hashProposal(targets, values, calldatas, descriptionHash));
411-
}
416+
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
412417

413-
/**
414-
* @dev Internal cancel mechanism: locks up the proposal timer, preventing it from being re-submitted. Marks it as
415-
* canceled to allow distinguishing it from executed proposals.
416-
*
417-
* Emits a {IGovernor-ProposalCanceled} event.
418-
*/
419-
function _cancel(uint256 proposalId) internal virtual returns (uint256) {
420418
ProposalState status = state(proposalId);
421419

422420
require(

‎contracts/governance/IGovernor.sol

+8-3
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,17 @@ abstract contract IGovernor is IERC165, IERC6372 {
235235
) public payable virtual returns (uint256 proposalId);
236236

237237
/**
238-
* @dev Cancel a proposal. This is restricted to Pending proposal (before the vote starts) and is restricted to
239-
* the proposal's proposer.
238+
* @dev Cancel a proposal. A proposal is cancellable by the proposer, but only while it is Pending state, i.e.
239+
* before the vote starts.
240240
*
241241
* Emits a {ProposalCanceled} event.
242242
*/
243-
function cancel(uint256 proposalId) public virtual;
243+
function cancel(
244+
address[] memory targets,
245+
uint256[] memory values,
246+
bytes[] memory calldatas,
247+
bytes32 descriptionHash
248+
) public virtual returns (uint256 proposalId);
244249

245250
/**
246251
* @dev Cast a vote

‎contracts/governance/compatibility/GovernorCompatibilityBravo.sol

+61-16
Original file line numberDiff line numberDiff line change
@@ -77,37 +77,63 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
7777
* @dev See {IGovernorCompatibilityBravo-queue}.
7878
*/
7979
function queue(uint256 proposalId) public virtual override {
80-
ProposalDetails storage details = _proposalDetails[proposalId];
81-
queue(
82-
details.targets,
83-
details.values,
84-
_encodeCalldata(details.signatures, details.calldatas),
85-
details.descriptionHash
86-
);
80+
(
81+
address[] memory targets,
82+
uint256[] memory values,
83+
bytes[] memory calldatas,
84+
bytes32 descriptionHash
85+
) = _getProposalParameters(proposalId);
86+
87+
queue(targets, values, calldatas, descriptionHash);
8788
}
8889

8990
/**
9091
* @dev See {IGovernorCompatibilityBravo-execute}.
9192
*/
9293
function execute(uint256 proposalId) public payable virtual override {
93-
ProposalDetails storage details = _proposalDetails[proposalId];
94-
execute(
95-
details.targets,
96-
details.values,
97-
_encodeCalldata(details.signatures, details.calldatas),
98-
details.descriptionHash
99-
);
94+
(
95+
address[] memory targets,
96+
uint256[] memory values,
97+
bytes[] memory calldatas,
98+
bytes32 descriptionHash
99+
) = _getProposalParameters(proposalId);
100+
101+
execute(targets, values, calldatas, descriptionHash);
100102
}
101103

102-
function cancel(uint256 proposalId) public virtual override(IGovernor, Governor) {
104+
/**
105+
* @dev Cancel a proposal with GovernorBravo logic.
106+
*/
107+
function cancel(uint256 proposalId) public virtual {
108+
(
109+
address[] memory targets,
110+
uint256[] memory values,
111+
bytes[] memory calldatas,
112+
bytes32 descriptionHash
113+
) = _getProposalParameters(proposalId);
114+
115+
cancel(targets, values, calldatas, descriptionHash);
116+
}
117+
118+
/**
119+
* @dev Cancel a proposal with GovernorBravo logic. At any moment a proposal can be cancelled, either by the
120+
* proposer, or by third parties if the proposer's voting power has dropped below the proposal threshold.
121+
*/
122+
function cancel(
123+
address[] memory targets,
124+
uint256[] memory values,
125+
bytes[] memory calldatas,
126+
bytes32 descriptionHash
127+
) public virtual override(IGovernor, Governor) returns (uint256) {
128+
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
103129
address proposer = _proposalDetails[proposalId].proposer;
104130

105131
require(
106132
_msgSender() == proposer || getVotes(proposer, clock() - 1) < proposalThreshold(),
107133
"GovernorBravo: proposer above threshold"
108134
);
109135

110-
_cancel(proposalId);
136+
return _cancel(targets, values, calldatas, descriptionHash);
111137
}
112138

113139
/**
@@ -128,6 +154,25 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
128154
return fullcalldatas;
129155
}
130156

157+
/**
158+
* @dev Retrieve proposal parameters by id, with fully encoded calldatas.
159+
*/
160+
function _getProposalParameters(
161+
uint256 proposalId
162+
)
163+
private
164+
view
165+
returns (address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash)
166+
{
167+
ProposalDetails storage details = _proposalDetails[proposalId];
168+
return (
169+
details.targets,
170+
details.values,
171+
_encodeCalldata(details.signatures, details.calldatas),
172+
details.descriptionHash
173+
);
174+
}
175+
131176
/**
132177
* @dev Store proposal metadata for later lookup
133178
*/

‎contracts/mocks/governance/GovernorCompatibilityBravoMock.sol

+7-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ abstract contract GovernorCompatibilityBravoMock is
6666
return super.execute(targets, values, calldatas, salt);
6767
}
6868

69-
function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) {
70-
super.cancel(proposalId);
69+
function cancel(
70+
address[] memory targets,
71+
uint256[] memory values,
72+
bytes[] memory calldatas,
73+
bytes32 descriptionHash
74+
) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) {
75+
return super.cancel(targets, values, calldatas, descriptionHash);
7176
}
7277

7378
function _execute(

‎contracts/mocks/wizard/MyGovernor3.sol

+7-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,13 @@ contract MyGovernor is
5454
return super.propose(targets, values, calldatas, description);
5555
}
5656

57-
function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) {
58-
super.cancel(proposalId);
57+
function cancel(
58+
address[] memory targets,
59+
uint256[] memory values,
60+
bytes[] memory calldatas,
61+
bytes32 descriptionHash
62+
) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) {
63+
return super.cancel(targets, values, calldatas, descriptionHash);
5964
}
6065

6166
function _execute(

‎contracts/utils/cryptography/ECDSA.sol

+10
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,14 @@ library ECDSA {
204204
data := keccak256(ptr, 0x42)
205205
}
206206
}
207+
208+
/**
209+
* @dev Returns an Ethereum Signed Data with intended validator, created from a
210+
* `validator` and `data` according to the version 0 of EIP-191.
211+
*
212+
* See {recover}.
213+
*/
214+
function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) {
215+
return keccak256(abi.encodePacked("\x19\x00", validator, data));
216+
}
207217
}

‎test/helpers/governance.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,13 @@ class GovernorHelper {
6868

6969
switch (visibility) {
7070
case 'external':
71-
return this.governor.methods['cancel(uint256)'](...concatOpts([proposal.id], opts));
71+
if (proposal.useCompatibilityInterface) {
72+
return this.governor.methods['cancel(uint256)'](...concatOpts([proposal.id], opts));
73+
} else {
74+
return this.governor.methods['cancel(address[],uint256[],bytes[],bytes32)'](
75+
...concatOpts(proposal.shortProposal, opts),
76+
);
77+
}
7278
case 'internal':
7379
return this.governor.methods['$_cancel(address[],uint256[],bytes[],bytes32)'](
7480
...concatOpts(proposal.shortProposal, opts),

‎test/helpers/sign.js

+16
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,21 @@ function toEthSignedMessageHash(messageHex) {
44
return web3.utils.sha3(Buffer.concat([prefix, messageBuffer]));
55
}
66

7+
/**
8+
* Create a signed data with intended validator according to the version 0 of EIP-191
9+
* @param validatorAddress The address of the validator
10+
* @param dataHex The data to be concatenated with the prefix and signed
11+
*/
12+
function toDataWithIntendedValidatorHash(validatorAddress, dataHex) {
13+
const validatorBuffer = Buffer.from(web3.utils.hexToBytes(validatorAddress));
14+
const dataBuffer = Buffer.from(web3.utils.hexToBytes(dataHex));
15+
const preambleBuffer = Buffer.from('\x19');
16+
const versionBuffer = Buffer.from('\x00');
17+
const ethMessage = Buffer.concat([preambleBuffer, versionBuffer, validatorBuffer, dataBuffer]);
18+
19+
return web3.utils.sha3(ethMessage);
20+
}
21+
722
/**
823
* Create a signer between a contract and a signer for a voucher of method, args, and redeemer
924
* Note that `method` is the web3 method, not the truffle-contract method
@@ -43,5 +58,6 @@ const getSignFor =
4358

4459
module.exports = {
4560
toEthSignedMessageHash,
61+
toDataWithIntendedValidatorHash,
4662
getSignFor,
4763
};

‎test/token/ERC20/extensions/ERC4626.t.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ contract ERC4626StdTest is ERC4626Test {
1212
_underlying_ = address(new ERC20Mock());
1313
_vault_ = address(new ERC4626Mock(_underlying_));
1414
_delta_ = 0;
15-
_vaultMayBeEmpty = false;
15+
_vaultMayBeEmpty = true;
1616
_unlimitedAmount = true;
1717
}
1818
}

‎test/utils/cryptography/ECDSA.test.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const { expectRevert } = require('@openzeppelin/test-helpers');
2-
const { toEthSignedMessageHash } = require('../../helpers/sign');
2+
const { toEthSignedMessageHash, toDataWithIntendedValidatorHash } = require('../../helpers/sign');
33

44
const { expect } = require('chai');
55

@@ -8,6 +8,7 @@ const ECDSA = artifacts.require('$ECDSA');
88
const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
99
const WRONG_MESSAGE = web3.utils.sha3('Nope');
1010
const NON_HASH_MESSAGE = '0x' + Buffer.from('abcd').toString('hex');
11+
const RANDOM_ADDRESS = web3.utils.toChecksumAddress(web3.utils.randomHex(20));
1112

1213
function to2098Format(signature) {
1314
const long = web3.utils.hexToBytes(signature);
@@ -248,4 +249,12 @@ contract('ECDSA', function (accounts) {
248249
);
249250
});
250251
});
252+
253+
context('toDataWithIntendedValidatorHash', function () {
254+
it('returns the hash correctly', async function () {
255+
expect(
256+
await this.ecdsa.methods['$toDataWithIntendedValidatorHash(address,bytes)'](RANDOM_ADDRESS, NON_HASH_MESSAGE),
257+
).to.equal(toDataWithIntendedValidatorHash(RANDOM_ADDRESS, NON_HASH_MESSAGE));
258+
});
259+
});
251260
});

0 commit comments

Comments
 (0)