Disallow a 0x0 address beneficiary (#9283)

* Disallow address 0x0 beneficiaries

* Add function to remove payment delegation

* Add documentation

* Require deletion of payment delegation is done by account

* Specify deletePaymentDelegation as non privileged

packages/protocol/contracts/common/Accounts.sol

@@ -323,15 +323,27 @@ contract Accounts is
    * @param fraction The fraction of the validator's payment that should be
    * diverted to `beneficiary` every epoch, given as FixidyLib value. Must not
    * be greater than 1.
+   * @dev Use `deletePaymentDelegation` to unset the payment delegation.
    */
   function setPaymentDelegation(address beneficiary, uint256 fraction) public {
     require(isAccount(msg.sender), "Not an account");
+    require(beneficiary != address(0), "Beneficiary cannot be address 0x0");
     FixidityLib.Fraction memory f = FixidityLib.wrap(fraction);
     require(f.lte(FixidityLib.fixed1()), "Fraction must not be greater than 1");
     paymentDelegations[msg.sender] = PaymentDelegation(beneficiary, f);
     emit PaymentDelegationSet(beneficiary, fraction);
   }
 
+  /**
+   * @notice Removes a validator's payment delegation by setting benficiary and
+   * fraction to 0.
+   */
+  function deletePaymentDelegation() public {
+    require(isAccount(msg.sender), "Not an account");
+    paymentDelegations[msg.sender] = PaymentDelegation(address(0x0), FixidityLib.wrap(0));
+    emit PaymentDelegationSet(address(0x0), 0);
+  }
+
   /**
    * @notice Gets validator payment delegation settings.
    * @param account Account of the validator.

packages/protocol/specs/accountsPrivileged.spec

@@ -25,6 +25,7 @@ definition knownAsNonPrivileged(method f) returns bool  = false
   || f.selector == removeSigner(address,bytes32).selector
   || f.selector == setEip712DomainSeparator().selector
   || f.selector == setPaymentDelegation(address,uint256).selector
+  || f.selector == deletePaymentDelegation().selector
   ; 
 
 rule privilegedOperation(method f, address privileged)

packages/protocol/test/common/accounts.ts

@@ -487,6 +487,13 @@ contract('Accounts', (accounts: string[]) => {
         )
       })
 
+      it('should not allow a beneficiary with address 0x0', async () => {
+        await assertRevertWithReason(
+          accountsInstance.setPaymentDelegation(NULL_ADDRESS, fraction),
+          'Beneficiary cannot be address 0x0'
+        )
+      })
+
       it('emits a PaymentDelegationSet event', async () => {
         const resp = await accountsInstance.setPaymentDelegation(beneficiary, fraction)
         assertLogMatches2(resp.logs[0], {
@@ -497,6 +504,40 @@ contract('Accounts', (accounts: string[]) => {
     })
   })
 
+  describe('#deletePaymentDelegation', () => {
+    const beneficiary = accounts[1]
+    const fraction = toFixed(0.2)
+
+    beforeEach(async () => {
+      await accountsInstance.createAccount()
+      await accountsInstance.setPaymentDelegation(beneficiary, fraction)
+    })
+
+    it('should not be callable by a non-account', async () => {
+      await assertRevertWithReason(
+        accountsInstance.setPaymentDelegation(beneficiary, fraction, { from: accounts[2] }),
+        'Not an account'
+      )
+    })
+
+    it('should set the address and beneficiary to 0', async () => {
+      await accountsInstance.deletePaymentDelegation()
+      const [realBeneficiary, realFraction] = await accountsInstance.getPaymentDelegation.call(
+        accounts[0]
+      )
+      assert.equal(realBeneficiary, NULL_ADDRESS)
+      assertEqualBN(realFraction, new BigNumber(0))
+    })
+
+    it('emits a PaymentDelegationSet event', async () => {
+      const resp = await accountsInstance.deletePaymentDelegation()
+      assertLogMatches2(resp.logs[0], {
+        event: 'PaymentDelegationSet',
+        args: { beneficiary: NULL_ADDRESS, fraction: new BigNumber(0) },
+      })
+    })
+  })
+
   describe('#setName', () => {
     describe('when the account has not been created', () => {
       it('should revert', async () => {