allow fee withdrawal to given receiver - just in case the owner is a …

…contract which can't receive or otherwise handle ETH. refs omni#1
lab10-coop · Feb 26, 2019 · 27c26a2 · 27c26a2
1 parent f3ee829
commit 27c26a2
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 11 deletions.
diff --git a/contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol b/contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol
@@ -12,7 +12,7 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
 
     /// Event created on money withdraw.
     event UserRequestForAffirmation(address recipient, uint256 value);
-    event FeeWithdrawal(uint256 amount);
+    event FeeWithdrawal(address receiver, uint256 amount);
 
     function initialize(
         address _validatorContract,
@@ -67,12 +67,12 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
          */
     }
 
-    function withdrawCollectedFees() external onlyIfOwnerOfProxy {
+    function withdrawCollectedFeesTo(address receiver) external onlyIfOwnerOfProxy {
         uint256 amount = uintStorage[keccak256(abi.encodePacked("feeDeposits", owner()))];
         require(amount > 0, "nothing to claim");
         uintStorage[keccak256(abi.encodePacked("feeDeposits", owner()))] = 0;
-        emit FeeWithdrawal(amount);
-        msg.sender.transfer(amount); // throws on failure
+        emit FeeWithdrawal(receiver, amount);
+        receiver.transfer(amount); // throws on failure
     }
 
     function getBridgeMode() public pure returns(bytes4 _data) {

diff --git a/test/native_to_erc/foreign_bridge_test.js b/test/native_to_erc/foreign_bridge_test.js
@@ -156,13 +156,18 @@ contract('ForeignBridge', async (accounts) => {
       true.should.be.equal(await foreignBridge.relayedMessages(transactionHash))
     })
 
-    it('should allow owner only to claim collected tx fees', async () => {
-      const ownerBalanceBefore = await web3.eth.getBalance(owner);
-      await foreignBridge.withdrawCollectedFees({from: accounts[1]}).should.be.rejected
-      await foreignBridge.withdrawCollectedFees({from: owner}).should.be.fulfilled
-      const ownerBalanceAfter = await web3.eth.getBalance(owner);
-      ownerBalanceAfter.should.be.bignumber.above(ownerBalanceBefore)
-      await foreignBridge.withdrawCollectedFees({from: owner}).should.be.rejected // nothing left to withdraw
+    it('should allow owner only to withdraw collected tx fees', async () => {
+      const receiver = accounts[6];
+      let collectedFees = await foreignBridge.feeDepositOf(owner);
+      collectedFees.should.be.bignumber.above(0);
+      const receiverBalanceBefore = await web3.eth.getBalance(receiver);
+      await foreignBridge.withdrawCollectedFeesTo(receiver, {from: accounts[1]}).should.be.rejected
+      await foreignBridge.withdrawCollectedFeesTo(receiver, {from: owner}).should.be.fulfilled
+      const receiverBalanceAfter = await web3.eth.getBalance(receiver);
+      receiverBalanceAfter.should.be.bignumber.equal(receiverBalanceBefore.add(collectedFees));
+      collectedFees = await foreignBridge.feeDepositOf(owner);
+      collectedFees.should.be.bignumber.equal(0); // nothing left
+      await foreignBridge.withdrawCollectedFeesTo(receiver, {from: owner}).should.be.rejected // nothing left to withdraw
     })
   })