Require beneficiary to be set for withdraw

solidity/contracts/KeepBonding.sol

@@ -49,7 +49,11 @@ contract KeepBonding {
     mapping(address => mapping(address => bool)) internal authorizedPools;
 
     event UnbondedValueDeposited(address indexed operator, uint256 amount);
-    event UnbondedValueWithdrawn(address indexed operator, uint256 amount);
+    event UnbondedValueWithdrawn(
+        address indexed operator,
+        address indexed beneficiary,
+        uint256 amount
+    );
     event BondCreated(
         address indexed operator,
         address indexed holder,
@@ -371,11 +375,18 @@ contract KeepBonding {
 
         unbondedValue[operator] = unbondedValue[operator].sub(amount);
 
-        (bool success, ) = tokenStaking.beneficiaryOf(operator).call.value(
-            amount
-        )("");
+        address beneficiary = tokenStaking.beneficiaryOf(operator);
+        // Following check protects from a situation when a bonding value has
+        // been deposited before defining a beneficiary for the operator in the
+        // TokenStaking contract.
+        require(
+            beneficiary != address(0),
+            "Beneficiary not defined for the operator"
+        );
+
+        (bool success, ) = beneficiary.call.value(amount)("");
         require(success, "Transfer failed");
 
-        emit UnbondedValueWithdrawn(operator, amount);
+        emit UnbondedValueWithdrawn(operator, beneficiary, amount);
     }
 }

solidity/test/BondedECDSAKeepFactoryTest.js

@@ -1523,6 +1523,7 @@ contract("BondedECDSAKeepFactory", async (accounts) => {
       await keepBonding.authorizeSortitionPoolContract(members[i], signerPool, {
         from: authorizers[i],
       })
+      await tokenStaking.setBeneficiary(members[i], members[i])
     }
 
     const unbondedAmount = unbondedValue || minimumBond

solidity/test/BondedECDSAKeepTest.js

@@ -973,9 +973,9 @@ contract("BondedECDSAKeep", (accounts) => {
       const memberStake = web3.utils.toBN("100000000000000000000000")
       // setting a value other then the min stake for testing purposes
       await keep.setMemberStake(memberStake)
-      
+
       assert.isFalse(
-        await keep.isFradulentPreimageSet(preimage1), 
+        await keep.isFradulentPreimageSet(preimage1),
         "fradulent preimage should not have been set"
       )
 
@@ -988,10 +988,10 @@ contract("BondedECDSAKeep", (accounts) => {
       )
 
       assert.isTrue(
-        await keep.isFradulentPreimageSet(preimage1), 
+        await keep.isFradulentPreimageSet(preimage1),
         "fradulent preimage should have been set"
       )
-      
+
       await keep.submitSignatureFraud(
         signature1.V,
         signature1.R,
@@ -1005,7 +1005,10 @@ contract("BondedECDSAKeep", (accounts) => {
           members[i],
           constants.ZERO_ADDRESS
         )
-        expect(actualStake).to.eq.BN(minimumStake.sub(memberStake), `incorrect stake for member ${i}`)
+        expect(actualStake).to.eq.BN(
+          minimumStake.sub(memberStake),
+          `incorrect stake for member ${i}`
+        )
       }
     })
 
@@ -1563,13 +1566,18 @@ contract("BondedECDSAKeep", (accounts) => {
   describe("withdraw", async () => {
     const singleValue = new BN(1000)
     const ethValue = singleValue.mul(new BN(members.length))
+    const beneficiary = accounts[4]
 
     beforeEach(async () => {
       await keep.distributeETHReward({value: ethValue})
     })
 
     it("correctly transfers value", async () => {
-      const initialMemberBalance = new BN(await web3.eth.getBalance(members[0]))
+      const initialMemberBalance = new BN(
+        await web3.eth.getBalance(beneficiary)
+      )
+
+      await tokenStaking.setBeneficiary(members[0], beneficiary)
 
       await keep.withdraw(members[0])
 
@@ -1584,17 +1592,16 @@ contract("BondedECDSAKeep", (accounts) => {
       ).to.eq.BN(0)
 
       expect(
-        await web3.eth.getBalance(members[0]),
+        await web3.eth.getBalance(beneficiary),
         "incorrect member account balance"
       ).to.eq.BN(initialMemberBalance.add(singleValue))
     })
 
     it("sends ETH to beneficiary", async () => {
       const valueWithRemainder = ethValue.add(new BN(1))
 
-      const member1 = accounts[2]
-      const member2 = accounts[3]
-      const beneficiary = accounts[4]
+      const member1 = members[0]
+      const member2 = members[1]
 
       const testMembers = [member1, member2]
 
@@ -1659,7 +1666,8 @@ contract("BondedECDSAKeep", (accounts) => {
       const etherReceiver = await TestEtherReceiver.new()
       await etherReceiver.setShouldFail(true)
 
-      const member = etherReceiver.address // a receiver which we expect to reject the transfer
+      const member = members[0]
+      await tokenStaking.setBeneficiary(member, etherReceiver.address) // a receiver which we expect to reject the transfer
 
       const keep = await newKeep(
         owner,
@@ -1676,9 +1684,9 @@ contract("BondedECDSAKeep", (accounts) => {
 
       await expectRevert(keep.withdraw(member), "Transfer failed")
 
-      // Check balances of keep member's account.
+      // Check balances of keep members's beneficiary account.
       expect(
-        await web3.eth.getBalance(member),
+        await web3.eth.getBalance(etherReceiver.address),
         "incorrect member's account balance"
       ).to.eq.BN(0)
 
@@ -1696,6 +1704,10 @@ contract("BondedECDSAKeep", (accounts) => {
 
     beforeEach(async () => {
       token = await TestToken.new()
+
+      for (let i = 0; i < members.length; i++) {
+        await tokenStaking.setBeneficiary(members[i], members[i])
+      }
     })
 
     it("correctly distributes ERC20", async () => {

solidity/test/KeepBondingTest.js

@@ -7,7 +7,11 @@ const ManagedGrant = artifacts.require("./ManagedGrantStub.sol")
 const KeepBonding = artifacts.require("./KeepBonding.sol")
 const TestEtherReceiver = artifacts.require("./TestEtherReceiver.sol")
 
-const {expectEvent, expectRevert} = require("@openzeppelin/test-helpers")
+const {
+  constants,
+  expectEvent,
+  expectRevert,
+} = require("@openzeppelin/test-helpers")
 
 const BN = web3.utils.BN
 
@@ -90,6 +94,8 @@ contract("KeepBonding", (accounts) => {
     const value = new BN(1000)
 
     beforeEach(async () => {
+      await tokenStaking.setBeneficiary(operator, beneficiary)
+
       await keepBonding.deposit(operator, {value: value})
     })
 
@@ -157,10 +163,20 @@ contract("KeepBonding", (accounts) => {
       })
       expectEvent(receipt, "UnbondedValueWithdrawn", {
         operator: operator,
+        beneficiary: beneficiary,
         amount: value,
       })
     })
 
+    it("reverts if beneficiary is not defined", async () => {
+      await tokenStaking.setBeneficiary(operator, constants.ZERO_ADDRESS)
+
+      await expectRevert(
+        keepBonding.withdraw(value, operator, {from: operator}),
+        "Beneficiary not defined for the operator"
+      )
+    })
+
     it("reverts if insufficient unbonded value", async () => {
       const invalidValue = value.add(new BN(1))
 
@@ -188,6 +204,7 @@ contract("KeepBonding", (accounts) => {
 
     beforeEach(async () => {
       await keepBonding.deposit(operator, {value: value})
+      await tokenStaking.setBeneficiary(operator, beneficiary)
 
       managedGrant = await ManagedGrant.new(managedGrantee)
       await tokenGrant.setGranteeOperator(managedGrant.address, operator)

solidity/test/contracts/TokenStakingStub.sol

@@ -45,16 +45,18 @@ contract TokenStakingStub is IStaking {
         return stakes[_operator];
     }
 
-    function setBeneficiary(address _operator, address payable _beneficiary) public {
+    function setBeneficiary(address _operator, address payable _beneficiary)
+        public
+    {
         operatorToBeneficiary[_operator] = _beneficiary;
     }
 
-    function beneficiaryOf(address _operator) public view returns (address payable) {
-        address payable beneficiary = operatorToBeneficiary[_operator];
-        if (beneficiary == address(0)) {
-            return address(uint160(_operator));
-        }
-        return beneficiary;
+    function beneficiaryOf(address _operator)
+        public
+        view
+        returns (address payable)
+    {
+        return operatorToBeneficiary[_operator];
     }
 
     function slash(uint256 _amount, address[] memory _misbehavedOperators)