chore(protocol-contracts): add withdraw function to oapp receiver (#1306)

Author: jatZama

protocol-contracts/governance/contracts/GovernanceOAppReceiver.sol

@@ -25,6 +25,10 @@ contract GovernanceOAppReceiver is OAppReceiver {
     error AdminSafeModuleNotSet();
     /// @notice Thrown when trying to set the adminSafeModule to the null address.
     error AdminSafeModuleIsNull();
+    /// @notice Thrown when failing to withdraw ETH from the contract.
+    error FailedToWithdrawETH();
+    /// @notice Thrown when recipient is the null address.
+    error InvalidNullRecipient();
 
     /// @notice Emitted when a new Safe AdminModule has been setup.
     event AdminSafeModuleUpdated(address indexed oldModule, address indexed newModule);
@@ -38,6 +42,8 @@ contract GovernanceOAppReceiver is OAppReceiver {
         bytes[] datas,
         Operation[] operations
     );
+    /// @notice Emitted when ETH has been successfully withdrawn from the contract.
+    event WithdrawnETH(address indexed recipient, uint256 amount);
 
     /// @notice Initialize with Endpoint V2 and owner address.
     /// @param endpoint The local chain's LayerZero Endpoint V2 address.
@@ -90,4 +96,17 @@ contract GovernanceOAppReceiver is OAppReceiver {
 
         emit ProposalExecuted(origin, guid, targets, values, functionSignatures, datas, operations);
     }
+
+    /// @dev Allows the owner to withdraw ETH held by the contract.
+    /// @param amount Amount of withdrawn ETH.
+    /// @param recipient Receiver of the withdrawn ETH, should be non-null.
+    function withdrawETH(uint256 amount, address recipient) external onlyOwner {
+        if (recipient == address(0)) revert InvalidNullRecipient();
+
+        (bool success, ) = recipient.call{ value: amount }("");
+        if (!success) {
+            revert FailedToWithdrawETH();
+        }
+        emit WithdrawnETH(recipient, amount);
+    }
 }

protocol-contracts/governance/test/GovernanceOApp.test.ts

@@ -237,7 +237,7 @@ describe('GovernanceOApp Test', function () {
         expect(BigInt(await gatewayConfigMockBis.value())).to.equal(2n)
     })
 
-    it('owner can wihdraw ETH from prefunded governanceOAppSender', async function () {
+    it('owner can wihdraw ETH from prefunded GovernanceOAppSender', async function () {
         await owner.sendTransaction({
             to: governanceOAppSender.address,
             value: ethers.utils.parseEther('1'),
@@ -259,7 +259,7 @@ describe('GovernanceOApp Test', function () {
         expect(diff <= tolerance).to.equal(true)
     })
 
-    it('should not send recovered funds to null address', async function () {
+    it('should not send recovered funds from GovernanceOAppSender to null address', async function () {
         await owner.sendTransaction({
             to: governanceOAppSender.address,
             value: ethers.utils.parseEther('1'),
@@ -278,6 +278,49 @@ describe('GovernanceOApp Test', function () {
         }
     })
 
+    it('owner can wihdraw ETH from prefunded GovernanceOAppReceiver', async function () {
+        await ethers.provider.send('hardhat_setBalance', [
+            // on a real network, funds could be sent to GovernanceOAppReceiver contract via the payable lzReceive method
+            governanceOAppReceiver.address,
+            '0xde0b6b3a7640000', // 1 ETH in hex
+        ])
+
+        const balanceOwnerBefore = await ethers.provider.getBalance(owner.address)
+        const balanceGovReceiverBefore = await ethers.provider.getBalance(governanceOAppReceiver.address)
+        expect(balanceGovReceiverBefore.toBigInt()).to.equal(ethers.utils.parseEther('1').toBigInt())
+
+        await governanceOAppReceiver.withdrawETH(ethers.utils.parseEther('1'), owner.address)
+
+        const balanceOwnerAfter = await ethers.provider.getBalance(owner.address)
+        const balanceGovReceiverAfter = await ethers.provider.getBalance(governanceOAppReceiver.address)
+        expect(balanceGovReceiverAfter.toBigInt()).to.equal(0n)
+        const received = balanceOwnerAfter.sub(balanceOwnerBefore).toBigInt()
+        const expected = ethers.utils.parseEther('1').toBigInt()
+        const tolerance = ethers.utils.parseEther('0.0001').toBigInt() // account gas used for the tx
+        const diff = received > expected ? received - expected : expected - received
+        expect(diff <= tolerance).to.equal(true)
+    })
+
+    it('should not send recovered funds from GovernanceOAppReceiver to null address', async function () {
+        await ethers.provider.send('hardhat_setBalance', [
+            // on a real network, funds could be sent to GovernanceOAppReceiver contract via the payable lzReceive method
+            governanceOAppReceiver.address,
+            '0xde0b6b3a7640000', // 1 ETH in hex
+        ])
+        const tx = governanceOAppReceiver
+            .connect(owner)
+            .withdrawETH(ethers.utils.parseEther('1'), ethers.constants.AddressZero)
+        try {
+            await tx
+            expect.fail('withdrawETH should have reverted with InvalidNullRecipient')
+        } catch (err: any) {
+            const data = err.data
+            const selector = data.slice(0, 10)
+            const expected = governanceOAppReceiver.interface.getSighash('InvalidNullRecipient()')
+            expect(selector).to.equal(expected)
+        }
+    })
+
     it('should send a payable remote proposal', async function () {
         expect(BigInt(await gatewayConfigMock.value())).to.equal(0n)
         await owner.sendTransaction({