Merge pull request #17 from provable-things/fix-weth-transfer

fix(wETH-pegouts): forward all available gas on during peg outs

contracts/CONTRACT_WITH_EXPENSIVE_FALLBACK_FXN.sol

@@ -0,0 +1,11 @@
+pragma solidity ^0.6.0;
+
+contract CONTRACT_WITH_EXPENSIVE_FALLBACK_FXN {
+    mapping(uint256 => address) pointlessMapping;
+
+    fallback() external payable {
+        for (uint256 i; i < 10; i++) {
+            pointlessMapping[i] = msg.sender;
+        }
+    }
+}

contracts/Erc20Vault.sol

@@ -181,7 +181,11 @@ contract Erc20Vault is Withdrawable, IERC777Recipient {
     {
         if (_tokenAddress == address(weth)) {
             weth.withdraw(_tokenAmount);
-            _tokenRecipient.transfer(_tokenAmount);
+            // NOTE: This is the latest recommendation (@ time of writing) for transferring ETH. This no longer relies
+            // on the provided 2300 gas stipend and instead forwards all available gas onwards.
+            // SOURCE: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now
+            (bool success, ) = _tokenRecipient.call.value(_tokenAmount)("");
+            require(success, "ETH transfer failed when pegging out wETH!");
         } else {
             IERC20(_tokenAddress).safeTransfer(_tokenRecipient, _tokenAmount);
         }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ptokens-perc20-smart-contract",
-  "version": "1.6.0",
+  "version": "1.7.0",
   "description": "The pToken pERC20-on-EOS-vault smart-contract",
   "main": "index.js",
   "scripts": {

test/erc20-vault-tests.js

@@ -6,6 +6,7 @@ const ERC20_ARTIFACT = artifacts.require('ERC20_TOKEN')
 const ERC777_ARTIFACT = artifacts.require('ERC777_TOKEN')
 const VaultArtifact = artifacts.require('Erc20Vault')
 const { expectRevert } = require('@openzeppelin/test-helpers')
+const CONTRACT_WITH_EXPENSIVE_FALLBACK_FXN_ARTIFACT = artifacts.require('CONTRACT_WITH_EXPENSIVE_FALLBACK_FXN')
 
 const GAS_LIMIT = 3e6
 
@@ -331,7 +332,7 @@ contract('Erc20Vault', ([PNETWORK_ADDRESS, NON_PNETWORK_ADDRESS, TOKEN_HOLDER_AD
     assert.strictEqual(await web3.eth.getBalance(VAULT_ADDRESS), '0', 'eth balance must be 0')
   })
 
-  it('Should peg out WETH_CONTRACT', async () => {
+  it('Should peg out wETH', async () => {
     await addTokenSupport(VAULT_METHODS, WETH_ADDRESS, PNETWORK_ADDRESS)
     await VAULT_METHODS.pegInEth(DESTINATION_ADDRESS).send({ from: TOKEN_HOLDER_ADDRESS, value: TOKEN_AMOUNT })
     const ethBalanceBefore = await web3.eth.getBalance(TOKEN_HOLDER_ADDRESS)
@@ -450,4 +451,22 @@ contract('Erc20Vault', ([PNETWORK_ADDRESS, NON_PNETWORK_ADDRESS, TOKEN_HOLDER_AD
     ))
     vaultTokenBalancesAfter.map(_balance => assert.strictEqual(parseInt(_balance), 0))
   })
+
+  it('Should peg out wETH to smart-contract w/ expensive fallback function', async () => {
+    const PEG_OUT_GAS_LIMIT = 450e3
+    const expensiveFallbackContract = await getContract(web3, CONTRACT_WITH_EXPENSIVE_FALLBACK_FXN_ARTIFACT)
+    const expensiveFallbackContractAddress = expensiveFallbackContract._address
+    await addTokenSupport(VAULT_METHODS, WETH_ADDRESS, PNETWORK_ADDRESS)
+    const expensiveContractEthBalanceBeforePegout = await web3.eth.getBalance(expensiveFallbackContractAddress)
+    assert.strictEqual(expensiveContractEthBalanceBeforePegout, '0')
+    await VAULT_METHODS.pegInEth(DESTINATION_ADDRESS).send({ from: TOKEN_HOLDER_ADDRESS, value: TOKEN_AMOUNT })
+    await VAULT_METHODS
+      .pegOut(expensiveFallbackContractAddress, WETH_ADDRESS, TOKEN_AMOUNT)
+      .send({ from: PNETWORK_ADDRESS, gas: PEG_OUT_GAS_LIMIT })
+    assert.strictEqual(await WETH_CONTRACT.methods.balanceOf(VAULT_ADDRESS).call(), '0')
+    const expensiveContractEthBalanceAfterPegout = await web3.eth.getBalance(expensiveFallbackContractAddress)
+    assert.strictEqual(expensiveContractEthBalanceAfterPegout, `${TOKEN_AMOUNT}`)
+    const vaultEthBalanceAfterPegOut = await web3.eth.getBalance(VAULT_ADDRESS)
+    assert.strictEqual(vaultEthBalanceAfterPegOut, '0', 'Vault\'s ETH balance after peg out must be 0!')
+  })
 })