Skip to content

O
openzeppelin-contracts-upgradeable
Commit 79cb606f by Francisco Giordano
Options

Merge remote-tracking branch 'vanilla/release-v4.4' into patched/release-v4.4

parents 5fcb6c72 b53c4324
Showing with 156 additions and 136 deletions
CHANGELOG.md
# Changelog
## 4.4.2 (2022-01-11)
### Bugfixes
* `GovernorCompatibilityBravo`: Fix error in the encoding of calldata for proposals submitted through the compatibility interface with explicit signatures. ([#3100](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3100))
## 4.4.1 (2021-12-14)
* `Initializable`: change the existing `initializer` modifier and add a new `onlyInitializing` modifier to prevent reentrancy risk. ([#3006](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3006))
......
contracts/governance/compatibility/GovernorCompatibilityBravo.sol
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts v4.4.1 (governance/compatibility/GovernorCompatibilityBravo.sol)
// OpenZeppelin Contracts (last updated v4.4.2) (governance/compatibility/GovernorCompatibilityBravo.sol)
pragma solidity ^0.8.0;
......@@ -132,7 +132,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
for (uint256 i = 0; i < signatures.length; ++i) {
fullcalldatas[i] = bytes(signatures[i]).length == 0
? calldatas[i]
: abi.encodeWithSignature(signatures[i], calldatas[i]);
: abi.encodePacked(bytes4(keccak256(bytes(signatures[i]))), calldatas[i]);
}
return fullcalldatas;
......
contracts/mocks/CallReceiverMock.sol
......@@ -6,6 +6,7 @@ contract CallReceiverMock {
string public sharedAnswer;
event MockFunctionCalled();
event MockFunctionCalledWithArgs(uint256 a, uint256 b);
uint256[] private _array;
......@@ -15,6 +16,12 @@ contract CallReceiverMock {
return "0x1234";
}
function mockFunctionWithArgs(uint256 a, uint256 b) public payable returns (string memory) {
emit MockFunctionCalledWithArgs(a, b);
return "0x1234";
}
function mockFunctionNonPayable() public returns (string memory) {
emit MockFunctionCalled();
......
contracts/package.json
{
"name": "@openzeppelin/contracts-upgradeable",
"description": "Secure Smart Contract library for Solidity",
"version": "4.4.1",
"version": "4.4.2",
"files": [
"**/*.sol",
"/build/contracts/*.json",
......
package.json
......@@ -2,7 +2,7 @@
"private": true,
"name": "openzeppelin-solidity",
"description": "Secure Smart Contract library for Solidity",
"version": "4.4.1",
"version": "4.4.2",
"files": [
"/contracts/**/*.sol",
"/build/contracts/*.json",
......@@ -79,6 +79,7 @@
"prettier": "^2.3.0",
"prettier-plugin-solidity": "^1.0.0-beta.16",
"rimraf": "^3.0.2",
"semver": "^7.3.5",
"solhint": "^3.3.6",
"solidity-ast": "^0.4.25",
"solidity-coverage": "^0.7.11",
......
scripts/release/update-comment.js
#!/usr/bin/env node
const fs = require('fs');
const glob = require('glob');
const proc = require('child_process');
const semver = require('semver');
const run = (cmd, ...args) => proc.execFileSync(cmd, args, { encoding: 'utf8' }).trim();
const gitStatus = proc.execFileSync('git', ['status', '--porcelain', '-uno', 'contracts/**/*.sol']);
const gitStatus = run('git', 'status', '--porcelain', '-uno', 'contracts/**/*.sol');
if (gitStatus.length > 0) {
console.error('Contracts directory is not clean');
process.exit(1);
......@@ -13,15 +12,24 @@ if (gitStatus.length > 0) {
const { version } = require('../../package.json');
const files = glob.sync('contracts/!(mocks)/**/*.sol');
// Get latest tag according to semver.
const [ tag ] = run('git', 'tag')
.split(/\r?\n/)
.filter(v => semver.lt(semver.coerce(v), version)) // only consider older tags, ignore current prereleases
.sort(semver.rcompare);
// Ordering tag → HEAD is important here.
const files = run('git', 'diff', tag, 'HEAD', '--name-only', 'contracts/**/*.sol')
.split(/\r?\n/)
.filter(file => file && !file.match(/mock/i));
for (const file of files) {
const current = fs.readFileSync(file, 'utf8');
const updated = current.replace(
/(\/\/ SPDX-License-Identifier:.*)$(\n\/\/ OpenZeppelin Contracts v.*$)?/m,
`$1\n// OpenZeppelin Contracts v${version} (${file.replace('contracts/', '')})`,
/(\/\/ SPDX-License-Identifier:.*)$(\n\/\/ OpenZeppelin Contracts .*$)?/m,
`$1\n// OpenZeppelin Contracts (last updated v${version}) (${file.replace('contracts/', '')})`,
);
fs.writeFileSync(file, updated);
}
proc.execFileSync('git', ['add', '--update', 'contracts']);
run('git', 'add', '--update', 'contracts');
test/governance/GovernorWorkflow.behavior.js
......@@ -18,11 +18,47 @@ function tryGet (obj, path = '') {
}
}
function zip (...args) {
return Array(Math.max(...args.map(array => array.length)))
.fill()
.map((_, i) => args.map(array => array[i]));
}
function concatHex (...args) {
return web3.utils.bytesToHex([].concat(...args.map(h => web3.utils.hexToBytes(h || '0x'))));
}
function runGovernorWorkflow () {
beforeEach(async function () {
this.receipts = {};
// distinguish depending on the proposal length
// - length 4: propose(address[], uint256[], bytes[], string) → GovernorCore
// - length 5: propose(address[], uint256[], string[], bytes[], string) → GovernorCompatibilityBravo
this.useCompatibilityInterface = this.settings.proposal.length === 5;
// compute description hash
this.descriptionHash = web3.utils.keccak256(this.settings.proposal.slice(-1).find(Boolean));
this.id = await this.mock.hashProposal(...this.settings.proposal.slice(0, -1), this.descriptionHash);
// condensed proposal, used for queue and execute operation
this.settings.shortProposal = [
// targets
this.settings.proposal[0],
// values
this.settings.proposal[1],
// calldata (prefix selector if necessary)
this.useCompatibilityInterface
? zip(
this.settings.proposal[2].map(selector => selector && web3.eth.abi.encodeFunctionSignature(selector)),
this.settings.proposal[3],
).map(hexs => concatHex(...hexs))
: this.settings.proposal[2],
// descriptionHash
this.descriptionHash,
];
// proposal id
this.id = await this.mock.hashProposal(...this.settings.shortProposal);
});
it('run', async function () {
......@@ -38,7 +74,11 @@ function runGovernorWorkflow () {
// propose
if (this.mock.propose && tryGet(this.settings, 'steps.propose.enable') !== false) {
this.receipts.propose = await getReceiptOrRevert(
this.mock.methods['propose(address[],uint256[],bytes[],string)'](
this.mock.methods[
this.useCompatibilityInterface
? 'propose(address[],uint256[],string[],bytes[],string)'
: 'propose(address[],uint256[],bytes[],string)'
](
...this.settings.proposal,
{ from: this.settings.proposer },
),
......@@ -98,9 +138,13 @@ function runGovernorWorkflow () {
// queue
if (this.mock.queue && tryGet(this.settings, 'steps.queue.enable') !== false) {
this.receipts.queue = await getReceiptOrRevert(
this.mock.methods['queue(address[],uint256[],bytes[],bytes32)'](
...this.settings.proposal.slice(0, -1),
this.descriptionHash,
this.useCompatibilityInterface
? this.mock.methods['queue(uint256)'](
this.id,
{ from: this.settings.queuer },
)
: this.mock.methods['queue(address[],uint256[],bytes[],bytes32)'](
...this.settings.shortProposal,
{ from: this.settings.queuer },
),
tryGet(this.settings, 'steps.queue.error'),
......@@ -114,9 +158,13 @@ function runGovernorWorkflow () {
// execute
if (this.mock.execute && tryGet(this.settings, 'steps.execute.enable') !== false) {
this.receipts.execute = await getReceiptOrRevert(
this.mock.methods['execute(address[],uint256[],bytes[],bytes32)'](
...this.settings.proposal.slice(0, -1),
this.descriptionHash,
this.useCompatibilityInterface
? this.mock.methods['execute(uint256)'](
this.id,
{ from: this.settings.executer },
)
: this.mock.methods['execute(address[],uint256[],bytes[],bytes32)'](
...this.settings.shortProposal,
{ from: this.settings.executer },
),
tryGet(this.settings, 'steps.execute.error'),
......
test/governance/compatibility/GovernorCompatibilityBravo.test.js
const { BN, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers');
const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
const Enums = require('../../helpers/enums');
const RLP = require('rlp');
......@@ -11,24 +11,6 @@ const Timelock = artifacts.require('CompTimelock');
const Governor = artifacts.require('GovernorCompatibilityBravoMock');
const CallReceiver = artifacts.require('CallReceiverMock');
async function getReceiptOrRevert (promise, error = undefined) {
if (error) {
await expectRevert(promise, error);
return undefined;
} else {
const { receipt } = await promise;
return receipt;
}
}
function tryGet (obj, path = '') {
try {
return path.split('.').reduce((o, k) => o[k], obj);
} catch (_) {
return undefined;
}
}
function makeContractAddress (creator, nonce) {
return web3.utils.toChecksumAddress(web3.utils.sha3(RLP.encode([creator, nonce])).slice(12).substring(14));
}
......@@ -194,6 +176,67 @@ contract('GovernorCompatibilityBravo', function (accounts) {
runGovernorWorkflow();
});
describe('with function selector and arguments', function () {
beforeEach(async function () {
this.settings = {
proposal: [
Array(4).fill(this.receiver.address),
Array(4).fill(web3.utils.toWei('0')),
[
'',
'',
'mockFunctionNonPayable()',
'mockFunctionWithArgs(uint256,uint256)',
],
[
this.receiver.contract.methods.mockFunction().encodeABI(),
this.receiver.contract.methods.mockFunctionWithArgs(17, 42).encodeABI(),
'0x',
web3.eth.abi.encodeParameters(['uint256', 'uint256'], [18, 43]),
],
'<proposal description>', // description
],
proposer,
tokenHolder: owner,
voters: [
{
voter: voter1,
weight: web3.utils.toWei('10'),
support: Enums.VoteType.For,
},
],
steps: {
queue: { delay: 7 * 86400 },
},
};
});
runGovernorWorkflow();
afterEach(async function () {
await expectEvent.inTransaction(
this.receipts.execute.transactionHash,
this.receiver,
'MockFunctionCalled',
);
await expectEvent.inTransaction(
this.receipts.execute.transactionHash,
this.receiver,
'MockFunctionCalled',
);
await expectEvent.inTransaction(
this.receipts.execute.transactionHash,
this.receiver,
'MockFunctionCalledWithArgs',
{ a: '17', b: '42' },
);
await expectEvent.inTransaction(
this.receipts.execute.transactionHash,
this.receiver,
'MockFunctionCalledWithArgs',
{ a: '18', b: '43' },
);
});
});
describe('proposalThreshold not reached', function () {
beforeEach(async function () {
this.settings = {
......@@ -266,8 +309,8 @@ contract('GovernorCompatibilityBravo', function (accounts) {
proposal: [
[ this.receiver.address ], // targets
[ web3.utils.toWei('0') ], // values
[ '' ], // signatures
[ this.receiver.contract.methods.mockFunction().encodeABI() ], // calldatas
[ 'mockFunction()' ], // signatures
[ '0x' ], // calldatas
'<proposal description>', // description
],
proposer,
......@@ -351,8 +394,8 @@ contract('GovernorCompatibilityBravo', function (accounts) {
proposer,
targets: this.settings.proposal[0],
// values: this.settings.proposal[1].map(value => new BN(value)),
signatures: this.settings.proposal[2],
calldatas: this.settings.proposal[3],
signatures: this.settings.proposal[2].map(_ => ''),
calldatas: this.settings.shortProposal[2],
startBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay),
endBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay).add(this.votingPeriod),
description: this.settings.proposal[4],
......@@ -378,98 +421,6 @@ contract('GovernorCompatibilityBravo', function (accounts) {
'MockFunctionCalled',
);
});
it('run', async function () {
// transfer tokens
if (tryGet(this.settings, 'voters')) {
for (const voter of this.settings.voters) {
if (voter.weight) {
await this.token.transfer(voter.voter, voter.weight, { from: this.settings.tokenHolder });
}
}
}
// propose
if (this.mock.propose && tryGet(this.settings, 'steps.propose.enable') !== false) {
this.receipts.propose = await getReceiptOrRevert(
this.mock.methods['propose(address[],uint256[],string[],bytes[],string)'](
...this.settings.proposal,
{ from: this.settings.proposer },
),
tryGet(this.settings, 'steps.propose.error'),
);
if (tryGet(this.settings, 'steps.propose.error') === undefined) {
this.id = this.receipts.propose.logs.find(({ event }) => event === 'ProposalCreated').args.proposalId;
this.snapshot = await this.mock.proposalSnapshot(this.id);
this.deadline = await this.mock.proposalDeadline(this.id);
}
if (tryGet(this.settings, 'steps.propose.delay')) {
await time.increase(tryGet(this.settings, 'steps.propose.delay'));
}
if (
tryGet(this.settings, 'steps.propose.error') === undefined &&
tryGet(this.settings, 'steps.propose.noadvance') !== true
) {
await time.advanceBlockTo(this.snapshot);
}
}
// vote
if (tryGet(this.settings, 'voters')) {
this.receipts.castVote = [];
for (const voter of this.settings.voters) {
if (!voter.signature) {
this.receipts.castVote.push(
await getReceiptOrRevert(
this.mock.castVote(this.id, voter.support, { from: voter.voter }),
voter.error,
),
);
} else {
const { v, r, s } = await voter.signature({ proposalId: this.id, support: voter.support });
this.receipts.castVote.push(
await getReceiptOrRevert(
this.mock.castVoteBySig(this.id, voter.support, v, r, s),
voter.error,
),
);
}
if (tryGet(voter, 'delay')) {
await time.increase(tryGet(voter, 'delay'));
}
}
}
// fast forward
if (tryGet(this.settings, 'steps.wait.enable') !== false) {
await time.advanceBlockTo(this.deadline);
}
// queue
if (this.mock.queue && tryGet(this.settings, 'steps.queue.enable') !== false) {
this.receipts.queue = await getReceiptOrRevert(
this.mock.methods['queue(uint256)'](this.id, { from: this.settings.queuer }),
tryGet(this.settings, 'steps.queue.error'),
);
this.eta = await this.mock.proposalEta(this.id);
if (tryGet(this.settings, 'steps.queue.delay')) {
await time.increase(tryGet(this.settings, 'steps.queue.delay'));
}
}
// execute
if (this.mock.execute && tryGet(this.settings, 'steps.execute.enable') !== false) {
this.receipts.execute = await getReceiptOrRevert(
this.mock.methods['execute(uint256)'](this.id, { from: this.settings.executer }),
tryGet(this.settings, 'steps.execute.error'),
);
if (tryGet(this.settings, 'steps.execute.delay')) {
await time.increase(tryGet(this.settings, 'steps.execute.delay'));
}
}
});
runGovernorWorkflow();
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment