diff --git a/CHANGELOG.md b/CHANGELOG.md index e582c71bd..e45e1a3b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977)) * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986)) + * `Governor`: add a relay function to help recover assets sent to a governor that is not its own executor (e.g. when using a timelock). ([#2926](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2926)) ## 4.4.0 (2021-11-25) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 235b76891..32e77bc76 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -347,6 +347,20 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { return weight; } + /** + * @dev Relays a transaction or function call to an arbitrary target. In cases where the governance executor + * is some contract other than the governor itself, like when using a timelock, this function can be invoked + * in a governance proposal to recover tokens or Ether that was sent to the governor contract by mistake. + * Note that if the executor is simply the governor itself, use of `relay` is redundant. + */ + function relay( + address target, + uint256 value, + bytes calldata data + ) external onlyGovernance { + Address.functionCallWithValue(target, data, value); + } + /** * @dev Address through which the governor executes action. Will be overloaded by module that execute actions * through another contract such as a timelock. diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index 303cc97e9..bc4f15c99 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -21,7 +21,7 @@ function makeContractAddress (creator, nonce) { } contract('GovernorTimelockCompound', function (accounts) { - const [ admin, voter ] = accounts; + const [ admin, voter, other ] = accounts; const name = 'OZ-Governor'; // const version = '1'; @@ -328,6 +328,57 @@ contract('GovernorTimelockCompound', function (accounts) { runGovernorWorkflow(); }); + describe('relay', function () { + beforeEach(async function () { + await this.token.mint(this.mock.address, 1); + this.call = [ + this.token.address, + 0, + this.token.contract.methods.transfer(other, 1).encodeABI(), + ]; + }); + + it('protected', async function () { + await expectRevert( + this.mock.relay(...this.call), + 'Governor: onlyGovernance', + ); + }); + + describe('using workflow', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ + this.mock.address, + ], + [ + web3.utils.toWei('0'), + ], + [ + this.mock.contract.methods.relay(...this.call).encodeABI(), + ], + '', + ], + voters: [ + { voter: voter, support: Enums.VoteType.For }, + ], + steps: { + queue: { delay: 7 * 86400 }, + }, + }; + + expect(await this.token.balanceOf(this.mock.address), 1); + expect(await this.token.balanceOf(other), 0); + }); + afterEach(async function () { + expect(await this.token.balanceOf(this.mock.address), 0); + expect(await this.token.balanceOf(other), 1); + }); + runGovernorWorkflow(); + }); + }); + describe('updateTimelock', function () { beforeEach(async function () { this.newTimelock = await Timelock.new(this.mock.address, 7 * 86400); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index fe20fb61c..35242740b 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -16,7 +16,7 @@ const Governor = artifacts.require('GovernorTimelockControlMock'); const CallReceiver = artifacts.require('CallReceiverMock'); contract('GovernorTimelockControl', function (accounts) { - const [ admin, voter ] = accounts; + const [ admin, voter, other ] = accounts; const name = 'OZ-Governor'; // const version = '1'; @@ -321,6 +321,57 @@ contract('GovernorTimelockControl', function (accounts) { runGovernorWorkflow(); }); + describe('relay', function () { + beforeEach(async function () { + await this.token.mint(this.mock.address, 1); + this.call = [ + this.token.address, + 0, + this.token.contract.methods.transfer(other, 1).encodeABI(), + ]; + }); + + it('protected', async function () { + await expectRevert( + this.mock.relay(...this.call), + 'Governor: onlyGovernance', + ); + }); + + describe('using workflow', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ + this.mock.address, + ], + [ + web3.utils.toWei('0'), + ], + [ + this.mock.contract.methods.relay(...this.call).encodeABI(), + ], + '', + ], + voters: [ + { voter: voter, support: Enums.VoteType.For }, + ], + steps: { + queue: { delay: 7 * 86400 }, + }, + }; + + expect(await this.token.balanceOf(this.mock.address), 1); + expect(await this.token.balanceOf(other), 0); + }); + afterEach(async function () { + expect(await this.token.balanceOf(this.mock.address), 0); + expect(await this.token.balanceOf(other), 1); + }); + runGovernorWorkflow(); + }); + }); + describe('cancel on timelock is forwarded in state', function () { beforeEach(async function () { this.settings = {