Add support for EOA target in Governor.relay (OpenZeppelin#3730)

JulissaDantes · Nov 3, 2022 · aec6327 · aec6327
1 parent 180fd9c
commit aec6327
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,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))

diff --git 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");
     }
 
     /**

diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js
@@ -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(),
+          },
+        ], '<proposal description>');
+
+        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,