Add a relay mechanism in the governor (#2926)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>

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)
 

contracts/governance/Governor.sol

@@ -348,6 +348,20 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
     }
 
     /**
+     * @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.
      */

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(),
+            ],
+            '<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);

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(),
+            ],
+            '<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 = {