diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 878279aa0..05a19045c 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -58,7 +58,7 @@ contract BreakInvariantBounty is PullPayment, Ownable { * @dev Transfers the current balance to the owner and terminates the contract. */ function destroy() public onlyOwner { - selfdestruct(owner); + selfdestruct(owner()); } /** diff --git a/contracts/drafts/TokenVesting.sol b/contracts/drafts/TokenVesting.sol index 4d9f00575..2377073e3 100644 --- a/contracts/drafts/TokenVesting.sol +++ b/contracts/drafts/TokenVesting.sol @@ -93,7 +93,7 @@ contract TokenVesting is Ownable { revoked[_token] = true; - _token.safeTransfer(owner, refund); + _token.safeTransfer(owner(), refund); emit Revoked(); } diff --git a/contracts/lifecycle/Pausable.sol b/contracts/lifecycle/Pausable.sol index 0a2767aa6..37caf1e0a 100644 --- a/contracts/lifecycle/Pausable.sol +++ b/contracts/lifecycle/Pausable.sol @@ -12,14 +12,21 @@ contract Pausable is Ownable { event Paused(); event Unpaused(); - bool public paused = false; + bool private paused_ = false; + /** + * @return true if the contract is paused, false otherwise. + */ + function paused() public view returns(bool) { + return paused_; + } + /** * @dev Modifier to make a function callable only when the contract is not paused. */ modifier whenNotPaused() { - require(!paused); + require(!paused_); _; } @@ -27,7 +34,7 @@ contract Pausable is Ownable { * @dev Modifier to make a function callable only when the contract is paused. */ modifier whenPaused() { - require(paused); + require(paused_); _; } @@ -35,7 +42,7 @@ contract Pausable is Ownable { * @dev called by the owner to pause, triggers stopped state */ function pause() public onlyOwner whenNotPaused { - paused = true; + paused_ = true; emit Paused(); } @@ -43,7 +50,7 @@ contract Pausable is Ownable { * @dev called by the owner to unpause, returns to normal state */ function unpause() public onlyOwner whenPaused { - paused = false; + paused_ = false; emit Unpaused(); } } diff --git a/contracts/ownership/CanReclaimToken.sol b/contracts/ownership/CanReclaimToken.sol index 858a1db84..e4f7e88a6 100644 --- a/contracts/ownership/CanReclaimToken.sol +++ b/contracts/ownership/CanReclaimToken.sol @@ -20,7 +20,7 @@ contract CanReclaimToken is Ownable { */ function reclaimToken(IERC20 _token) external onlyOwner { uint256 balance = _token.balanceOf(this); - _token.safeTransfer(owner, balance); + _token.safeTransfer(owner(), balance); } } diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index 1fbf571bf..f89013a1a 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -7,7 +7,7 @@ pragma solidity ^0.4.24; * functions, this simplifies the implementation of "user permissions". */ contract Ownable { - address public owner; + address private owner_; event OwnershipRenounced(address indexed previousOwner); @@ -22,14 +22,21 @@ contract Ownable { * account. */ constructor() public { - owner = msg.sender; + owner_ = msg.sender; + } + + /** + * @return the address of the owner. + */ + function owner() public view returns(address) { + return owner_; } /** * @dev Throws if called by any account other than the owner. */ modifier onlyOwner() { - require(msg.sender == owner); + require(msg.sender == owner_); _; } @@ -40,8 +47,8 @@ contract Ownable { * modifier anymore. */ function renounceOwnership() public onlyOwner { - emit OwnershipRenounced(owner); - owner = address(0); + emit OwnershipRenounced(owner_); + owner_ = address(0); } /** @@ -58,7 +65,7 @@ contract Ownable { */ function _transferOwnership(address _newOwner) internal { require(_newOwner != address(0)); - emit OwnershipTransferred(owner, _newOwner); - owner = _newOwner; + emit OwnershipTransferred(owner_, _newOwner); + owner_ = _newOwner; } } diff --git a/contracts/ownership/Superuser.sol b/contracts/ownership/Superuser.sol index d8d081d03..b1f1fa4b5 100644 --- a/contracts/ownership/Superuser.sol +++ b/contracts/ownership/Superuser.sol @@ -27,7 +27,7 @@ contract Superuser is Ownable, RBAC { } modifier onlyOwnerOrSuperuser() { - require(msg.sender == owner || isSuperuser(msg.sender)); + require(msg.sender == owner() || isSuperuser(msg.sender)); _; } diff --git a/contracts/payment/RefundEscrow.sol b/contracts/payment/RefundEscrow.sol index 9ba5fc600..802dd4711 100644 --- a/contracts/payment/RefundEscrow.sol +++ b/contracts/payment/RefundEscrow.sol @@ -16,8 +16,8 @@ contract RefundEscrow is Ownable, ConditionalEscrow { event Closed(); event RefundsEnabled(); - State public state; - address public beneficiary; + State private state_; + address private beneficiary_; /** * @dev Constructor. @@ -25,8 +25,22 @@ contract RefundEscrow is Ownable, ConditionalEscrow { */ constructor(address _beneficiary) public { require(_beneficiary != address(0)); - beneficiary = _beneficiary; - state = State.Active; + beneficiary_ = _beneficiary; + state_ = State.Active; + } + + /** + * @return the current state of the escrow. + */ + function state() public view returns(State) { + return state_; + } + + /** + * @return the beneficiary of the escrow. + */ + function beneficiary() public view returns(address) { + return beneficiary_; } /** @@ -34,7 +48,7 @@ contract RefundEscrow is Ownable, ConditionalEscrow { * @param _refundee The address funds will be sent to if a refund occurs. */ function deposit(address _refundee) public payable { - require(state == State.Active); + require(state_ == State.Active); super.deposit(_refundee); } @@ -43,8 +57,8 @@ contract RefundEscrow is Ownable, ConditionalEscrow { * further deposits. */ function close() public onlyOwner { - require(state == State.Active); - state = State.Closed; + require(state_ == State.Active); + state_ = State.Closed; emit Closed(); } @@ -52,8 +66,8 @@ contract RefundEscrow is Ownable, ConditionalEscrow { * @dev Allows for refunds to take place, rejecting further deposits. */ function enableRefunds() public onlyOwner { - require(state == State.Active); - state = State.Refunding; + require(state_ == State.Active); + state_ = State.Refunding; emit RefundsEnabled(); } @@ -61,14 +75,14 @@ contract RefundEscrow is Ownable, ConditionalEscrow { * @dev Withdraws the beneficiary's funds. */ function beneficiaryWithdraw() public { - require(state == State.Closed); - beneficiary.transfer(address(this).balance); + require(state_ == State.Closed); + beneficiary_.transfer(address(this).balance); } /** * @dev Returns whether refundees can withdraw their deposits (be refunded). */ function withdrawalAllowed(address _payee) public view returns (bool) { - return state == State.Refunding; + return state_ == State.Refunding; } } diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index 576dba0fe..0d4a2e095 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -11,12 +11,12 @@ import "../math/SafeMath.sol"; contract SplitPayment { using SafeMath for uint256; - uint256 public totalShares = 0; - uint256 public totalReleased = 0; + uint256 private totalShares_ = 0; + uint256 private totalReleased_ = 0; - mapping(address => uint256) public shares; - mapping(address => uint256) public released; - address[] public payees; + mapping(address => uint256) private shares_; + mapping(address => uint256) private released_; + address[] private payees_; /** * @dev Constructor @@ -35,25 +35,60 @@ contract SplitPayment { */ function () external payable {} + /** + * @return the total shares of the contract. + */ + function totalShares() public view returns(uint256) { + return totalShares_; + } + + /** + * @return the total amount already released. + */ + function totalReleased() public view returns(uint256) { + return totalReleased_; + } + + /** + * @return the shares of an account. + */ + function shares(address _account) public view returns(uint256) { + return shares_[_account]; + } + + /** + * @return the amount already released to an account. + */ + function released(address _account) public view returns(uint256) { + return released_[_account]; + } + + /** + * @return the address of a payee. + */ + function payee(uint256 index) public view returns(address) { + return payees_[index]; + } + /** * @dev Release one of the payee's proportional payment. * @param _payee Whose payments will be released. */ function release(address _payee) public { - require(shares[_payee] > 0); + require(shares_[_payee] > 0); - uint256 totalReceived = address(this).balance.add(totalReleased); + uint256 totalReceived = address(this).balance.add(totalReleased_); uint256 payment = totalReceived.mul( - shares[_payee]).div( - totalShares).sub( - released[_payee] + shares_[_payee]).div( + totalShares_).sub( + released_[_payee] ); require(payment != 0); assert(address(this).balance >= payment); - released[_payee] = released[_payee].add(payment); - totalReleased = totalReleased.add(payment); + released_[_payee] = released_[_payee].add(payment); + totalReleased_ = totalReleased_.add(payment); _payee.transfer(payment); } @@ -66,10 +101,10 @@ contract SplitPayment { function _addPayee(address _payee, uint256 _shares) internal { require(_payee != address(0)); require(_shares > 0); - require(shares[_payee] == 0); + require(shares_[_payee] == 0); - payees.push(_payee); - shares[_payee] = _shares; - totalShares = totalShares.add(_shares); + payees_.push(_payee); + shares_[_payee] = _shares; + totalShares_ = totalShares_.add(_shares); } } diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index 0ba200cdf..46d69813e 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/contracts/token/ERC20/ERC20Mintable.sol @@ -22,7 +22,7 @@ contract ERC20Mintable is ERC20, Ownable { } modifier hasMintPermission() { - require(msg.sender == owner); + require(msg.sender == owner()); _; } diff --git a/test/payment/RefundEscrow.test.js b/test/payment/RefundEscrow.test.js index e74d68f67..7730d8242 100644 --- a/test/payment/RefundEscrow.test.js +++ b/test/payment/RefundEscrow.test.js @@ -28,6 +28,11 @@ contract('RefundEscrow', function ([_, owner, beneficiary, refundee1, refundee2] }); context('active state', function () { + it('has beneficiary and state', async function () { + (await this.escrow.beneficiary()).should.be.equal(beneficiary); + (await this.escrow.state()).should.be.bignumber.equal(0); + }); + it('accepts deposits', async function () { await this.escrow.deposit(refundee1, { from: owner, value: amount }); diff --git a/test/payment/SplitPayment.test.js b/test/payment/SplitPayment.test.js index c5a0bf617..788296016 100644 --- a/test/payment/SplitPayment.test.js +++ b/test/payment/SplitPayment.test.js @@ -46,6 +46,17 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, this.contract = await SplitPayment.new(this.payees, this.shares); }); + it('should have total shares', async function () { + (await this.contract.totalShares()).should.be.bignumber.equal(20 + 10 + 70); + }); + + it('should have payees', async function () { + this.payees.forEach(async (payee, index) => { + (await this.payee(index)).should.be.equal(payee); + (await this.contract.released(payee)).should.be.bignumber.equal(0); + }); + }); + it('should accept payments', async function () { await ethSendTransaction({ from: owner, to: this.contract.address, value: amount }); @@ -53,11 +64,11 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should store shares if address is payee', async function () { - (await this.contract.shares.call(payee1)).should.be.bignumber.not.equal(0); + (await this.contract.shares(payee1)).should.be.bignumber.not.equal(0); }); it('should not store shares if address is not payee', async function () { - (await this.contract.shares.call(nonpayee1)).should.be.bignumber.equal(0); + (await this.contract.shares(nonpayee1)).should.be.bignumber.equal(0); }); it('should throw if no funds to claim', async function () { @@ -96,7 +107,7 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, (await ethGetBalance(this.contract.address)).should.be.bignumber.equal(0); // check correct funds released accounting - (await this.contract.totalReleased.call()).should.be.bignumber.equal(initBalance); + (await this.contract.totalReleased()).should.be.bignumber.equal(initBalance); }); }); });