Withdraw helper contract to support collecting funds from channels (#454)

abarmat · abarmat · commit 9e8a41fc8402 · 2021-05-17T11:32:41.000-03:00
Introduces a contract called GRTWithdrawHelper. The main goal of this contract is to encode custom logic in the process of withdrawing funds from the Vector Channel Multisigs. The contract receives funds from a Channel Multisig, then it forwards them to the Staking contract related to an Allocation.

- Add a withdraw helper contract to support collecting funds from channels.
- Try catch revert on the Staking contract and return funds if that happens.
- Add an explicit parties defined return address and wrap approve call to return funds in case of revert.
diff --git a/cli/defaults.ts b/cli/defaults.ts
@@ -9,8 +9,8 @@ export const local = {
   accountNumber: '0',
 }
 export const defaultOverrides: Overrides = {
-//  gasPrice: utils.parseUnits('25', 'gwei'), // auto
-//  gasLimit: 2000000, // auto
+  //  gasPrice: utils.parseUnits('25', 'gwei'), // auto
+  //  gasLimit: 2000000, // auto
 }
 
 export const cliOpts = {
diff --git a/contracts/statechannels/GRTWithdrawHelper.sol b/contracts/statechannels/GRTWithdrawHelper.sol
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.7.3;
+pragma experimental ABIEncoderV2;
+
+import "../token/IGraphToken.sol";
+
+import "./WithdrawHelper.sol";
+
+/**
+ * @title WithdrawHelper contract for GRT tokens
+ * @notice This contract encodes the logic that connects the transfer of funds from a
+ * Channel Multisig to the protocol in the context of a withdrawal.
+ * A Channel Multisig will atomically transfer the tokens to the WithdrawHelper and then
+ * these tokens will get pulled from the Staking contract using the `allocationID` passed
+ * in the `callData` of the WithdrawCommitment.
+ * Tokens transferred are associated to a particular allocation in the Staking contract.
+ * This contract is not meant to hold funds, as they can be stolen by presenting a
+ * handcrafted WithdrawalCommitment.
+ */
+contract GRTWithdrawHelper is WithdrawHelper {
+    struct CollectData {
+        address staking;
+        address allocationID;
+        address returnAddress;
+    }
+
+    bytes4 private constant APPROVE_SELECTOR = bytes4(keccak256("approve(address,uint256)"));
+    bytes4 private constant COLLECT_SELECTOR = bytes4(keccak256("collect(uint256,address)"));
+
+    // -- State --
+
+    address public immutable tokenAddress;
+
+    /**
+     * @notice Contract constructor.
+     * @param _tokenAddress Token address to use for transfers
+     */
+    constructor(address _tokenAddress) {
+        tokenAddress = _tokenAddress;
+    }
+
+    /**
+     * @notice Returns the ABI encoded representation of a CollectData struct.
+     * @param _collectData CollectData struct with information about how to collect funds
+     */
+    function getCallData(CollectData calldata _collectData) public pure returns (bytes memory) {
+        return abi.encode(_collectData);
+    }
+
+    /**
+     * @notice Execute hook used by a channel to send funds to the protocol.
+     * @param _wd WithdrawData struct for the withdrawal commitment
+     * @param _actualAmount Amount to transfer to the Staking contract
+     */
+    function execute(WithdrawData calldata _wd, uint256 _actualAmount) external override {
+        require(_wd.assetId == tokenAddress, "GRTWithdrawHelper: !token");
+
+        // Decode and validate collect data
+        CollectData memory collectData = abi.decode(_wd.callData, (CollectData));
+        require(collectData.staking != address(0), "GRTWithdrawHelper: !staking");
+        require(collectData.allocationID != address(0), "GRTWithdrawHelper: !allocationID");
+        require(collectData.returnAddress != address(0), "GRTWithdrawHelper: !returnAddress");
+
+        // Approve the staking contract to pull the transfer amount
+        (bool success1, ) =
+            tokenAddress.call(
+                abi.encodeWithSelector(APPROVE_SELECTOR, collectData.staking, _actualAmount)
+            );
+
+        // If the call fails return the funds to the return address and bail
+        if (!success1) {
+            _sendTokens(collectData.returnAddress, _actualAmount);
+            return;
+        }
+
+        // Call the Staking contract to collect funds from this contract
+        (bool success2, ) =
+            collectData.staking.call(
+                abi.encodeWithSelector(COLLECT_SELECTOR, _actualAmount, collectData.allocationID)
+            );
+
+        // If the call fails return the funds to the return address
+        if (!success2) {
+            _sendTokens(collectData.returnAddress, _actualAmount);
+        }
+    }
+
+    /**
+     * @notice Send tokens out of the contract.
+     * @param _to Destination address
+     * @param _amount Amount to transfer
+     */
+    function _sendTokens(address _to, uint256 _amount) private {
+        IGraphToken(tokenAddress).transfer(_to, _amount);
+    }
+}
diff --git a/contracts/statechannels/ICMCWithdraw.sol b/contracts/statechannels/ICMCWithdraw.sol
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.7.3;
+pragma experimental ABIEncoderV2;
+
+struct WithdrawData {
+    address channelAddress;
+    address assetId;
+    address payable recipient;
+    uint256 amount;
+    uint256 nonce;
+    address callTo;
+    bytes callData;
+}
+
+interface ICMCWithdraw {
+    function getWithdrawalTransactionRecord(WithdrawData calldata wd) external view returns (bool);
+
+    function withdraw(
+        WithdrawData calldata wd,
+        bytes calldata aliceSignature,
+        bytes calldata bobSignature
+    ) external;
+}
diff --git a/contracts/statechannels/WithdrawHelper.sol b/contracts/statechannels/WithdrawHelper.sol
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.7.3;
+pragma experimental ABIEncoderV2;
+
+import "./ICMCWithdraw.sol";
+
+interface WithdrawHelper {
+    function execute(WithdrawData calldata wd, uint256 actualAmount) external;
+}
diff --git a/test/lib/testHelpers.ts b/test/lib/testHelpers.ts
@@ -1,6 +1,6 @@
 import hre from 'hardhat'
 import { providers, utils, BigNumber, Signer, Wallet } from 'ethers'
-import { formatUnits } from 'ethers/lib/utils'
+import { formatUnits, getAddress } from 'ethers/lib/utils'
 
 import { EpochManager } from '../../build/typechain/contracts/EpochManager'
 
@@ -17,6 +17,7 @@ export const logStake = (stakes: any): void => {
     console.log(k, ':', parseEther(v as string))
   })
 }
+export const randomAddress = (): string => getAddress(randomHexBytes(20))
 
 // Network
 
diff --git a/test/payments/withdrawHelper.test.ts b/test/payments/withdrawHelper.test.ts
@@ -0,0 +1,188 @@
+import { expect } from 'chai'
+import { constants } from 'ethers'
+
+import { GRTWithdrawHelper } from '../../build/typechain/contracts/GRTWithdrawHelper'
+import { GraphToken } from '../../build/typechain/contracts/GraphToken'
+import { Staking } from '../../build/typechain/contracts/Staking'
+
+import { NetworkFixture } from '../lib/fixtures'
+import * as deployment from '../lib/deployment'
+import {
+  deriveChannelKey,
+  getAccounts,
+  randomAddress,
+  randomHexBytes,
+  toGRT,
+  Account,
+} from '../lib/testHelpers'
+
+const { AddressZero } = constants
+
+describe('WithdrawHelper', () => {
+  let cmc: Account
+  let governor: Account
+  let indexer: Account
+
+  let fixture: NetworkFixture
+
+  let grt: GraphToken
+  let staking: Staking
+  let withdrawHelper: GRTWithdrawHelper
+
+  function createWithdrawData(callData: string) {
+    return {
+      amount: 0,
+      assetId: grt.address,
+      callData,
+      callTo: withdrawHelper.address,
+      channelAddress: randomAddress(),
+      nonce: 1,
+      recipient: withdrawHelper.address,
+    }
+  }
+
+  before(async function () {
+    ;[cmc, governor, indexer] = await getAccounts()
+
+    fixture = new NetworkFixture()
+    ;({ grt, staking } = await fixture.load(governor.signer))
+    withdrawHelper = ((await deployment.deployContract(
+      'GRTWithdrawHelper',
+      governor.signer,
+      grt.address,
+    )) as unknown) as GRTWithdrawHelper
+
+    // Give some funds to the indexer and approve staking contract to use funds on indexer behalf
+    const indexerTokens = toGRT('100000')
+    await grt.connect(governor.signer).mint(indexer.address, indexerTokens)
+    await grt.connect(indexer.signer).approve(staking.address, indexerTokens)
+
+    // Give some funds to the CMC multisig fake account
+    const cmcTokens = toGRT('2000')
+    await grt.connect(governor.signer).mint(cmc.address, cmcTokens)
+
+    // Allow WithdrawHelper to call the Staking contract
+    await staking.connect(governor.signer).setAssetHolder(withdrawHelper.address, true)
+  })
+
+  beforeEach(async function () {
+    await fixture.setUp()
+  })
+
+  afterEach(async function () {
+    await fixture.tearDown()
+  })
+
+  describe('execute withdrawal', function () {
+    it('withdraw tokens from the CMC to staking contract through WithdrawHelper', async function () {
+      // Generate test data
+      const channelKey = deriveChannelKey()
+      const allocationID = channelKey.address
+      const subgraphDeploymentID = randomHexBytes(32)
+      const metadata = randomHexBytes(32)
+
+      // Setup staking
+      const stakeTokens = toGRT('100000')
+      await staking.connect(indexer.signer).stake(stakeTokens)
+      await staking
+        .connect(indexer.signer)
+        .allocate(
+          subgraphDeploymentID,
+          stakeTokens,
+          allocationID,
+          metadata,
+          await channelKey.generateProof(indexer.address),
+        )
+
+      // Initiate a withdrawal
+      // For the purpose of the test we skip the CMC and call WithdrawHelper
+      // directly. Transfer tokens from the CMC -> WithdrawHelper being the recipient
+      const actualAmount = toGRT('2000') // <- withdraw amount
+      await grt.connect(cmc.signer).transfer(withdrawHelper.address, actualAmount)
+
+      // Simulate callTo from the CMC to the WithdrawHelper
+      const callData = await withdrawHelper.getCallData({
+        staking: staking.address,
+        allocationID,
+        returnAddress: randomAddress(),
+      })
+      const withdrawData = {
+        amount: 0,
+        assetId: grt.address,
+        callData,
+        callTo: withdrawHelper.address,
+        channelAddress: cmc.address,
+        nonce: 1,
+        recipient: withdrawHelper.address,
+      }
+      await withdrawHelper.connect(cmc.signer).execute(withdrawData, actualAmount)
+
+      // Allocation must have collected the withdrawn tokens
+      const allocation = await staking.allocations(allocationID)
+      expect(allocation.collectedFees).eq(actualAmount)
+
+      // CMC should not have more funds
+      expect(await grt.balanceOf(cmc.address)).eq(0)
+    })
+
+    it('withdraw tokens from the CMC to staking contract through WithdrawHelper (invalid allocation)', async function () {
+      // Use an invalid allocation
+      const allocationID = '0xfefefefefefefefefefefefefefefefefefefefe'
+      const returnAddress = randomAddress()
+
+      // Initiate a withdrawal
+      // For the purpose of the test we skip the CMC and call WithdrawHelper
+      // directly. Transfer tokens from the CMC -> WithdrawHelper being the recipient
+      const actualAmount = toGRT('2000') // <- withdraw amount
+      await grt.connect(cmc.signer).transfer(withdrawHelper.address, actualAmount)
+
+      // Simulate callTo from the CMC to the WithdrawHelper
+      const callData = await withdrawHelper.getCallData({
+        staking: staking.address,
+        allocationID,
+        returnAddress,
+      })
+      const withdrawData = {
+        amount: 0,
+        assetId: grt.address,
+        callData,
+        callTo: withdrawHelper.address,
+        channelAddress: cmc.address,
+        nonce: 1,
+        recipient: withdrawHelper.address,
+      }
+
+      // This reverts!
+      await withdrawHelper.connect(cmc.signer).execute(withdrawData, actualAmount)
+
+      // There should not be collected fees
+      const allocation = await staking.allocations(allocationID)
+      expect(allocation.collectedFees).eq(0)
+
+      // CMC should have the funds returned
+      expect(await grt.balanceOf(returnAddress)).eq(actualAmount)
+    })
+
+    it('reject collect data with no staking address', async function () {
+      // Simulate callTo from the CMC to the WithdrawHelper
+      const callData = await withdrawHelper.getCallData({
+        staking: AddressZero,
+        allocationID: randomAddress(),
+        returnAddress: randomAddress(),
+      })
+      const tx = withdrawHelper.execute(createWithdrawData(callData), toGRT('100'))
+      await expect(tx).revertedWith('GRTWithdrawHelper: !staking')
+    })
+
+    it('reject collect data with no allocation', async function () {
+      // Simulate callTo from the CMC to the WithdrawHelper
+      const callData = await withdrawHelper.getCallData({
+        staking: randomAddress(),
+        allocationID: AddressZero,
+        returnAddress: randomAddress(),
+      })
+      const tx = withdrawHelper.execute(createWithdrawData(callData), toGRT('100'))
+      await expect(tx).revertedWith('GRTWithdrawHelper: !allocationID')
+    })
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -9,8 +9,8 @@ export const local = {`
`9`	`9`	`accountNumber: '0',`
`10`	`10`	`}`
`11`	`11`	`export const defaultOverrides: Overrides = {`
`12`		`-// gasPrice: utils.parseUnits('25', 'gwei'), // auto`
`13`		`-// gasLimit: 2000000, // auto`
	`12`	`+ // gasPrice: utils.parseUnits('25', 'gwei'), // auto`
	`13`	`+ // gasLimit: 2000000, // auto`
`14`	`14`	`}`
`15`	`15`
`16`	`16`	`export const cliOpts = {`