Merge pull request #4 from JakeLin/feature/improve-contract-part4

Improve contract part4 - @xavierlepretre thanks, I will address all your comments in the next PR with GUI.

contracts/Ownable.sol

@@ -0,0 +1,75 @@
+pragma solidity ^0.5.0;
+
+/**
+ * @dev Contract module which provides a basic access control mechanism, where
+ * there is an account (an owner) that can be granted exclusive access to
+ * specific functions.
+ *
+ * This module is used through inheritance. It will make available the modifier
+ * `onlyOwner`, which can be applied to your functions to restrict their use to
+ * the owner.
+ */
+contract Ownable {
+    address private _owner;
+
+    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
+
+    /**
+     * @dev Initializes the contract setting the deployer as the initial owner.
+     */
+    constructor () internal {
+        _owner = msg.sender;
+        emit OwnershipTransferred(address(0), _owner);
+    }
+
+    /**
+     * @dev Returns the address of the current owner.
+     */
+    function owner() public view returns (address) {
+        return _owner;
+    }
+
+    /**
+     * @dev Throws if called by any account other than the owner.
+     */
+    modifier onlyOwner() {
+        require(isOwner(), "Ownable: caller is not the owner");
+        _;
+    }
+
+    /**
+     * @dev Returns true if the caller is the current owner.
+     */
+    function isOwner() public view returns (bool) {
+        return msg.sender == _owner;
+    }
+
+    /**
+     * @dev Leaves the contract without owner. It will not be possible to call
+     * `onlyOwner` functions anymore. Can only be called by the current owner.
+     *
+     * NOTE: Renouncing ownership will leave the contract without an owner,
+     * thereby removing any functionality that is only available to the owner.
+     */
+    function renounceOwnership() public onlyOwner {
+        emit OwnershipTransferred(_owner, address(0));
+        _owner = address(0);
+    }
+
+    /**
+     * @dev Transfers ownership of the contract to a new account (`newOwner`).
+     * Can only be called by the current owner.
+     */
+    function transferOwnership(address newOwner) public onlyOwner {
+        _transferOwnership(newOwner);
+    }
+
+    /**
+     * @dev Transfers ownership of the contract to a new account (`newOwner`).
+     */
+    function _transferOwnership(address newOwner) internal {
+        require(newOwner != address(0), "Ownable: new owner is the zero address");
+        emit OwnershipTransferred(_owner, newOwner);
+        _owner = newOwner;
+    }
+}

contracts/Splitter.sol

@@ -1,25 +1,16 @@
 pragma solidity >=0.4.21 <0.6.0;
 
 import "./SafeMath.sol";
+import "./Ownable.sol";
 
-contract Splitter {
+contract Splitter is Ownable {
   using SafeMath for uint256;
 
   event LogSplitted(address indexed sender, uint256 amount, address indexed beneficiary1, address indexed beneficiary2);
   event LogWithdrawn(address indexed sender, uint256 amount);
 
-  address public owner;
   mapping (address => uint256) public balances;
 
-  constructor() public {
-    owner = msg.sender;
-  }
-
-  modifier onlyOwner() {
-    require (msg.sender == owner, "Only owner can split!");
-    _;
-  }
-
   function split(address _beneficiary1, address _beneficiary2) external payable onlyOwner() {
     require(_beneficiary1 != address(0) && _beneficiary2 != address(0), "Beneficiary's address must not be zero!");
     require(msg.value > 0, "Must split more than zero ether!");

migrations/2_deploy_contracts.js

@@ -3,5 +3,4 @@ var Splitter = artifacts.require("./Splitter.sol");
 
 module.exports = function(deployer) {
   deployer.deploy(Splitter);
-  deployer.link(Splitter, SafeMath);
 };

test/splitter.js

@@ -76,7 +76,7 @@ contract('Splitter', accounts => {
       // Act & Assert
       await truffleAssert.reverts(
         contract.methods.split(bob, carol).send({from: someoneElse, value: 2}),
-        'Only owner can split!'
+        'Ownable: caller is not the owner'
       );
     });
   });
@@ -95,7 +95,7 @@ contract('Splitter', accounts => {
 
     it('should withdraw the ether to Bob\'s account', async () => {
       // Assert
-      const gasPrice = toBN(await web3.eth.getGasPrice());
+      const gasPrice = toBN((await web3.eth.getTransaction(tx.transactionHash)).gasPrice);
       const gasFee = toBN(tx.gasUsed).mul(gasPrice);
       assert.strictEqual(toBN((await web3.eth.getBalance(bob))).sub(bobBeforeWithdrawBalance).toString(10), toBN(toWei('0.03', 'ether')).sub(gasFee).toString(10));
     });