Use a pull model to withdraw deposit ETH balances

solidity/contracts/deposit/Deposit.sol

@@ -402,4 +402,18 @@ contract Deposit is DepositFactoryAuthority {
         self.notifyCourtesyTimeout();
         return true;
     }
+
+    /// @notice     Withdraw caller's allowance.
+    /// @dev        Withdrawals can only happen when a contract is in an end-state.
+    /// @return     True if successful, otherwise revert.
+    function withdrawFunds() public returns (bool) {
+        self.withdrawFunds();
+        return true;
+    }
+
+    /// @notice     Get caller's withdraw allowance.
+    /// @return     The withdraw allowance in wei.
+    function getWithdrawAllowance() public returns (uint256) {
+        return self.getWithdrawAllowance();
+    }
 }

solidity/contracts/deposit/DepositFunding.sol

@@ -78,13 +78,6 @@ library DepositFunding {
         return true;
     }
 
-    /// @notice     Seizes signer bonds and distributes them to the funder.
-    /// @dev        This is only called as part of funding fraud flow.
-    function distributeSignerBondsToFunder(DepositUtils.Deposit storage _d) internal {
-        uint256 _seized = _d.seizeSignerBonds();
-        _d.depositOwner().transfer(_seized);  // Transfer whole amount
-    }
-
     /// @notice     Anyone may notify the contract that signing group setup has timed out.
     /// @param  _d  Deposit storage pointer.
     function notifySignerSetupFailure(DepositUtils.Deposit storage _d) public {
@@ -98,7 +91,7 @@ library DepositFunding {
         uint256 _seized = _d.seizeSignerBonds();
 
         /* solium-disable-next-line security/no-send */
-        _d.depositOwner().send(_d.keepSetupFee);
+        _d.enableWithdrawal(_d.depositOwner(), _d.keepSetupFee);
         _d.pushFundsToKeepGroup(_seized.sub(_d.keepSetupFee));
 
         _d.setFailedSetup();
@@ -169,7 +162,11 @@ library DepositFunding {
         bool _isFraud = _d.submitSignatureFraud(_v, _r, _s, _signedDigest, _preimage);
         require(_isFraud, "Signature is not fraudulent");
         _d.logFraudDuringSetup();
-        distributeSignerBondsToFunder(_d);
+
+        // Allow deposit owner to withdraw seized bonds after contract termination.
+        uint256 _seized = _d.seizeSignerBonds();
+        _d.enableWithdrawal(_d.depositOwner(), _seized);
+
         fundingFraudTeardown(_d);
         _d.setFailedSetup();
         _d.logSetupFailed();

solidity/contracts/deposit/DepositLiquidation.sol

@@ -74,7 +74,7 @@ library DepositLiquidation {
         if (_d.auctionTBTCAmount() == 0) {
             // we came from the redemption flow
             _d.setLiquidated();
-            _d.redeemerAddress.transfer(_seized);
+            _d.enableWithdrawal(_d.redeemerAddress, _seized);
             _d.logLiquidated();
             return;
         }
@@ -167,8 +167,8 @@ library DepositLiquidation {
 
         // send the TBTC to the TDT holder. If the TDT holder is the Vending Machine, burn it to maintain the peg.
         address tdtHolder = _d.depositOwner();
-
         uint256 lotSizeTbtc = _d.lotSizeTbtc();
+
         require(_d.tbtcToken.balanceOf(msg.sender) >= lotSizeTbtc, "Not enough TBTC to cover outstanding debt");
 
         if(tdtHolder == _d.vendingMachineAddress){
@@ -179,8 +179,8 @@ library DepositLiquidation {
         }
 
         // Distribute funds to auction buyer
-        uint256 _valueToDistribute = _d.auctionValue();
-        msg.sender.transfer(_valueToDistribute);
+        uint256 valueToDistribute = _d.auctionValue();
+        _d.enableWithdrawal(msg.sender, valueToDistribute);
 
         // Send any TBTC left to the Fee Rebate Token holder
         _d.distributeFeeRebate();
@@ -195,16 +195,15 @@ library DepositLiquidation {
         if (initiator == address(0)){
             initiator = address(0xdead);
         }
-        if (contractEthBalance > 1) {
+        if (contractEthBalance > valueToDistribute + 1) {
+            uint256 remainingUnallocated = contractEthBalance.sub(valueToDistribute);
             if (_wasFraud) {
-                /* solium-disable-next-line security/no-send */
-                initiator.send(contractEthBalance);
+                _d.enableWithdrawal(initiator, remainingUnallocated);
             } else {
                 // There will always be a liquidation initiator.
-                uint256 split = contractEthBalance.div(2);
+                uint256 split = remainingUnallocated.div(2);
                 _d.pushFundsToKeepGroup(split);
-                /* solium-disable-next-line security/no-send */
-                initiator.send(address(this).balance);
+                _d.enableWithdrawal(initiator, remainingUnallocated.sub(split));
             }
         }
     }

solidity/contracts/deposit/DepositUtils.sol

@@ -66,9 +66,10 @@ library DepositUtils {
         uint256 fundedAt; // timestamp when funding proof was received
         bytes utxoOutpoint;  // the 36-byte outpoint of the custodied UTXO
 
-        /// @notice Map of timestamps for transaction digests approved for signing
-        /// @dev Holds a timestamp from the moment when the transaction digest
-        /// was approved for signing
+        /// @dev Map of ETH balances an address can withdraw after contract reaches ends-state.
+        mapping(address => uint256) withdrawalAllowances;
+
+        /// @dev Map of timestamps representing when transaction digests were approved for signing
         mapping (bytes32 => uint256) approvedDigests;
     }
 
@@ -396,6 +397,34 @@ library DepositUtils {
         return _postCallBalance.sub(_preCallBalance);
     }
 
+    /// @notice     Adds a given amount to the withdraw allowance for the address.
+    /// @dev        Withdrawals can only happen when a contract is in an end-state.
+    function enableWithdrawal(DepositUtils.Deposit storage _d, address _withdrawer, uint256 _amount) internal {
+        _d.withdrawalAllowances[_withdrawer] = _d.withdrawalAllowances[_withdrawer].add(_amount);
+    }
+
+    /// @notice     Withdraw caller's allowance.
+    /// @dev        Withdrawals can only happen when a contract is in an end-state.
+    function withdrawFunds(DepositUtils.Deposit storage _d) internal {
+        uint256 available = _d.withdrawalAllowances[msg.sender];
+
+        require(_d.inEndState(), "Contract not yet terminated");
+        require(available > 0, "Nothing to withdraw");
+        require(address(this).balance >= available, "Insufficient contract balance");
+
+        // zero-out to prevent reentrancy
+        _d.withdrawalAllowances[msg.sender] = 0;
+
+        /* solium-disable-next-line security/no-call-value */
+        msg.sender.call.value(available)("");
+    }
+
+    /// @notice     Get the caller's withdraw allowance.
+    /// @return     The caller's withdraw allowance in wei.
+    function getWithdrawAllowance(DepositUtils.Deposit storage _d) internal returns (uint256) {
+        return _d.withdrawalAllowances[msg.sender];
+    }
+
     /// @notice     Distributes the fee rebate to the Fee Rebate Token owner.
     /// @dev        Whenever this is called we are shutting down.
     function distributeFeeRebate(Deposit storage _d) internal {

solidity/contracts/test/deposit/TestDeposit.sol

@@ -254,4 +254,9 @@ contract TestDeposit is Deposit {
     function getAuctionBasePercentage() public view returns (uint256) {
         return self.getAuctionBasePercentage();
     }
+
+    function auctionValue() public view returns (uint256) {
+        return self.auctionValue();
+    }
+
 }

solidity/contracts/test/deposit/TestDepositUtils.sol

@@ -49,10 +49,6 @@ contract TestDepositUtils is TestDeposit {
         return self.signerPKH();
     }
 
-    function auctionValue() public view returns (uint256) {
-        return self.auctionValue();
-    }
-
     function utxoSize() public view returns (uint256) {
         return self.utxoSize();
     }
@@ -84,6 +80,10 @@ contract TestDepositUtils is TestDeposit {
     function pushFundsToKeepGroup(uint256 _ethValue) public returns (bool) {
         return self.pushFundsToKeepGroup(_ethValue);
     }
+
+    function enableWithdrawal(address _withdrawer, uint256 _amount) public {
+        self.enableWithdrawal(_withdrawer, _amount);
+    }
 }
 
 // Separate contract for testing SPV proofs, as putting this in the main

solidity/test/DepositFraudTest.js

@@ -56,7 +56,6 @@ describe("DepositFraud", async function() {
 
     it("updates to awaiting fraud funding proof, distributes signer bond to funder, and logs FraudDuringSetup", async () => {
       const blockNumber = await web3.eth.getBlock("latest").number
-      const initialBalance = await web3.eth.getBalance(beneficiary)
 
       await testDeposit.provideFundingECDSAFraudProof(
         0,
@@ -67,11 +66,12 @@ describe("DepositFraud", async function() {
       )
 
       await assertBalance.eth(ecdsaKeepStub.address, new BN(0))
+      await assertBalance.eth(testDeposit.address, new BN(bond))
 
-      await assertBalance.eth(
-        beneficiary,
-        new BN(initialBalance).add(new BN(bond)),
-      )
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: beneficiary,
+      })
+      expect(withdrawable).to.eq.BN(new BN(bond))
 
       const depositState = await testDeposit.getState.call()
       expect(depositState).to.eq.BN(states.FAILED_SETUP)
@@ -156,18 +156,17 @@ describe("DepositFraud", async function() {
 
       const currentBond = await web3.eth.getBalance(ecdsaKeepStub.address)
       const block = await web3.eth.getBlock("latest")
-      const initialBalance = await web3.eth.getBalance(owner)
       await testDeposit.startSignerFraudLiquidation()
 
       const events = await tbtcSystemStub.getPastEvents("Liquidated", {
         fromBlock: block.number,
         toBlock: "latest",
       })
 
-      await assertBalance.eth(
-        owner,
-        new BN(initialBalance).add(new BN(currentBond)),
-      )
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: owner,
+      })
+      expect(withdrawable).to.eq.BN(new BN(currentBond))
 
       expect(events[0].returnValues[0]).to.equal(testDeposit.address)
 

solidity/test/DepositFundingTest.js

@@ -218,16 +218,18 @@ describe("DepositFunding", async function() {
     })
 
     it("updates state to setup failed, deconstes state, logs SetupFailed, and refunds TDT owner", async () => {
-      const initialFunderBalance = await web3.eth.getBalance(owner)
       const blockNumber = await web3.eth.getBlock("latest").number
-      await testDeposit.notifySignerSetupFailure()
+      await testDeposit.notifySignerSetupFailure({from: owner})
 
       const signingGroupRequestedAt = await testDeposit.getSigningGroupRequestedAt.call()
-      const finalFunderBalance = await web3.eth.getBalance(owner)
 
-      expect(
-        new BN(finalFunderBalance).sub(new BN(initialFunderBalance)),
-      ).to.eq.BN(openKeepFee)
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: owner,
+      })
+
+      const depositBalance = await web3.eth.getBalance(testDeposit.address)
+      expect(withdrawable).to.eq.BN(new BN(openKeepFee))
+      expect(withdrawable).to.eq.BN(new BN(depositBalance))
 
       expect(
         signingGroupRequestedAt,

solidity/test/DepositLiquidationTest.js

@@ -163,23 +163,30 @@ describe("DepositLiquidation", async function() {
       ).to.eq.BN(tokenCheck)
     })
 
-    it("distributes value to the buyer", async () => {
-      const value = 10000000000000000
+    it("awards withdrawable value to the buyer", async () => {
+      const value = new BN("10000000000000000")
       const block = await web3.eth.getBlock("latest")
       const notifiedTime = block.timestamp
-      const initialBalance = await web3.eth.getBalance(buyer)
 
       await ecdsaKeepStub.pushFundsFromKeep(testDeposit.address, {value: value})
 
       await testDeposit.setLiquidationAndCourtesyInitated(notifiedTime, 0)
+      const auctionValue = await testDeposit.auctionValue.call()
+
       await testDeposit.purchaseSignerBondsAtAuction({from: buyer})
 
-      const finalBalance = await web3.eth.getBalance(buyer)
+      // calculate the split of the un-purchased signer bond
+      const split = value.sub(auctionValue).div(new BN(2))
 
-      expect(
-        new BN(finalBalance),
-        "buyer balance should increase",
-      ).to.be.gte.BN(initialBalance)
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: buyer,
+      })
+      const depositBalance = await web3.eth.getBalance(testDeposit.address)
+
+      expect(depositBalance).to.eq.BN(auctionValue.add(split))
+      expect(withdrawable, "buyer should have a withdrawable balance").to.eq.BN(
+        auctionValue,
+      )
     })
 
     it("splits funds between liquidation triggerer and signers if not fraud", async () => {
@@ -191,36 +198,35 @@ describe("DepositLiquidation", async function() {
 
       await ecdsaKeepStub.pushFundsFromKeep(testDeposit.address, {value: value})
 
-      const initialInitiatorBalance = await web3.eth.getBalance(
-        liquidationInitiator,
-      )
       const initialSignerBalance = await web3.eth.getBalance(
         ecdsaKeepStub.address,
       )
 
       await testDeposit.setLiquidationInitiator(liquidationInitiator)
       await testDeposit.setLiquidationAndCourtesyInitated(notifiedTime, 0)
+
+      const auctionValue = await testDeposit.auctionValue.call()
       // Buy auction immediately. No scaling taken place. Auction value is base percentage of signer bond.
       await testDeposit.purchaseSignerBondsAtAuction({from: buyer})
 
-      const finalInitiatorBalance = await web3.eth.getBalance(
-        liquidationInitiator,
-      )
       const finalSignerBalance = await web3.eth.getBalance(
         ecdsaKeepStub.address,
       )
 
-      const initiatorBalanceDiff = new BN(finalInitiatorBalance).sub(
-        new BN(initialInitiatorBalance),
-      )
       const signerBalanceDiff = new BN(finalSignerBalance).sub(
         new BN(initialSignerBalance),
       )
 
       const totalReward = (value * (100 - basePercentage)) / 100
       const split = totalReward / 2
 
-      expect(new BN(split)).to.eq.BN(initiatorBalanceDiff)
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: liquidationInitiator,
+      })
+      const depositBalance = await web3.eth.getBalance(testDeposit.address)
+
+      expect(depositBalance).to.eq.BN(auctionValue.add(new BN(split)))
+      expect(new BN(split)).to.eq.BN(withdrawable)
       expect(new BN(split)).to.eq.BN(signerBalanceDiff)
     })
 
@@ -233,9 +239,6 @@ describe("DepositLiquidation", async function() {
 
       await ecdsaKeepStub.pushFundsFromKeep(testDeposit.address, {value: value})
 
-      const initialInitiatorBalance = await web3.eth.getBalance(
-        liquidationInitiator,
-      )
       const initialSignerBalance = await web3.eth.getBalance(
         ecdsaKeepStub.address,
       )
@@ -246,24 +249,24 @@ describe("DepositLiquidation", async function() {
       // Buy auction immediately. No scaling taken place. Auction value is base percentage of signer bond.
       await testDeposit.purchaseSignerBondsAtAuction({from: buyer})
 
-      const finalInitiatorBalance = await web3.eth.getBalance(
-        liquidationInitiator,
-      )
       const finalSignerBalance = await web3.eth.getBalance(
         ecdsaKeepStub.address,
       )
 
-      const initiatorBalanceDiff = new BN(finalInitiatorBalance).sub(
-        new BN(initialInitiatorBalance),
-      )
       const signerBalanceDiff = new BN(finalSignerBalance).sub(
         new BN(initialSignerBalance),
       )
 
+      const withdrawable = await testDeposit.getWithdrawAllowance.call({
+        from: liquidationInitiator,
+      })
+
       const totalReward = (value * (100 - basePercentage)) / 100
+      const depositBalance = await web3.eth.getBalance(testDeposit.address)
 
+      expect(depositBalance).to.eq.BN(new BN(value))
       expect(new BN(signerBalanceDiff)).to.eq.BN(0)
-      expect(new BN(initiatorBalanceDiff)).to.eq.BN(totalReward)
+      expect(new BN(withdrawable)).to.eq.BN(totalReward)
     })
   })