Skip to content

O
openzeppelin-contracts-upgradeable
Commit 06ab69e5 by github-actions
Options

Transpile 79cb606f

parent 3ff38d85
Showing with 160 additions and 140 deletions
CHANGELOG.md
# Changelog # 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) ## 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)) * `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/GovernorCompatibilityBravoUpgradeable.sol
// SPDX-License-Identifier: MIT // 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; pragma solidity ^0.8.0;
...@@ -144,7 +144,7 @@ abstract contract GovernorCompatibilityBravoUpgradeable is Initializable, IGover ...@@ -144,7 +144,7 @@ abstract contract GovernorCompatibilityBravoUpgradeable is Initializable, IGover
for (uint256 i = 0; i < signatures.length; ++i) { for (uint256 i = 0; i < signatures.length; ++i) {
fullcalldatas[i] = bytes(signatures[i]).length == 0 fullcalldatas[i] = bytes(signatures[i]).length == 0
? calldatas[i] ? calldatas[i]
: abi.encodeWithSignature(signatures[i], calldatas[i]); : abi.encodePacked(bytes4(keccak256(bytes(signatures[i]))), calldatas[i]);
} }
return fullcalldatas; return fullcalldatas;
......
contracts/mocks/CallReceiverMockUpgradeable.sol
...@@ -13,6 +13,7 @@ contract CallReceiverMockUpgradeable is Initializable { ...@@ -13,6 +13,7 @@ contract CallReceiverMockUpgradeable is Initializable {
string public sharedAnswer; string public sharedAnswer;
event MockFunctionCalled(); event MockFunctionCalled();
event MockFunctionCalledWithArgs(uint256 a, uint256 b);
uint256[] private _array; uint256[] private _array;
...@@ -22,6 +23,12 @@ contract CallReceiverMockUpgradeable is Initializable { ...@@ -22,6 +23,12 @@ contract CallReceiverMockUpgradeable is Initializable {
return "0x1234"; 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) { function mockFunctionNonPayable() public returns (string memory) {
emit MockFunctionCalled(); emit MockFunctionCalled();
......
contracts/package.json
{ {
"name": "@openzeppelin/contracts-upgradeable", "name": "@openzeppelin/contracts-upgradeable",
"description": "Secure Smart Contract library for Solidity", "description": "Secure Smart Contract library for Solidity",
"version": "4.4.1", "version": "4.4.2",
"files": [ "files": [
"**/*.sol", "**/*.sol",
"/build/contracts/*.json", "/build/contracts/*.json",
......
package.json
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
"private": true, "private": true,
"name": "openzeppelin-solidity", "name": "openzeppelin-solidity",
"description": "Secure Smart Contract library for Solidity", "description": "Secure Smart Contract library for Solidity",
"version": "4.4.1", "version": "4.4.2",
"files": [ "files": [
"/contracts/**/*.sol", "/contracts/**/*.sol",
"/build/contracts/*.json", "/build/contracts/*.json",
...@@ -79,6 +79,7 @@ ...@@ -79,6 +79,7 @@
"prettier": "^2.3.0", "prettier": "^2.3.0",
"prettier-plugin-solidity": "^1.0.0-beta.16", "prettier-plugin-solidity": "^1.0.0-beta.16",
"rimraf": "^3.0.2", "rimraf": "^3.0.2",
"semver": "^7.3.5",
"solhint": "^3.3.6", "solhint": "^3.3.6",
"solidity-ast": "^0.4.25", "solidity-ast": "^0.4.25",
"solidity-coverage": "^0.7.11", "solidity-coverage": "^0.7.11",
......
scripts/release/update-comment.js
#!/usr/bin/env node #!/usr/bin/env node
const fs = require('fs'); const fs = require('fs');
const glob = require('glob');
const proc = require('child_process'); 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) { if (gitStatus.length > 0) {
console.error('Contracts directory is not clean'); console.error('Contracts directory is not clean');
process.exit(1); process.exit(1);
...@@ -13,15 +12,24 @@ if (gitStatus.length > 0) { ...@@ -13,15 +12,24 @@ if (gitStatus.length > 0) {
const { version } = require('../../package.json'); 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) { for (const file of files) {
const current = fs.readFileSync(file, 'utf8'); const current = fs.readFileSync(file, 'utf8');
const updated = current.replace( const updated = current.replace(
/(\/\/ SPDX-License-Identifier:.*)$(\n\/\/ OpenZeppelin Contracts v.*$)?/m, /(\/\/ SPDX-License-Identifier:.*)$(\n\/\/ OpenZeppelin Contracts .*$)?/m,
`$1\n// OpenZeppelin Contracts v${version} (${file.replace('contracts/', '')})`, `$1\n// OpenZeppelin Contracts (last updated v${version}) (${file.replace('contracts/', '')})`,
); );
fs.writeFileSync(file, updated); 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 = '') { ...@@ -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 () { function runGovernorWorkflow () {
beforeEach(async function () { beforeEach(async function () {
this.receipts = {}; 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.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 () { it('run', async function () {
...@@ -38,7 +74,11 @@ function runGovernorWorkflow () { ...@@ -38,7 +74,11 @@ function runGovernorWorkflow () {
// propose // propose
if (this.mock.propose && tryGet(this.settings, 'steps.propose.enable') !== false) { if (this.mock.propose && tryGet(this.settings, 'steps.propose.enable') !== false) {
this.receipts.propose = await getReceiptOrRevert( 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, ...this.settings.proposal,
{ from: this.settings.proposer }, { from: this.settings.proposer },
), ),
...@@ -98,11 +138,15 @@ function runGovernorWorkflow () { ...@@ -98,11 +138,15 @@ function runGovernorWorkflow () {
// queue // queue
if (this.mock.queue && tryGet(this.settings, 'steps.queue.enable') !== false) { if (this.mock.queue && tryGet(this.settings, 'steps.queue.enable') !== false) {
this.receipts.queue = await getReceiptOrRevert( this.receipts.queue = await getReceiptOrRevert(
this.mock.methods['queue(address[],uint256[],bytes[],bytes32)']( this.useCompatibilityInterface
...this.settings.proposal.slice(0, -1), ? this.mock.methods['queue(uint256)'](
this.descriptionHash, this.id,
{ from: this.settings.queuer }, { 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'), tryGet(this.settings, 'steps.queue.error'),
); );
this.eta = await this.mock.proposalEta(this.id); this.eta = await this.mock.proposalEta(this.id);
...@@ -114,11 +158,15 @@ function runGovernorWorkflow () { ...@@ -114,11 +158,15 @@ function runGovernorWorkflow () {
// execute // execute
if (this.mock.execute && tryGet(this.settings, 'steps.execute.enable') !== false) { if (this.mock.execute && tryGet(this.settings, 'steps.execute.enable') !== false) {
this.receipts.execute = await getReceiptOrRevert( this.receipts.execute = await getReceiptOrRevert(
this.mock.methods['execute(address[],uint256[],bytes[],bytes32)']( this.useCompatibilityInterface
...this.settings.proposal.slice(0, -1), ? this.mock.methods['execute(uint256)'](
this.descriptionHash, this.id,
{ from: this.settings.executer }, { 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'), tryGet(this.settings, 'steps.execute.error'),
); );
if (tryGet(this.settings, 'steps.execute.delay')) { if (tryGet(this.settings, 'steps.execute.delay')) {
......
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 Enums = require('../../helpers/enums');
const RLP = require('rlp'); const RLP = require('rlp');
...@@ -11,24 +11,6 @@ const Timelock = artifacts.require('CompTimelock'); ...@@ -11,24 +11,6 @@ const Timelock = artifacts.require('CompTimelock');
const Governor = artifacts.require('GovernorCompatibilityBravoMock'); const Governor = artifacts.require('GovernorCompatibilityBravoMock');
const CallReceiver = artifacts.require('CallReceiverMock'); 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) { function makeContractAddress (creator, nonce) {
return web3.utils.toChecksumAddress(web3.utils.sha3(RLP.encode([creator, nonce])).slice(12).substring(14)); return web3.utils.toChecksumAddress(web3.utils.sha3(RLP.encode([creator, nonce])).slice(12).substring(14));
} }
...@@ -194,6 +176,67 @@ contract('GovernorCompatibilityBravo', function (accounts) { ...@@ -194,6 +176,67 @@ contract('GovernorCompatibilityBravo', function (accounts) {
runGovernorWorkflow(); 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 () { describe('proposalThreshold not reached', function () {
beforeEach(async function () { beforeEach(async function () {
this.settings = { this.settings = {
...@@ -266,8 +309,8 @@ contract('GovernorCompatibilityBravo', function (accounts) { ...@@ -266,8 +309,8 @@ contract('GovernorCompatibilityBravo', function (accounts) {
proposal: [ proposal: [
[ this.receiver.address ], // targets [ this.receiver.address ], // targets
[ web3.utils.toWei('0') ], // values [ web3.utils.toWei('0') ], // values
[ '' ], // signatures [ 'mockFunction()' ], // signatures
[ this.receiver.contract.methods.mockFunction().encodeABI() ], // calldatas [ '0x' ], // calldatas
'<proposal description>', // description '<proposal description>', // description
], ],
proposer, proposer,
...@@ -351,8 +394,8 @@ contract('GovernorCompatibilityBravo', function (accounts) { ...@@ -351,8 +394,8 @@ contract('GovernorCompatibilityBravo', function (accounts) {
proposer, proposer,
targets: this.settings.proposal[0], targets: this.settings.proposal[0],
// values: this.settings.proposal[1].map(value => new BN(value)), // values: this.settings.proposal[1].map(value => new BN(value)),
signatures: this.settings.proposal[2], signatures: this.settings.proposal[2].map(_ => ''),
calldatas: this.settings.proposal[3], calldatas: this.settings.shortProposal[2],
startBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay), startBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay),
endBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay).add(this.votingPeriod), endBlock: new BN(this.receipts.propose.blockNumber).add(this.votingDelay).add(this.votingPeriod),
description: this.settings.proposal[4], description: this.settings.proposal[4],
...@@ -378,98 +421,6 @@ contract('GovernorCompatibilityBravo', function (accounts) { ...@@ -378,98 +421,6 @@ contract('GovernorCompatibilityBravo', function (accounts) {
'MockFunctionCalled', 'MockFunctionCalled',
); );
}); });
runGovernorWorkflow();
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'));
}
}
});
}); });
}); });
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