ZepplinOS: Where to set owner of the contract?

Question

ZepplinOS contracts do not use constructors in it's zos implementation, and instead uses an initialization function that it imports from zos-lib/contracts/Initializable.sol package.

In order for the contract to have upgradeable state, the state variables need to be declared within the initialization function like so:

  bytes32[] public data;

  function initialize(bytes32[] _data) initializer public {
    data = _data;
  }

If I want to set the owner of the contract, would I simply set an owner variable in the initialize function? Not sure what the best practice is here.

pragma solidity ^0.5.0;

import "zos-lib/contracts/Initializable.sol";

contract Verifications is Initializable { 

  mapping (bytes32 => bytes32) public data;
  address public owner;

  function initialize(bytes32[] _data) initializer public {
    owner = msg.sender;
    data = _data;
  }
}

Thoughts? Is this ok?

Answer 1

I think it'd better to explicit the owner via input param like below, instead of using msg.sender since msg.sender might be not what we intend to set because zos use contract proxy to make it upgradable.

pragma solidity ^0.5.0;

import "zos-lib/contracts/Initializable.sol";

/**
 * @title Ownable
 * @dev The Ownable contract has an owner address, and provides basic authorization control
 * functions, this simplifies the implementation of "user permissions".
 */
contract Ownable is Initializable {
    address private _owner;

    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

    /**
     * @dev The Ownable constructor sets the original `owner` of the contract to the sender
     * account.
     */
    function initialize(address sender) public initializer {
        _owner = sender;
        emit OwnershipTransferred(address(0), _owner);
    }

    /**
     * @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(isOwner());
        _;
    }

    /**
     * @return true if `msg.sender` is the owner of the contract.
     */
    function isOwner() public view returns (bool) {
        return msg.sender == _owner;
    }

    /**
     * @dev Allows the current owner to relinquish control of the contract.
     * @notice Renouncing to ownership will leave the contract without an owner.
     * It will not be possible to call the functions with the `onlyOwner`
     * modifier anymore.
     */
    function renounceOwnership() public onlyOwner {
        emit OwnershipTransferred(_owner, address(0));
        _owner = address(0);
    }

    /**
     * @dev Allows the current owner to transfer control of the contract to a newOwner.
     * @param newOwner The address to transfer ownership to.
     */
    function transferOwnership(address newOwner) public onlyOwner {
        _transferOwnership(newOwner);
    }

    /**
     * @dev Transfers control of the contract to a newOwner.
     * @param newOwner The address to transfer ownership to.
     */
    function _transferOwnership(address newOwner) internal {
        require(newOwner != address(0));
        emit OwnershipTransferred(_owner, newOwner);
        _owner = newOwner;
    }

    uint256[50] private ______gap;
}