True Ownership (#1247)

* Added barebones Secondary.

* Added transferPrimary

* Escrow is now Secondary instead of Ownable.

* Now reverting on transfers to 0.

* The Secondary's primary is now private.

contracts/mocks/SecondaryMock.sol

@@ -0,0 +1,9 @@
+pragma solidity ^0.4.24;
+
+import "../ownership/Secondary.sol";
+
+
+contract SecondaryMock is Secondary {
+  function onlyPrimaryMock() public view onlyPrimary {
+  }
+}

contracts/ownership/Secondary.sol

@@ -0,0 +1,35 @@
+pragma solidity ^0.4.24;
+
+
+/**
+ * @title Secondary
+ * @dev A Secondary contract can only be used by its primary account (the one that created it)
+ */
+contract Secondary {
+  address private primary_;
+
+  /**
+   * @dev Sets the primary account to the one that is creating the Secondary contract.
+   */
+  constructor() public {
+    primary_ = msg.sender;
+  }
+
+  /**
+   * @dev Reverts if called from any account other than the primary.
+   */
+  modifier onlyPrimary() {
+    require(msg.sender == primary_);
+    _;
+  }
+
+  function primary() public view returns (address) {
+    return primary_;
+  }
+
+  function transferPrimary(address _recipient) public onlyPrimary {
+    require(_recipient != address(0));
+
+    primary_ = _recipient;
+  }
+}

contracts/payment/Escrow.sol

@@ -1,17 +1,17 @@
 pragma solidity ^0.4.23;
 
 import "../math/SafeMath.sol";
-import "../ownership/Ownable.sol";
+import "../ownership/Secondary.sol";
 
 
 /**
  * @title Escrow
  * @dev Base escrow contract, holds funds destinated to a payee until they
  * withdraw them. The contract that uses the escrow as its payment method
- * should be its owner, and provide public methods redirecting to the escrow's
+ * should be its primary, and provide public methods redirecting to the escrow's
  * deposit and withdraw.
  */
-contract Escrow is Ownable {
+contract Escrow is Secondary {
   using SafeMath for uint256;
 
   event Deposited(address indexed payee, uint256 weiAmount);
@@ -27,7 +27,7 @@ contract Escrow is Ownable {
   * @dev Stores the sent amount as credit to be withdrawn.
   * @param _payee The destination address of the funds.
   */
-  function deposit(address _payee) public onlyOwner payable {
+  function deposit(address _payee) public onlyPrimary payable {
     uint256 amount = msg.value;
     deposits[_payee] = deposits[_payee].add(amount);
 
@@ -38,7 +38,7 @@ contract Escrow is Ownable {
   * @dev Withdraw accumulated balance for a payee.
   * @param _payee The address whose funds will be withdrawn and transferred to.
   */
-  function withdraw(address _payee) public onlyOwner {
+  function withdraw(address _payee) public onlyPrimary {
     uint256 payment = deposits[_payee];
     assert(address(this).balance >= payment);
 

test/ownership/Secondary.test.js

@@ -0,0 +1,57 @@
+const { assertRevert } = require('../helpers/assertRevert');
+
+const SecondaryMock = artifacts.require('SecondaryMock');
+
+require('chai')
+  .should();
+
+contract('Secondary', function ([_, primary, newPrimary, anyone]) {
+  const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
+
+  beforeEach(async function () {
+    this.secondary = await SecondaryMock.new({ from: primary });
+  });
+
+  it('stores the primary\'s address', async function () {
+    (await this.secondary.primary()).should.equal(primary);
+  });
+
+  describe('onlyPrimary', function () {
+    it('allows the primary account to call onlyPrimary functions', async function () {
+      await this.secondary.onlyPrimaryMock({ from: primary });
+    });
+
+    it('reverts when anyone calls onlyPrimary functions', async function () {
+      await assertRevert(this.secondary.onlyPrimaryMock({ from: anyone }));
+    });
+  });
+
+  describe('transferPrimary', function () {
+    it('makes the recipient the new primary', async function () {
+      await this.secondary.transferPrimary(newPrimary, { from: primary });
+      (await this.secondary.primary()).should.equal(newPrimary);
+    });
+
+    it('reverts when transfering to the null address', async function () {
+      await assertRevert(this.secondary.transferPrimary(ZERO_ADDRESS, { from: primary }));
+    });
+
+    it('reverts when called by anyone', async function () {
+      await assertRevert(this.secondary.transferPrimary(newPrimary, { from: anyone }));
+    });
+
+    context('with new primary', function () {
+      beforeEach(async function () {
+        await this.secondary.transferPrimary(newPrimary, { from: primary });
+      });
+
+      it('allows the new primary account to call onlyPrimary functions', async function () {
+        await this.secondary.onlyPrimaryMock({ from: newPrimary });
+      });
+
+      it('reverts when the old primary account calls onlyPrimary functions', async function () {
+        await assertRevert(this.secondary.onlyPrimaryMock({ from: primary }));
+      });
+    });
+  });
+});

test/payment/Escrow.behavior.js

@@ -9,46 +9,46 @@ require('chai')
   .use(require('chai-bignumber')(BigNumber))
   .should();
 
-function shouldBehaveLikeEscrow (owner, [payee1, payee2]) {
+function shouldBehaveLikeEscrow (primary, [payee1, payee2]) {
   const amount = web3.toWei(42.0, 'ether');
 
   describe('as an escrow', function () {
     describe('deposits', function () {
       it('can accept a single deposit', async function () {
-        await this.escrow.deposit(payee1, { from: owner, value: amount });
+        await this.escrow.deposit(payee1, { from: primary, value: amount });
 
         (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount);
 
         (await this.escrow.depositsOf(payee1)).should.be.bignumber.equal(amount);
       });
 
       it('can accept an empty deposit', async function () {
-        await this.escrow.deposit(payee1, { from: owner, value: 0 });
+        await this.escrow.deposit(payee1, { from: primary, value: 0 });
       });
 
-      it('only the owner can deposit', async function () {
+      it('only the primary account can deposit', async function () {
         await expectThrow(this.escrow.deposit(payee1, { from: payee2 }), EVMRevert);
       });
 
       it('emits a deposited event', async function () {
-        const receipt = await this.escrow.deposit(payee1, { from: owner, value: amount });
+        const receipt = await this.escrow.deposit(payee1, { from: primary, value: amount });
 
         const event = expectEvent.inLogs(receipt.logs, 'Deposited', { payee: payee1 });
         event.args.weiAmount.should.be.bignumber.equal(amount);
       });
 
       it('can add multiple deposits on a single account', async function () {
-        await this.escrow.deposit(payee1, { from: owner, value: amount });
-        await this.escrow.deposit(payee1, { from: owner, value: amount * 2 });
+        await this.escrow.deposit(payee1, { from: primary, value: amount });
+        await this.escrow.deposit(payee1, { from: primary, value: amount * 2 });
 
         (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount * 3);
 
         (await this.escrow.depositsOf(payee1)).should.be.bignumber.equal(amount * 3);
       });
 
       it('can track deposits to multiple accounts', async function () {
-        await this.escrow.deposit(payee1, { from: owner, value: amount });
-        await this.escrow.deposit(payee2, { from: owner, value: amount * 2 });
+        await this.escrow.deposit(payee1, { from: primary, value: amount });
+        await this.escrow.deposit(payee2, { from: primary, value: amount * 2 });
 
         (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount * 3);
 
@@ -62,8 +62,8 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) {
       it('can withdraw payments', async function () {
         const payeeInitialBalance = await ethGetBalance(payee1);
 
-        await this.escrow.deposit(payee1, { from: owner, value: amount });
-        await this.escrow.withdraw(payee1, { from: owner });
+        await this.escrow.deposit(payee1, { from: primary, value: amount });
+        await this.escrow.withdraw(payee1, { from: primary });
 
         (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(0);
 
@@ -74,16 +74,16 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) {
       });
 
       it('can do an empty withdrawal', async function () {
-        await this.escrow.withdraw(payee1, { from: owner });
+        await this.escrow.withdraw(payee1, { from: primary });
       });
 
-      it('only the owner can withdraw', async function () {
+      it('only the primary account can withdraw', async function () {
         await expectThrow(this.escrow.withdraw(payee1, { from: payee1 }), EVMRevert);
       });
 
       it('emits a withdrawn event', async function () {
-        await this.escrow.deposit(payee1, { from: owner, value: amount });
-        const receipt = await this.escrow.withdraw(payee1, { from: owner });
+        await this.escrow.deposit(payee1, { from: primary, value: amount });
+        const receipt = await this.escrow.withdraw(payee1, { from: primary });
 
         const event = expectEvent.inLogs(receipt.logs, 'Withdrawn', { payee: payee1 });
         event.args.weiAmount.should.be.bignumber.equal(amount);

test/payment/Escrow.test.js

@@ -2,10 +2,10 @@ const { shouldBehaveLikeEscrow } = require('./Escrow.behavior');
 
 const Escrow = artifacts.require('Escrow');
 
-contract('Escrow', function ([_, owner, ...otherAccounts]) {
+contract('Escrow', function ([_, primary, ...otherAccounts]) {
   beforeEach(async function () {
-    this.escrow = await Escrow.new({ from: owner });
+    this.escrow = await Escrow.new({ from: primary });
   });
 
-  shouldBehaveLikeEscrow(owner, otherAccounts);
+  shouldBehaveLikeEscrow(primary, otherAccounts);
 });