Skip to content

Commit

Permalink
Add a relay mechanism in the governor (#2926)
Browse files Browse the repository at this point in the history
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
  • Loading branch information
Amxx and frangio committed Nov 30, 2021
1 parent 3536587 commit 6089f11
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 14 additions & 0 deletions contracts/governance/Governor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 52 additions & 1 deletion test/governance/extensions/GovernorTimelockCompound.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(),
],
'<proposal description>',
],
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);
Expand Down
53 changes: 52 additions & 1 deletion test/governance/extensions/GovernorTimelockControl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(),
],
'<proposal description>',
],
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 = {
Expand Down

0 comments on commit 6089f11

Please sign in to comment.