Modifiers on solidity

Question

So, as part of the bitdegree course on solidity I'm looking to create a modifier named onlyOwner and assign it to changePrice function. I must make sure the modifier allows function to be executed only if the sender's address matches the address of the owner. The sender's address can be obtained using msg.sender.

I have tried entering this to create the modifier but its not working for me and I'm not sure why. Any help/recommended code would be greatly appreciated!

pragma solidity ^0.4.17;

contract ModifiersTutorial {

address public owner;
uint256 public price = 0;
address public sender=msg.sender;

//
modifier onlyOwner(sender){
if (owner==sender);
}
//

// Use your modifier on the function below
function changePrice(uint256 _price) public onlyOwner {
    price = _price;
}

function ModifiersTutorial () {
    owner = msg.sender; // msg.sender in constructor equals to the address that created the contract
}

}

Answer 1

Your modifier code is incorrect. You need an underscore to proceed.

modifier onlyOwner(sender){
  if (owner==sender) _; // Note the underscore
}

Also, for security reasons, you really should just use msg.sender instead of passing it in.

modifier onlyOwner() {
  if (owner == msg.sender) _;
}

Answer 2

pragma solidity ^0.4.17;

contract ModifiersTutorial {

    address public owner;
    uint256 public price = 0;

    modifier onlyOwner(){
    if( owner == msg.sender ) _;
    }

    function changePrice(uint256 _price) public onlyOwner{
        price = _price;
    }

    function ModifiersTutorial () {
        owner = msg.sender; 
    }
}

Answer 3

Not sure if it conflicts with the spec you've been given, but an alternative practice would be to require(owner == msg.sender) rather than use an if statement -- the former tells the user what happened, while the latter simply fails silently. This is what it could look like:

modifier onlyOwner(){
  require(owner == msg.sender, "error-only-owner");
  _;
}