From 2157c51ac5152a8f06cc801a9ed18408ca11e23d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Sep 2022 10:22:23 +0200 Subject: [PATCH 1/5] Add support for target EOA in Governor:relay --- contracts/governance/Governor.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 5083165b17f..10113882160 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -544,8 +544,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive address target, uint256 value, bytes calldata data - ) external virtual onlyGovernance { - Address.functionCallWithValue(target, data, value); + ) external payable virtual onlyGovernance { + (bool success, bytes memory returndata) = target.call{value: value}(data); + Address.verifyCallResult(success, returndata, "Governor: relay reverted without message"); } /** From b981b9d9bdb216cbd4b5a46cf2b9ba90d75548f2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 26 Sep 2022 19:27:18 +0200 Subject: [PATCH 2/5] add governor relay payability test --- .../GovernorTimelockControl.test.js | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 45e26c93595..72ee81ab50c 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -1,4 +1,4 @@ -const { BN, constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); +const { constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); const Enums = require('../../helpers/enums'); const { GovernorHelper } = require('../../helpers/governance'); @@ -24,10 +24,10 @@ contract('GovernorTimelockControl', function (accounts) { // const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; - const tokenSupply = web3.utils.toWei('100'); - const votingDelay = new BN(4); - const votingPeriod = new BN(16); - const value = web3.utils.toWei('1'); + const tokenSupply = web3.utils.toBN(web3.utils.toWei('100')); + const votingDelay = web3.utils.toBN(4); + const votingPeriod = web3.utils.toBN(16); + const value = web3.utils.toBN(web3.utils.toWei('1')); beforeEach(async function () { const [ deployer ] = await web3.eth.getAccounts(); @@ -293,6 +293,39 @@ contract('GovernorTimelockControl', function (accounts) { ); }); + it('is payable and can transfer eth to EOA', async function () { + const t2g = web3.utils.toBN(128); // timelock to governor + const g2o = web3.utils.toBN(100); // governor to eoa (other) + + this.helper.setProposal([ + { + target: this.mock.address, + value: t2g, + data: this.mock.contract.methods.relay( + other, + g2o, + "0x", + ).encodeABI(), + }, + ], ''); + + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + const timelockBalance = await web3.eth.getBalance(this.timelock.address).then(web3.utils.toBN); + const otherBalance = await web3.eth.getBalance(other).then(web3.utils.toBN); + + await this.helper.propose(); + await this.helper.waitForSnapshot(); + await this.helper.vote({ support: Enums.VoteType.For }, { from: voter1 }); + await this.helper.waitForDeadline(); + await this.helper.queue(); + await this.helper.waitForEta(); + await this.helper.execute(); + + expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(timelockBalance.sub(t2g)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(t2g.sub(g2o)); + expect(await web3.eth.getBalance(other)).to.be.bignumber.equal(otherBalance.add(g2o)); + }); + it('protected against other proposers', async function () { await this.timelock.schedule( this.mock.address, From e8c9bb2996a758584935e82d2afce2102d1aa1e0 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 26 Sep 2022 19:28:45 +0200 Subject: [PATCH 3/5] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecc6f02d7b6..c8d8e254a03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * `ERC165Checker`: add `supportsERC165InterfaceUnchecked` for consulting individual interfaces without the full ERC165 protocol. ([#3339](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3339)) * `Address`: optimize `functionCall` by calling `functionCallWithValue` directly. ([#3468](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3468)) * `Address`: optimize `functionCall` functions by checking contract size only if there is no returned data. ([#3469](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3469)) + * `Governor`: make the `relay` function payable, and add support for EOA payments. ([#3730](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3730)) * `GovernorCompatibilityBravo`: remove unused `using` statements. ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506)) * `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513)) * `ERC20Votes`, `ERC721Votes`: optimize `getPastVotes` for looking up recent checkpoints. ([#3673](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3673)) From 6f41e2450d7ca9512daaebf4f2c848e136f04f98 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 26 Sep 2022 19:30:39 +0200 Subject: [PATCH 4/5] minimize changes --- .../extensions/GovernorTimelockControl.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 72ee81ab50c..30c58a0f5e9 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -1,4 +1,4 @@ -const { constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); +const { BN, constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); const Enums = require('../../helpers/enums'); const { GovernorHelper } = require('../../helpers/governance'); @@ -24,10 +24,10 @@ contract('GovernorTimelockControl', function (accounts) { // const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; - const tokenSupply = web3.utils.toBN(web3.utils.toWei('100')); - const votingDelay = web3.utils.toBN(4); - const votingPeriod = web3.utils.toBN(16); - const value = web3.utils.toBN(web3.utils.toWei('1')); + const tokenSupply = web3.utils.toWei('100'); + const votingDelay = new BN(4); + const votingPeriod = new BN(16); + const value = web3.utils.toWei('1'); beforeEach(async function () { const [ deployer ] = await web3.eth.getAccounts(); From 76c97544f8f9df5f3ac7c80d1d19f9418bcbffe0 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 26 Sep 2022 19:32:45 +0200 Subject: [PATCH 5/5] fix lint --- test/governance/extensions/GovernorTimelockControl.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 30c58a0f5e9..edff5610883 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -304,7 +304,7 @@ contract('GovernorTimelockControl', function (accounts) { data: this.mock.contract.methods.relay( other, g2o, - "0x", + '0x', ).encodeABI(), }, ], '');